[Helm]: Duplication of common chart template causes conflicts

[krtk6160]

Describe the bug
The apollo router chart overrides the common.tplvalues.render template function from the bitnami common chart, instead of using the common chart as a dependency. This causes a problem when the router chart is a dependency of a chart that also has an indirect dependency to the common chart.

To explain with an example:
if chart A has router and chart B as its dependencies, and chart B in turn has the common chart as its dependency, then the template definition from the router chart will be used when chart B makes a call to the template function. This won't be a problem if the definitions are identical, but there have been a few changes to the common chart that changed the definition. This caused one of our charts, which is in the same situation as chart A in the example above, to break.

To Reproduce
Steps to reproduce the behavior:

1. Create a chart A that has router:1.25.0 and bitnami/redis:18.1.2 as its dependencies.
2. Run helm template on chart A.
3. See error.

Expected behavior
The router chart shouldn't override anything from the common chart, especially since the common chart is widely used as a dependency for numerous charts in the helm community. It should instead use the common chart as a dependency.

Output

Error: template: galoy/charts/redis/templates/sentinel/statefulset.yaml:13:14: executing 

"galoy/charts/redis/templates/sentinel/statefulset.yaml" at <include "common.labels.standard" (dict "customLabels" 

.Values.commonLabels "context" $)>: error calling include: template: 

galoy/charts/mongodb/charts/common/templates/_tplvalues.tpl:35:11: executing "common.tplvalues.merge" at <include 

"common.tplvalues.render" (dict "value" . "context" $.context "scope" $.scope)>: error calling include: template: 

galoy/charts/router/templates/_helpers.tpl:91:12: executing "common.tplvalues.render" at <tpl (.value | toYaml) .context>: 

error calling tpl: cannot retrieve Template.Basepath from values inside tpl function: {}: "BasePath" is not a value

Additional context
You can see the error in one of our github actions.

[garypen]

I don't think the way you have described the problem is quite correct.

In the router chart _helpers.tpl there is a comment which gives credit to bitnami for the approach used to solve a templating problem. Nothing from bitnami is included or overridden by the router chart. I think the issue here is actually caused by a name clash.

Perhaps that could be resolved by renaming:

common.tplvalues.render

to

apollo.tplvalues.router

in the router templates: _helpers.tpl.

@krtk6160 What do you think?

[krtk6160]

I don't think the way you have described the problem is quite correct.

In the router chart _helpers.tpl there is a comment which gives credit to bitnami for the approach used to solve a templating problem. Nothing from bitnami is included or overridden by the router chart. I think the issue here is actually caused by a name clash.

Yes, what I meant was that because the template function (or part of it) was copied from the common chart without name change, the result is effectively an override, albeit unintentional. Helm gives precedence to the definition in the apollo chart for the common.tplvalues.render function.

Perhaps that could be resolved by renaming:

common.tplvalues.render

to

apollo.tplvalues.router

in the router templates: _helpers.tpl.

@krtk6160 What do you think?

Yes, I tried that change locally and it does fix the problem.