diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/placement.go similarity index 81% rename from pkg/utils/validator/clusterresourceplacement.go rename to pkg/utils/validator/placement.go index 29fa95e4f..86da654b4 100644 --- a/pkg/utils/validator/clusterresourceplacement.go +++ b/pkg/utils/validator/placement.go @@ -14,24 +14,30 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package validator provides utils to validate cluster resource placement resource. +// Package validator provides utils to validate all fleet custom resources. package validator import ( + "context" "errors" "fmt" + "net/http" "sort" "strings" + admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" apiErrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/klog/v2" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" "github.com/kubefleet-dev/kubefleet/pkg/propertyprovider" @@ -48,20 +54,26 @@ var ( invalidTolerationValueErrFmt = "invalid toleration value %+v: %s" uniqueTolerationErrFmt = "toleration %+v already exists, tolerations must be unique" + // Webhook validation message format strings + AllowUpdateOldInvalidFmt = "allow update on old invalid v1beta1 %s with DeletionTimestamp set" + DenyUpdateOldInvalidFmt = "deny update on old invalid v1beta1 %s with DeletionTimestamp not set %s" + DenyCreateUpdateInvalidFmt = "deny create/update v1beta1 %s has invalid fields %s" + AllowModifyFmt = "any user is allowed to modify v1beta1 %s" + // Below is the map of supported capacity types. supportedResourceCapacityTypesMap = map[string]bool{propertyprovider.AllocatableCapacityName: true, propertyprovider.AvailableCapacityName: true, propertyprovider.TotalCapacityName: true} resourceCapacityTypes = supportedResourceCapacityTypes() ) -// ValidateClusterResourcePlacement validates a ClusterResourcePlacement object. -func ValidateClusterResourcePlacement(clusterResourcePlacement *placementv1beta1.ClusterResourcePlacement) error { +// validatePlacement validates a placement object (either ClusterResourcePlacement or ResourcePlacement). +func validatePlacement(name string, resourceSelectors []placementv1beta1.ResourceSelectorTerm, policy *placementv1beta1.PlacementPolicy, strategy placementv1beta1.RolloutStrategy, isClusterScoped bool) error { allErr := make([]error, 0) - if len(clusterResourcePlacement.Name) > validation.DNS1035LabelMaxLength { + if len(name) > validation.DNS1035LabelMaxLength { allErr = append(allErr, fmt.Errorf("the name field cannot have length exceeding %d", validation.DNS1035LabelMaxLength)) } - for _, selector := range clusterResourcePlacement.Spec.ResourceSelectors { + for _, selector := range resourceSelectors { if selector.LabelSelector != nil { if len(selector.Name) != 0 { allErr = append(allErr, fmt.Errorf("the labelSelector and name fields are mutually exclusive in selector %+v", selector)) @@ -84,9 +96,15 @@ func ValidateClusterResourcePlacement(clusterResourcePlacement *placementv1beta1 Version: selector.Version, Kind: selector.Kind, } - if !ResourceInformer.IsClusterScopedResources(gvk) { + // Only check cluster scope for ClusterResourcePlacement + if isClusterScoped && !ResourceInformer.IsClusterScopedResources(gvk) { allErr = append(allErr, fmt.Errorf("the resource is not found in schema (please retry) or it is not a cluster scoped resource: %v", gvk)) } + + // Only check namespace scope for ResourcePlacement + if !isClusterScoped && ResourceInformer.IsClusterScopedResources(gvk) { + allErr = append(allErr, fmt.Errorf("the resource is not found in schema (please retry) or it is a cluster scoped resource: %v", gvk)) + } } else { err := fmt.Errorf("cannot perform resource scope check for now, please retry") klog.ErrorS(controller.NewUnexpectedBehaviorError(err), "resource informer is nil") @@ -94,19 +112,41 @@ func ValidateClusterResourcePlacement(clusterResourcePlacement *placementv1beta1 } } - if clusterResourcePlacement.Spec.Policy != nil { - if err := validatePlacementPolicy(clusterResourcePlacement.Spec.Policy); err != nil { + if policy != nil { + if err := validatePlacementPolicy(policy); err != nil { allErr = append(allErr, fmt.Errorf("the placement policy field is invalid: %w", err)) } } - if err := validateRolloutStrategy(clusterResourcePlacement.Spec.Strategy); err != nil { + if err := validateRolloutStrategy(strategy); err != nil { allErr = append(allErr, fmt.Errorf("the rollout Strategy field is invalid: %w", err)) } return apiErrors.NewAggregate(allErr) } +// ValidateClusterResourcePlacement validates a ClusterResourcePlacement object. +func ValidateClusterResourcePlacement(clusterResourcePlacement *placementv1beta1.ClusterResourcePlacement) error { + return validatePlacement( + clusterResourcePlacement.Name, + clusterResourcePlacement.Spec.ResourceSelectors, + clusterResourcePlacement.Spec.Policy, + clusterResourcePlacement.Spec.Strategy, + true, // isClusterScoped + ) +} + +// ValidateResourcePlacement validates a ResourcePlacement object. +func ValidateResourcePlacement(resourcePlacement *placementv1beta1.ResourcePlacement) error { + return validatePlacement( + resourcePlacement.Name, + resourcePlacement.Spec.ResourceSelectors, + resourcePlacement.Spec.Policy, + resourcePlacement.Spec.Strategy, + false, // isClusterScoped + ) +} + func IsPlacementPolicyTypeUpdated(oldPolicy, currentPolicy *placementv1beta1.PlacementPolicy) bool { if oldPolicy == nil && currentPolicy != nil { // if placement policy is left blank, by default PickAll is chosen. @@ -509,3 +549,58 @@ func supportedResourceCapacityTypes() []string { sort.Strings(capacityTypes) return capacityTypes } + +// HandlePlacementValidation provides consolidated webhook validation logic for placement objects. +// This function accepts higher-order functions for type-specific operations. +func HandlePlacementValidation( + ctx context.Context, + req admission.Request, + decoder webhook.AdmissionDecoder, + resourceType string, + decodeFunc func(admission.Request, webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error), + decodeOldFunc func(admission.Request, webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error), + validateFunc func(placementv1beta1.PlacementObj) error, +) admission.Response { + if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { + klog.V(2).InfoS("handling placement", "resourceType", resourceType, "operation", req.Operation, "namespacedName", types.NamespacedName{Name: req.Name, Namespace: req.Namespace}) + + placement, err := decodeFunc(req, decoder) + if err != nil { + klog.ErrorS(err, "failed to decode v1beta1 placement object for create/update operation", "resourceType", resourceType, "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups) + return admission.Errored(http.StatusBadRequest, err) + } + + if req.Operation == admissionv1.Update { + oldPlacement, err := decodeOldFunc(req, decoder) + if err != nil { + return admission.Errored(http.StatusBadRequest, err) + } + + // Special case: allow updates to old placement objects with invalid fields so that we can + // update the placement to remove finalizer then delete it. + if err := validateFunc(oldPlacement); err != nil { + if placement.GetDeletionTimestamp() != nil { + return admission.Allowed(fmt.Sprintf(AllowUpdateOldInvalidFmt, resourceType)) + } + return admission.Denied(fmt.Sprintf(DenyUpdateOldInvalidFmt, resourceType, err)) + } + + // Handle update case where placement type should be immutable. + if IsPlacementPolicyTypeUpdated(oldPlacement.GetPlacementSpec().Policy, placement.GetPlacementSpec().Policy) { + return admission.Denied("placement type is immutable") + } + + // Handle update case where existing tolerations were updated/deleted + if IsTolerationsUpdatedOrDeleted(oldPlacement.GetPlacementSpec().Tolerations(), placement.GetPlacementSpec().Tolerations()) { + return admission.Denied("tolerations have been updated/deleted, only additions to tolerations are allowed") + } + } + + if err := validateFunc(placement); err != nil { + klog.V(2).InfoS("v1beta1 placement has invalid fields, request is denied", "resourceType", resourceType, "operation", req.Operation, "namespacedName", types.NamespacedName{Name: placement.GetName(), Namespace: req.Namespace}) + return admission.Denied(fmt.Sprintf(DenyCreateUpdateInvalidFmt, resourceType, err)) + } + } + + return admission.Allowed(fmt.Sprintf(AllowModifyFmt, resourceType)) +} diff --git a/pkg/utils/validator/clusterresourceplacement_test.go b/pkg/utils/validator/placement_test.go similarity index 92% rename from pkg/utils/validator/clusterresourceplacement_test.go rename to pkg/utils/validator/placement_test.go index aa92c5b15..6afd0ae7f 100644 --- a/pkg/utils/validator/clusterresourceplacement_test.go +++ b/pkg/utils/validator/placement_test.go @@ -167,6 +167,29 @@ func TestValidateClusterResourcePlacement(t *testing.T) { wantErr: true, wantErrMsg: "cannot perform resource scope check for now, please retry", }, + "CRP with namespaced resource should fail": { + crp: &placementv1beta1.ClusterResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-crp", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + { + Group: "apps", + Version: "v1", + Kind: "Deployment", + Name: "test-deployment", + }, + }, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, // Deployment is namespaced + }, + wantErr: true, + wantErrMsg: "resource is not found in schema (please retry) or it is not a cluster scoped resource", + }, } for testName, testCase := range tests { t.Run(testName, func(t *testing.T) { @@ -1719,3 +1742,130 @@ func TestIsTolerationsUpdatedOrDeleted(t *testing.T) { }) } } + +func TestValidateResourcePlacement(t *testing.T) { + tests := map[string]struct { + rp *placementv1beta1.ResourcePlacement + resourceInformer informer.Manager + wantErr bool + wantErrMsg string + }{ + "RP with invalid placement policy": { + rp: &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + { + Group: "apps", + Version: "v1", + Kind: "Deployment", + Name: "test-deployment", + }, + }, + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickFixedPlacementType, + ClusterNames: []string{}, // Empty cluster names for PickFixed type + }, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + wantErr: true, + wantErrMsg: "cluster names cannot be empty for policy type PickFixed", + }, + "RP with invalid rollout strategy": { + rp: &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + { + Group: "apps", + Version: "v1", + Kind: "Deployment", + Name: "test-deployment", + }, + }, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1}, // Negative value + }, + }, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + wantErr: true, + wantErrMsg: "maxUnavailable must be greater than or equal to 0", + }, + "RP with cluster scoped resource should fail": { + rp: &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Namespace: "test-namespace", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + { + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + Name: "test-cluster-role", + }, + }, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.ClusterRoleGVK: true}, + IsClusterScopedResource: true, // ClusterRole is cluster-scoped + }, + wantErr: true, + wantErrMsg: "resource is not found in schema (please retry) or it is a cluster scoped resource", + }, + "RP with namespaced resource should succeed": { + rp: &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Namespace: "test-namespace", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + { + Group: "apps", + Version: "v1", + Kind: "Deployment", + Name: "test-deployment", + }, + }, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, // Deployment is namespaced + }, + wantErr: false, + }, + } + + for testName, testCase := range tests { + t.Run(testName, func(t *testing.T) { + RestMapper = utils.TestMapper{} + ResourceInformer = testCase.resourceInformer + gotErr := ValidateResourcePlacement(testCase.rp) + if (gotErr != nil) != testCase.wantErr { + t.Errorf("ValidateResourcePlacement() error = %v, wantErr %v", gotErr, testCase.wantErr) + } + if testCase.wantErr && !strings.Contains(gotErr.Error(), testCase.wantErrMsg) { + t.Errorf("ValidateResourcePlacement() got %v, should contain want %s", gotErr, testCase.wantErrMsg) + } + }) + } +} diff --git a/pkg/webhook/add_handler.go b/pkg/webhook/add_handler.go index e25d92255..08380e6b8 100644 --- a/pkg/webhook/add_handler.go +++ b/pkg/webhook/add_handler.go @@ -10,6 +10,7 @@ import ( "github.com/kubefleet-dev/kubefleet/pkg/webhook/pod" "github.com/kubefleet-dev/kubefleet/pkg/webhook/replicaset" "github.com/kubefleet-dev/kubefleet/pkg/webhook/resourceoverride" + "github.com/kubefleet-dev/kubefleet/pkg/webhook/resourceplacement" ) func init() { @@ -18,6 +19,7 @@ func init() { // AddToManagerFuncs is a list of functions to register webhook validators and mutators to the webhook server AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.AddMutating) AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add) + AddToManagerFuncs = append(AddToManagerFuncs, resourceplacement.Add) AddToManagerFuncs = append(AddToManagerFuncs, pod.Add) AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add) AddToManagerFuncs = append(AddToManagerFuncs, membercluster.Add) diff --git a/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook.go b/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook.go index 7bc326103..f5815cfa5 100644 --- a/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook.go +++ b/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook.go @@ -20,11 +20,7 @@ package clusterresourceplacement import ( "context" "fmt" - "net/http" - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -34,12 +30,6 @@ import ( "github.com/kubefleet-dev/kubefleet/pkg/utils/validator" ) -const ( - allowUpdateOldInvalidCRPFmt = "allow update on old invalid v1beta1 CRP with DeletionTimestamp set" - denyUpdateOldInvalidCRPFmt = "deny update on old invalid v1beta1 CRP with DeletionTimestamp not set %s" - denyCreateUpdateInvalidCRPFmt = "deny create/update v1beta1 CRP has invalid fields %s" -) - var ( // ValidationPath is the webhook service path which admission requests are routed to for validating v1beta1 CRP resources. ValidationPath = fmt.Sprintf(utils.ValidationPathFmt, placementv1beta1.GroupVersion.Group, placementv1beta1.GroupVersion.Version, "clusterresourceplacement") @@ -57,41 +47,23 @@ func Add(mgr manager.Manager) error { } // Handle clusterResourcePlacementValidator handles create, update CRP requests. -func (v *clusterResourcePlacementValidator) Handle(_ context.Context, req admission.Request) admission.Response { - var crp placementv1beta1.ClusterResourcePlacement - if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update { - klog.V(2).InfoS("handling CRP", "operation", req.Operation, "namespacedName", types.NamespacedName{Name: req.Name}) - if err := v.decoder.Decode(req, &crp); err != nil { - klog.ErrorS(err, "failed to decode v1beta1 CRP object for create/update operation", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Errored(http.StatusBadRequest, err) - } - if req.Operation == admissionv1.Update { +func (v *clusterResourcePlacementValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + return validator.HandlePlacementValidation(ctx, req, v.decoder, + "CRP", + // decodeFunc + func(req admission.Request, decoder webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error) { + var crp placementv1beta1.ClusterResourcePlacement + err := decoder.Decode(req, &crp) + return &crp, err + }, + // decodeOldFunc + func(req admission.Request, decoder webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error) { var oldCRP placementv1beta1.ClusterResourcePlacement - if err := v.decoder.DecodeRaw(req.OldObject, &oldCRP); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - // this is a special case where we allow updates to old v1beta1 CRP with invalid fields so that we can - // update the CRP to remove finalizer then delete CRP. - if err := validator.ValidateClusterResourcePlacement(&oldCRP); err != nil { - if crp.DeletionTimestamp != nil { - return admission.Allowed(allowUpdateOldInvalidCRPFmt) - } - return admission.Denied(fmt.Sprintf(denyUpdateOldInvalidCRPFmt, err)) - } - // handle update case where placement type should be immutable. - if validator.IsPlacementPolicyTypeUpdated(oldCRP.Spec.Policy, crp.Spec.Policy) { - return admission.Denied("placement type is immutable") - } - // handle update case where existing tolerations were updated/deleted - if validator.IsTolerationsUpdatedOrDeleted(oldCRP.Spec.Tolerations(), crp.Spec.Tolerations()) { - return admission.Denied("tolerations have been updated/deleted, only additions to tolerations are allowed") - } - } - if err := validator.ValidateClusterResourcePlacement(&crp); err != nil { - klog.V(2).InfoS("v1beta1 cluster resource placement has invalid fields, request is denied", "operation", req.Operation, "namespacedName", types.NamespacedName{Name: crp.Name}) - return admission.Denied(fmt.Sprintf(denyCreateUpdateInvalidCRPFmt, err)) - } - } - klog.V(2).InfoS("user is allowed to modify v1beta1 cluster resource placement", "operation", req.Operation, "user", req.UserInfo.Username, "group", req.UserInfo.Groups, "namespacedName", types.NamespacedName{Name: crp.Name}) - return admission.Allowed("any user is allowed to modify v1beta1 CRP") + err := decoder.DecodeRaw(req.OldObject, &oldCRP) + return &oldCRP, err + }, + // validateFunc + func(obj placementv1beta1.PlacementObj) error { + return validator.ValidateClusterResourcePlacement(obj.(*placementv1beta1.ClusterResourcePlacement)) + }) } diff --git a/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go b/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go index 30909093d..a41ea297a 100644 --- a/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go +++ b/pkg/webhook/clusterresourceplacement/v1beta1_clusterresourceplacement_validating_webhook_test.go @@ -279,7 +279,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Allowed("any user is allowed to modify v1beta1 CRP"), + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowModifyFmt, "CRP")), }, "deny CRP create - invalid CRP object": { req: admission.Request{ @@ -304,7 +304,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Denied(fmt.Sprintf(denyCreateUpdateInvalidCRPFmt, errString)), + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyCreateUpdateInvalidFmt, "CRP", errString)), }, "allow CRP update - valid update": { req: admission.Request{ @@ -333,7 +333,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Allowed("any user is allowed to modify v1beta1 CRP"), + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowModifyFmt, "CRP")), }, "allow CRP update - invalid old CRP object, invalid new CRP is deleting, finalizer removed": { req: admission.Request{ @@ -362,7 +362,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Allowed(allowUpdateOldInvalidCRPFmt), + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowUpdateOldInvalidFmt, "CRP")), }, "allow CRP update - invalid old CRP object, valid new CRP is deleting, finalizer removed, spec updated": { req: admission.Request{ @@ -391,7 +391,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Allowed(allowUpdateOldInvalidCRPFmt), + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowUpdateOldInvalidFmt, "CRP")), }, "deny CRP update - invalid old CRP, invalid new CRP is not deleting, finalizer removed": { req: admission.Request{ @@ -420,7 +420,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Denied(fmt.Sprintf(denyUpdateOldInvalidCRPFmt, errString)), + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyUpdateOldInvalidFmt, "CRP", errString)), }, "allow CRP update - invalid old CRP, invalid new CRP is deleting, finalizer not removed": { req: admission.Request{ @@ -449,7 +449,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Allowed(allowUpdateOldInvalidCRPFmt), + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowUpdateOldInvalidFmt, "CRP")), }, "deny CRP update - invalid old CRP, invalid new CRP, label is updated": { req: admission.Request{ @@ -478,7 +478,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Denied(fmt.Sprintf(denyUpdateOldInvalidCRPFmt, errString)), + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyUpdateOldInvalidFmt, "CRP", errString)), }, "deny CRP update - invalid old CRP, valid new CRP, spec updated": { req: admission.Request{ @@ -507,7 +507,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Denied(fmt.Sprintf(denyUpdateOldInvalidCRPFmt, errString)), + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyUpdateOldInvalidFmt, "CRP", errString)), }, "deny CRP update - valid old CRP, invalid new CRP, spec updated": { req: admission.Request{ @@ -536,7 +536,7 @@ func TestHandle(t *testing.T) { resourceValidator: clusterResourcePlacementValidator{ decoder: decoder, }, - wantResponse: admission.Denied(fmt.Sprintf(denyCreateUpdateInvalidCRPFmt, errString)), + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyCreateUpdateInvalidFmt, "CRP", errString)), }, "deny CRP update - new CRP immutable placement type": { req: admission.Request{ diff --git a/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook.go b/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook.go new file mode 100644 index 000000000..2389c0495 --- /dev/null +++ b/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook.go @@ -0,0 +1,73 @@ +/* +Copyright 2025 The KubeFleet 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 resourceplacement implements the webhook for v1beta1 ResourcePlacement. +package resourceplacement + +import ( + "context" + "fmt" + + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" + "github.com/kubefleet-dev/kubefleet/pkg/utils" + "github.com/kubefleet-dev/kubefleet/pkg/utils/validator" +) + +var ( + // ValidationPath is the webhook service path which admission requests are routed to for validating v1beta1 RP resources. + ValidationPath = fmt.Sprintf(utils.ValidationPathFmt, placementv1beta1.GroupVersion.Group, placementv1beta1.GroupVersion.Version, "resourceplacement") +) + +type resourcePlacementValidator struct { + decoder webhook.AdmissionDecoder +} + +// Add registers the webhook for K8s bulit-in object types. +func Add(mgr manager.Manager) error { + hookServer := mgr.GetWebhookServer() + hookServer.Register(ValidationPath, &webhook.Admission{Handler: &resourcePlacementValidator{admission.NewDecoder(mgr.GetScheme())}}) + return nil +} + +// Handle resourcePlacementValidator handles create, update RP requests. +func (v *resourcePlacementValidator) Handle(ctx context.Context, req admission.Request) admission.Response { + return validator.HandlePlacementValidation( + ctx, + req, + v.decoder, + "RP", + // decodeFunc + func(req admission.Request, decoder webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error) { + var rp placementv1beta1.ResourcePlacement + err := decoder.Decode(req, &rp) + return &rp, err + }, + // decodeOldFunc + func(req admission.Request, decoder webhook.AdmissionDecoder) (placementv1beta1.PlacementObj, error) { + var oldRP placementv1beta1.ResourcePlacement + err := decoder.DecodeRaw(req.OldObject, &oldRP) + return &oldRP, err + }, + // validateFunc + func(obj placementv1beta1.PlacementObj) error { + return validator.ValidateResourcePlacement(obj.(*placementv1beta1.ResourcePlacement)) + }, + ) +} diff --git a/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook_test.go b/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook_test.go new file mode 100644 index 000000000..2a0a7a591 --- /dev/null +++ b/pkg/webhook/resourceplacement/v1beta1_resourceplacement_validating_webhook_test.go @@ -0,0 +1,432 @@ +package resourceplacement + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "testing" + "time" + + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" + "github.com/kubefleet-dev/kubefleet/pkg/utils" + "github.com/kubefleet-dev/kubefleet/pkg/utils/informer" + "github.com/kubefleet-dev/kubefleet/pkg/utils/validator" + testinformer "github.com/kubefleet-dev/kubefleet/test/utils/informer" + "github.com/stretchr/testify/assert" +) + +var ( + resourceSelector = placementv1beta1.ResourceSelectorTerm{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + Name: "test-deployment", + } + errString = "the rollout Strategy field is invalid: maxUnavailable must be greater than or equal to 0, got `-1`" +) + +func TestHandle(t *testing.T) { + invalidRPObject := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer}, + }, + Spec: placementv1beta1.PlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1}, + }, + }, + }, + } + + invalidRPObjectDeletingFinalizersRemoved := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Finalizers: []string{}, + DeletionTimestamp: ptr.To(metav1.NewTime(time.Now())), + }, + Spec: placementv1beta1.PlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1}, + }, + }, + }, + } + + invalidRPObjectFinalizersRemoved := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Finalizers: []string{}, + }, + Spec: placementv1beta1.PlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + RollingUpdate: &placementv1beta1.RollingUpdateConfig{ + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: -1}, + }, + }, + }, + } + + validRPObject := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + Finalizers: []string{placementv1beta1.PlacementCleanupFinalizer}, + }, + Spec: placementv1beta1.PlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + }, + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + }, + }, + } + + validRPObjectWithTolerations := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + }, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickAllPlacementType, + Tolerations: []placementv1beta1.Toleration{ + { + Key: "key1", + Value: "value1", + }, + }, + }, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + }, + }, + } + + updatedPlacementTypeRPObject := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rp", + }, + Spec: placementv1beta1.PlacementSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + PlacementType: placementv1beta1.PickNPlacementType, + NumberOfClusters: ptr.To(int32(2)), + }, + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{resourceSelector}, + Strategy: placementv1beta1.RolloutStrategy{ + Type: placementv1beta1.RollingUpdateRolloutStrategyType, + }, + }, + } + + validRPObjectBytes, err := json.Marshal(validRPObject) + assert.Nil(t, err) + invalidRPObjectBytes, err := json.Marshal(invalidRPObject) + assert.Nil(t, err) + invalidRPObjectDeletingFinalizersRemovedBytes, err := json.Marshal(invalidRPObjectDeletingFinalizersRemoved) + assert.Nil(t, err) + invalidRPObjectFinalizersRemovedBytes, err := json.Marshal(invalidRPObjectFinalizersRemoved) + assert.Nil(t, err) + updatedPlacementTypeRPObjectBytes, err := json.Marshal(updatedPlacementTypeRPObject) + assert.Nil(t, err) + validRPObjectWithTolerationsBytes, err := json.Marshal(validRPObjectWithTolerations) + assert.Nil(t, err) + + scheme := runtime.NewScheme() + err = placementv1beta1.AddToScheme(scheme) + assert.Nil(t, err) + decoder := admission.NewDecoder(scheme) + assert.Nil(t, err) + + testCases := map[string]struct { + req admission.Request + resourceValidator resourcePlacementValidator + resourceInformer informer.Manager + wantResponse admission.Response + }{ + "allow RP create": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + Object: runtime.RawExtension{ + Raw: validRPObjectBytes, + Object: validRPObject, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Create, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowModifyFmt, "RP")), + }, + "deny RP create - invalid RP object": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + Object: runtime.RawExtension{ + Raw: invalidRPObjectBytes, + Object: invalidRPObject, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Create, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyCreateUpdateInvalidFmt, "RP", errString)), + }, + "allow RP update - invalid old RP object, invalid new RP is deleting, finalizer removed": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + OldObject: runtime.RawExtension{ + Raw: invalidRPObjectBytes, + Object: invalidRPObject, + }, + Object: runtime.RawExtension{ + Raw: invalidRPObjectDeletingFinalizersRemovedBytes, + Object: invalidRPObjectDeletingFinalizersRemoved, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Allowed(fmt.Sprintf(validator.AllowUpdateOldInvalidFmt, "RP")), + }, + "deny RP update - invalid old RP, invalid new RP is not deleting, finalizer removed": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + OldObject: runtime.RawExtension{ + Raw: invalidRPObjectBytes, + Object: invalidRPObject, + }, + Object: runtime.RawExtension{ + Raw: invalidRPObjectFinalizersRemovedBytes, + Object: invalidRPObjectFinalizersRemoved, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyUpdateOldInvalidFmt, "RP", errString)), + }, + "deny RP update - valid old RP, invalid new RP, spec updated": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + OldObject: runtime.RawExtension{ + Raw: validRPObjectBytes, + Object: validRPObject, + }, + Object: runtime.RawExtension{ + Raw: invalidRPObjectBytes, + Object: invalidRPObject, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Denied(fmt.Sprintf(validator.DenyCreateUpdateInvalidFmt, "RP", errString)), + }, + "deny RP update - new RP immutable placement type": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + OldObject: runtime.RawExtension{ + Raw: validRPObjectBytes, + Object: validRPObject, + }, + Object: runtime.RawExtension{ + Raw: updatedPlacementTypeRPObjectBytes, + Object: updatedPlacementTypeRPObject, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Denied("placement type is immutable"), + }, + "deny RP update - new RP tolerations updated": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + OldObject: runtime.RawExtension{ + Raw: validRPObjectWithTolerationsBytes, + Object: validRPObjectWithTolerations, + }, + Object: runtime.RawExtension{ + Raw: validRPObjectBytes, + Object: validRPObject, + }, + UserInfo: authenticationv1.UserInfo{ + Username: "test-user", + Groups: []string{"system:masters"}, + }, + RequestKind: &utils.ClusterResourcePlacementMetaGVK, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Denied("tolerations have been updated/deleted, only additions to tolerations are allowed"), + }, + "decode error for main object": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + Object: runtime.RawExtension{ + Raw: []byte(`{"invalid": "json"`), // Invalid JSON + }, + Operation: admissionv1.Create, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Errored(http.StatusBadRequest, fmt.Errorf("couldn't get version/kind; json parse error: unexpected end of JSON input")), + }, + "decode error for old object during update": { + req: admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Name: "test-rp", + Object: runtime.RawExtension{ + Raw: func() []byte { + validRP := &placementv1beta1.ResourcePlacement{ + ObjectMeta: metav1.ObjectMeta{Name: "test-rp"}, + Spec: placementv1beta1.PlacementSpec{ + ResourceSelectors: []placementv1beta1.ResourceSelectorTerm{ + {Group: "apps", Version: "v1", Kind: "Deployment", Name: "test"}, + }, + }, + } + validRPBytes, _ := json.Marshal(validRP) + return validRPBytes + }(), + }, + OldObject: runtime.RawExtension{ + Raw: []byte(`{"invalid": "json"`), // Invalid JSON for old object + }, + Operation: admissionv1.Update, + }, + }, + resourceInformer: &testinformer.FakeManager{ + APIResources: map[schema.GroupVersionKind]bool{utils.DeploymentGVK: true}, + IsClusterScopedResource: false, + }, + resourceValidator: resourcePlacementValidator{ + decoder: decoder, + }, + wantResponse: admission.Errored(http.StatusBadRequest, fmt.Errorf("couldn't get version/kind; json parse error: unexpected end of JSON input")), + }, + } + + for testName, testCase := range testCases { + t.Run(testName, func(t *testing.T) { + validator.RestMapper = utils.TestMapper{} + validator.ResourceInformer = testCase.resourceInformer + gotResult := testCase.resourceValidator.Handle(context.Background(), testCase.req) + assert.Equal(t, testCase.wantResponse, gotResult, utils.TestCaseMsg, testName) + }) + } +}