Support specifying custom values files #899
Conversation
Helm has code for this here: https://github.com/helm/helm/blob/master/cmd/helm/install.go#L358 It would probably be a better approach to try move their code for it to k8s.io/helm/pkg/chartutil, and then use that here. For now I just copied out the necessary bits, because their version supports things like custom protocols, like values files from ftp or http, which I don't think we need. Though matching what helm supports is probably a good idea. TODO: I still need to add tests for this. Closes https://github.com/Azure/draft/issues/818 Closes https://github.com/Azure/draft/issues/856 Signed-off-by: William Stewart <zoidbergwill@gmail.com> Signed-off-by: William Stewart <zoidbergwill@gmail.com>
|
|
| @@ -250,14 +251,67 @@ func loadArchive(ctx *Context) (err error) { | |||
| return nil | |||
| } | |||
|
|
|||
| // Merges source and destination map, preferring values from the source map | |||
| func mergeValues(dest map[string]interface{}, src map[string]interface{}) map[string]interface{} { | |||
There was a problem hiding this comment.
cc @bacongobbler, @michelleN
Are there any other projects you are aware of that re-implement the merging of value files from Helm?
Would it make sense to expose this in the Helm pkg?
There was a problem hiding this comment.
Google's skaffold, for example, just calls helm binary directly and passes all valuesFiles as arguments to it, i.e. it doesn't deal with merging them at all — but even if this is the first occurrence, I think it makes sense to expose this in the Helm.
There was a problem hiding this comment.
Should I make a PR for helm? Would chartutil be the right package for it?
There was a problem hiding this comment.
I'm pretty sure this already exists in pkg/chartutil called MergeInto: https://github.com/helm/helm/blob/403af2c3895e4fa0e04713d94135bf3438d9ce9c/pkg/chartutil/values.go#L97-L111
Probably would be good to use that instead
There was a problem hiding this comment.
There was a problem hiding this comment.
From what I could see, parts where supported by chartutil already, but not other parts, will take a look at refining it, whether it's in Helm or here.
Related to https://github.com/Azure/draft/pull/899 Signed-off-by: William Stewart <zoidbergwill@gmail.com>
|
Any word when, or if, this will be merged? This is a must-have feature for my particular use case. |
|
Is there any way I can help get this pull request back alive? I can contribute to the code if necessary, but like @cws-credsimple, this is very important for my use case |
Helm has code for this here: https://github.com/helm/helm/blob/master/cmd/helm/install.go#L358
It would probably be a better approach to try move their code for it to
k8s.io/helm/pkg/chartutil, and then use that here. For now I just copied out
the necessary bits, because their version supports things like custom protocols,
like values files from ftp or http, which I don't think we need. Though
matching what helm supports is probably a better idea.
TODO: I still need to add tests for this.
Closes https://github.com/Azure/draft/issues/818
Closes https://github.com/Azure/draft/issues/856