Support dry run in admission webhooks

pkg/apis/admission/types.go

@@ -73,6 +73,11 @@ type AdmissionRequest struct {
 	// OldObject is the existing object. Only populated for UPDATE requests.
 	// +optional
 	OldObject runtime.Object
+	// DryRun indicates that modifications will definitely not be persisted for this request.
+	// Calls to webhooks must have no side effects if DryRun is true.
+	// Defaults to false.
+	// +optional
+	DryRun *bool
 }
 
 // AdmissionResponse describes an admission response.

pkg/apis/admissionregistration/fuzzer/fuzzer.go

@@ -30,6 +30,8 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 			c.FuzzNoCustom(obj) // fuzz self without calling this function again
 			p := admissionregistration.FailurePolicyType("Fail")
 			obj.FailurePolicy = &p
+			s := admissionregistration.SideEffectClassUnknown
+			obj.SideEffects = &s
 		},
 	}
 }

pkg/apis/admissionregistration/types.go

@@ -112,6 +112,22 @@ const (
 	Fail FailurePolicyType = "Fail"
 )
 
+type SideEffectClass string
+
+const (
+	// SideEffectClassUnknown means that no information is known about the side effects of calling the webhook.
+	// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
+	SideEffectClassUnknown SideEffectClass = "Unknown"
+	// SideEffectClassNone means that calling the webhook will have no side effects.
+	SideEffectClassNone SideEffectClass = "None"
+	// SideEffectClassSome means that calling the webhook will possibly have side effects.
+	// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
+	SideEffectClassSome SideEffectClass = "Some"
+	// SideEffectClassNoneOnDryRun means that calling the webhook will possibly have side effects, but if the
+	// request being reviewed has the dry-run attribute, the side effects will be suppressed.
+	SideEffectClassNoneOnDryRun SideEffectClass = "NoneOnDryRun"
+)
+
 // +genclient
 // +genclient:nonNamespaced
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -235,6 +251,15 @@ type Webhook struct {
 	// Default to the empty LabelSelector, which matches everything.
 	// +optional
 	NamespaceSelector *metav1.LabelSelector
+
+	// SideEffects states whether this webhookk has side effects.
+	// Acceptable values are: Unknown, None, Some, NoneOnDryRun
+	// Webhooks with side effects MUST implement a reconciliation system, since a request may be
+	// rejected by a future step in the admission change and the side effects therefore need to be undone.
+	// Requests with the dryRun attribute will be auto-rejected if they match a webhook with
+	// sideEffects == Unknown or Some. Defaults to Unknown.
+	// +optional
+	SideEffects *SideEffectClass
 }
 
 // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make

pkg/apis/admissionregistration/v1beta1/defaults.go

@@ -35,4 +35,9 @@ func SetDefaults_Webhook(obj *admissionregistrationv1beta1.Webhook) {
 		selector := metav1.LabelSelector{}
 		obj.NamespaceSelector = &selector
 	}
+	if obj.SideEffects == nil {
+		// TODO: revisit/remove this default and possibly make the field required when promoting to v1
+		unknown := admissionregistrationv1beta1.SideEffectClassUnknown
+		obj.SideEffects = &unknown
+	}
 }

pkg/apis/admissionregistration/validation/validation.go

@@ -192,6 +192,9 @@ func validateWebhook(hook *admissionregistration.Webhook, fldPath *field.Path) f
 	if hook.FailurePolicy != nil && !supportedFailurePolicies.Has(string(*hook.FailurePolicy)) {
 		allErrors = append(allErrors, field.NotSupported(fldPath.Child("failurePolicy"), *hook.FailurePolicy, supportedFailurePolicies.List()))
 	}
+	if hook.SideEffects != nil && !supportedSideEffectClasses.Has(string(*hook.SideEffects)) {
+		allErrors = append(allErrors, field.NotSupported(fldPath.Child("sideEffects"), *hook.SideEffects, supportedSideEffectClasses.List()))
+	}
 
 	if hook.NamespaceSelector != nil {
 		allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
@@ -291,6 +294,13 @@ var supportedFailurePolicies = sets.NewString(
 	string(admissionregistration.Fail),
 )
 
+var supportedSideEffectClasses = sets.NewString(
+	string(admissionregistration.SideEffectClassUnknown),
+	string(admissionregistration.SideEffectClassNone),
+	string(admissionregistration.SideEffectClassSome),
+	string(admissionregistration.SideEffectClassNoneOnDryRun),
+)
+
 var supportedOperations = sets.NewString(
 	string(admissionregistration.OperationAll),
 	string(admissionregistration.Create),

pkg/apis/admissionregistration/validation/validation_test.go

@@ -500,6 +500,21 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 			expectedError: `webhooks[0].failurePolicy: Unsupported value: "other": supported values: "Fail", "Ignore"`,
 		},
 		{
+			name: "SideEffects can only be \"Unknown\", \"None\", \"Some\", or \"NoneOnDryRun\"",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name:         "webhook.k8s.io",
+						ClientConfig: validClientConfig,
+						SideEffects: func() *admissionregistration.SideEffectClass {
+							r := admissionregistration.SideEffectClass("other")
+							return &r
+						}(),
+					},
+				}),
+			expectedError: `webhooks[0].sideEffects: Unsupported value: "other": supported values: "None", "NoneOnDryRun", "Some", "Unknown"`,
+		},
+		{
 			name: "both service and URL missing",
 			config: newValidatingWebhookConfiguration(
 				[]admissionregistration.Webhook{

staging/src/k8s.io/api/admission/v1beta1/types.go

@@ -71,6 +71,10 @@ type AdmissionRequest struct {
 	// OldObject is the existing object. Only populated for UPDATE requests.
 	// +optional
 	OldObject runtime.RawExtension `json:"oldObject,omitempty" protobuf:"bytes,10,opt,name=oldObject"`
+	// DryRun indicates that modifications will definitely not be persisted for this request.
+	// Defaults to false.
+	// +optional
+	DryRun *bool `json:"dryRun,omitempty" protobuf:"varint,11,opt,name=dryRun"`
 }
 
 // AdmissionResponse describes an admission response.

staging/src/k8s.io/api/admissionregistration/v1beta1/types.go

@@ -60,6 +60,22 @@ const (
 	Fail FailurePolicyType = "Fail"
 )
 
+type SideEffectClass string
+
+const (
+	// SideEffectClassUnknown means that no information is known about the side effects of calling the webhook.
+	// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
+	SideEffectClassUnknown SideEffectClass = "Unknown"
+	// SideEffectClassNone means that calling the webhook will have no side effects.
+	SideEffectClassNone SideEffectClass = "None"
+	// SideEffectClassSome means that calling the webhook will possibly have side effects.
+	// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
+	SideEffectClassSome SideEffectClass = "Some"
+	// SideEffectClassNoneOnDryRun means that calling the webhook will possibly have side effects, but if the
+	// request being reviewed has the dry-run attribute, the side effects will be suppressed.
+	SideEffectClassNoneOnDryRun SideEffectClass = "NoneOnDryRun"
+)
+
 // +genclient
 // +genclient:nonNamespaced
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
@@ -191,6 +207,15 @@ type Webhook struct {
 	// Default to the empty LabelSelector, which matches everything.
 	// +optional
 	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty" protobuf:"bytes,5,opt,name=namespaceSelector"`
+
+	// SideEffects states whether this webhookk has side effects.
+	// Acceptable values are: Unknown, None, Some, NoneOnDryRun
+	// Webhooks with side effects MUST implement a reconciliation system, since a request may be
+	// rejected by a future step in the admission change and the side effects therefore need to be undone.
+	// Requests with the dryRun attribute will be auto-rejected if they match a webhook with
+	// sideEffects == Unknown or Some. Defaults to Unknown.
+	// +optional
+	SideEffects *SideEffectClass `json:"sideEffects,omitempty" protobuf:"bytes,6,opt,name=sideEffects,casttype=SideEffectClass"`
 }
 
 // RuleWithOperations is a tuple of Operations and Resources. It is recommended to make

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/dispatcher.go

@@ -83,8 +83,12 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr *generic.Version
 // note that callAttrMutatingHook updates attr
 func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta1.Webhook, attr *generic.VersionedAttributes) error {
 	if attr.IsDryRun() {
-		// TODO: support this
-		return webhookerrors.NewDryRunUnsupportedErr(h.Name)
+		if h.SideEffects == nil {
+			return &webhookerrors.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")}
+		}
+		if !(*h.SideEffects == v1beta1.SideEffectClassNone || *h.SideEffects == v1beta1.SideEffectClassNoneOnDryRun) {
+			return webhookerrors.NewDryRunUnsupportedErr(h.Name)
+		}
 	}
 
 	// Make the webhook request

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/request/admissionreview.go

@@ -36,6 +36,7 @@ func CreateAdmissionReview(attr *generic.VersionedAttributes) admissionv1beta1.A
 		UID:      aUserInfo.GetUID(),
 		Username: aUserInfo.GetName(),
 	}
+	dryRun := attr.IsDryRun()
 
 	// Convert the extra information in the user object
 	for key, val := range aUserInfo.GetExtra() {
@@ -66,6 +67,7 @@ func CreateAdmissionReview(attr *generic.VersionedAttributes) admissionv1beta1.A
 			OldObject: runtime.RawExtension{
 				Object: attr.VersionedOldObject,
 			},
+			DryRun: &dryRun,
 		},
 	}
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/testcase.go

@@ -43,6 +43,11 @@ var matchEverythingRules = []registrationv1beta1.RuleWithOperations{{
 	},
 }}
 
+var sideEffectsUnknown registrationv1beta1.SideEffectClass = registrationv1beta1.SideEffectClassUnknown
+var sideEffectsNone registrationv1beta1.SideEffectClass = registrationv1beta1.SideEffectClassNone
+var sideEffectsSome registrationv1beta1.SideEffectClass = registrationv1beta1.SideEffectClassSome
+var sideEffectsNoneOnDryRun registrationv1beta1.SideEffectClass = registrationv1beta1.SideEffectClassNoneOnDryRun
+
 // NewFakeDataSource returns a mock client and informer returning the given webhooks.
 func NewFakeDataSource(name string, webhooks []registrationv1beta1.Webhook, mutating bool, stopCh <-chan struct{}) (clientset kubernetes.Interface, factory informers.SharedInformerFactory) {
 	var objs = []runtime.Object{
@@ -388,27 +393,67 @@ func NewNonMutatingTestCases(url *url.URL) []Test {
 			Name: "no match dry run",
 			Webhooks: []registrationv1beta1.Webhook{{
 				Name:         "nomatch",
-				ClientConfig: ccfgSVC("disallow"),
+				ClientConfig: ccfgSVC("allow"),
 				Rules: []registrationv1beta1.RuleWithOperations{{
 					Operations: []registrationv1beta1.OperationType{registrationv1beta1.Create},
 				}},
 				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsSome,
 			}},
 			IsDryRun:    true,
 			ExpectAllow: true,
 		},
 		{
-			Name: "match dry run",
+			Name: "match dry run side effects Unknown",
+			Webhooks: []registrationv1beta1.Webhook{{
+				Name:              "allow",
+				ClientConfig:      ccfgSVC("allow"),
+				Rules:             matchEverythingRules,
+				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsUnknown,
+			}},
+			IsDryRun:      true,
+			ErrorContains: "does not support dry run",
+		},
+		{
+			Name: "match dry run side effects None",
+			Webhooks: []registrationv1beta1.Webhook{{
+				Name:              "allow",
+				ClientConfig:      ccfgSVC("allow"),
+				Rules:             matchEverythingRules,
+				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsNone,
+			}},
+			IsDryRun:          true,
+			ExpectAllow:       true,
+			ExpectAnnotations: map[string]string{"allow/key1": "value1"},
+		},
+		{
+			Name: "match dry run side effects Some",
 			Webhooks: []registrationv1beta1.Webhook{{
 				Name:              "allow",
 				ClientConfig:      ccfgSVC("allow"),
 				Rules:             matchEverythingRules,
 				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsSome,
 			}},
 			IsDryRun:      true,
 			ErrorContains: "does not support dry run",
 		},
 		{
+			Name: "match dry run side effects NoneOnDryRun",
+			Webhooks: []registrationv1beta1.Webhook{{
+				Name:              "allow",
+				ClientConfig:      ccfgSVC("allow"),
+				Rules:             matchEverythingRules,
+				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsNoneOnDryRun,
+			}},
+			IsDryRun:          true,
+			ExpectAllow:       true,
+			ExpectAnnotations: map[string]string{"allow/key1": "value1"},
+		},
+		{
 			Name: "illegal annotation format",
 			Webhooks: []registrationv1beta1.Webhook{{
 				Name:              "invalidAnnotation",
@@ -489,12 +534,13 @@ func NewMutatingTestCases(url *url.URL) []Test {
 			ErrorContains: "invalid character",
 		},
 		{
-			Name: "match & remove label dry run",
+			Name: "match & remove label dry run unsupported",
 			Webhooks: []registrationv1beta1.Webhook{{
 				Name:              "removeLabel",
 				ClientConfig:      ccfgSVC("removeLabel"),
 				Rules:             matchEverythingRules,
 				NamespaceSelector: &metav1.LabelSelector{},
+				SideEffects:       &sideEffectsUnknown,
 			}},
 			IsDryRun:      true,
 			ErrorContains: "does not support dry run",

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/dispatcher.go

@@ -98,8 +98,12 @@ 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
-		return webhookerrors.NewDryRunUnsupportedErr(h.Name)
+		if h.SideEffects == nil {
+			return &webhookerrors.ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Webhook SideEffects is nil")}
+		}
+		if !(*h.SideEffects == v1beta1.SideEffectClassNone || *h.SideEffects == v1beta1.SideEffectClassNoneOnDryRun) {
+			return webhookerrors.NewDryRunUnsupportedErr(h.Name)
+		}
 	}
 
 	// Make the webhook request