feat: add rp validating webhook (#312)

Author: weng271190436

pkg/utils/validator/clusterresourceplacement.go renamed to 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,29 +96,57 @@ 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")
 			allErr = append(allErr, fmt.Errorf("cannot perform resource scope check for now, please retry"))
 		}
 	}
 
-	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))
+}

pkg/utils/validator/clusterresourceplacement_test.go renamed to 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)
+			}
+		})
+	}
+}

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)