ZEP-0015: Helm Value Style Variable Interfaces #16
Conversation
Racer159
commented
Feb 3, 2025
Racer159
commented
- One-line PR description: Adds a proposal for Zarf Variables to be handled more like Helm Values.
- Issue link: Helm Values Style Variable Interfaces #15
- Other comments:
Signed-off-by: Wayne Starr <[email protected]>
Racer159
force-pushed
the
0015-helm-value-style-variable-interfaces
branch
from
a871ff9 to
3ee8cfd
Compare
Signed-off-by: Wayne Starr <[email protected]>
|
|
||
| ### Risks and Mitigations | ||
|
|
||
| This will require some additional processing of the `zarf-config` files to allow them to be processed properly because of Viper's long-running [case-insensitive keys issue](https://github.com/spf13/viper/issues/1014). This could be performed similar to UDS CLI's implementation of this feature and / or by deprecating some of the existing less-used formats for configuration. |
There was a problem hiding this comment.
Yeah I'm not a fan of having four different formats. I doubt many are using .ini or .json formats. IMO it would be best to deprecate at least these two.
I think the argument can be made to keep only one format around. However, yaml and toml both have their use cases. toml is nice since Zarf tends to use hierarchies a lot (e.g. [zarf.package.create]), yaml is more intuitive when defining variables for Helm values files.
|
|
||
| This will require some additional processing of the `zarf-config` files to allow them to be processed properly because of Viper's long-running [case-insensitive keys issue](https://github.com/spf13/viper/issues/1014). This could be performed similar to UDS CLI's implementation of this feature and / or by deprecating some of the existing less-used formats for configuration. | ||
|
|
||
| The `--set` syntax will change somewhat how variables are interpreted on the CLI (i.e. `--set VAR=100` will no monger represent `"100"` and instead will just be `100` internally). For `###` templates this can be mitigated by simply representing the value as a string for backwards compatibility though this will likely need to either be marked as breaking for Helm overrides or have additional flag changes / opt ins for this specific feature. |
There was a problem hiding this comment.
I would argue the existing behavior of Helm overrides is not ideal and should just be changed. It helps that the feature is in "alpha" (though the Zarf team really needs to better track the state of our features). This comment gives a good example of the unexpected behavior when everything is converted to a string zarf-dev/zarf#2783 (comment)
There was a problem hiding this comment.
I am more worried about the risk of a breaking change on the --set flag, but I would still lean towards introducing this as a breaking change and avoiding the overhead of command line flags. Do you know of any situations in Helm charts where a user is likely to want their integer (or other type) represented as a string?
Co-authored-by: Austin Abro <[email protected]>
