Skip to content

Resolve conflicts in helm templates/values #504

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

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

a-hilaly
Copy link
Member

Fixes: aws-controllers-k8s/community#2014

Prior to this patch, we observed some weird conflicts when installing multiple
ACK helm charts (as sub charts), e.g ack-chart, where the last controller
installation would override values/templates from the previous ones.
This issue stemmed from the asumption that helm "template" names are
unique across installations, which was a miss from the ACK team.

This patch addresses the conflict by prefixing all the helm template
variables with a ack-$SERVICE-controller string.

This solution comes with the cost of adding complexity to the already
complex helm "templates templates generator"... However the least we
could do is to leverage Go template "Funcs" to reduce the complexity of
writing those templates. For example instead of writing something like
{{ "{{ \"define \"ack- }}" }}{{ .ServicePackageName }}{{ "-controller.rbac-rules\"" }},
we could "simply" write a {{ DefineTemplate "rbac-rules" }}

Signed-off-by: Amine Hilaly hilalyamine@gmail.com

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Fixes: aws-controllers-k8s/community#2014

Prior to this patch, we observed conflicts when installing multiple ACK
helm charts (as sub charts), e.g `ack-chart`, where the last controller
installation would override values/templates from the previous ones.
This issue stemmed from the asumption that helm "template" names are
unique across installations, which was a miss by ACK team.

This patch addresses the conflict by prefixing all the helm template
variables with a `ack-$SERVICE-controller` string.

This solution comes with the cost of adding complexity to the already
complex helm "templates templates generator"... However the least we
could do is to leverage Go template "Funcs" to reduce the complexity of
writing those templates. For example instead of writing something like
`{{ "{{ \"define \"ack- }}" }}{{ .ServicePackageName }}{{
"-controller.rbac-rules\"" }}`, we could "simply" write a `{{
DefineTemplate "rbac-rules" }}`

Signed-off-by: Amine Hilaly <hilalyamine@gmail.com>
@ack-prow ack-prow bot requested review from jlbutler and jljaco February 13, 2024 04:57
@ack-prow ack-prow bot added the approved label Feb 13, 2024
@a-hilaly
Copy link
Member Author

/retest

4 similar comments
@a-hilaly
Copy link
Member Author

/retest

@a-hilaly
Copy link
Member Author

/retest

@a-hilaly
Copy link
Member Author

/retest

@a-hilaly
Copy link
Member Author

/retest

@ack-bot
Copy link
Collaborator

ack-bot commented Feb 13, 2024

/lgtm

@ack-prow ack-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 13, 2024
Copy link

ack-prow bot commented Feb 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: a-hilaly, ack-bot

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow bot merged commit 0013c75 into aws-controllers-k8s:main Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Identical Helm named template across different charts causes wrong controller RBAC permissions
2 participants