Adding option to set mh config from CLI, same as helm --set #12
Conversation
|
Thanks, @prabdeb! It looks very cool! Any way we can mirror helm's set semantics, such that Edit: If the above is too difficult, I'll not block merging this. It's good work as-is. |
|
Thanks @josdotso , yes having it merged with "," solves the the issues partially. I have fixed it now as per - https://github.com/kubernetes/helm/blob/daeb458302e95863fc75c31afc05c03f0121012e/cmd/helm/install.go#L365 |
cmd/apply.go
Outdated
| @@ -39,6 +39,13 @@ apps, mh acts on all apps in your mh config.`, | |||
| PrintRendered: viper.GetBool("printRendered"), | |||
| } | |||
|
|
|||
| // Add values passed via CLI using --set | |||
| if viper.GetStringSlice("set") != nil { | |||
| envCLIConfig = lib.MHConfig{ | |||
There was a problem hiding this comment.
You are overwriting envCLIConfig instead of extending it.
There was a problem hiding this comment.
Also, the condition is unnecessary here, GetStringSlice will return an empty slice if no parameters are given on the command line. It will in never return nil.
cmd/simulate.go
Outdated
| @@ -39,6 +39,13 @@ apps, mh acts on all apps in your mh config.`, | |||
| PrintRendered: viper.GetBool("printRendered"), | |||
| } | |||
|
|
|||
| // Add values passed via CLI using --set | |||
| if viper.GetStringSlice("set") != nil { | |||
| envCLIConfig = lib.MHConfig{ | |||
There was a problem hiding this comment.
Also overwriting envCLIConfig here.
mhlib/mhConfig.go
Outdated
| @@ -27,6 +27,7 @@ type MHConfig struct { | |||
| Simulate bool `yaml:"simulate"` | |||
| TargetContext string `yaml:"targetContext"` | |||
| Team string `yaml:"team"` | |||
| CLIValues []string `yaml:"cliValues"` | |||
There was a problem hiding this comment.
Please rename this to something more descriptive, CLIValues gives the impression of this field holding all CLI flags. E.g. SETValues like you did in cmd/.
mhlib/app.go
Outdated
| @@ -228,6 +229,11 @@ func (a *App) render(configFile string) (*string, *string, *[]byte, error) { | |||
| }, | |||
| } | |||
|
|
|||
| // Add config via --set command | |||
| for _, value := range a.MHConfig.CLIValues { | |||
| strvals.ParseInto(value, config) | |||
There was a problem hiding this comment.
Please check for errors here.
|
@simt2 thanks for your review, I have done them, can you please have a look! |
mhlib/mhConfig.go
Outdated
| @@ -27,6 +27,7 @@ type MHConfig struct { | |||
| Simulate bool `yaml:"simulate"` | |||
| TargetContext string `yaml:"targetContext"` | |||
| Team string `yaml:"team"` | |||
| SETValues []string `yaml:"setValues"` | |||
There was a problem hiding this comment.
One more thing @prabdeb , please remove the yaml tag here, I don't think it should be possible to set those values in a MH config file.
|
Done @simt2 |
|
LGTM. Let's wait for @simt2 to approve as well. |
Fixes #11