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

fix: Fix Helm chart template logic #6

Merged
merged 1 commit into from
May 1, 2024

Conversation

juanjjaramillo
Copy link
Contributor

Description

While working in the creation of the Docker registry, I found an issue blocking the operator from being installed.

After triaging, I found the issue is that the Helm chart only works for the release name specified in the docs. This is a critical flaw, since the customer controls the release name, not us.

Specifically, changing the release name broke the logic for the creation and usage of the service account, which impacted all aspects of the chart: deployment of pods, cluster roles, RBAC rules. Additionally, the certificate hard-coded the secret name, while other entities used the templatized name.

The challenge for debugging the issue is that the Helm chart is a hack that used a template with unused variables that were not removed, masking the root cause and leading to multiple red herrings.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • New feature / enhancement (non-breaking change which adds functionality)
  • Security fix
  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Documentation has been updated
  • This change requires changes in testing:
    • unit tests
    • E2E tests

@juanjjaramillo juanjjaramillo marked this pull request as ready for review May 1, 2024 18:37
@juanjjaramillo juanjjaramillo requested a review from a team May 1, 2024 18:37
@juanjjaramillo juanjjaramillo merged commit ca492e8 into main May 1, 2024
4 checks passed
@juanjjaramillo juanjjaramillo deleted the juanjjaramillo/fix_helm_chart branch May 1, 2024 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants