-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
filter out 'turn this on' config structs for admission #16639
filter out 'turn this on' config structs for admission #16639
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
return delegate(config2) | ||
} | ||
|
||
// if it was a DefaultAdmissionConfig, then it must have said "enabled" and it wasn't really meant for the |
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.
why does it say "enabled"? it can be disabled too.
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.
why does it say "enabled"? it can be disabled too.
Not if you get here, right? It will have already run the "ispluginenabled" check by the time this method is called.
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.
@deads2k agree, noticed later.
@deads2k I am fine if you want to go with this PR and if carry is fine. And then you/i could close my PR. |
Ok, I'm going to do that and then focus on eliminating more of our special cases. @aveshagarwal are you interested in reviewing more admission code changes we go through? I'd very happy of someone else being familiar with it. |
Yeah sure. |
Automatic merge from submit-queue (batch tested with PRs 16657, 16607, 16647, 16639, 16655). |
Alternative to #16505 to allow our enablement of config. I think this aligns more closely with a goal of calling the "normal" https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/server/options/admission.go#L78 path.