Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New clusterRoleName helper seems to be causing a template error #180

Closed
nikki-quant opened this issue Oct 28, 2024 · 4 comments
Closed

New clusterRoleName helper seems to be causing a template error #180

nikki-quant opened this issue Oct 28, 2024 · 4 comments

Comments

@nikki-quant
Copy link
Contributor

Hi folks, thanks for maintaining CoreDNS!

I'm looking to update from version 1.30.0 to 1.36.0 of the CoreDNS chart.

When I run helm template against the new version without making any changes to values files I get the following error:

		STDERR:
		  Error: template: coredns/templates/_helpers.tpl:233:38: executing "coredns.clusterRoleName" at <.Values.clusterRole.nameOverride>: nil pointer evaluating interface {}.nameOverride
		  Use --debug flag to render out invalid YAML

This looks to be caused by the following snippet in the helper templates for the chart:

{{/*
Create the name of the service account to use
*/}}
{{- define "coredns.clusterRoleName" -}}
{{- if and .Values.clusterRole .Values.clusterRole.nameOverride -}}
    {{ .Values.clusterRole.nameOverride }}
{{- else -}}
    {{ template "coredns.fullname" . }}
{{- end -}}
{{- end -}}

Without the clusterRole key set in the values, Helm errors when trying to evaluate whether .Values.clusterRole.nameOverride is defined, because it is trying to access a child property of a parent which does not exist. As far as I understand, this is the expected behaviour in Helm and their recommendation is to prefer using relatively flat variable names: helm/helm#8026 (comment))

Adding an empty clusterRole: {} key in a values file lets me template the chart successfully.

As best I can tell, clusterRole was currently only used as a parent for the new clusterRole.nameOverride. Should it be added to the chart default values as an empty dictionary to avoid running into this templating error? ie:

# /charts/coredns/values.yaml

clusterRole: {}

Thanks,
Nikki

@hagaibarel
Copy link
Collaborator

Hi @nikki-quant , thanks for raising this.

I think you're correct and the lack of a default value causes a nil pointer exception. The right value should be an empty string ("") as we expect a string vaule there not object.

I would be happy to review a PR with the fix

@nikki-quant
Copy link
Contributor Author

Thank for taking a look - I'll put a PR in :)

@nikki-quant
Copy link
Contributor Author

Hi @hagaibarel , I've put in a pull request. Very happy to make updates if that doesn't look right, or if there's styleguide type things you'd prefer done differently for the changelog or comments.

@hagaibarel
Copy link
Collaborator

Fixed in #181

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants