[Proposal] Support merging of `extra*` fields in a backward compatible way
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 https://github.com/helm/helm/issues/3767 or https://github.com/helm/helm/issues/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 (https://github.com/backstage/charts/pull/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.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Note that https://github.com/helm/helm/pull/30632 might provide another alternative using Helm, once/if it is merged. In that case, I guess the proposal here won't be needed anymore.