diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD index b8f9bb55274..390ea55183b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/BUILD @@ -50,6 +50,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion.go index 11f704f736a..f0e0ed79c1b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion.go @@ -41,3 +41,72 @@ func ConvertToGVK(obj runtime.Object, gvk schema.GroupVersionKind, o admission.O out.GetObjectKind().SetGroupVersionKind(gvk) return out, nil } + +// NewVersionedAttributes returns versioned attributes with the old and new object (if non-nil) converted to the requested kind +func NewVersionedAttributes(attr admission.Attributes, gvk schema.GroupVersionKind, o admission.ObjectInterfaces) (*VersionedAttributes, error) { + // convert the old and new objects to the requested version + versionedAttr := &VersionedAttributes{ + Attributes: attr, + VersionedKind: gvk, + } + if oldObj := attr.GetOldObject(); oldObj != nil { + out, err := ConvertToGVK(oldObj, gvk, o) + if err != nil { + return nil, err + } + versionedAttr.VersionedOldObject = out + } + if obj := attr.GetObject(); obj != nil { + out, err := ConvertToGVK(obj, gvk, o) + if err != nil { + return nil, err + } + versionedAttr.VersionedObject = out + } + return versionedAttr, nil +} + +// ConvertVersionedAttributes converts VersionedObject and VersionedOldObject to the specified kind, if needed. +// If attr.VersionedKind already matches the requested kind, no conversion is performed. +// If conversion is required: +// * attr.VersionedObject is used as the source for the new object if Dirty=true (and is round-tripped through attr.Attributes.Object, clearing Dirty in the process) +// * attr.Attributes.Object is used as the source for the new object if Dirty=false +// * attr.Attributes.OldObject is used as the source for the old object +func ConvertVersionedAttributes(attr *VersionedAttributes, gvk schema.GroupVersionKind, o admission.ObjectInterfaces) error { + // we already have the desired kind, we're done + if attr.VersionedKind == gvk { + return nil + } + + // convert the original old object to the desired GVK + if oldObj := attr.Attributes.GetOldObject(); oldObj != nil { + out, err := ConvertToGVK(oldObj, gvk, o) + if err != nil { + return err + } + attr.VersionedOldObject = out + } + + if attr.VersionedObject != nil { + // convert the existing versioned object to internal + if attr.Dirty { + err := o.GetObjectConvertor().Convert(attr.VersionedObject, attr.Attributes.GetObject(), nil) + if err != nil { + return err + } + } + + // and back to external + out, err := ConvertToGVK(attr.Attributes.GetObject(), gvk, o) + if err != nil { + return err + } + attr.VersionedObject = out + } + + // Remember we converted to this version + attr.VersionedKind = gvk + attr.Dirty = false + + return nil +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion_test.go index b37c6158938..4e609f060ef 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/conversion_test.go @@ -17,6 +17,7 @@ limitations under the License. package generic import ( + "encoding/json" "reflect" "testing" @@ -26,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/apis/example" examplev1 "k8s.io/apiserver/pkg/apis/example/v1" @@ -160,3 +162,203 @@ func TestRuntimeSchemeConvert(t *testing.T) { t.Errorf("unexpected mutation of self-converted Unstructured: obj=%#v, clone=%#v", obj, clone) } } + +func TestConvertVersionedAttributes(t *testing.T) { + scheme := initiateScheme(t) + o := admission.NewObjectInterfacesFromScheme(scheme) + + gvk := func(g, v, k string) schema.GroupVersionKind { + return schema.GroupVersionKind{g, v, k} + } + attrs := func(obj, oldObj runtime.Object) admission.Attributes { + return admission.NewAttributesRecord(obj, oldObj, schema.GroupVersionKind{}, "", "", schema.GroupVersionResource{}, "", "", nil, false, nil) + } + u := func(data string) *unstructured.Unstructured { + t.Helper() + m := map[string]interface{}{} + if err := json.Unmarshal([]byte(data), &m); err != nil { + t.Fatal(err) + } + return &unstructured.Unstructured{Object: m} + } + testcases := []struct { + Name string + Attrs *VersionedAttributes + GVK schema.GroupVersionKind + ExpectedAttrs *VersionedAttributes + }{ + { + Name: "noop", + Attrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + VersionedObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpodversioned"}}, + VersionedKind: examplev1.SchemeGroupVersion.WithKind("Pod"), + Dirty: true, + }, + GVK: examplev1.SchemeGroupVersion.WithKind("Pod"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + VersionedObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpodversioned"}}, + VersionedKind: examplev1.SchemeGroupVersion.WithKind("Pod"), + Dirty: true, + }, + }, + { + Name: "clean, typed", + Attrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + VersionedObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpodversioned"}}, + VersionedKind: gvk("g", "v", "k"), + }, + GVK: examplev1.SchemeGroupVersion.WithKind("Pod"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + // name gets overwritten from converted attributes, type gets set explicitly + VersionedObject: &examplev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "example.apiserver.k8s.io/v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + VersionedOldObject: &examplev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "example.apiserver.k8s.io/v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + VersionedKind: examplev1.SchemeGroupVersion.WithKind("Pod"), + }, + }, + { + Name: "clean, unstructured", + Attrs: &VersionedAttributes{ + Attributes: attrs( + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobj"}}`), + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + ), + VersionedObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobjversioned"}}`), + VersionedOldObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobjversioned"}}`), + VersionedKind: gvk("g", "v", "k"), // claim a different current version to trigger conversion + }, + GVK: gvk("mygroup.k8s.io", "v1", "Flunder"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobj"}}`), + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + ), + VersionedObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobj"}}`), + VersionedOldObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + VersionedKind: gvk("mygroup.k8s.io", "v1", "Flunder"), + }, + }, + { + Name: "dirty, typed", + Attrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + VersionedObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpodversioned"}}, + VersionedKind: gvk("g", "v", "k"), // claim a different current version to trigger conversion + Dirty: true, + }, + GVK: examplev1.SchemeGroupVersion.WithKind("Pod"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + ), + // new name gets preserved from versioned object, type gets set explicitly + VersionedObject: &examplev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "example.apiserver.k8s.io/v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + // old name gets overwritten from converted attributes, type gets set explicitly + VersionedOldObject: &examplev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "example.apiserver.k8s.io/v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{Name: "oldpod"}}, + VersionedKind: examplev1.SchemeGroupVersion.WithKind("Pod"), + Dirty: false, + }, + }, + { + Name: "dirty, unstructured", + Attrs: &VersionedAttributes{ + Attributes: attrs( + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobj"}}`), + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + ), + VersionedObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobjversioned"}}`), + VersionedOldObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobjversioned"}}`), + VersionedKind: gvk("g", "v", "k"), // claim a different current version to trigger conversion + Dirty: true, + }, + GVK: gvk("mygroup.k8s.io", "v1", "Flunder"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobjversioned"}}`), + u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + ), + // new name gets preserved from versioned object, type gets set explicitly + VersionedObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"newobjversioned"}}`), + // old name gets overwritten from converted attributes, type gets set explicitly + VersionedOldObject: u(`{"apiVersion": "mygroup.k8s.io/v1","kind": "Flunder","metadata":{"name":"oldobj"}}`), + VersionedKind: gvk("mygroup.k8s.io", "v1", "Flunder"), + Dirty: false, + }, + }, + { + Name: "nil old object", + Attrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpod"}}, + nil, + ), + VersionedObject: &examplev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: nil, + VersionedKind: gvk("g", "v", "k"), // claim a different current version to trigger conversion + Dirty: true, + }, + GVK: examplev1.SchemeGroupVersion.WithKind("Pod"), + ExpectedAttrs: &VersionedAttributes{ + Attributes: attrs( + &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + nil, + ), + // new name gets preserved from versioned object, type gets set explicitly + VersionedObject: &examplev1.Pod{TypeMeta: metav1.TypeMeta{APIVersion: "example.apiserver.k8s.io/v1", Kind: "Pod"}, ObjectMeta: metav1.ObjectMeta{Name: "newpodversioned"}}, + VersionedOldObject: nil, + VersionedKind: examplev1.SchemeGroupVersion.WithKind("Pod"), + Dirty: false, + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + err := ConvertVersionedAttributes(tc.Attrs, tc.GVK, o) + if err != nil { + t.Fatal(err) + } + if e, a := tc.ExpectedAttrs.Attributes.GetObject(), tc.Attrs.Attributes.GetObject(); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + if e, a := tc.ExpectedAttrs.Attributes.GetOldObject(), tc.Attrs.Attributes.GetOldObject(); !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + if e, a := tc.ExpectedAttrs.VersionedKind, tc.Attrs.VersionedKind; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + if e, a := tc.ExpectedAttrs.VersionedObject, tc.Attrs.VersionedObject; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + if e, a := tc.ExpectedAttrs.VersionedOldObject, tc.Attrs.VersionedOldObject; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + if e, a := tc.ExpectedAttrs.Dirty, tc.Attrs.Dirty; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff:\n%s", diff.ObjectReflectDiff(e, a)) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/interfaces.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/interfaces.go index d998b6b71ec..c57189543fb 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/interfaces.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic/interfaces.go @@ -21,6 +21,7 @@ import ( "k8s.io/api/admissionregistration/v1beta1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" ) @@ -33,13 +34,32 @@ type Source interface { // VersionedAttributes is a wrapper around the original admission attributes, adding versioned // variants of the object and old object. type VersionedAttributes struct { - admission.Attributes + // Attributes holds the original admission attributes + Attributes admission.Attributes + // VersionedOldObject holds Attributes.OldObject (if non-nil), converted to VersionedKind. + // It must never be mutated. VersionedOldObject runtime.Object - VersionedObject runtime.Object + // VersionedObject holds Attributes.Object (if non-nil), converted to VersionedKind. + // If mutated, Dirty must be set to true by the mutator. + VersionedObject runtime.Object + // VersionedKind holds the fully qualified kind + VersionedKind schema.GroupVersionKind + // Dirty indicates VersionedObject has been modified since being converted from Attributes.Object + Dirty bool +} + +// WebhookInvocation describes how to call a webhook, including the resource and subresource the webhook registered for, +// and the kind that should be sent to the webhook. +type WebhookInvocation struct { + Webhook *v1beta1.Webhook + + Resource schema.GroupVersionResource + Subresource string + Kind schema.GroupVersionKind } // Dispatcher dispatches webhook call to a list of webhooks with admission attributes as argument. type Dispatcher interface { // Dispatch a request to the webhooks using the given webhooks. A non-nil error means the request is rejected. - Dispatch(ctx context.Context, a *VersionedAttributes, o admission.ObjectInterfaces, hooks []*v1beta1.Webhook) error + Dispatch(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces, hooks []*WebhookInvocation) error } 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 c44f67346c2..92c3de2caa1 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 @@ -125,21 +125,34 @@ func (a *Webhook) ValidateInitialization() error { return nil } -// ShouldCallHook makes a decision on whether to call the webhook or not by the attribute. -func (a *Webhook) ShouldCallHook(h *v1beta1.Webhook, attr admission.Attributes) (bool, *apierrors.StatusError) { - var matches bool +// shouldCallHook returns invocation details if the webhook should be called, nil if the webhook should not be called, +// or an error if an error was encountered during evaluation. +func (a *Webhook) shouldCallHook(h *v1beta1.Webhook, attr admission.Attributes) (*WebhookInvocation, *apierrors.StatusError) { + var err *apierrors.StatusError + var invocation *WebhookInvocation for _, r := range h.Rules { m := rules.Matcher{Rule: r, Attr: attr} if m.Matches() { - matches = true + invocation = &WebhookInvocation{ + Webhook: h, + Resource: attr.GetResource(), + Subresource: attr.GetSubresource(), + Kind: attr.GetKind(), + } break } } - if !matches { - return false, nil + + if invocation == nil { + return nil, nil } - return a.namespaceMatcher.MatchNamespaceSelector(h, attr) + matches, err := a.namespaceMatcher.MatchNamespaceSelector(h, attr) + if !matches || err != nil { + return nil, err + } + + return invocation, nil } // Dispatch is called by the downstream Validate or Admit methods. @@ -154,14 +167,14 @@ func (a *Webhook) Dispatch(attr admission.Attributes, o admission.ObjectInterfac // TODO: Figure out if adding one second timeout make sense here. ctx := context.TODO() - var relevantHooks []*v1beta1.Webhook + var relevantHooks []*WebhookInvocation for i := range hooks { - call, err := a.ShouldCallHook(&hooks[i], attr) + invocation, err := a.shouldCallHook(&hooks[i], attr) if err != nil { return err } - if call { - relevantHooks = append(relevantHooks, &hooks[i]) + if invocation != nil { + relevantHooks = append(relevantHooks, invocation) } } @@ -170,23 +183,5 @@ func (a *Webhook) Dispatch(attr admission.Attributes, o admission.ObjectInterfac return nil } - // convert the object to the external version before sending it to the webhook - versionedAttr := VersionedAttributes{ - Attributes: attr, - } - if oldObj := attr.GetOldObject(); oldObj != nil { - out, err := ConvertToGVK(oldObj, attr.GetKind(), o) - if err != nil { - return apierrors.NewInternalError(err) - } - versionedAttr.VersionedOldObject = out - } - if obj := attr.GetObject(); obj != nil { - out, err := ConvertToGVK(obj, attr.GetKind(), o) - if err != nil { - return apierrors.NewInternalError(err) - } - versionedAttr.VersionedObject = out - } - return a.dispatcher.Dispatch(ctx, &versionedAttr, o, relevantHooks) + return a.dispatcher.Dispatch(ctx, attr, o, relevantHooks) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index 1a6530395f4..903afb8753b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD @@ -33,25 +33,14 @@ go_library( go_test( name = "go_default_test", - srcs = [ - "dispatcher_test.go", - "plugin_test.go", - ], + srcs = ["plugin_test.go"], embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/generic:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/apis/example:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/apis/example2/v1:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", - "//vendor/github.com/stretchr/testify/require:go_default_library", ], ) 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 80449dc6106..33190ff1214 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 @@ -55,11 +55,26 @@ func newMutatingDispatcher(p *Plugin) func(cm *webhook.ClientManager) generic.Di var _ generic.Dispatcher = &mutatingDispatcher{} -func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr *generic.VersionedAttributes, o admission.ObjectInterfaces, relevantHooks []*v1beta1.Webhook) error { - for _, hook := range relevantHooks { +func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces, relevantHooks []*generic.WebhookInvocation) error { + var versionedAttr *generic.VersionedAttributes + for _, invocation := range relevantHooks { + hook := invocation.Webhook + if versionedAttr == nil { + // First webhook, create versioned attributes + var err error + if versionedAttr, err = generic.NewVersionedAttributes(attr, invocation.Kind, o); err != nil { + return apierrors.NewInternalError(err) + } + } else { + // Subsequent webhook, convert existing versioned attributes to this webhook's version + if err := generic.ConvertVersionedAttributes(versionedAttr, invocation.Kind, o); err != nil { + return apierrors.NewInternalError(err) + } + } + t := time.Now() - err := a.callAttrMutatingHook(ctx, hook, attr, o) - admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr.Attributes, "admit", hook.Name) + err := a.callAttrMutatingHook(ctx, invocation, versionedAttr, o) + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "admit", hook.Name) if err == nil { continue } @@ -77,16 +92,17 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr *generic.Version return err } - // convert attr.VersionedObject to the internal version in the underlying admission.Attributes - if attr.VersionedObject != nil { - return o.GetObjectConvertor().Convert(attr.VersionedObject, attr.Attributes.GetObject(), nil) + // convert versionedAttr.VersionedObject to the internal version in the underlying admission.Attributes + if versionedAttr != nil && versionedAttr.VersionedObject != nil && versionedAttr.Dirty { + return o.GetObjectConvertor().Convert(versionedAttr.VersionedObject, versionedAttr.Attributes.GetObject(), nil) } return nil } // note that callAttrMutatingHook updates attr -func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.Webhook, attr *generic.VersionedAttributes, o admission.ObjectInterfaces) error { - if attr.IsDryRun() { +func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes, o admission.ObjectInterfaces) error { + h := invocation.Webhook + if attr.Attributes.IsDryRun() { if h.SideEffects == nil { return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")} } @@ -102,7 +118,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta } // Make the webhook request - request := request.CreateAdmissionReview(attr) + request := request.CreateAdmissionReview(attr, invocation) client, err := a.cm.HookClient(util.HookClientConfigForWebhook(h)) if err != nil { return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: err} @@ -122,7 +138,7 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta for k, v := range response.Response.AuditAnnotations { key := h.Name + "/" + k - if err := attr.AddAnnotation(key, v); err != nil { + 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) } } @@ -164,16 +180,17 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta // They are represented as Unstructured. newVersionedObject = &unstructured.Unstructured{} } else { - newVersionedObject, err = o.GetObjectCreater().New(attr.GetKind()) + newVersionedObject, err = o.GetObjectCreater().New(attr.VersionedKind) if err != nil { return apierrors.NewInternalError(err) } } // TODO: if we have multiple mutating webhooks, we can remember the json // instead of encoding and decoding for each one. - if _, _, err := jsonSerializer.Decode(patchedJS, nil, newVersionedObject); err != nil { + if newVersionedObject, _, err = jsonSerializer.Decode(patchedJS, nil, newVersionedObject); err != nil { return apierrors.NewInternalError(err) } + attr.Dirty = true attr.VersionedObject = newVersionedObject o.GetObjectDefaulter().Default(attr.VersionedObject) return nil diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher_test.go deleted file mode 100644 index 2e7268532fd..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher_test.go +++ /dev/null @@ -1,137 +0,0 @@ -/* -Copyright 2018 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 mutating - -import ( - "context" - "reflect" - "testing" - - "github.com/stretchr/testify/require" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/admission/plugin/webhook/generic" - "k8s.io/apiserver/pkg/apis/example" - examplev1 "k8s.io/apiserver/pkg/apis/example/v1" - example2v1 "k8s.io/apiserver/pkg/apis/example2/v1" -) - -var sampleCRD = unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "mygroup.k8s.io/v1", - "kind": "Flunder", - "data": map[string]interface{}{ - "Key": "Value", - }, - }, -} - -func TestDispatch(t *testing.T) { - scheme := runtime.NewScheme() - require.NoError(t, example.AddToScheme(scheme)) - require.NoError(t, examplev1.AddToScheme(scheme)) - require.NoError(t, example2v1.AddToScheme(scheme)) - objectInterfaces := admission.NewObjectInterfacesFromScheme(scheme) - - tests := []struct { - name string - in runtime.Object - out runtime.Object - expectedObj runtime.Object - }{ - { - name: "convert example/v1#Pod to example#Pod", - in: &examplev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - Labels: map[string]string{ - "key": "value", - }, - }, - Spec: examplev1.PodSpec{ - RestartPolicy: examplev1.RestartPolicy("never"), - }, - }, - out: &example.Pod{}, - expectedObj: &example.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod1", - Labels: map[string]string{ - "key": "value", - }, - }, - Spec: example.PodSpec{ - RestartPolicy: example.RestartPolicy("never"), - }, - }, - }, - { - name: "convert example2/v1#replicaset to example#replicaset", - in: &example2v1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rs1", - Labels: map[string]string{ - "key": "value", - }, - }, - Spec: example2v1.ReplicaSetSpec{ - Replicas: func() *int32 { var i int32; i = 1; return &i }(), - }, - }, - out: &example.ReplicaSet{}, - expectedObj: &example.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "rs1", - Labels: map[string]string{ - "key": "value", - }, - }, - Spec: example.ReplicaSetSpec{ - Replicas: 1, - }, - }, - }, - { - name: "no conversion if the object is the same", - in: &sampleCRD, - out: &sampleCRD, - expectedObj: &sampleCRD, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - a := &mutatingDispatcher{ - plugin: &Plugin{}, - } - attr := generic.VersionedAttributes{ - Attributes: admission.NewAttributesRecord(test.out, nil, schema.GroupVersionKind{}, "", "", schema.GroupVersionResource{}, "", admission.Operation(""), nil, false, nil), - VersionedOldObject: nil, - VersionedObject: test.in, - } - if err := a.Dispatch(context.TODO(), &attr, objectInterfaces, nil); err != nil { - t.Fatalf("%s: unexpected error: %v", test.name, err) - } - if !reflect.DeepEqual(attr.Attributes.GetObject(), test.expectedObj) { - t.Errorf("\nexpected:\n%#v\ngot:\n %#v\n", test.expectedObj, test.out) - } - }) - } -} 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 51a45a36a70..46e2854fb9a 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 @@ -26,9 +26,14 @@ import ( ) // CreateAdmissionReview creates an AdmissionReview for the provided admission.Attributes -func CreateAdmissionReview(attr *generic.VersionedAttributes) admissionv1beta1.AdmissionReview { - gvk := attr.GetKind() - gvr := attr.GetResource() +func CreateAdmissionReview(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), @@ -56,16 +61,27 @@ func CreateAdmissionReview(attr *generic.VersionedAttributes) admissionv1beta1.A Resource: gvr.Resource, Version: gvr.Version, }, - SubResource: attr.GetSubresource(), - Name: attr.GetName(), - Namespace: attr.GetNamespace(), - Operation: admissionv1beta1.Operation(attr.GetOperation()), - UserInfo: userInfo, + 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: admissionv1beta1.Operation(attr.GetOperation()), + UserInfo: userInfo, Object: runtime.RawExtension{ - Object: attr.VersionedObject, + Object: versionedAttributes.VersionedObject, }, OldObject: runtime.RawExtension{ - Object: attr.VersionedOldObject, + Object: versionedAttributes.VersionedOldObject, }, DryRun: &dryRun, Options: runtime.RawExtension{ diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index f9f1c668eb7..b005e484bd7 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -14,6 +14,7 @@ go_library( "//staging/src/k8s.io/api/admission/v1beta1:go_default_library", "//staging/src/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/configuration:go_default_library", 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 7889b4f5da4..d7421e2b53b 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 @@ -27,6 +27,7 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apiserver/pkg/admission" admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" @@ -47,17 +48,33 @@ func newValidatingDispatcher(cm *webhook.ClientManager) generic.Dispatcher { var _ generic.Dispatcher = &validatingDispatcher{} -func (d *validatingDispatcher) Dispatch(ctx context.Context, attr *generic.VersionedAttributes, o admission.ObjectInterfaces, relevantHooks []*v1beta1.Webhook) error { +func (d *validatingDispatcher) Dispatch(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces, relevantHooks []*generic.WebhookInvocation) error { + // Construct all the versions we need to call our webhooks + versionedAttrs := map[schema.GroupVersionKind]*generic.VersionedAttributes{} + for _, call := range relevantHooks { + // If we already have this version, continue + if _, ok := versionedAttrs[call.Kind]; ok { + continue + } + versionedAttr, err := generic.NewVersionedAttributes(attr, call.Kind, o) + if err != nil { + return apierrors.NewInternalError(err) + } + versionedAttrs[call.Kind] = versionedAttr + } + wg := sync.WaitGroup{} errCh := make(chan error, len(relevantHooks)) wg.Add(len(relevantHooks)) for i := range relevantHooks { - go func(hook *v1beta1.Webhook) { + go func(invocation *generic.WebhookInvocation) { defer wg.Done() + hook := invocation.Webhook + versionedAttr := versionedAttrs[invocation.Kind] t := time.Now() - err := d.callHook(ctx, hook, attr) - admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, attr.Attributes, "validating", hook.Name) + err := d.callHook(ctx, invocation, versionedAttr) + admissionmetrics.Metrics.ObserveWebhook(time.Since(t), err != nil, versionedAttr.Attributes, "validating", hook.Name) if err == nil { return } @@ -98,8 +115,9 @@ func (d *validatingDispatcher) Dispatch(ctx context.Context, attr *generic.Versi return errs[0] } -func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, attr *generic.VersionedAttributes) error { - if attr.IsDryRun() { +func (d *validatingDispatcher) callHook(ctx context.Context, invocation *generic.WebhookInvocation, attr *generic.VersionedAttributes) error { + h := invocation.Webhook + if attr.Attributes.IsDryRun() { if h.SideEffects == nil { return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")} } @@ -115,7 +133,7 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, } // Make the webhook request - request := request.CreateAdmissionReview(attr) + request := request.CreateAdmissionReview(attr, invocation) client, err := d.cm.HookClient(util.HookClientConfigForWebhook(h)) if err != nil { return &webhook.ErrCallingWebhook{WebhookName: h.Name, Reason: err} @@ -134,7 +152,7 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, } for k, v := range response.Response.AuditAnnotations { key := h.Name + "/" + k - if err := attr.AddAnnotation(key, v); err != nil { + 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) } }