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(dataplane): move DataPlane ports validation to ValidationAdmissionPolicy and ValidationAdmissionPolicyBinding #1007

Merged
merged 6 commits into from
Jan 17, 2025

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Jan 13, 2025

What this PR does / why we need it:

Which issue this PR fixes

Part of #949.

This will get an accompanying charts PR where ValidationAdmissionPolicy and ValidationAdmissionPolicyBinding` are added there.

This all sets the max items on the DataPlaneServicePort array to 4 to potentially cut the added validations cost down.

An initial attempt on this was to add CEL rules on the CRD but API server was complaining about the cost of these. With ValidationAdmissionPolicy rules it was possible.

NOTE: This will require k8s 1.30 as ValidatingAdmissionPolicy graduated to stable in 1.30 but users will only get this deployed in the helm chart when .Capabilities.APIVersions.Has check will pass, ref Kong/charts#1215.

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 significant changes

@pmalek pmalek added this to the KGO v1.5.x milestone Jan 13, 2025
@pmalek pmalek self-assigned this Jan 13, 2025
@pmalek pmalek force-pushed the dataplane-ports-validation-migrate-to-validation-policy branch 3 times, most recently from 4f5da6f to 22cffd6 Compare January 15, 2025 14:07
@pmalek pmalek marked this pull request as ready for review January 15, 2025 14:10
@pmalek pmalek requested a review from a team as a code owner January 15, 2025 14:10
czeslavo
czeslavo previously approved these changes Jan 16, 2025
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

Given that we plan on making this repo a source of truth for ValidationAdmissionPolicies, should we add tests verifying them in here?

@pmalek pmalek force-pushed the dataplane-ports-validation-migrate-to-validation-policy branch 2 times, most recently from e112d0a to b03db6a Compare January 16, 2025 17:17
@pmalek
Copy link
Member Author

pmalek commented Jan 16, 2025

Given that we plan on making this repo a source of truth for ValidationAdmissionPolicies, should we add tests verifying them in here?

I agree, as agreed on slack I added tests by extending the crd validation tests in b03db6a

PTAL.

@pmalek
Copy link
Member Author

pmalek commented Jan 17, 2025

@czeslavo This is ready for review now. Feel free to take a look and comment.

@pmalek pmalek force-pushed the dataplane-ports-validation-migrate-to-validation-policy branch from 8afcb73 to 4ffde37 Compare January 17, 2025 10:45
@pmalek pmalek requested a review from czeslavo January 17, 2025 10:46
@pmalek pmalek force-pushed the dataplane-ports-validation-migrate-to-validation-policy branch 5 times, most recently from 41ebe42 to 886290f Compare January 17, 2025 13:23
@pmalek pmalek force-pushed the dataplane-ports-validation-migrate-to-validation-policy branch from 341246f to 8c3adfb Compare January 17, 2025 15:31
@pmalek pmalek requested a review from programmer04 January 17, 2025 15:38
@pmalek pmalek enabled auto-merge (squash) January 17, 2025 15:41
@pmalek pmalek merged commit dfc12c8 into main Jan 17, 2025
26 checks passed
@pmalek pmalek deleted the dataplane-ports-validation-migrate-to-validation-policy branch January 17, 2025 15:45
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.

3 participants