diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go index 72d2091ecf5..073856a1a31 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go @@ -40,7 +40,7 @@ type webhookConverterFactory struct { } func newWebhookConverterFactory(serviceResolver webhook.ServiceResolver, authResolverWrapper webhook.AuthenticationInfoResolverWrapper) (*webhookConverterFactory, error) { - clientManager, err := webhook.NewClientManager(v1beta1.SchemeGroupVersion, v1beta1.AddToScheme) + clientManager, err := webhook.NewClientManager([]schema.GroupVersion{v1beta1.SchemeGroupVersion}, v1beta1.AddToScheme) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go index e88125ff5b8..51453a35895 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/webhook.go @@ -21,6 +21,7 @@ import ( "fmt" "io" + admissionv1 "k8s.io/api/admission/v1" admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -65,7 +66,14 @@ func NewWebhook(handler *admission.Handler, configFile io.Reader, sourceFactory return nil, err } - cm, err := webhookutil.NewClientManager(admissionv1beta1.SchemeGroupVersion, admissionv1beta1.AddToScheme) + cm, err := webhookutil.NewClientManager( + []schema.GroupVersion{ + admissionv1beta1.SchemeGroupVersion, + admissionv1.SchemeGroupVersion, + }, + admissionv1beta1.AddToScheme, + admissionv1.AddToScheme, + ) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go index 5c5c41aedbd..494c81800ea 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go @@ -27,7 +27,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/klog" - admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admission/v1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -39,7 +39,7 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" - "k8s.io/apiserver/pkg/admission/plugin/webhook/request" + webhookrequest "k8s.io/apiserver/pkg/admission/plugin/webhook/request" "k8s.io/apiserver/pkg/admission/plugin/webhook/util" webhookutil "k8s.io/apiserver/pkg/util/webhook" ) @@ -163,20 +163,16 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta } } - // Currently dispatcher only supports `v1beta1` AdmissionReview - // TODO: Make the dispatcher capable of sending multiple AdmissionReview versions - if !util.HasAdmissionReviewVersion(v1beta1.SchemeGroupVersion.Version, invocation.Webhook) { - return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReview")} + uid, request, response, err := webhookrequest.CreateAdmissionObjects(attr, invocation) + if err != nil { + return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - // Make the webhook request - request := request.CreateAdmissionReview(attr, invocation) client, err := a.cm.HookClient(util.HookClientConfigForWebhook(invocation.Webhook)) if err != nil { return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - response := &admissionv1beta1.AdmissionReview{} - r := client.Post().Context(ctx).Body(&request) + r := client.Post().Context(ctx).Body(request) if h.TimeoutSeconds != nil { r = r.Timeout(time.Duration(*h.TimeoutSeconds) * time.Second) } @@ -184,26 +180,26 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - if response.Response == nil { - return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")} + result, err := webhookrequest.VerifyAdmissionResponse(uid, true, response) + if err != nil { + return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - for k, v := range response.Response.AuditAnnotations { + for k, v := range result.AuditAnnotations { key := h.Name + "/" + k if err := attr.Attributes.AddAnnotation(key, v); err != nil { klog.Warningf("Failed to set admission audit annotation %s to %s for mutating webhook %s: %v", key, v, h.Name, err) } } - if !response.Response.Allowed { - return false, webhookerrors.ToStatusErr(h.Name, response.Response.Result) + if !result.Allowed { + return false, webhookerrors.ToStatusErr(h.Name, result.Result) } - patchJS := response.Response.Patch - if len(patchJS) == 0 { + if len(result.Patch) == 0 { return false, nil } - patchObj, err := jsonpatch.DecodePatch(patchJS) + patchObj, err := jsonpatch.DecodePatch(result.Patch) if err != nil { return false, apierrors.NewInternalError(err) } @@ -216,14 +212,21 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta return false, apierrors.NewInternalError(fmt.Errorf("admission webhook %q attempted to modify the object, which is not supported for this operation", h.Name)) } + var patchedJS []byte jsonSerializer := json.NewSerializer(json.DefaultMetaFactory, o.GetObjectCreater(), o.GetObjectTyper(), false) - objJS, err := runtime.Encode(jsonSerializer, attr.VersionedObject) - if err != nil { - return false, apierrors.NewInternalError(err) - } - patchedJS, err := patchObj.Apply(objJS) - if err != nil { - return false, apierrors.NewInternalError(err) + switch result.PatchType { + // VerifyAdmissionResponse normalizes to v1 patch types, regardless of the AdmissionReview version used + case admissionv1.PatchTypeJSONPatch: + objJS, err := runtime.Encode(jsonSerializer, attr.VersionedObject) + if err != nil { + return false, apierrors.NewInternalError(err) + } + patchedJS, err = patchObj.Apply(objJS) + if err != nil { + return false, apierrors.NewInternalError(err) + } + default: + return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("unsupported patch type %q", result.PatchType)} } var newVersionedObject runtime.Object diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go index 46e2854fb9a..5ea28d446b5 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go @@ -17,16 +17,138 @@ limitations under the License. package request import ( + "fmt" + + admissionv1 "k8s.io/api/admission/v1" admissionv1beta1 "k8s.io/api/admission/v1beta1" authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" ) -// CreateAdmissionReview creates an AdmissionReview for the provided admission.Attributes -func CreateAdmissionReview(versionedAttributes *generic.VersionedAttributes, invocation *generic.WebhookInvocation) admissionv1beta1.AdmissionReview { +// AdmissionResponse contains the fields extracted from an AdmissionReview response +type AdmissionResponse struct { + AuditAnnotations map[string]string + Allowed bool + Patch []byte + PatchType admissionv1.PatchType + Result *metav1.Status +} + +// VerifyAdmissionResponse checks the validity of the provided admission review object, and returns the +// audit annotations, whether the response allowed the request, any provided patch/patchType/status, +// or an error if the provided admission review was not valid. +func VerifyAdmissionResponse(uid types.UID, mutating bool, review runtime.Object) (*AdmissionResponse, error) { + switch r := review.(type) { + case *admissionv1.AdmissionReview: + if r.Response == nil { + return nil, fmt.Errorf("webhook response was absent") + } + + // Verify UID matches + if r.Response.UID != uid { + return nil, fmt.Errorf("expected response.uid=%q, got %q", uid, r.Response.UID) + } + + // Verify GVK + v1GVK := admissionv1.SchemeGroupVersion.WithKind("AdmissionReview") + if r.GroupVersionKind() != v1GVK { + return nil, fmt.Errorf("expected webhook response of %v, got %v", v1GVK.String(), r.GroupVersionKind().String()) + } + + patch := []byte(nil) + patchType := admissionv1.PatchType("") + + if mutating { + // Ensure a mutating webhook provides both patch and patchType together + if len(r.Response.Patch) > 0 && r.Response.PatchType == nil { + return nil, fmt.Errorf("webhook returned response.patch but not response.patchType") + } + if len(r.Response.Patch) == 0 && r.Response.PatchType != nil { + return nil, fmt.Errorf("webhook returned response.patchType but not response.patch") + } + patch = r.Response.Patch + if r.Response.PatchType != nil { + patchType = *r.Response.PatchType + if len(patchType) == 0 { + return nil, fmt.Errorf("webhook returned invalid response.patchType of %q", patchType) + } + } + } else { + // Ensure a validating webhook doesn't return patch or patchType + if len(r.Response.Patch) > 0 { + return nil, fmt.Errorf("validating webhook may not return response.patch") + } + if r.Response.PatchType != nil { + return nil, fmt.Errorf("validating webhook may not return response.patchType") + } + } + + return &AdmissionResponse{ + AuditAnnotations: r.Response.AuditAnnotations, + Allowed: r.Response.Allowed, + Patch: patch, + PatchType: patchType, + Result: r.Response.Result, + }, nil + + case *admissionv1beta1.AdmissionReview: + if r.Response == nil { + return nil, fmt.Errorf("webhook response was absent") + } + + // Response GVK and response.uid were not verified in v1beta1 handling, allow any + + patch := []byte(nil) + patchType := admissionv1.PatchType("") + if mutating { + patch = r.Response.Patch + if len(r.Response.Patch) > 0 { + // patch type was not verified in v1beta1 admissionreview handling. pin to only supported version if a patch is provided. + patchType = admissionv1.PatchTypeJSONPatch + } + } + + return &AdmissionResponse{ + AuditAnnotations: r.Response.AuditAnnotations, + Allowed: r.Response.Allowed, + Patch: patch, + PatchType: patchType, + Result: r.Response.Result, + }, nil + + default: + return nil, fmt.Errorf("unexpected response type %T", review) + } +} + +// CreateAdmissionObjects returns the unique request uid, the AdmissionReview object to send the webhook and to decode the response into, +// or an error if the webhook does not support receiving any of the admission review versions we know to send +func CreateAdmissionObjects(versionedAttributes *generic.VersionedAttributes, invocation *generic.WebhookInvocation) (uid types.UID, request, response runtime.Object, err error) { + for _, version := range invocation.Webhook.GetAdmissionReviewVersions() { + switch version { + case admissionv1.SchemeGroupVersion.Version: + uid := types.UID(uuid.NewUUID()) + request := CreateV1AdmissionReview(uid, versionedAttributes, invocation) + response := &admissionv1.AdmissionReview{} + return uid, request, response, nil + + case admissionv1beta1.SchemeGroupVersion.Version: + uid := types.UID(uuid.NewUUID()) + request := CreateV1beta1AdmissionReview(uid, versionedAttributes, invocation) + response := &admissionv1beta1.AdmissionReview{} + return uid, request, response, nil + + } + } + return "", nil, nil, fmt.Errorf("webhook does not accept known AdmissionReview versions (v1, v1beta1)") +} + +// CreateV1AdmissionReview creates an AdmissionReview for the provided admission.Attributes +func CreateV1AdmissionReview(uid types.UID, versionedAttributes *generic.VersionedAttributes, invocation *generic.WebhookInvocation) *admissionv1.AdmissionReview { attr := versionedAttributes.Attributes gvk := invocation.Kind gvr := invocation.Resource @@ -48,9 +170,75 @@ func CreateAdmissionReview(versionedAttributes *generic.VersionedAttributes, inv userInfo.Extra[key] = authenticationv1.ExtraValue(val) } - return admissionv1beta1.AdmissionReview{ + return &admissionv1.AdmissionReview{ + Request: &admissionv1.AdmissionRequest{ + UID: uid, + Kind: metav1.GroupVersionKind{ + Group: gvk.Group, + Kind: gvk.Kind, + Version: gvk.Version, + }, + Resource: metav1.GroupVersionResource{ + Group: gvr.Group, + Resource: gvr.Resource, + Version: gvr.Version, + }, + SubResource: subresource, + RequestKind: &metav1.GroupVersionKind{ + Group: requestGVK.Group, + Kind: requestGVK.Kind, + Version: requestGVK.Version, + }, + RequestResource: &metav1.GroupVersionResource{ + Group: requestGVR.Group, + Resource: requestGVR.Resource, + Version: requestGVR.Version, + }, + RequestSubResource: requestSubResource, + Name: attr.GetName(), + Namespace: attr.GetNamespace(), + Operation: admissionv1.Operation(attr.GetOperation()), + UserInfo: userInfo, + Object: runtime.RawExtension{ + Object: versionedAttributes.VersionedObject, + }, + OldObject: runtime.RawExtension{ + Object: versionedAttributes.VersionedOldObject, + }, + DryRun: &dryRun, + Options: runtime.RawExtension{ + Object: attr.GetOperationOptions(), + }, + }, + } +} + +// CreateV1beta1AdmissionReview creates an AdmissionReview for the provided admission.Attributes +func CreateV1beta1AdmissionReview(uid types.UID, versionedAttributes *generic.VersionedAttributes, invocation *generic.WebhookInvocation) *admissionv1beta1.AdmissionReview { + attr := versionedAttributes.Attributes + gvk := invocation.Kind + gvr := invocation.Resource + subresource := invocation.Subresource + requestGVK := attr.GetKind() + requestGVR := attr.GetResource() + requestSubResource := attr.GetSubresource() + aUserInfo := attr.GetUserInfo() + userInfo := authenticationv1.UserInfo{ + Extra: make(map[string]authenticationv1.ExtraValue), + Groups: aUserInfo.GetGroups(), + UID: aUserInfo.GetUID(), + Username: aUserInfo.GetName(), + } + dryRun := attr.IsDryRun() + + // Convert the extra information in the user object + for key, val := range aUserInfo.GetExtra() { + userInfo.Extra[key] = authenticationv1.ExtraValue(val) + } + + return &admissionv1beta1.AdmissionReview{ Request: &admissionv1beta1.AdmissionRequest{ - UID: uuid.NewUUID(), + UID: uid, Kind: metav1.GroupVersionKind{ Group: gvk.Group, Kind: gvk.Kind, diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview_test.go new file mode 100644 index 00000000000..0aeaae885ac --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview_test.go @@ -0,0 +1,618 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package request + +import ( + "reflect" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + + admissionv1 "k8s.io/api/admission/v1" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1" + appsv1 "k8s.io/api/apps/v1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/admission/plugin/webhook" + "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" + "k8s.io/apiserver/pkg/authentication/user" + utilpointer "k8s.io/utils/pointer" +) + +func TestVerifyAdmissionResponse(t *testing.T) { + v1beta1JSONPatch := admissionv1beta1.PatchTypeJSONPatch + v1JSONPatch := admissionv1.PatchTypeJSONPatch + + emptyv1beta1Patch := admissionv1beta1.PatchType("") + emptyv1Patch := admissionv1.PatchType("") + + invalidv1beta1Patch := admissionv1beta1.PatchType("Foo") + invalidv1Patch := admissionv1.PatchType("Foo") + + testcases := []struct { + name string + uid types.UID + mutating bool + review runtime.Object + + expectAuditAnnotations map[string]string + expectAllowed bool + expectPatch []byte + expectPatchType admissionv1.PatchType + expectResult *metav1.Status + expectErr string + }{ + // Allowed validating + { + name: "v1beta1 allowed validating", + uid: "123", + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{Allowed: true}, + }, + expectAllowed: true, + }, + { + name: "v1 allowed validating", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{UID: "123", Allowed: true}, + }, + expectAllowed: true, + }, + // Allowed mutating + { + name: "v1beta1 allowed mutating", + uid: "123", + mutating: true, + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{Allowed: true}, + }, + expectAllowed: true, + }, + { + name: "v1 allowed mutating", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{UID: "123", Allowed: true}, + }, + expectAllowed: true, + }, + + // Audit annotations + { + name: "v1beta1 auditAnnotations", + uid: "123", + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: true, + AuditAnnotations: map[string]string{"foo": "bar"}, + }, + }, + expectAllowed: true, + expectAuditAnnotations: map[string]string{"foo": "bar"}, + }, + { + name: "v1 auditAnnotations", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + AuditAnnotations: map[string]string{"foo": "bar"}, + }, + }, + expectAllowed: true, + expectAuditAnnotations: map[string]string{"foo": "bar"}, + }, + + // Patch + { + name: "v1beta1 patch", + uid: "123", + mutating: true, + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + }, + }, + expectAllowed: true, + expectPatch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + expectPatchType: "JSONPatch", + }, + { + name: "v1 patch", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &v1JSONPatch, + }, + }, + expectAllowed: true, + expectPatch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + expectPatchType: "JSONPatch", + }, + + // Result + { + name: "v1beta1 result", + uid: "123", + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{Status: "Failure", Message: "Foo", Code: 401}, + }, + }, + expectAllowed: false, + expectResult: &metav1.Status{Status: "Failure", Message: "Foo", Code: 401}, + }, + { + name: "v1 result", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: false, + Result: &metav1.Status{Status: "Failure", Message: "Foo", Code: 401}, + }, + }, + expectAllowed: false, + expectResult: &metav1.Status{Status: "Failure", Message: "Foo", Code: 401}, + }, + + // Missing response + { + name: "v1beta1 no response", + uid: "123", + review: &admissionv1beta1.AdmissionReview{}, + expectErr: "response was absent", + }, + { + name: "v1 no response", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + }, + expectErr: "response was absent", + }, + + // v1 invalid responses + { + name: "v1 wrong group", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io2/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + }, + }, + expectErr: "expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview", + }, + { + name: "v1 wrong version", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v2", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + }, + }, + expectErr: "expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview", + }, + { + name: "v1 wrong kind", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview2"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + }, + }, + expectErr: "expected webhook response of admission.k8s.io/v1, Kind=AdmissionReview", + }, + { + name: "v1 wrong uid", + uid: "123", + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "1234", + Allowed: true, + }, + }, + expectErr: `expected response.uid="123"`, + }, + { + name: "v1 patch without patch type", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + }, + }, + expectErr: `webhook returned response.patch but not response.patchType`, + }, + { + name: "v1 patch type without patch", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + PatchType: &v1JSONPatch, + }, + }, + expectErr: `webhook returned response.patchType but not response.patch`, + }, + { + name: "v1 empty patch type", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &emptyv1Patch, + }, + }, + expectErr: `webhook returned invalid response.patchType of ""`, + }, + { + name: "v1 invalid patch type", + uid: "123", + mutating: true, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &invalidv1Patch, + }, + }, + expectAllowed: true, + expectPatch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + expectPatchType: invalidv1Patch, // invalid patch types are caught when the mutating dispatcher evaluates the patch + }, + { + name: "v1 patch for validating webhook", + uid: "123", + mutating: false, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + }, + }, + expectErr: `validating webhook may not return response.patch`, + }, + { + name: "v1 patch type for validating webhook", + uid: "123", + mutating: false, + review: &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io/v1", Kind: "AdmissionReview"}, + Response: &admissionv1.AdmissionResponse{ + UID: "123", + Allowed: true, + PatchType: &invalidv1Patch, + }, + }, + expectErr: `validating webhook may not return response.patchType`, + }, + + // v1beta1 invalid responses that we have to allow/fixup for compatibility + { + name: "v1beta1 wrong group/version/kind", + uid: "123", + review: &admissionv1beta1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "admission.k8s.io2/v2", Kind: "AdmissionReview2"}, + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: true, + }, + }, + expectAllowed: true, + }, + { + name: "v1beta1 wrong uid", + uid: "123", + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + UID: "1234", + Allowed: true, + }, + }, + expectAllowed: true, + }, + { + name: "v1beta1 validating returns patch/patchType", + uid: "123", + mutating: false, + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + UID: "1234", + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &v1beta1JSONPatch, + }, + }, + expectAllowed: true, + }, + { + name: "v1beta1 empty patch type", + uid: "123", + mutating: true, + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &emptyv1beta1Patch, + }, + }, + expectAllowed: true, + expectPatch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + expectPatchType: admissionv1.PatchTypeJSONPatch, + }, + { + name: "v1beta1 invalid patchType", + uid: "123", + mutating: true, + review: &admissionv1beta1.AdmissionReview{ + Response: &admissionv1beta1.AdmissionResponse{ + Allowed: true, + Patch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + PatchType: &invalidv1beta1Patch, + }, + }, + expectAllowed: true, + expectPatch: []byte(`[{"op":"add","path":"/foo","value":"bar"}]`), + expectPatchType: admissionv1.PatchTypeJSONPatch, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + result, err := VerifyAdmissionResponse(tc.uid, tc.mutating, tc.review) + if err != nil { + if len(tc.expectErr) > 0 { + if !strings.Contains(err.Error(), tc.expectErr) { + t.Errorf("expected error '%s', got %v", tc.expectErr, err) + } + } else { + t.Errorf("unexpected error %v", err) + } + return + } else if len(tc.expectErr) > 0 { + t.Errorf("expected error '%s', got none", tc.expectErr) + return + } + + if e, a := tc.expectAuditAnnotations, result.AuditAnnotations; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + if e, a := tc.expectAllowed, result.Allowed; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + if e, a := tc.expectPatch, result.Patch; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + if e, a := tc.expectPatchType, result.PatchType; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + if e, a := tc.expectResult, result.Result; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + }) + } +} + +func TestCreateAdmissionObjects(t *testing.T) { + internalObj := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "2", Name: "myname", Namespace: "myns"}} + internalObjOld := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1", Name: "myname", Namespace: "myns"}} + versionedObj := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "2", Name: "myname", Namespace: "myns"}} + versionedObjOld := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1", Name: "myname", Namespace: "myns"}} + userInfo := &user.DefaultInfo{ + Name: "myuser", + Groups: []string{"mygroup"}, + UID: "myuid", + Extra: map[string][]string{"extrakey": {"value1", "value2"}}, + } + attrs := admission.NewAttributesRecord( + internalObj.DeepCopyObject(), + internalObjOld.DeepCopyObject(), + schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + "myns", + "myname", + schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, + "", + admission.Update, + &metav1.UpdateOptions{FieldManager: "foo"}, + false, + userInfo, + ) + + testcases := []struct { + name string + attrs *generic.VersionedAttributes + invocation *generic.WebhookInvocation + + expectRequest func(uid types.UID) runtime.Object + expectResponse runtime.Object + expectErr string + }{ + { + name: "no supported versions", + invocation: &generic.WebhookInvocation{ + Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", &admissionregistrationv1beta1.MutatingWebhook{}), + }, + expectErr: "webhook does not accept known AdmissionReview versions", + }, + { + name: "no known supported versions", + invocation: &generic.WebhookInvocation{ + Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", &admissionregistrationv1beta1.MutatingWebhook{ + AdmissionReviewVersions: []string{"vX"}, + }), + }, + expectErr: "webhook does not accept known AdmissionReview versions", + }, + { + name: "v1", + attrs: &generic.VersionedAttributes{ + VersionedObject: versionedObj.DeepCopyObject(), + VersionedOldObject: versionedObjOld.DeepCopyObject(), + Attributes: attrs, + }, + invocation: &generic.WebhookInvocation{ + Resource: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, + Subresource: "", + Kind: schema.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"}, + Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", &admissionregistrationv1beta1.MutatingWebhook{ + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + }), + }, + expectRequest: func(uid types.UID) runtime.Object { + return &admissionv1.AdmissionReview{ + Request: &admissionv1.AdmissionRequest{ + UID: uid, + Kind: metav1.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"}, + Resource: metav1.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, + SubResource: "", + RequestKind: &metav1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + RequestResource: &metav1.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, + RequestSubResource: "", + Name: "myname", + Namespace: "myns", + Operation: "UPDATE", + UserInfo: authenticationv1.UserInfo{ + Username: "myuser", + UID: "myuid", + Groups: []string{"mygroup"}, + Extra: map[string]authenticationv1.ExtraValue{"extrakey": {"value1", "value2"}}, + }, + Object: runtime.RawExtension{Object: versionedObj}, + OldObject: runtime.RawExtension{Object: versionedObjOld}, + DryRun: utilpointer.BoolPtr(false), + Options: runtime.RawExtension{Object: &metav1.UpdateOptions{FieldManager: "foo"}}, + }, + } + }, + expectResponse: &admissionv1.AdmissionReview{}, + }, + { + name: "v1beta1", + attrs: &generic.VersionedAttributes{ + VersionedObject: versionedObj.DeepCopyObject(), + VersionedOldObject: versionedObjOld.DeepCopyObject(), + Attributes: attrs, + }, + invocation: &generic.WebhookInvocation{ + Resource: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, + Subresource: "", + Kind: schema.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"}, + Webhook: webhook.NewMutatingWebhookAccessor("mywebhook", &admissionregistrationv1beta1.MutatingWebhook{ + AdmissionReviewVersions: []string{"v1beta1", "v1"}, + }), + }, + expectRequest: func(uid types.UID) runtime.Object { + return &admissionv1beta1.AdmissionReview{ + Request: &admissionv1beta1.AdmissionRequest{ + UID: uid, + Kind: metav1.GroupVersionKind{Group: "extensions", Version: "v1beta1", Kind: "Deployment"}, + Resource: metav1.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, + SubResource: "", + RequestKind: &metav1.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"}, + RequestResource: &metav1.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, + RequestSubResource: "", + Name: "myname", + Namespace: "myns", + Operation: "UPDATE", + UserInfo: authenticationv1.UserInfo{ + Username: "myuser", + UID: "myuid", + Groups: []string{"mygroup"}, + Extra: map[string]authenticationv1.ExtraValue{"extrakey": {"value1", "value2"}}, + }, + Object: runtime.RawExtension{Object: versionedObj}, + OldObject: runtime.RawExtension{Object: versionedObjOld}, + DryRun: utilpointer.BoolPtr(false), + Options: runtime.RawExtension{Object: &metav1.UpdateOptions{FieldManager: "foo"}}, + }, + } + }, + expectResponse: &admissionv1beta1.AdmissionReview{}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + uid, request, response, err := CreateAdmissionObjects(tc.attrs, tc.invocation) + if err != nil { + if len(tc.expectErr) > 0 { + if !strings.Contains(err.Error(), tc.expectErr) { + t.Errorf("expected error '%s', got %v", tc.expectErr, err) + } + } else { + t.Errorf("unexpected error %v", err) + } + return + } else if len(tc.expectErr) > 0 { + t.Errorf("expected error '%s', got none", tc.expectErr) + return + } + + if len(uid) == 0 { + t.Errorf("expected uid, got none") + } + if e, a := tc.expectRequest(uid), request; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + if e, a := tc.expectResponse, response; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected: %v", cmp.Diff(e, a)) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go index e73b6b04732..ee728a535ba 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go @@ -487,7 +487,7 @@ func NewNonMutatingTestCases(url *url.URL) []ValidatingTest { AdmissionReviewVersions: []string{"v1beta1"}, }}, ExpectStatusCode: http.StatusInternalServerError, - ErrorContains: "Webhook response was absent", + ErrorContains: "webhook response was absent", }, { Name: "no match dry run", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go index dda52c90e57..9a8aa3390da 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go @@ -22,7 +22,6 @@ import ( "sync" "time" - admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" @@ -32,7 +31,7 @@ import ( "k8s.io/apiserver/pkg/admission/plugin/webhook" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" - "k8s.io/apiserver/pkg/admission/plugin/webhook/request" + webhookrequest "k8s.io/apiserver/pkg/admission/plugin/webhook/request" "k8s.io/apiserver/pkg/admission/plugin/webhook/util" webhookutil "k8s.io/apiserver/pkg/util/webhook" "k8s.io/klog" @@ -145,20 +144,16 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Validati } } - // Currently dispatcher only supports `v1beta1` AdmissionReview - // TODO: Make the dispatcher capable of sending multiple AdmissionReview versions - if !util.HasAdmissionReviewVersion(v1beta1.SchemeGroupVersion.Version, invocation.Webhook) { - return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("webhook does not accept v1beta1 AdmissionReviewRequest")} + uid, request, response, err := webhookrequest.CreateAdmissionObjects(attr, invocation) + if err != nil { + return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - // Make the webhook request - request := request.CreateAdmissionReview(attr, invocation) client, err := d.cm.HookClient(util.HookClientConfigForWebhook(invocation.Webhook)) if err != nil { return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - response := &admissionv1beta1.AdmissionReview{} - r := client.Post().Context(ctx).Body(&request) + r := client.Post().Context(ctx).Body(request) if h.TimeoutSeconds != nil { r = r.Timeout(time.Duration(*h.TimeoutSeconds) * time.Second) } @@ -166,17 +161,19 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Validati return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - if response.Response == nil { - return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook response was absent")} + result, err := webhookrequest.VerifyAdmissionResponse(uid, false, response) + if err != nil { + return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - for k, v := range response.Response.AuditAnnotations { + + for k, v := range result.AuditAnnotations { key := h.Name + "/" + k if err := attr.Attributes.AddAnnotation(key, v); err != nil { klog.Warningf("Failed to set admission audit annotation %s to %s for validating webhook %s: %v", key, v, h.Name, err) } } - if response.Response.Allowed { + if result.Allowed { return nil } - return webhookerrors.ToStatusErr(h.Name, response.Response.Result) + return webhookerrors.ToStatusErr(h.Name, result.Result) } diff --git a/staging/src/k8s.io/apiserver/pkg/util/webhook/client.go b/staging/src/k8s.io/apiserver/pkg/util/webhook/client.go index a689ecf681f..fd6a83c1c01 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/webhook/client.go +++ b/staging/src/k8s.io/apiserver/pkg/util/webhook/client.go @@ -62,19 +62,21 @@ type ClientManager struct { } // NewClientManager creates a clientManager. -func NewClientManager(gv schema.GroupVersion, addToSchemaFunc func(s *runtime.Scheme) error) (ClientManager, error) { +func NewClientManager(gvs []schema.GroupVersion, addToSchemaFuncs ...func(s *runtime.Scheme) error) (ClientManager, error) { cache, err := lru.New(defaultCacheSize) if err != nil { return ClientManager{}, err } hookScheme := runtime.NewScheme() - if err := addToSchemaFunc(hookScheme); err != nil { - return ClientManager{}, err + for _, addToSchemaFunc := range addToSchemaFuncs { + if err := addToSchemaFunc(hookScheme); err != nil { + return ClientManager{}, err + } } return ClientManager{ cache: cache, negotiatedSerializer: serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{ - Serializer: serializer.NewCodecFactory(hookScheme).LegacyCodec(gv), + Serializer: serializer.NewCodecFactory(hookScheme).LegacyCodec(gvs...), }), }, nil } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/audit/dynamic/dynamic.go b/staging/src/k8s.io/apiserver/plugin/pkg/audit/dynamic/dynamic.go index b52c7c85285..80004a885b8 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/audit/dynamic/dynamic.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/audit/dynamic/dynamic.go @@ -28,6 +28,7 @@ import ( auditregv1alpha1 "k8s.io/api/auditregistration/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" auditinternal "k8s.io/apiserver/pkg/apis/audit" auditinstall "k8s.io/apiserver/pkg/apis/audit/install" @@ -101,7 +102,7 @@ func NewBackend(c *Config) (audit.Backend, error) { if c.BufferedConfig == nil { c.BufferedConfig = NewDefaultWebhookBatchConfig() } - cm, err := webhook.NewClientManager(auditv1.SchemeGroupVersion, func(s *runtime.Scheme) error { + cm, err := webhook.NewClientManager([]schema.GroupVersion{auditv1.SchemeGroupVersion}, func(s *runtime.Scheme) error { auditinstall.Install(s) return nil }) diff --git a/test/integration/apiserver/admissionwebhook/admission_test.go b/test/integration/apiserver/admissionwebhook/admission_test.go index 7e7075f2fb3..953feac6cb4 100644 --- a/test/integration/apiserver/admissionwebhook/admission_test.go +++ b/test/integration/apiserver/admissionwebhook/admission_test.go @@ -30,7 +30,9 @@ import ( "testing" "time" + admissionreviewv1 "k8s.io/api/admission/v1" "k8s.io/api/admission/v1beta1" + admissionv1 "k8s.io/api/admissionregistration/v1" admissionv1beta1 "k8s.io/api/admissionregistration/v1beta1" appsv1beta1 "k8s.io/api/apps/v1beta1" corev1 "k8s.io/api/core/v1" @@ -151,6 +153,8 @@ var ( ) type webhookOptions struct { + version string + // phase indicates whether this is a mutating or validating webhook phase string // converted indicates if this webhook makes use of matchPolicy:equivalent and expects conversion. @@ -165,7 +169,7 @@ type holder struct { t *testing.T recordGVR metav1.GroupVersionResource - recordOperation v1beta1.Operation + recordOperation string recordNamespace string recordName string @@ -182,7 +186,7 @@ type holder struct { // When a converted request is recorded, gvrToConvertedGVR[expectGVK] is compared to the GVK seen by the webhook. gvrToConvertedGVK map[metav1.GroupVersionResource]schema.GroupVersionKind - recorded map[webhookOptions]*v1beta1.AdmissionRequest + recorded map[webhookOptions]*admissionRequest } func (h *holder) reset(t *testing.T) { @@ -200,10 +204,12 @@ func (h *holder) reset(t *testing.T) { h.expectOptions = false // Set up the recorded map with nil records for all combinations - h.recorded = map[webhookOptions]*v1beta1.AdmissionRequest{} + h.recorded = map[webhookOptions]*admissionRequest{} for _, phase := range []string{mutation, validation} { for _, converted := range []bool{true, false} { - h.recorded[webhookOptions{phase: phase, converted: converted}] = nil + for _, version := range []string{"v1", "v1beta1"} { + h.recorded[webhookOptions{version: version, phase: phase, converted: converted}] = nil + } } } } @@ -217,7 +223,7 @@ func (h *holder) expect(gvr schema.GroupVersionResource, gvk, optionsGVK schema. defer h.lock.Unlock() h.recordGVR = metav1.GroupVersionResource{Group: gvr.Group, Version: gvr.Version, Resource: gvr.Resource} h.expectGVK = gvk - h.recordOperation = operation + h.recordOperation = string(operation) h.recordName = name h.recordNamespace = namespace h.expectObject = object @@ -226,14 +232,28 @@ func (h *holder) expect(gvr schema.GroupVersionResource, gvk, optionsGVK schema. h.expectOptions = options // Set up the recorded map with nil records for all combinations - h.recorded = map[webhookOptions]*v1beta1.AdmissionRequest{} + h.recorded = map[webhookOptions]*admissionRequest{} for _, phase := range []string{mutation, validation} { for _, converted := range []bool{true, false} { - h.recorded[webhookOptions{phase: phase, converted: converted}] = nil + for _, version := range []string{"v1", "v1beta1"} { + h.recorded[webhookOptions{version: version, phase: phase, converted: converted}] = nil + } } } } -func (h *holder) record(phase string, converted bool, request *v1beta1.AdmissionRequest) { + +type admissionRequest struct { + Operation string + Resource metav1.GroupVersionResource + SubResource string + Namespace string + Name string + Object runtime.RawExtension + OldObject runtime.RawExtension + Options runtime.RawExtension +} + +func (h *holder) record(version string, phase string, converted bool, request *admissionRequest) { h.lock.Lock() defer h.lock.Unlock() @@ -286,9 +306,9 @@ func (h *holder) record(phase string, converted bool, request *v1beta1.Admission } if debug { - h.t.Logf("recording: %#v = %s %#v %v", webhookOptions{phase: phase, converted: converted}, request.Operation, request.Resource, request.SubResource) + h.t.Logf("recording: %#v = %s %#v %v", webhookOptions{version: version, phase: phase, converted: converted}, request.Operation, request.Resource, request.SubResource) } - h.recorded[webhookOptions{phase: phase, converted: converted}] = request + h.recorded[webhookOptions{version: version, phase: phase, converted: converted}] = request } func (h *holder) verify(t *testing.T) { @@ -297,12 +317,12 @@ func (h *holder) verify(t *testing.T) { for options, value := range h.recorded { if err := h.verifyRequest(options.converted, value); err != nil { - t.Errorf("phase:%v, converted:%v error: %v", options.phase, options.converted, err) + t.Errorf("version: %v, phase:%v, converted:%v error: %v", options.version, options.phase, options.converted, err) } } } -func (h *holder) verifyRequest(converted bool, request *v1beta1.AdmissionRequest) error { +func (h *holder) verifyRequest(converted bool, request *admissionRequest) error { // Check if current resource should be exempted from Admission processing if admissionExemptResources[gvr(h.recordGVR.Group, h.recordGVR.Version, h.recordGVR.Resource)] { if request == nil { @@ -366,8 +386,8 @@ func (h *holder) verifyOptions(options runtime.Object) error { return nil } -// TestWebhookV1beta1 tests communication between API server and webhook process. -func TestWebhookV1beta1(t *testing.T) { +// TestWebhookAdmission tests communication between API server and webhook process. +func TestWebhookAdmission(t *testing.T) { // holder communicates expectations to webhooks, and results from webhooks holder := &holder{ t: t, @@ -386,10 +406,17 @@ func TestWebhookV1beta1(t *testing.T) { } webhookMux := http.NewServeMux() - webhookMux.Handle("/"+mutation, newWebhookHandler(t, holder, mutation, false)) - webhookMux.Handle("/convert/"+mutation, newWebhookHandler(t, holder, mutation, true)) - webhookMux.Handle("/"+validation, newWebhookHandler(t, holder, validation, false)) - webhookMux.Handle("/convert/"+validation, newWebhookHandler(t, holder, validation, true)) + webhookMux.Handle("/v1beta1/"+mutation, newV1beta1WebhookHandler(t, holder, mutation, false)) + webhookMux.Handle("/v1beta1/convert/"+mutation, newV1beta1WebhookHandler(t, holder, mutation, true)) + webhookMux.Handle("/v1beta1/"+validation, newV1beta1WebhookHandler(t, holder, validation, false)) + webhookMux.Handle("/v1beta1/convert/"+validation, newV1beta1WebhookHandler(t, holder, validation, true)) + webhookMux.Handle("/v1/"+mutation, newV1WebhookHandler(t, holder, mutation, false)) + webhookMux.Handle("/v1/convert/"+mutation, newV1WebhookHandler(t, holder, mutation, true)) + webhookMux.Handle("/v1/"+validation, newV1WebhookHandler(t, holder, validation, false)) + webhookMux.Handle("/v1/convert/"+validation, newV1WebhookHandler(t, holder, validation, true)) + webhookMux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + holder.t.Errorf("unexpected request to %v", req.URL.Path) + })) webhookServer := httptest.NewUnstartedServer(webhookMux) webhookServer.TLS = &tls.Config{ RootCAs: roots, @@ -488,7 +515,8 @@ func TestWebhookV1beta1(t *testing.T) { // Note: this only works because there are no overlapping resource names in-process that are not co-located convertedResources := map[string]schema.GroupVersionResource{} // build the webhook rules enumerating the specific group/version/resources we want - convertedRules := []admissionv1beta1.RuleWithOperations{} + convertedV1beta1Rules := []admissionv1beta1.RuleWithOperations{} + convertedV1Rules := []admissionv1.RuleWithOperations{} for _, gvr := range gvrsToTest { metaGVR := metav1.GroupVersionResource{Group: gvr.Group, Version: gvr.Version, Resource: gvr.Resource} @@ -499,10 +527,14 @@ func TestWebhookV1beta1(t *testing.T) { convertedGVR = gvr convertedResources[gvr.Resource] = gvr // add an admission rule indicating we can receive this version - convertedRules = append(convertedRules, admissionv1beta1.RuleWithOperations{ + convertedV1beta1Rules = append(convertedV1beta1Rules, admissionv1beta1.RuleWithOperations{ Operations: []admissionv1beta1.OperationType{admissionv1beta1.OperationAll}, Rule: admissionv1beta1.Rule{APIGroups: []string{gvr.Group}, APIVersions: []string{gvr.Version}, Resources: []string{gvr.Resource}}, }) + convertedV1Rules = append(convertedV1Rules, admissionv1.RuleWithOperations{ + Operations: []admissionv1.OperationType{admissionv1.OperationAll}, + Rule: admissionv1.Rule{APIGroups: []string{gvr.Group}, APIVersions: []string{gvr.Version}, Resources: []string{gvr.Resource}}, + }) } // record the expected resource and kind @@ -510,10 +542,16 @@ func TestWebhookV1beta1(t *testing.T) { holder.gvrToConvertedGVK[metaGVR] = schema.GroupVersionKind{Group: resourcesByGVR[convertedGVR].Group, Version: resourcesByGVR[convertedGVR].Version, Kind: resourcesByGVR[convertedGVR].Kind} } - if err := createV1beta1MutationWebhook(client, webhookServer.URL+"/"+mutation, webhookServer.URL+"/convert/"+mutation, convertedRules); err != nil { + if err := createV1beta1MutationWebhook(client, webhookServer.URL+"/v1beta1/"+mutation, webhookServer.URL+"/v1beta1/convert/"+mutation, convertedV1beta1Rules); err != nil { t.Fatal(err) } - if err := createV1beta1ValidationWebhook(client, webhookServer.URL+"/"+validation, webhookServer.URL+"/convert/"+validation, convertedRules); err != nil { + if err := createV1beta1ValidationWebhook(client, webhookServer.URL+"/v1beta1/"+validation, webhookServer.URL+"/v1beta1/convert/"+validation, convertedV1beta1Rules); err != nil { + t.Fatal(err) + } + if err := createV1MutationWebhook(client, webhookServer.URL+"/v1/"+mutation, webhookServer.URL+"/v1/convert/"+mutation, convertedV1Rules); err != nil { + t.Fatal(err) + } + if err := createV1ValidationWebhook(client, webhookServer.URL+"/v1/"+validation, webhookServer.URL+"/v1/convert/"+validation, convertedV1Rules); err != nil { t.Fatal(err) } @@ -1118,7 +1156,7 @@ func testNoPruningCustomFancy(c *testContext) { // utility methods // -func newWebhookHandler(t *testing.T, holder *holder, phase string, converted bool) http.Handler { +func newV1beta1WebhookHandler(t *testing.T, holder *holder, phase string, converted bool) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() data, err := ioutil.ReadAll(r.Body) @@ -1176,18 +1214,124 @@ func newWebhookHandler(t *testing.T, holder *holder, phase string, converted boo if review.Request.UserInfo.Username == testClientUsername { // only record requests originating from this integration test's client - holder.record(phase, converted, review.Request) + reviewRequest := &admissionRequest{ + Operation: string(review.Request.Operation), + Resource: review.Request.Resource, + SubResource: review.Request.SubResource, + Namespace: review.Request.Namespace, + Name: review.Request.Name, + Object: review.Request.Object, + OldObject: review.Request.OldObject, + Options: review.Request.Options, + } + holder.record("v1beta1", phase, converted, reviewRequest) } review.Response = &v1beta1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{Message: "admitted"}, + } + + // v1beta1 webhook handler tolerated these not being set. verify the server continues to accept these as unset. + review.APIVersion = "" + review.Kind = "" + review.Response.UID = "" + + // If we're mutating, and have an object, return a patch to exercise conversion + if phase == mutation && len(review.Request.Object.Raw) > 0 { + review.Response.Patch = []byte(`[{"op":"add","path":"/foo","value":"test"}]`) + jsonPatch := v1beta1.PatchTypeJSONPatch + review.Response.PatchType = &jsonPatch + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(review); err != nil { + t.Errorf("Marshal of response failed with error: %v", err) + } + }) +} + +func newV1WebhookHandler(t *testing.T, holder *holder, phase string, converted bool) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + data, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Error(err) + return + } + + if contentType := r.Header.Get("Content-Type"); contentType != "application/json" { + t.Errorf("contentType=%s, expect application/json", contentType) + return + } + + review := admissionreviewv1.AdmissionReview{} + if err := json.Unmarshal(data, &review); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(data), err) + http.Error(w, err.Error(), 400) + return + } + + if review.GetObjectKind().GroupVersionKind() != gvk("admission.k8s.io", "v1", "AdmissionReview") { + err := fmt.Errorf("Invalid admission review kind: %#v", review.GetObjectKind().GroupVersionKind()) + t.Error(err) + http.Error(w, err.Error(), 400) + return + } + + if len(review.Request.Object.Raw) > 0 { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(review.Request.Object.Raw, u); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(review.Request.Object.Raw), err) + http.Error(w, err.Error(), 400) + return + } + review.Request.Object.Object = u + } + if len(review.Request.OldObject.Raw) > 0 { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(review.Request.OldObject.Raw, u); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(review.Request.OldObject.Raw), err) + http.Error(w, err.Error(), 400) + return + } + review.Request.OldObject.Object = u + } + + if len(review.Request.Options.Raw) > 0 { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(review.Request.Options.Raw, u); err != nil { + t.Errorf("Fail to deserialize options object: %s for admission request %#+v with error: %v", string(review.Request.Options.Raw), review.Request, err) + http.Error(w, err.Error(), 400) + return + } + review.Request.Options.Object = u + } + + if review.Request.UserInfo.Username == testClientUsername { + // only record requests originating from this integration test's client + reviewRequest := &admissionRequest{ + Operation: string(review.Request.Operation), + Resource: review.Request.Resource, + SubResource: review.Request.SubResource, + Namespace: review.Request.Namespace, + Name: review.Request.Name, + Object: review.Request.Object, + OldObject: review.Request.OldObject, + Options: review.Request.Options, + } + holder.record("v1", phase, converted, reviewRequest) + } + + review.Response = &admissionreviewv1.AdmissionResponse{ Allowed: true, UID: review.Request.UID, Result: &metav1.Status{Message: "admitted"}, } // If we're mutating, and have an object, return a patch to exercise conversion if phase == mutation && len(review.Request.Object.Raw) > 0 { - review.Response.Patch = []byte(`[{"op":"add","path":"/foo","value":"test"}]`) - jsonPatch := v1beta1.PatchTypeJSONPatch + review.Response.Patch = []byte(`[{"op":"add","path":"/bar","value":"test"}]`) + jsonPatch := admissionreviewv1.PatchTypeJSONPatch review.Response.PatchType = &jsonPatch } @@ -1358,6 +1502,84 @@ func createV1beta1MutationWebhook(client clientset.Interface, endpoint, converte return err } +func createV1ValidationWebhook(client clientset.Interface, endpoint, convertedEndpoint string, convertedRules []admissionv1.RuleWithOperations) error { + fail := admissionv1.Fail + equivalent := admissionv1.Equivalent + none := admissionv1.SideEffectClassNone + // Attaching Admission webhook to API server + _, err := client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Create(&admissionv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "admissionv1.integration.test"}, + Webhooks: []admissionv1.ValidatingWebhook{ + { + Name: "admissionv1.integration.test", + ClientConfig: admissionv1.WebhookClientConfig{ + URL: &endpoint, + CABundle: localhostCert, + }, + Rules: []admissionv1.RuleWithOperations{{ + Operations: []admissionv1.OperationType{admissionv1.OperationAll}, + Rule: admissionv1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*/*"}}, + }}, + FailurePolicy: &fail, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + SideEffects: &none, + }, + { + Name: "admissionv1.integration.testconversion", + ClientConfig: admissionv1.WebhookClientConfig{ + URL: &convertedEndpoint, + CABundle: localhostCert, + }, + Rules: convertedRules, + FailurePolicy: &fail, + MatchPolicy: &equivalent, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + SideEffects: &none, + }, + }, + }) + return err +} + +func createV1MutationWebhook(client clientset.Interface, endpoint, convertedEndpoint string, convertedRules []admissionv1.RuleWithOperations) error { + fail := admissionv1.Fail + equivalent := admissionv1.Equivalent + none := admissionv1.SideEffectClassNone + // Attaching Mutation webhook to API server + _, err := client.AdmissionregistrationV1().MutatingWebhookConfigurations().Create(&admissionv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "mutationv1.integration.test"}, + Webhooks: []admissionv1.MutatingWebhook{ + { + Name: "mutationv1.integration.test", + ClientConfig: admissionv1.WebhookClientConfig{ + URL: &endpoint, + CABundle: localhostCert, + }, + Rules: []admissionv1.RuleWithOperations{{ + Operations: []admissionv1.OperationType{admissionv1.OperationAll}, + Rule: admissionv1.Rule{APIGroups: []string{"*"}, APIVersions: []string{"*"}, Resources: []string{"*/*"}}, + }}, + FailurePolicy: &fail, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + SideEffects: &none, + }, + { + Name: "mutationv1.integration.testconversion", + ClientConfig: admissionv1.WebhookClientConfig{ + URL: &convertedEndpoint, + CABundle: localhostCert, + }, + Rules: convertedRules, + FailurePolicy: &fail, + MatchPolicy: &equivalent, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + SideEffects: &none, + }, + }, + }) + return err +} + // localhostCert was generated from crypto/tls/generate_cert.go with the following command: // go run generate_cert.go --rsa-bits 512 --host 127.0.0.1,::1,example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h var localhostCert = []byte(`-----BEGIN CERTIFICATE-----