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: only set matchConditions on webhook when not empty #3412

Merged
merged 4 commits into from
Jul 16, 2024

Conversation

martijnvdp
Copy link
Contributor

@martijnvdp martijnvdp commented Jun 3, 2024

What this PR does / why we need it:
only set matchConditions on webhooks when the list in values is not empty

empty matchConditions gets ommited by kubernetes and when deploying with argoCD it results in a drift on this property

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #3410

Special notes for your reviewer:

@martijnvdp martijnvdp requested a review from a team as a code owner June 3, 2024 15:45
@martijnvdp martijnvdp closed this Jun 3, 2024
@martijnvdp martijnvdp reopened this Jun 3, 2024
@ritazh
Copy link
Member

ritazh commented Jun 4, 2024

@martijnvdp Thanks for the PR! Gatekeeper helm chart is autogenerated when a release is cut.
Please refer to https://open-policy-agent.github.io/gatekeeper/website/docs/help#contributing-to-helm-chart to make the change in the necessary place and run make manifests. Thank you!

@martijnvdp martijnvdp force-pushed the matchConditions-optional branch 2 times, most recently from 5df5d37 to 6444ba6 Compare June 4, 2024 14:37
@martijnvdp
Copy link
Contributor Author

thanks i have updated the PR

@martijnvdp martijnvdp force-pushed the matchConditions-optional branch 2 times, most recently from 7562c9e to 06d8819 Compare June 4, 2024 14:44
Signed-off-by: martijnvdp <m.vanderploeg@nl.ccv.eu>
@Sir-Jacques
Copy link

Any updates on this?

We are running the ignoreDifferences workaround for now, but would like to get rid of that if possible.

@codecov-commenter
Copy link

codecov-commenter commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.74%. Comparing base (3350319) to head (843bdeb).
Report is 95 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (3350319) and HEAD (843bdeb). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (3350319) HEAD (843bdeb)
unittests 2 1
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3412      +/-   ##
==========================================
- Coverage   54.49%   46.74%   -7.76%     
==========================================
  Files         134      218      +84     
  Lines       12329    14784    +2455     
==========================================
+ Hits         6719     6911     +192     
- Misses       5116     7069    +1953     
- Partials      494      804     +310     
Flag Coverage Δ
unittests 46.74% <ø> (-7.76%) ⬇️

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

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

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

@ritazh ritazh requested a review from a team July 9, 2024 13:54
@ritazh ritazh added this to the v3.17.0 milestone Jul 9, 2024
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

LGTM

@sozercan sozercan requested a review from maxsmythe July 10, 2024 22:32
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

@ritazh ritazh merged commit 0f20484 into open-policy-agent:master Jul 16, 2024
20 checks passed
@JonnieDoe
Copy link

Any updates on this?

We are running the ignoreDifferences workaround for now, but would like to get rid of that if possible.

We are also impacted on 3.16.x

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.

Change chart to only set matchConditions on webhooks when the value parameter is not empty
7 participants