Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set Helm chart Values via full path as string instead of nesting objects. #2852

Open
morgan-cromell opened this issue Feb 28, 2024 · 13 comments
Labels
area/helm kind/enhancement Improvements or new features

Comments

@morgan-cromell
Copy link

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Setting deeply nested values in a helm chart can be very cumbersome in some languages. Needing to define multiple dictionaries to set a deep value. I suggest the option to set the values by their full key instead. So example instead of setting a value like this:

Values = new Dictionary<string, object>
                    {
                        ["kubernetesIngress"] = new Dictionary<string, object>
                        {
                            ["publishedService"] = new Dictionary<string, object>
                            {
                                ["enabled"] = true
                            }
                        }
                    }

You could set it like this

Values = new new Dictionary<string, object>
{
	["kubernetesIngress.publishedService.enabled"] = true
}

This would also better mirror values in the helm documentation.

Affected area/feature

@morgan-cromell morgan-cromell added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Feb 28, 2024
@mjeffryes
Copy link
Member

Thanks for the suggestion @morgan-cromell! I agree that the nested structure for setting values can be cumbersome especially in dotnet. We'll consider adding this for the next revision of Release and Chart

@mjeffryes mjeffryes removed the needs-triage Needs attention from the triage team label Feb 28, 2024
@mmisztal1980
Copy link

@mjeffryes to be completely honest this feature seems to be counter-effective. We should be able to supply lists of values*.yaml files as a primary source of configuration and have the same experience as any person who uses HELM on a daily bases. The existing feature should only be used to set overrides

@morgan-cromell
Copy link
Author

@mmisztal1980 I don't think this feature is counter-effective. Even if you could supply a list of values file and override them you would still need a nested structure to override a nested value. I think the feature to supply values*.yaml file is a good idea but it is a separate feature from this.

@mmisztal1980
Copy link

@morgan-cromell I agree with you that we should have a more convenient way of referencing nested structures, however I'll still stand by my opinion that the primary source of configuration data for HELM charts, should originate from value-files, not from overrides.

@morgan-cromell
Copy link
Author

@mmisztal1980 my main use case is using helm templates from artifact hub and setting values that are connected to outputs from other resources. Dont see how i can do this with value files.

@mmisztal1980
Copy link

mmisztal1980 commented Mar 6, 2024

I'm having a similar use case (we also rely on artifact hub, but we need to place charts in private-network registries), we tend to leave little to chance and prefer to have values files under source control (we've had values changed in upstream before, therefore we want strict control). We know that the chart contains defaults, however we prefer not to rely on them and conciously configure the entire chart in the way we prefer.

I hope that sheds some light.

Regarding your case, it's still perfectly legit to apply overrides onto the values files. This is why helm's CLI supports:

helm upgrade -f values1.yaml -f values.2 yaml --set foo=bar --set nested.foo=otherBar (...)

In the 1st place 😉

@morgan-cromell
Copy link
Author

@mmisztal1980 So what we are saying is that we want both features?

@mmisztal1980
Copy link

Pretty much, in order for the Chart functionality to be "complete"

@morgan-cromell
Copy link
Author

@mmisztal1980 You should probably create a separate issue if you haven't already for value files as it will probably be missed otherwise,

@mmisztal1980
Copy link

will do

@blampe
Copy link
Contributor

blampe commented May 6, 2024

We should be able to supply lists of values*.yaml files as a primary source of configuration and have the same experience as any person who uses HELM on a daily bases.

@morgan-cromell @mmisztal1980 the upcoming Chart v4 resource (#2947) will support valueYamlFiles, please stay tuned!

Regarding the original ask (specifying overrides by full path), our intention with the new resource is to better align it with Helm's upstream behavior. At the moment this means your values are effectively treated the same as if they had been marshaled to YAML and provided via -f values.yaml.

@morgan-cromell
Copy link
Author

@blampe great news being able to use values.yaml files.

Would still like to see setting values with full path. This would properly mirror the - -set option.

@blampe
Copy link
Contributor

blampe commented May 9, 2024

Would still like to see setting values with full path. This would properly mirror the - -set option.

Yep, makes sense. One way to do that could be through a new ValuesSet field, although we'd need to make sure we get the precedence between that and the other values right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

4 participants