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: add CRD validation expression for KongPlugin and KongClusterPlugin config and configFrom fields #5119

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Nov 8, 2023

What this PR does / why we need it:

This PR adds CRD validation expression to KongPlugin and KongClusterPlugin to enforce only one of config and configFrom fields to be set using the CRD validation expressions.

Which issue this PR fixes:

Related to #5062 (just a small part of it).

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@pmalek pmalek added the area/CRD Changes in existing CRDs or introduction of new ones label Nov 8, 2023
@pmalek pmalek added this to the KIC v3.1.0 milestone Nov 8, 2023
@pmalek pmalek self-assigned this Nov 8, 2023
@pmalek pmalek force-pushed the crd-validation-expressions-plugins-can-only-set-config-or-configFrom branch from 0030a65 to 51f13ca Compare November 8, 2023 11:33
@pull-request-size pull-request-size bot added size/S and removed size/XS labels Nov 8, 2023
@pmalek pmalek changed the title feat: add CRD validation expression for KongPlugin and KongClusterPlugin config and configFrom fields feat: add CRD validation expression for KongPlugin and KongClusterPlugin config and configFrom fields Nov 8, 2023
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8d7419a) 75.7% compared to head (c4a266e) 77.8%.
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #5119     +/-   ##
=======================================
+ Coverage   75.7%   77.8%   +2.1%     
=======================================
  Files        167     167             
  Lines      18911   18908      -3     
=======================================
+ Hits       14320   14718    +398     
+ Misses      3770    3359    -411     
- Partials     821     831     +10     
Files Coverage Δ
internal/admission/validator.go 72.5% <ø> (-0.3%) ⬇️

... and 17 files with indirect coverage changes

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

@pmalek pmalek force-pushed the crd-validation-expressions-plugins-can-only-set-config-or-configFrom branch from 51f13ca to e3fcc28 Compare November 8, 2023 12:18
@pull-request-size pull-request-size bot added size/L and removed size/S labels Nov 8, 2023
@team-k8s-bot team-k8s-bot marked this pull request as ready for review November 8, 2023 13:43
@team-k8s-bot team-k8s-bot requested a review from a team as a code owner November 8, 2023 13:43
internal/admission/validator.go Outdated Show resolved Hide resolved
pkg/apis/configuration/v1/kongclusterplugin_types.go Outdated Show resolved Hide resolved
@pmalek pmalek requested a review from czeslavo November 8, 2023 16:51
czeslavo
czeslavo previously approved these changes Nov 8, 2023
Copy link
Contributor

@pangruoran pangruoran left a comment

Choose a reason for hiding this comment

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

LGTM

This marks a fresh beginning for CEL in KIC.

@pmalek pmalek requested a review from czeslavo November 9, 2023 09:40
@pmalek pmalek enabled auto-merge (squash) November 9, 2023 09:40
@pmalek pmalek merged commit 314951c into main Nov 9, 2023
35 checks passed
@pmalek pmalek deleted the crd-validation-expressions-plugins-can-only-set-config-or-configFrom branch November 9, 2023 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CRD Changes in existing CRDs or introduction of new ones size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move CRD validation that's possible to be done in CEL, from webhook to CRD Validation Expressions
3 participants