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-chart: controller-manager wh name flags #2879

Merged
merged 3 commits into from
Jul 28, 2023

Conversation

ugur99
Copy link
Contributor

@ugur99 ugur99 commented Jul 15, 2023

Signed-off-by: Ugur Can Ozturk ugurozturk918@gmail.com

What this PR does / why we need it:
This PR fixes missing flags problems of the gatekeeper-controller-manager

Which issue(s) this PR fixes
Fixes #2878

Signed-off-by: Ugur <ugurozturk918@gmail.com>
@ugur99 ugur99 changed the title fix(helm-chart) controller-manager flags fix: helm-chart controller-manager flags Jul 15, 2023
@sozercan
Copy link
Member

@ugur99 thanks for the PR! Gatekeeper helm chart is autogenerated and these changes will be clobbered in the next release. Please refer to Contributing to Helm Chart for modifying the Helm chart.

Signed-off-by: Ugur Ozturk <ugurozturk918@gmail.com>
@ugur99
Copy link
Contributor Author

ugur99 commented Jul 17, 2023

@ugur99 thanks for the PR! Gatekeeper helm chart is autogenerated and these changes will be clobbered in the next release. Please refer to Contributing to Helm Chart for modifying the Helm chart.

Thank you for the quick reply @sozercan! I hope it's now fixed and compatible with the contrib. document 🤞

@acpana acpana changed the title fix: helm-chart controller-manager flags fix: helm-chart: controller-manager wh name flags Jul 19, 2023
Copy link
Contributor

@acpana acpana left a comment

Choose a reason for hiding this comment

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

Someone correct me if I am wrong but I think we should also update the name of the mutating webhook in the rbac role.

cmd/build/helmify/kustomize-for-helm.yaml Show resolved Hide resolved
@ugur99 ugur99 requested a review from acpana July 23, 2023 23:21
Copy link
Contributor

@acpana acpana left a comment

Choose a reason for hiding this comment

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

there's already a template for the cluster role parametrization.

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04% ⚠️

Comparison is base (ea842cd) 53.13% compared to head (8fb7294) 53.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2879      +/-   ##
==========================================
- Coverage   53.13%   53.10%   -0.04%     
==========================================
  Files         135      135              
  Lines       11790    11790              
==========================================
- Hits         6265     6261       -4     
- Misses       5041     5044       +3     
- Partials      484      485       +1     
Flag Coverage Δ
unittests 53.10% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ritazh ritazh left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for the fix! 🎉

@ritazh ritazh merged commit 33efb28 into open-policy-agent:master Jul 28, 2023
16 checks passed
@ugur99
Copy link
Contributor Author

ugur99 commented Sep 20, 2023

Hello team! I think this fix has not been released yet, do you have any ideas when or in which version it will be released?

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

Successfully merging this pull request may close these issues.

opa-gatekeeper can not work properly with different mutatingWebhookName
6 participants