-
Notifications
You must be signed in to change notification settings - Fork 98
Description
Hi there,
I'm part of a team that maintains a derived chart using this chart as a direct dependency: https://github.com/redhat-developer/rhdh-chart/tree/main/charts/backstage
In our chart, we have included some default values in the extra* fields, like some env vars that we consider as defaults. See https://github.com/redhat-developer/rhdh-chart/blob/197a1a4f4af94c0197eb13caf13f2800b728280b/charts/backstage/values.yaml#L126-L136
The problem is that, if users of our chart want to customize this in their own values.yaml files, for example by adding some additional env vars, they need to replicate all the default ones that we have, besides adding their own.
This creates a lot of confusion for users of our chart, as they would assume (rightfully) that the extra* fields meant adding extra stuff, not replicating the defaults.
I am aware that this could be a technical limitation in how Helm itself handles arrays, as reported in helm/helm#3767 or helm/helm#3486.
But I wanted to propose/discuss a possible approach for supporting this here, in a backward-compatible way.
Proposal
Since Helm natively supports merging maps, I was thinking of changing the schema to make all the extra* fields either a map or an array (using the oneOf JSON Schema specification). And then update the templates such that they can check the field type and :
- if it is a map, make sure to use the value in the logic
- if the type is a slice, proceed as currently. So this should have no impact whatsoever on the current behavior.
I've put together a draft POC PR (#270) to illustrate this proposal using the simple extraEnvVars field. See https://github.com/backstage/charts/pull/270/files#diff-3484be37a646b3db74d7bd80ecd11ae378e6a2626f939ceda79aba27c4eab8d2 for an example of how I'm planning to handle that in the templates.
And this branch in our derived chart illustrates how we would define these extra env vars in the default values.yaml file, so that users can easily provide their own values.yaml extending these env vars.
Alternatives
Here are a few other alternatives I've been thinking about to fix this.
Additional extra*Derived fields
The idea here is that for each extra* field, we add a new extra*Derived field (better naming to find out). For example, having extraEnvVars and extraEnvVarsDerived, where extraEnvVars would remain empty, even in the derived chart.
So the derived chart, like ours, would add their default env vars into extraEnvVarsDerived.
This way, it won't change anything for the user: they would continue to fill in the extraEnvVars field.
Maybe this is simpler to manage here? It would be up to the derived charts to keep the extra* fields empty from the user standpoint.
If this makes sense to you, I can go ahead and try to update all the extra* fields accordingly. Or if you have a better suggestion for solving this, I would be more than happy to collaborate to implement it.
Please do let me know your thoughts about this.
Thanks.