lunny/helm-chart
1
0
Fork 0
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

Ingress annotation inputs should always be quoted #483

New Issue
Closed
opened 2023-08-03 10:57:11 +00:00 by tobiasbp · 12 comments
tobiasbp commented 2023-08-03 10:57:11 +00:00 (Migrated from gitea.com)
Copy Link

In the Ingress object, I would like to use this annotation:
ingress.annotations.nginx.ingress.kubernetes.io/use-regex: true

When adding the annotation above, the Chart breaks. If I quote the value like this \'true\', the value is accepted, but the value in the Kubernetes Ingress object becomes '''true'''. I'm not sure if that will evaluate to the boolean value of true?

Maybe the values should be quoted here ?
https://gitea.com/gitea/helm-chart/src/branch/main/templates/gitea/ingress.yaml#L20

I'm using Helmsman to deploy the Gitea chart.

Any suggestions much appreciated. Thanks.

In the Ingress object, I would like to use this annotation: _ingress.annotations.nginx.ingress.kubernetes.io/use-regex: true_ When adding the annotation above, the Chart breaks. If I quote the value like this _\\'true\\'_, the value is accepted, but the value in the Kubernetes Ingress object becomes _'''true'''_. I'm not sure if that will evaluate to the boolean value of _true_? Maybe the values should be quoted here ? https://gitea.com/gitea/helm-chart/src/branch/main/templates/gitea/ingress.yaml#L20 I'm using Helmsman to deploy the Gitea chart. Any suggestions much appreciated. Thanks.
pat-s commented 2023-08-03 12:21:02 +00:00 (Migrated from gitea.com)
Copy Link

The following works for me

ingress:
  enabled: true
  annotations:
    nginx.ingress.kubernetes.io/use-regex: "true"
The following works for me ```yml ingress: enabled: true annotations: nginx.ingress.kubernetes.io/use-regex: "true" ```
tobiasbp commented 2023-08-03 17:27:08 +00:00 (Migrated from gitea.com)
Copy Link

It's probably related to me using Helmsman. If quoting the values for annotations in the chart solves the problem, would you accept a PR for that?

It's probably related to me using Helmsman. If quoting the values for annotations in the chart solves the problem, would you accept a PR for that?
pat-s commented 2023-08-04 07:04:02 +00:00 (Migrated from gitea.com)
Copy Link

I don't see how the chart is the issue here. The values are forwarded verbatim, so you should be able to supply any kind of quoting yourself. In the end it's YAML and any tool should interpret it the same way.

Can you double-check this? I don't know helmsman but I also suspect it might not be the issue. Last, you could try directly via helm and and possibly sort out the issue with helmsman then.

I don't see how the chart is the issue here. The values are forwarded verbatim, so you should be able to supply any kind of quoting yourself. In the end it's YAML and any tool should interpret it the same way. Can you double-check this? I don't know helmsman but I also suspect it might not be the issue. Last, you could try directly via helm and and possibly sort out the issue with helmsman then.
👍 1
tobiasbp commented 2023-08-04 07:09:32 +00:00 (Migrated from gitea.com)
Copy Link

Thanks. I'll investigate and report back.

Thanks. I'll investigate and report back.
pat-s commented 2023-08-22 20:56:34 +00:00 (Migrated from gitea.com)
Copy Link

@tobiasbp any news here?

@tobiasbp any news here?
tobiasbp commented 2023-08-24 08:04:39 +00:00 (Migrated from gitea.com)
Copy Link

I'm using Helmsman to deploy. Helmsman uses the --set option in Helm to set values in the chart.

The following demonstrates the issue by using Helm to add an annotation to the Ingress object.

Only string values accepted

Setting an annotation in the Ingress object with a string value works:

helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=hello

Setting an annotation in the Ingress object with a integer value does not work:

helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=100

Error: INSTALLATION FAILED: unable to build kubernetes objects from release manifest: unable to decode "": json: cannot unmarshal number into Go struct field ObjectMeta.metadata.annotations of type string

Setting an annotation in the Ingress object with a boolean value does not work:

helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=true

Error: INSTALLATION FAILED: unable to build kubernetes objects from release manifest: unable to decode "": json: cannot unmarshal bool into Go struct field ObjectMeta.metadata.annotations of type string

Using quoted non string values

I can put quotes around a non string value, and the annotation will be added:

helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=\"100\"

This will add this annotation to the Ingress object:

  annotations:
    foo-bar/some-parameter: '"100"'

That value is not honored as 100. I have to manually edit the Ingress object in the Kubernetes cluster to make it look like this (A single set of "), to make the annotation work (ingress uses the value):

  annotations:
    foo-bar/some-parameter: "100"

Proposed solution

Current version of the Chart adds annotations like this:

....
  {{- with .Values.ingress.annotations }}
  annotations:
    {{- toYaml . | nindent 4 }}
  {{- end }}
....

Adding annotations like this, allows all types of values to be used in annotations:

....
  annotations:
    {{- range $key, $value := .Values.ingress.annotations }}
      {{ $key }}: {{ $value | quote }}
    {{- end }}
...

Example of using an integer as value with proposed solution:

helm install --dry-run --generate-name ./gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-proxy=100 | grep proxy

      foo-bar/some-proxy: "100"

Example of setting a boolen value for an annotation with dots in name:

helm install --dry-run --generate-name ./gitea --set ingress.enabled=true --set ingress.annotations.foo\\.bar/some-proxy=true | grep proxy

      foo.bar/some-proxy: "true"

According to the ingress-nginx documentation values for annotations must be quoted.

I'm using _Helmsman_ to deploy. _Helmsman_ uses the _--set_ option in _Helm_ to set values in the chart. The following demonstrates the issue by using _Helm_ to add an annotation to the _Ingress object_. # Only string values accepted Setting an annotation in the Ingress object with a _string_ value works: ``` helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=hello ``` Setting an annotation in the Ingress object with a _integer_ value does **not** work: ``` helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=100 Error: INSTALLATION FAILED: unable to build kubernetes objects from release manifest: unable to decode "": json: cannot unmarshal number into Go struct field ObjectMeta.metadata.annotations of type string ``` Setting an annotation in the Ingress object with a _boolean_ value does **not** work: ``` helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=true Error: INSTALLATION FAILED: unable to build kubernetes objects from release manifest: unable to decode "": json: cannot unmarshal bool into Go struct field ObjectMeta.metadata.annotations of type string ``` # Using quoted non string values I can put quotes around a non string value, and the annotation will be added: ``` helm install --dry-run --generate-name gitea/gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-parameter=\"100\" ``` This will add this annotation to the Ingress object: ``` annotations: foo-bar/some-parameter: '"100"' ``` That value is not honored as _100_. I have to manually edit the Ingress object in the Kubernetes cluster to make it look like this (A single set of "), to make the annotation work (ingress uses the value): ``` annotations: foo-bar/some-parameter: "100" ``` # Proposed solution Current version of the Chart adds annotations like this: ``` .... {{- with .Values.ingress.annotations }} annotations: {{- toYaml . | nindent 4 }} {{- end }} .... ``` Adding annotations like this, allows all types of values to be used in annotations: ``` .... annotations: {{- range $key, $value := .Values.ingress.annotations }} {{ $key }}: {{ $value | quote }} {{- end }} ... ``` Example of using an _integer_ as value with proposed solution: ``` helm install --dry-run --generate-name ./gitea --set ingress.enabled=true --set ingress.annotations.foo-bar/some-proxy=100 | grep proxy foo-bar/some-proxy: "100" ``` Example of setting a _boolen_ value for an annotation with dots in name: ``` helm install --dry-run --generate-name ./gitea --set ingress.enabled=true --set ingress.annotations.foo\\.bar/some-proxy=true | grep proxy foo.bar/some-proxy: "true" ``` According to the [ingress-nginx documentation](https://github.com/kubernetes/ingress-nginx/blob/main/docs/user-guide/nginx-configuration/annotations.md#annotations) values for annotations **must** be quoted.
pat-s commented 2023-08-26 11:11:52 +00:00 (Migrated from gitea.com)
Copy Link

Thanks for the extensive research.

I haven't found any ingress annotation definition of mine where I needed a numeric values being treated as such. All numeric inputs (e.g. IP definitions) were correctly used behind the scenes when quoted. Yet I have nginx.ingress.kubernetes.io/use-regex: "true" in some definitions and using quoted inputs for booleans seems to also work as desired behind the scenes.

I wonder what influence helmsman plays here and to what degree this is caused by helm itself? Why can't you use unescaped quotes in helmsman? Ref your example --set ingress.annotations.foo-bar/some-parameter=\"100\". Does this restriction only apply to numerics but not to booleans, i.e. the latter still work as desired?

Adding annotations like this, allows all types of values to be used in annotations:

I am ok with this change as it should only make things more flexible AFAICS. Do you want to create a PR?

Thanks for the extensive research. I haven't found any ingress annotation definition of mine where I needed a numeric values being treated as such. All numeric inputs (e.g. IP definitions) were correctly used behind the scenes when quoted. Yet I have `nginx.ingress.kubernetes.io/use-regex: "true"` in some definitions and using quoted inputs for booleans seems to also work as desired behind the scenes. I wonder what influence `helmsman` plays here and to what degree this is caused by `helm` itself? Why can't you use unescaped quotes in `helmsman`? Ref your example `--set ingress.annotations.foo-bar/some-parameter=\"100\"`. Does this restriction only apply to numerics but not to booleans, i.e. the latter still work as desired? > Adding annotations like this, allows all types of values to be used in annotations: I am ok with this change as it should only make things more flexible AFAICS. Do you want to create a PR?
tobiasbp commented 2023-08-29 13:53:06 +00:00 (Migrated from gitea.com)
Copy Link

I'd like to add a PR, however my private SSH key (ssh-rsa) is not accepted when i try to add it to my gitea.com account?
Cannot verify your SSH key: key length is not enough: got 2024, needs 2047

https://github.com/go-gitea/gitea/issues/20249

Feel free to add my suggested change since I can not easily do it.

I'd like to add a PR, however my private SSH key (ssh-rsa) is not accepted when i try to add it to my _gitea.com_ account? `Cannot verify your SSH key: key length is not enough: got 2024, needs 2047` https://github.com/go-gitea/gitea/issues/20249 Feel free to add my suggested change since I can not easily do it.
pat-s commented 2023-08-30 08:53:57 +00:00 (Migrated from gitea.com)
Copy Link

What key type is it? The linked issue is very old, I doubt this one applies.
Might be worth opening a new issue or creating a new keypair for gitea.com?

What key type is it? The linked issue is very old, I doubt this one applies. Might be worth opening a new issue or creating a new keypair for gitea.com?
tobiasbp commented 2023-08-31 10:17:37 +00:00 (Migrated from gitea.com)
Copy Link

What key type is it? The linked issue is very old, I doubt this one applies.
Might be worth opening a new issue or creating a new keypair for gitea.com?

I have created a new ssh-rsa keypair with ssh-keygen -t rsa. I have added the new public key to my https://gitea.com/gitea account. The public key I use in other places is also ssh-rsa. The new public key is longer than the older one.

I'll create a PR with the changes to ingress annotations.

> What key type is it? The linked issue is very old, I doubt this one applies. > Might be worth opening a new issue or creating a new keypair for gitea.com? I have created a new _ssh-rsa_ keypair with `ssh-keygen -t rsa`. I have added the new public key to my _https://gitea.com/gitea_ account. The public key I use in other places is also _ssh-rsa_. The new public key is longer than the older one. I'll create a PR with the changes to _ingress annotations_.
pat-s commented 2023-08-31 12:00:55 +00:00 (Migrated from gitea.com)
Copy Link

Why -t rsa and not use the new default Ed25519 if you have issues with rsa?

Why `-t rsa` and not use the new default Ed25519 if you have issues with rsa?
tobiasbp commented 2023-08-31 12:18:38 +00:00 (Migrated from gitea.com)
Copy Link

Added a PR: https://gitea.com/gitea/helm-chart/pulls/497

Added a PR: https://gitea.com/gitea/helm-chart/pulls/497
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
kind/enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#483
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?