-
Notifications
You must be signed in to change notification settings - Fork 26
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
Make the admission webhook run #3
Conversation
Scheme: Scheme, | ||
ParameterCodec: metav1.ParameterCodec, | ||
NegotiatedSerializer: Codecs, | ||
SubresourceGroupVersionKind: map[string]schema.GroupVersionKind{ |
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.
@smarterclayton this tricks the REST handler code into decoding the object we want, but it is a side-effect, not intent. I think we should officially allow the intent.
@@ -31,6 +31,7 @@ func NewNamespaceReservationServerOptions(out, errOut io.Writer) *NamespaceReser | |||
StdOut: out, | |||
StdErr: errOut, | |||
} | |||
o.RecommendedOptions.Etcd = nil |
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.
Thanks @sttts
} | ||
|
||
admittingObjectName := &NamedThing{} | ||
err := json.Unmarshal(admissionReview.Spec.Object.Raw, admittingObjectName) |
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.
Can't use a normal metadata path because the admission webhook is broken.
return nil, errors.NewBadRequest(err.Error()) | ||
} | ||
|
||
if len(admittingObjectName.Name) == 0 { |
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.
fail closed on empty name. might be generated later, but this fails anyway.
return admissionReview, nil | ||
} | ||
|
||
if admittingObjectName.Name == "fail-me" { |
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.
proof that it works. To be updated later
@dgoodwin let's get this closed off so I can tidy and plumb a client config in a smaller pull. We'll finally be to the meat. |
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.
Assuming todo's and tests to be addressed later this looks ok to me so far.
Automatic merge from submit-queue (batch tested with PRs 16861, 16438). make webhook admission kind of work This is done to support openshift/kubernetes-namespace-reservation#3 . It contains multiple fixes needed to make webhooks run at all. In addition, it changes validation rules and admission handling until we get changes like kubernetes/kubernetes#53826 into the API. This affects handling and compatibility of an alpha feature. @dgoodwin @abhgupta be ready for upgrade pain in this area. @bparees I turned this on in cluster-up. Surgery was relatively minor
This bumps the vendored level of kube for some changes we need, then updates the templates to wire the admission hook (note that this requires patches apiserver-side), and sets up the apiserver to accept our input.