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(helm): should not set webhook caBundle when cert-manager enabled #4395

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erikgb
Copy link

@erikgb erikgb commented Apr 15, 2024

What problem does this PR solve?

We use FluxCD to install chaos-mesh, and prefer to use cert-manager for all PKI-related in our clusters. When enabling cert-manager in chaos-mesh Helm chart, the templates for the validating and mutating webhook set a dummy value for the webhook caBundle field. This is not correct IMO, and causes Flux and cert-manager to "fight" about managing the field - since both are reconciling the resources periodically.

What's changed and how it works?

This PR removes the dummy value set for webhook caBundle field when cert-manager enabled, which will allow cert-manager (ca-injector) to fully manage this particular field in the resources.

Related changes

  • This change also requires further updates to the website (e.g. docs)
  • This change also requires further updates to the UI interface

Cherry-pick to release branches (optional)

This PR should be cherry-picked to the following release branches:

  • release-2.6
  • release-2.5

Checklist

CHANGELOG

Must include at least one of them.

  • I have updated the CHANGELOG.md
  • I have labeled this PR with "no-need-update-changelog"

Tests

Must include at least one of them.

  • Unit test
  • E2E test
  • Manual test

Side effects

  • Breaking backward compatibility

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

Signed-off-by: Erik Godding Boye <egboye@gmail.com>
@STRRL STRRL self-assigned this Apr 16, 2024
@STRRL STRRL self-requested a review April 16, 2024 14:09
@Gallardot Gallardot self-requested a review April 19, 2024 15:45
Copy link
Member

@Gallardot Gallardot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikgb Thank you for your contribution. LGTM, but please make the CI happy.

@erikgb
Copy link
Author

erikgb commented Apr 20, 2024

Thanks @Gallardot! The failing integration test seems to be unrelated (a timeout). For the changelog check: do you think this change requires a changelog entry or not?

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.

None yet

3 participants