roboll / helmfile

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

helmfile --state-values-set unable to set booleans #1347

Open Arabus opened 4 years ago

Arabus commented 4 years ago

When trying to enable/disable a release from my helmfile via the CLI with --state-values-set foo.enabled=true I get

$ helmfile --selector name=two --state-values-set second.enabled=true  template --skip-deps
in ./helmfile.yaml: error during helmfile.yaml.part.0 parsing: cannot append two different types (string, bool)

Is there a way I'm forgetting or is this not possible at the moment?

Relevant Metadata

Helmfile version

$ helmfile version
helmfile version v0.120.0

Helm version & Plugins

$ helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"dirty", GoVersion:"go1.14.3"}

$ helm plugin list
NAME            VERSION DESCRIPTION
diff            3.1.1   Preview helm upgrade changes as a diff
helm-git        0.7.0   Get non-packaged Charts directly from Git.
inject          0.1.8   inject plugin for Helm
kubeval         0.13.0  "Validate Helm charts against the Kubernetes schemas"
x               0.8.1   Turn Kubernetes manifests and Kustomization into Helm Release

go version

$ go version
go version go1.14.4 darwin/amd64

mac os version

$ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.14.6
turekp commented 3 years ago

Same issue with v.139.9 due to (probably) auto-wrapping of booleans and integers into quotes when using --state-values-set. Makes the "condition" feature unusable from CLI.

Arabus commented 2 years ago

I assume the problem happens here https://github.com/roboll/helmfile/blob/77e6268bcb38ca68deb75f1bc8b4ca258fb8473d/main.go#L715 Because this is a StringSlice all of its members are automagically converted to strings. It might be good to add a type converter here that converts the value part to its corresponding type unless it was quoted. We only need a bool converter for this ticket but I guess one for numbers might also not hurt. Alternativley autoconvert strings to the type required by the state-value

mumoshu commented 2 years ago

@Arabus Hey! Thanks for reporting. Implementing auto-converters would definitely help.

FYI, I thought Helm had the similar mechanisms for typing values passed via --set (and --set-string in case you want to disable the autoconverion so that it forces the value to be a string). Helm's implementation can be a good reference when implementing this in Helmfile.