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 64e6851f8f..0bda44562a 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 @@ -84,7 +84,7 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr *generic.Version func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.Webhook, attr *generic.VersionedAttributes) error { if attr.IsDryRun() { // TODO: support this - webhookerrors.NewDryRunUnsupportedErr(h.Name) + return webhookerrors.NewDryRunUnsupportedErr(h.Name) } // Make the webhook request diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go index ba8f44a661..165a8c8947 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/plugin_test.go @@ -79,9 +79,9 @@ func TestAdmit(t *testing.T) { var attr admission.Attributes if tt.IsCRD { - attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels) + attr = webhooktesting.NewAttributeUnstructured(ns, tt.AdditionalLabels, tt.IsDryRun) } else { - attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels) + attr = webhooktesting.NewAttribute(ns, tt.AdditionalLabels, tt.IsDryRun) } err = wh.Admit(attr) @@ -147,7 +147,7 @@ func TestAdmitCachedClient(t *testing.T) { continue } - err = wh.Admit(webhooktesting.NewAttribute(ns, nil)) + err = wh.Admit(webhooktesting.NewAttribute(ns, nil, false)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } 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 453ba11ee4..5fa385680e 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 @@ -76,7 +76,7 @@ func NewFakeDataSource(name string, webhooks []registrationv1beta1.Webhook, muta return client, informerFactory } -func newAttributesRecord(object metav1.Object, oldObject metav1.Object, kind schema.GroupVersionKind, namespace string, name string, resource string, labels map[string]string) admission.Attributes { +func newAttributesRecord(object metav1.Object, oldObject metav1.Object, kind schema.GroupVersionKind, namespace string, name string, resource string, labels map[string]string, dryRun bool) admission.Attributes { object.SetName(name) object.SetNamespace(namespace) objectLabels := map[string]string{resource + ".name": name} @@ -95,11 +95,11 @@ func newAttributesRecord(object metav1.Object, oldObject metav1.Object, kind sch UID: "webhook-test", } - return admission.NewAttributesRecord(object.(runtime.Object), oldObject.(runtime.Object), kind, namespace, name, gvr, subResource, admission.Update, false, &userInfo) + return admission.NewAttributesRecord(object.(runtime.Object), oldObject.(runtime.Object), kind, namespace, name, gvr, subResource, admission.Update, dryRun, &userInfo) } // NewAttribute returns static admission Attributes for testing. -func NewAttribute(namespace string, labels map[string]string) admission.Attributes { +func NewAttribute(namespace string, labels map[string]string, dryRun bool) admission.Attributes { // Set up a test object for the call object := corev1.Pod{ TypeMeta: metav1.TypeMeta{ @@ -111,11 +111,11 @@ func NewAttribute(namespace string, labels map[string]string) admission.Attribut kind := corev1.SchemeGroupVersion.WithKind("Pod") name := "my-pod" - return newAttributesRecord(&object, &oldObject, kind, namespace, name, "pod", labels) + return newAttributesRecord(&object, &oldObject, kind, namespace, name, "pod", labels, dryRun) } // NewAttributeUnstructured returns static admission Attributes for testing with custom resources. -func NewAttributeUnstructured(namespace string, labels map[string]string) admission.Attributes { +func NewAttributeUnstructured(namespace string, labels map[string]string, dryRun bool) admission.Attributes { // Set up a test object for the call object := unstructured.Unstructured{} object.SetKind("TestCRD") @@ -126,7 +126,7 @@ func NewAttributeUnstructured(namespace string, labels map[string]string) admiss kind := object.GroupVersionKind() name := "my-test-crd" - return newAttributesRecord(&object, &oldObject, kind, namespace, name, "crd", labels) + return newAttributesRecord(&object, &oldObject, kind, namespace, name, "crd", labels, dryRun) } type urlConfigGenerator struct { @@ -149,6 +149,7 @@ type Test struct { Webhooks []registrationv1beta1.Webhook Path string IsCRD bool + IsDryRun bool AdditionalLabels map[string]string ExpectLabels map[string]string ExpectAllow bool @@ -349,6 +350,30 @@ func NewNonMutatingTestCases(url *url.URL) []Test { }}, ErrorContains: "Webhook response was absent", }, + { + Name: "no match dry run", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "nomatch", + ClientConfig: ccfgSVC("disallow"), + Rules: []registrationv1beta1.RuleWithOperations{{ + Operations: []registrationv1beta1.OperationType{registrationv1beta1.Create}, + }}, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + IsDryRun: true, + ExpectAllow: true, + }, + { + Name: "match dry run", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "allow", + ClientConfig: ccfgSVC("allow"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + IsDryRun: true, + ErrorContains: "does not support dry run", + }, // No need to test everything with the url case, since only the // connection is different. } @@ -417,6 +442,17 @@ func NewMutatingTestCases(url *url.URL) []Test { }}, ErrorContains: "invalid character", }, + { + Name: "match & remove label dry run", + Webhooks: []registrationv1beta1.Webhook{{ + Name: "removeLabel", + ClientConfig: ccfgSVC("removeLabel"), + Rules: matchEverythingRules, + NamespaceSelector: &metav1.LabelSelector{}, + }}, + IsDryRun: true, + ErrorContains: "does not support dry run", + }, // No need to test everything with the url case, since only the // connection is different. } 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 74fd18680b..28cfb23b03 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 @@ -99,7 +99,7 @@ func (d *validatingDispatcher) Dispatch(ctx context.Context, attr *generic.Versi func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Webhook, attr *generic.VersionedAttributes) error { if attr.IsDryRun() { // TODO: support this - webhookerrors.NewDryRunUnsupportedErr(h.Name) + return webhookerrors.NewDryRunUnsupportedErr(h.Name) } // Make the webhook request diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go index 95b7d77bd9..60d7d9c368 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/plugin_test.go @@ -72,7 +72,7 @@ func TestValidate(t *testing.T) { continue } - err = wh.Validate(webhooktesting.NewAttribute(ns, nil)) + err = wh.Validate(webhooktesting.NewAttribute(ns, nil, tt.IsDryRun)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) } @@ -130,7 +130,7 @@ func TestValidateCachedClient(t *testing.T) { continue } - err = wh.Validate(webhooktesting.NewAttribute(ns, nil)) + err = wh.Validate(webhooktesting.NewAttribute(ns, nil, false)) if tt.ExpectAllow != (err == nil) { t.Errorf("%s: expected allowed=%v, but got err=%v", tt.Name, tt.ExpectAllow, err) }