roboll / helmfile

Deploy Kubernetes Helm Charts
MIT License
4.05k stars 565 forks source link

feat: Support YAML dotfile notation for helmfile #936

Open themanifold opened 5 years ago

themanifold commented 5 years ago

Looks like the yaml package you use (https://godoc.org/gopkg.in/yaml.v2) doesn't explicitly support dot file notation in your YAML. What I mean is:

a.b.c: "foo"

Instead of

a:
  b:
    c: "foo"

In some cases, when I want to access a deeply nested YAML structure, this would be great. It's also a shortcut supported in helm itself, so we should probably support it too.

As a small aside, it looks like helm uses sigs.k8s.io/yaml - we could just leverage that right? That supports dotfile notation.

mumoshu commented 5 years ago

@themanifold Hey! This sounds good. The only concern is that - how can you include dot in the key name if that's what you want?

In other words, does sigs.k8s.io/yaml allows you to differentiate a.b.c: foo for {"a":{"b":{"c":"foo"}}} and {"a.b.c": "foo"}?

themanifold commented 5 years ago

No, I think you need to escape the dots if you want them to actually be part of a key.

This is an example: https://stackoverflow.com/questions/49522776/helm-how-to-override-value-with-periods-in-name

mumoshu commented 5 years ago

@themanifold Thanks!

Okay, that makes sense for me, but it might be a breaking change for existing users who relies on that we don't need escaping dots in the key today.

Is there anyone relies it?

@osterman @gtaylor @sstarcher @bitsofinfo Does any of your helmfile modules use a.b.c-styled keys in values?

osterman commented 5 years ago

Dots in keys is very common for Kubernetes annotations. We use them all over the place. E.g. Kiam, external dns, cert-manager, nginx-ingress elb settings, etc. annotations are passed in the values.

I think this optimization to Helmfile will cause more problems than it’s worth, unless there is backwards compatibility somehow.

mumoshu commented 5 years ago

@osterman Thanks! That's a great point.

mumoshu commented 5 years ago

@themanifold To make this backward-compatible, we're likely to need another configuration field than values: that supports it. But honestly I'm not sure if that's worth the complexity gain.

Alternatively, can we consider this as a part of #869? That is, there's another config language that supports a shorthand syntax for declaring a deeply nested yaml dict. In CUE specifically, a b c: "foo" works.

I won't say Helmfile is committed to support e.g. CUE, but the more use-cases one config language gets(jsonnet, lua, cue) the more likely it is being integrated into helmfile.

themanifold commented 5 years ago

@mumoshu I think supporting it in CUE is fine, but whatever you decide, we should make it clear. Because this project dealt with helm charts, I was under the false impression that it would support similar features to helm charts.

So just a note to say we don't support the dot file syntax is fine (and easier).

bitsofinfo commented 5 years ago

I sometimes use literal keys w/ dots (i.e. think k8s labels are common usage of this) "some.label.name": "label-value" but I do have cases where i needed to set (via --set-values) a representation of a.b.c on the cmd line where c is a child of b is a child of a etc; but I don't reference it that way (i.e. flattened a.b.c: value in any YAML file directly, its always declared in a nested/indented child fashion.

recall: https://github.com/roboll/helmfile/issues/773

norman-zon commented 2 years ago

Has anything happened regarding this proposal?

mumoshu commented 2 years ago

@norman-zon Nothing. I believe this would add more problems than benefits.

mumoshu commented 2 years ago

BTW- Does Helm support this dots-in-yaml-dict-keys-as-separators? Isn't it just that Helm supports dotted notations in --set?