diff --git a/pkg/apis/admission/types.go b/pkg/apis/admission/types.go index 16cbd586955..fb704abdf90 100644 --- a/pkg/apis/admission/types.go +++ b/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. diff --git a/pkg/apis/admissionregistration/fuzzer/fuzzer.go b/pkg/apis/admissionregistration/fuzzer/fuzzer.go index 4275951b514..ba4f2c5e5bf 100644 --- a/pkg/apis/admissionregistration/fuzzer/fuzzer.go +++ b/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 }, } } diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index 1d71bc6947a..511bdd96d1c 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/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 diff --git a/pkg/apis/admissionregistration/v1beta1/defaults.go b/pkg/apis/admissionregistration/v1beta1/defaults.go index 907f7d9f31d..fa35267624d 100644 --- a/pkg/apis/admissionregistration/v1beta1/defaults.go +++ b/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 + } } diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 958ebf44022..14d3d799bf4 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/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), diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 5dd3fc551b7..2cf41fa5979 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -499,6 +499,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( diff --git a/staging/src/k8s.io/api/admission/v1beta1/types.go b/staging/src/k8s.io/api/admission/v1beta1/types.go index a64ec211154..653e847107f 100644 --- a/staging/src/k8s.io/api/admission/v1beta1/types.go +++ b/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. diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go index f209e7accc2..0b948ba1df9 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go +++ b/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 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 048afb3569f..c69e0159825 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 @@ -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 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 663349a4e4b..cec41315c2b 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 @@ -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, }, } } 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 b6ea69480c3..30af14e74f3 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 @@ -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,26 +393,66 @@ 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{{ @@ -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", 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 89884e8e1c3..8b6214e2830 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 @@ -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