diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index c2cbd13cc72..78209889d49 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -22,6 +22,7 @@ import ( genericvalidation "k8s.io/apimachinery/pkg/api/validation" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" @@ -194,28 +195,44 @@ func validateAdmissionReviewVersions(versions []string, requireRecognizedVersion return allErrors } -func ValidateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration) field.ErrorList { - return validateValidatingWebhookConfiguration(e, true) +// ValidateValidatingWebhookConfiguration validates a webhook before creation. +func ValidateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration, requestGV schema.GroupVersion) field.ErrorList { + return validateValidatingWebhookConfiguration(e, true, requireUniqueWebhookNames(requestGV)) } -func validateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration, requireRecognizedVersion bool) field.ErrorList { +func validateValidatingWebhookConfiguration(e *admissionregistration.ValidatingWebhookConfiguration, requireRecognizedVersion, requireUniqueWebhookNames bool) field.ErrorList { allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata")) + hookNames := sets.NewString() for i, hook := range e.Webhooks { allErrors = append(allErrors, validateValidatingWebhook(&hook, field.NewPath("webhooks").Index(i))...) allErrors = append(allErrors, validateAdmissionReviewVersions(hook.AdmissionReviewVersions, requireRecognizedVersion, field.NewPath("webhooks").Index(i).Child("admissionReviewVersions"))...) + if requireUniqueWebhookNames && len(hook.Name) > 0 { + if hookNames.Has(hook.Name) { + allErrors = append(allErrors, field.Duplicate(field.NewPath("webhooks").Index(i).Child("name"), hook.Name)) + } + hookNames.Insert(hook.Name) + } } return allErrors } -func ValidateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration) field.ErrorList { - return validateMutatingWebhookConfiguration(e, true) +// ValidateMutatingWebhookConfiguration validates a webhook before creation. +func ValidateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration, requestGV schema.GroupVersion) field.ErrorList { + return validateMutatingWebhookConfiguration(e, true, requireUniqueWebhookNames(requestGV)) } -func validateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration, requireRecognizedVersion bool) field.ErrorList { +func validateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration, requireRecognizedVersion, requireUniqueWebhookNames bool) field.ErrorList { allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata")) + hookNames := sets.NewString() for i, hook := range e.Webhooks { allErrors = append(allErrors, validateMutatingWebhook(&hook, field.NewPath("webhooks").Index(i))...) allErrors = append(allErrors, validateAdmissionReviewVersions(hook.AdmissionReviewVersions, requireRecognizedVersion, field.NewPath("webhooks").Index(i).Child("admissionReviewVersions"))...) + if requireUniqueWebhookNames && len(hook.Name) > 0 { + if hookNames.Has(hook.Name) { + allErrors = append(allErrors, field.Duplicate(field.NewPath("webhooks").Index(i).Child("name"), hook.Name)) + } + hookNames.Insert(hook.Name) + } } return allErrors } @@ -403,10 +420,47 @@ func validatingHasAcceptedAdmissionReviewVersions(webhooks []admissionregistrati return true } -func ValidateValidatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.ValidatingWebhookConfiguration) field.ErrorList { - return validateValidatingWebhookConfiguration(newC, validatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks)) +// mutatingHasUniqueWebhookNames returns true if all webhooks have unique names +func mutatingHasUniqueWebhookNames(webhooks []admissionregistration.MutatingWebhook) bool { + names := sets.NewString() + for _, hook := range webhooks { + if names.Has(hook.Name) { + return false + } + names.Insert(hook.Name) + } + return true } -func ValidateMutatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.MutatingWebhookConfiguration) field.ErrorList { - return validateMutatingWebhookConfiguration(newC, mutatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks)) +// validatingHasUniqueWebhookNames returns true if all webhooks have unique names +func validatingHasUniqueWebhookNames(webhooks []admissionregistration.ValidatingWebhook) bool { + names := sets.NewString() + for _, hook := range webhooks { + if names.Has(hook.Name) { + return false + } + names.Insert(hook.Name) + } + return true +} + +func ValidateValidatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.ValidatingWebhookConfiguration, requestGV schema.GroupVersion) field.ErrorList { + return validateValidatingWebhookConfiguration( + newC, + validatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks), + requireUniqueWebhookNames(requestGV) && validatingHasUniqueWebhookNames(oldC.Webhooks), + ) +} + +func ValidateMutatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.MutatingWebhookConfiguration, requestGV schema.GroupVersion) field.ErrorList { + return validateMutatingWebhookConfiguration( + newC, + mutatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks), + requireUniqueWebhookNames(requestGV) && mutatingHasUniqueWebhookNames(oldC.Webhooks), + ) +} + +// requireUniqueWebhookNames returns true for all requests except v1beta1 (for backwards compatibility) +func requireUniqueWebhookNames(requestGV schema.GroupVersion) bool { + return requestGV != (schema.GroupVersion{Group: admissionregistration.GroupName, Version: "v1beta1"}) } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 39ff7c58b83..33a2e054f62 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -21,6 +21,7 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/kubernetes/pkg/apis/admissionregistration" ) @@ -55,6 +56,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { tests := []struct { name string config *admissionregistration.ValidatingWebhookConfiguration + gv schema.GroupVersion expectedError string }{ { @@ -145,6 +147,40 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { }, true), expectedError: `webhooks[1].name: Invalid value: "k8s.io": should be a domain with at least three segments separated by dots, webhooks[2].name: Required value`, }, + { + name: "Webhooks must have unique names when not created via v1beta1", + config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + gv: schema.GroupVersion{Group: "foo", Version: "bar"}, + expectedError: `webhooks[1].name: Duplicate value: "webhook.k8s.io"`, + }, + { + name: "Webhooks can have duplicate names when created via v1beta1", + config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + gv: schema.GroupVersion{Group: "admissionregistration.k8s.io", Version: "v1beta1"}, + expectedError: ``, + }, { name: "Operations must not be empty or nil", config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ @@ -739,7 +775,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - errs := ValidateValidatingWebhookConfiguration(test.config) + errs := ValidateValidatingWebhookConfiguration(test.config, test.gv) err := errs.ToAggregate() if err != nil { if e, a := test.expectedError, err.Error(); !strings.Contains(a, e) || e == "" { @@ -765,6 +801,7 @@ func TestValidateValidatingWebhookConfigurationUpdate(t *testing.T) { name string oldconfig *admissionregistration.ValidatingWebhookConfiguration config *admissionregistration.ValidatingWebhookConfiguration + gv schema.GroupVersion expectedError string }{ { @@ -845,10 +882,87 @@ func TestValidateValidatingWebhookConfigurationUpdate(t *testing.T) { }, false), expectedError: `Invalid value: []string{"invalid-v1"}`, }, + { + name: "Webhooks must have unique names when not updated via v1beta1", + config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, false), + gv: schema.GroupVersion{Group: "foo", Version: "bar"}, + expectedError: `webhooks[1].name: Duplicate value: "webhook.k8s.io"`, + }, + { + name: "Webhooks can have duplicate names when old config has duplicate names", + config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + gv: schema.GroupVersion{Group: "foo", Version: "bar"}, + expectedError: ``, + }, + { + name: "Webhooks can have duplicate names when updated via v1beta1", + config: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, true), + oldconfig: newValidatingWebhookConfiguration([]admissionregistration.ValidatingWebhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + SideEffects: validSideEffect, + }, + }, false), + gv: schema.GroupVersion{Group: "admissionregistration.k8s.io", Version: "v1beta1"}, + expectedError: ``, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - errs := ValidateValidatingWebhookConfigurationUpdate(test.config, test.oldconfig) + errs := ValidateValidatingWebhookConfigurationUpdate(test.config, test.oldconfig, test.gv) err := errs.ToAggregate() if err != nil { if e, a := test.expectedError, err.Error(); !strings.Contains(a, e) || e == "" { diff --git a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go index 9c476102eef..0b316abe264 100644 --- a/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go +++ b/pkg/registry/admissionregistration/mutatingwebhookconfiguration/strategy.go @@ -21,7 +21,9 @@ import ( "reflect" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/admissionregistration" @@ -63,8 +65,13 @@ func (mutatingWebhookConfigurationStrategy) PrepareForUpdate(ctx context.Context // Validate validates a new mutatingWebhookConfiguration. func (mutatingWebhookConfigurationStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { + var groupVersion schema.GroupVersion + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + } + ic := obj.(*admissionregistration.MutatingWebhookConfiguration) - return validation.ValidateMutatingWebhookConfiguration(ic) + return validation.ValidateMutatingWebhookConfiguration(ic, groupVersion) } // Canonicalize normalizes the object after validation. @@ -78,7 +85,12 @@ func (mutatingWebhookConfigurationStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (mutatingWebhookConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateMutatingWebhookConfigurationUpdate(obj.(*admissionregistration.MutatingWebhookConfiguration), old.(*admissionregistration.MutatingWebhookConfiguration)) + var groupVersion schema.GroupVersion + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + } + + return validation.ValidateMutatingWebhookConfigurationUpdate(obj.(*admissionregistration.MutatingWebhookConfiguration), old.(*admissionregistration.MutatingWebhookConfiguration), groupVersion) } // AllowUnconditionalUpdate is the default update policy for mutatingWebhookConfiguration objects. Status update should diff --git a/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go b/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go index 6100ecc0cb0..494012e3d13 100644 --- a/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go +++ b/pkg/registry/admissionregistration/validatingwebhookconfiguration/strategy.go @@ -21,7 +21,9 @@ import ( "reflect" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/admissionregistration" @@ -63,7 +65,12 @@ func (validatingWebhookConfigurationStrategy) PrepareForUpdate(ctx context.Conte // Validate validates a new validatingWebhookConfiguration. func (validatingWebhookConfigurationStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { - return validation.ValidateValidatingWebhookConfiguration(obj.(*admissionregistration.ValidatingWebhookConfiguration)) + var groupVersion schema.GroupVersion + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + } + + return validation.ValidateValidatingWebhookConfiguration(obj.(*admissionregistration.ValidatingWebhookConfiguration), groupVersion) } // Canonicalize normalizes the object after validation. @@ -77,7 +84,12 @@ func (validatingWebhookConfigurationStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (validatingWebhookConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateValidatingWebhookConfigurationUpdate(obj.(*admissionregistration.ValidatingWebhookConfiguration), old.(*admissionregistration.ValidatingWebhookConfiguration)) + var groupVersion schema.GroupVersion + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + } + + return validation.ValidateValidatingWebhookConfigurationUpdate(obj.(*admissionregistration.ValidatingWebhookConfiguration), old.(*admissionregistration.ValidatingWebhookConfiguration), groupVersion) } // AllowUnconditionalUpdate is the default update policy for validatingWebhookConfiguration objects. Status update should