-
Notifications
You must be signed in to change notification settings - Fork 437
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
Add kubebuilder validations in proto for extauth AuthConfig #9481
Conversation
...thub.com/solo-io/gloo/projects/gloo/api/v1/enterprise/options/extauth/v1/extauth.proto.sk.md
Outdated
Show resolved
Hide resolved
…onfig state" This reverts commit c128fa8.
…g.configs-maxLength
Issues linked to changelog: |
I've brought this change to the attention of @DuncanDoyle @kcbabo and @sam-heilbron, we have their thumbs up on the change and we can merge this PR right after the 1.17 branch is cut. cc: @arianaw66 |
…/ protoc-gen-openapi
…g.configs-maxLength
…g.configs-maxLength
It appears the bump to solo-kit v0.35.1 breaks the build-bot tests |
...thub.com/solo-io/gloo/projects/gloo/api/v1/enterprise/options/extauth/v1/extauth.proto.sk.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving for now. There's some followup doc work I'd like to do, which will include 1) translating rules into user-friendly comments, and 2) figuring out some way to update solo-kit so that the cel rules themselves dont build into the published docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Approved"
- Checked the rules against the
CheckIfInvalidAuthConfig
function - Pulled changes into
solo-projects
and manually ran the in memory and kubernetes e2e ext-auth tests - Ran ./projects/gloo/pkg/syncer/extauth validation unit tests
Description
This PR adds kubebuilder markers and CEL validation rules to the AuthConfig CRD.
These are unit tested thoroughly in https://github.com/solo-io/gloo-mesh-enterprise/pull/16951.
The constraints are notated here, primarily in the first two commits, which come from the solo-projects validation code and are therefore guaranteed to be constraints that apply when using Edge.
API changes
No API fields have been changed, but this adds pre-admission validation which can affect customers' pipelines. The changes proposed here should only affect AuthConfig CRs which are not currently ACCEPTED by our translation.
Docs changes
The docs will unfortunately be updated with the kubebuilder markers, as there's no way to hide the additional proto comments from the docs.
The docs team will also add user-facing constraint descriptions according to the notes here.
Context
This is motivation by the validation milestone in progress for Gloo Platform.
Interesting decisions
Further discussion of the relevant factors and decisions can be found on the ExtAuthPolicy validation issue that is a part of that milestone.
Testing steps
See https://github.com/solo-io/gloo-mesh-enterprise/pull/16951 for testing steps
Checklist: