Helmfile: Feature Request: Add support for `helm --set-string`

[osterman]

what

• add support for --set-string

why

• Helm will cast certain values to integers (e.g. true)
• Certain fields need to be strings

use-case

Certain services like kube-lego expect a boolean string.

e.g.

ingress.annotations.kubernetes\.io/tls-acme=true

When passed with --set, this gets cast to an integer and causes the error:

failed to create patch: failed to get versionedObject: unable to convert unstructured object to extensions/v1beta1, Kind=Ingress: cannot convert int64 to string

Using --set-string everything works.

references

https://github.com/kubernetes/helm/pull/3599

what does not work

• --set ingress.annotations.kubernetes\.io/tls-acme="true"
• --set ingress.annotations.kubernetes\.io/tls-acme=\"true\"
• --set ingress.annotations.kubernetes\.io/tls-acme="!!bool true"

[osterman]

Here are a few possible interfaces:

How it works today:

set:
  name: "foo"
  value: "true"

option 1

Introduce a top-level set_string arg

set_string:
  name: "foo"
  value: "true"

option 2

Introduce a string parameter.

set_string:
  name: "foo"
  string: "true"

option 3 (preferred)

Introduce a type parameter.

set_string:
  name: "foo"
  value: "true"
  type: "string"

[mumoshu]

@osterman Sounds like a good idea 👍

Nit but you want set_string to be just set in option 2 and 3, right?

[mumoshu]

I "slightly" prefer option 1 over 3 just because it's easier to add validation for unexpected keys at go-yaml-level vs adding our own validation for type.

From another point of view, I slightly prefer option 1 over 2 and 3 because (1) it leaves possibility to add any value source at helmfile-level regardless of --set --set-string and --set-file and (2) it translates directly to respective helm flags.

So, probably I'd expect this in the future:

releases:
  - name: myapp
    set:
    - name: val1
      name: foo
    setString:
    - name: val2
      value: bar
    setFile:
    - name: val3
      value: file1

which is translated to:

helm upgrade --install --set val1=foo --set-string val2=bar --set-file val3=file1

WDYT?

[sstarcher]

Hmm so helm itself does not have this capability does anyone know why?

[mumoshu]

@sstarcher Excuse me but would you mind clarifying what you are referring? --set-string is actually supported by helm.

[sstarcher]

@mumoshu I'm not seeing it on my helm 2.8.2. For helm install I only see --set

      --repo string            chart repository url where to locate the requested chart
      --set stringArray        set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)
      --timeout int            time in seconds to wait for any individual Kubernetes operation (like Jobs for hooks) (default 300)

[mumoshu]

Ha! We were hurrying too much. It turns out we need 2.9.0-rc.4 or later for --set-string.

[osterman]

Nit but you want set_string to be just set in option 2 and 3, right?

oops. Yes, that's what I meant.

[osterman]

I'm not seeing it on my helm 2.8.2

Sorry! As you guys figured out, it's for the upcoming 2.9.0 release. My fault for not pointing that out and only linking to the PR.

Good time to start planning for it though! =)

[sstarcher]

Could we get around this by not using --set on the command line for the "sets" in

releases:
  - name: myapp
    set:
    - name: val1
      name: foo
    setString:
    - name: val2
      value: bar
    setFile:
    - name: val3
      value: file1

and instead dumping them out to a yaml file as is. Overall I dislike the inline ability of helmfile for set values and believe we should push people to putting them in yaml files that live separately. Of course one reason now for the set: structure is for interpolation, but we have a ticket to interpolate the yaml files also.

[sstarcher]

If we really want the set: to remain we could deprecate set and implement this all using something like.

releases:
  - name: myapp
    values:
      val1: foo
      val2: bar
      ingress.annotations.kubernetes\.io/tls-acme: true

Would would also allow us to get around your boolean issue and is much much cleaner and less verbose. Those values could get dumped to a yaml file and read into helm with a --values.

[mumoshu]

@sstarcher Thx. Yeah. That's also a viable option after #97!

I'd rather keep values a yaml array so that you can still merge values files and inlined values like:

releases:
- name: myrelease
   values:
   # My proposal for #97. Also address this feature request and save users from #109
  - myvalues.yaml
   # Your proposal above. It would also address #40
  - val1: foo
      val2: bar
      ingress.annotations.kubernetes\.io/tls-acme: true

This sounds nice in regard to backward-compatibility. What we need is just implement #97 and extend values[] to additionally accept map[string]interface as well as string.

WDYT?

[sstarcher]

@mumoshu ya, I like your suggestion. I entirely forgot values overlapped with the existing values.

[osterman]

https://github.com/kubernetes/helm/releases/tag/v2.9.0 released today

[osterman]

Could we get around this by not using --set on the command line for the "sets"

I'd also like to voice that I'm not a big fan of --set and --set-string per the aformentioned reasons. Relying on the the helm values file interface seems like the best bet.

instead dumping them out to a yaml file as is

Yes! This would be ideal

[mumoshu]

This is on the very beginning of my TODO list now 😃

[mumoshu]

@int128 is working on this at #139. Awesome!

[mumoshu]

I believe this is resolved via #139, by making it unnecessary to use set: in helmfile and hence --set-* family of helm. Please feel free to reopen or raise another issue if not!

[osterman]

@mumoshu awesome! we'll check this out.

[osterman]

This makes me want https://github.com/roboll/helmfile/issues/151 even more since our helmfile will just continue to grow. =)

[mumoshu]

@osterman Thanks for your feedback. I understand it 👍

[TheNotary]

It seems like the below works and doesn't require the creation of an intermediate file which makes me feel spectacular :)

helm install --name some-chart . --set-string  dbUser.password=$CASSANDRA_PASS

[mumoshu]

@TheNotary Thanks! Yeah, that's how helm works. But in helmfile's context we'd better use temporary values files via values: config.

The point is that helm's --set, --values, --set-string has no intuitive ordering. The ordering of values being merged is important to make layers of configurations work.

In contrast, --values hence helmfile's values: is merged in the order of definition while avoiding the type issues.