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

feat: use admission review v1 #5464

Merged
merged 15 commits into from
Nov 30, 2022
Merged

feat: use admission review v1 #5464

merged 15 commits into from
Nov 30, 2022

Conversation

eddycharly
Copy link
Member

@eddycharly eddycharly commented Nov 23, 2022

Explanation

This PR uses admission review v1 instead of v1beta1.
v1 requires:

  • uid in the response is the same as in the request
  • patchType must be set when returning patches

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #5464 (d0aca66) into main (1ea4a0d) will increase coverage by 0.01%.
The diff coverage is 32.60%.

@@            Coverage Diff             @@
##             main    #5464      +/-   ##
==========================================
+ Coverage   36.42%   36.44%   +0.01%     
==========================================
  Files         171      171              
  Lines       19133    19137       +4     
==========================================
+ Hits         6970     6974       +4     
  Misses      11371    11371              
  Partials      792      792              
Impacted Files Coverage Δ
pkg/controllers/webhook/controller.go 0.00% <0.00%> (ø)
pkg/webhooks/handlers/admission.go 0.00% <0.00%> (ø)
pkg/webhooks/handlers/error.go 0.00% <0.00%> (ø)
pkg/webhooks/handlers/protect.go 0.00% <0.00%> (ø)
pkg/webhooks/handlers/verify.go 0.00% <0.00%> (ø)
pkg/webhooks/policy/handlers.go 0.00% <0.00%> (ø)
pkg/webhooks/resource/utils.go 35.18% <0.00%> (ø)
pkg/webhooks/resource/handlers.go 48.88% <55.55%> (ø)
pkg/utils/admission/response.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
@eddycharly eddycharly marked this pull request as ready for review November 24, 2022 00:08
eddycharly and others added 6 commits November 24, 2022 01:16
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Copy link
Contributor

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

Do we need to test the upgrade once to see every thing is seamless ?. Ideally it should work but just in case.

prateekpandey14 and others added 5 commits November 25, 2022 13:24
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@realshuting
Copy link
Member

Do we need to test the upgrade once to see every thing is seamless ?. Ideally it should work but just in case.

Agreed, we need to test the upgrade and make sure it works as expected.

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@eddycharly
Copy link
Member Author

@realshuting I'm trying right now.

@eddycharly
Copy link
Member Author

@realshuting @prateekpandey14 worked like a charm :)

Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

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

/lgtm

@realshuting realshuting merged commit 83bbf87 into kyverno:main Nov 30, 2022
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