From 2dd2dab4583362a3f2f2c6e8f8932e2fab4d7feb Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Thu, 23 Oct 2025 19:41:26 +0000 Subject: [PATCH 1/2] remove CRP and MC validation Signed-off-by: Wei Weng --- cmd/hubagent/main.go | 4 +- cmd/memberagent/main.go | 2 - pkg/utils/apiresources.go | 4 - pkg/utils/apiresources_test.go | 10 - pkg/utils/common.go | 42 -- .../validator/clusterresourceplacement.go | 44 -- .../clusterresourceplacement_test.go | 167 ------- pkg/webhook/add_handler.go | 1 - ...terresourceplacement_validating_webhook.go | 52 -- .../fleetresourcehandler_webhook.go | 28 +- .../fleetresourcehandler_webhook_test.go | 456 +----------------- pkg/webhook/validation/uservalidation.go | 50 +- pkg/webhook/webhook.go | 42 +- pkg/webhook/webhook_test.go | 4 +- 14 files changed, 27 insertions(+), 879 deletions(-) delete mode 100644 pkg/webhook/clusterresourceplacement/v1alpha1_clusterresourceplacement_validating_webhook.go diff --git a/cmd/hubagent/main.go b/cmd/hubagent/main.go index 96a37fea8..1fc880851 100644 --- a/cmd/hubagent/main.go +++ b/cmd/hubagent/main.go @@ -44,7 +44,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1alpha1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/cmd/hubagent/options" "github.com/kubefleet-dev/kubefleet/cmd/hubagent/workload" mcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/membercluster/v1beta1" @@ -71,7 +70,6 @@ const ( func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(fleetv1alpha1.AddToScheme(scheme)) utilruntime.Must(workv1alpha1.AddToScheme(scheme)) utilruntime.Must(placementv1beta1.AddToScheme(scheme)) utilruntime.Must(clusterv1beta1.AddToScheme(scheme)) @@ -203,7 +201,7 @@ func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.Webho klog.ErrorS(err, "unable to add WebhookConfig") return err } - if err = webhook.AddToManager(mgr, whiteListedUsers, isFleetV1Beta1API, denyModifyMemberClusterLabels); err != nil { + if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels); err != nil { klog.ErrorS(err, "unable to register webhooks to the manager") return err } diff --git a/cmd/memberagent/main.go b/cmd/memberagent/main.go index ffdac9862..73f764683 100644 --- a/cmd/memberagent/main.go +++ b/cmd/memberagent/main.go @@ -51,7 +51,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" imcv1beta1 "github.com/kubefleet-dev/kubefleet/pkg/controllers/internalmembercluster/v1beta1" "github.com/kubefleet-dev/kubefleet/pkg/controllers/workapplier" workv1alpha1controller "github.com/kubefleet-dev/kubefleet/pkg/controllers/workv1alpha1" @@ -113,7 +112,6 @@ func init() { klog.InitFlags(nil) utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(fleetv1alpha1.AddToScheme(scheme)) utilruntime.Must(workv1alpha1.AddToScheme(scheme)) utilruntime.Must(clusterv1beta1.AddToScheme(scheme)) utilruntime.Must(placementv1beta1.AddToScheme(scheme)) diff --git a/pkg/utils/apiresources.go b/pkg/utils/apiresources.go index 71d817919..99cb45c73 100644 --- a/pkg/utils/apiresources.go +++ b/pkg/utils/apiresources.go @@ -28,7 +28,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" ) var ( @@ -187,9 +186,6 @@ func NewResourceConfig(isAllowList bool) *ResourceConfig { if r.isAllowList { return r } - // TODO: remove after we remove v1alpha1 support - // disable v1alpha1 related resources by default - r.AddGroup(fleetv1alpha1.GroupVersion.Group) r.AddGroupVersionKind(WorkV1Alpha1GVK) // disable cluster group by default diff --git a/pkg/utils/apiresources_test.go b/pkg/utils/apiresources_test.go index 0b3e52cf5..b01929ce4 100644 --- a/pkg/utils/apiresources_test.go +++ b/pkg/utils/apiresources_test.go @@ -284,16 +284,6 @@ func TestResourceConfigMixedParse(t *testing.T) { func TestDefaultResourceConfigGroupVersionKindParse(t *testing.T) { resourcesInDefaultDisabledList := []schema.GroupVersionKind{ corev1PodGVK, corev1NodeGVK, - { - Group: "fleet.azure.com", - Version: "v1beta1", - Kind: "MemberCluster", - }, - { - Group: "fleet.azure.com", - Version: "v1alpha1", - Kind: "MemberCluster", - }, { Group: "events.k8s.io", Version: "v1beta1", diff --git a/pkg/utils/common.go b/pkg/utils/common.go index 9b2d43d73..b78b7ef40 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -49,7 +49,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/utils/condition" "github.com/kubefleet-dev/kubefleet/pkg/utils/controller" "github.com/kubefleet-dev/kubefleet/pkg/utils/informer" @@ -113,11 +112,6 @@ const ( ) var ( - FleetRule = rbacv1.PolicyRule{ - Verbs: []string{"*"}, - APIGroups: []string{fleetv1alpha1.GroupVersion.Group}, - Resources: []string{"*"}, - } FleetClusterRule = rbacv1.PolicyRule{ Verbs: []string{"*"}, APIGroups: []string{clusterv1beta1.GroupVersion.Group}, @@ -147,18 +141,6 @@ var ( // Those are the GVR/GVKs in use by Fleet source code. var ( - ClusterResourcePlacementV1Alpha1GVK = schema.GroupVersionKind{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Kind: "ClusterResourcePlacement", - } - - ClusterResourcePlacementV1Alpha1GVR = schema.GroupVersionResource{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Resource: fleetv1alpha1.ClusterResourcePlacementResource, - } - ClusterResourcePlacementGVR = schema.GroupVersionResource{ Group: placementv1beta1.GroupVersion.Group, Version: placementv1beta1.GroupVersion.Version, @@ -249,12 +231,6 @@ var ( Kind: "Event", } - IMCV1Alpha1MetaGVK = metav1.GroupVersionKind{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Kind: "InternalMemberCluster", - } - IngressClassGVR = schema.GroupVersionResource{ Group: networkingv1.SchemeGroupVersion.Group, Version: networkingv1.SchemeGroupVersion.Version, @@ -285,24 +261,6 @@ var ( Resource: "limitranges", } - MCV1Alpha1MetaGVK = metav1.GroupVersionKind{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Kind: "MemberCluster", - } - - MCV1Alpha1GVK = schema.GroupVersionKind{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Kind: fleetv1alpha1.MemberClusterKind, - } - - MCV1Alpha1GVR = schema.GroupVersionResource{ - Group: fleetv1alpha1.GroupVersion.Group, - Version: fleetv1alpha1.GroupVersion.Version, - Resource: fleetv1alpha1.MemberClusterResource, - } - MCMetaGVK = metav1.GroupVersionKind{ Group: clusterv1beta1.GroupVersion.Group, Version: clusterv1beta1.GroupVersion.Version, diff --git a/pkg/utils/validator/clusterresourceplacement.go b/pkg/utils/validator/clusterresourceplacement.go index 804dd7e44..29fa95e4f 100644 --- a/pkg/utils/validator/clusterresourceplacement.go +++ b/pkg/utils/validator/clusterresourceplacement.go @@ -34,7 +34,6 @@ import ( "k8s.io/klog/v2" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/propertyprovider" "github.com/kubefleet-dev/kubefleet/pkg/utils/controller" "github.com/kubefleet-dev/kubefleet/pkg/utils/informer" @@ -54,49 +53,6 @@ var ( resourceCapacityTypes = supportedResourceCapacityTypes() ) -// ValidateClusterResourcePlacementAlpha validates a ClusterResourcePlacement v1alpha1 object. -func ValidateClusterResourcePlacementAlpha(clusterResourcePlacement *fleetv1alpha1.ClusterResourcePlacement) error { - allErr := make([]error, 0) - - // we leverage the informer manager to do the resource scope validation - if ResourceInformer == nil { - allErr = append(allErr, fmt.Errorf("cannot perform resource scope check for now, please retry")) - } - - for _, selector := range clusterResourcePlacement.Spec.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)) - } - if _, err := metav1.LabelSelectorAsSelector(selector.LabelSelector); err != nil { - allErr = append(allErr, fmt.Errorf("the labelSelector in resource selector %+v is invalid: %w", selector, err)) - } - } - if ResourceInformer != nil { - gvk := schema.GroupVersionKind{ - Group: selector.Group, - Version: selector.Version, - Kind: selector.Kind, - } - // TODO: Ensure gvk created from resource selector is valid. - if !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)) - } - } - } - - if clusterResourcePlacement.Spec.Policy != nil && clusterResourcePlacement.Spec.Policy.Affinity != nil && - clusterResourcePlacement.Spec.Policy.Affinity.ClusterAffinity != nil { - for _, selector := range clusterResourcePlacement.Spec.Policy.Affinity.ClusterAffinity.ClusterSelectorTerms { - if _, err := metav1.LabelSelectorAsSelector(&selector.LabelSelector); err != nil { - allErr = append(allErr, fmt.Errorf("the labelSelector in cluster selector %+v is invalid: %w", selector, err)) - } - } - } - - return apiErrors.NewAggregate(allErr) -} - // ValidateClusterResourcePlacement validates a ClusterResourcePlacement object. func ValidateClusterResourcePlacement(clusterResourcePlacement *placementv1beta1.ClusterResourcePlacement) error { allErr := make([]error, 0) diff --git a/pkg/utils/validator/clusterresourceplacement_test.go b/pkg/utils/validator/clusterresourceplacement_test.go index 356a8953c..aa92c5b15 100644 --- a/pkg/utils/validator/clusterresourceplacement_test.go +++ b/pkg/utils/validator/clusterresourceplacement_test.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/utils" "github.com/kubefleet-dev/kubefleet/pkg/utils/informer" testinformer "github.com/kubefleet-dev/kubefleet/test/utils/informer" @@ -43,172 +42,6 @@ var ( } ) -func TestValidateClusterResourcePlacementAlpha(t *testing.T) { - tests := map[string]struct { - crp *fleetv1alpha1.ClusterResourcePlacement - resourceInformer informer.Manager - wantErr bool - wantErrMsg string - }{ - "valid CRP": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - Group: "rbac.authorization.k8s.io", - Version: "v1", - Kind: "ClusterRole", - Name: "test-cluster-role", - }, - }, - }, - }, - resourceInformer: &testinformer.FakeManager{IsClusterScopedResource: false}, - wantErr: false, - }, - "invalid Resource Selector with name & label selector": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - Group: "rbac.authorization.k8s.io", - Version: "v1", - Kind: "ClusterRole", - Name: "test-cluster-role", - LabelSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"test-key": "test-value"}, - }, - }, - }, - }, - }, - resourceInformer: &testinformer.FakeManager{IsClusterScopedResource: false}, - wantErr: true, - wantErrMsg: "the labelSelector and name fields are mutually exclusive in selector", - }, - "invalid Resource Selector with invalid label selector": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "test-key", - Operator: metav1.LabelSelectorOpIn, - }, - }, - }, - }, - }, - }, - }, - resourceInformer: &testinformer.FakeManager{IsClusterScopedResource: false}, - wantErr: true, - wantErrMsg: "for 'in', 'notin' operators, values set can't be empty", - }, - "invalid Resource Selector with invalid cluster resource selector": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - Group: "rbac.authorization.k8s.io", - Version: "v1", - Kind: "Role", - Name: "test-role", - }, - }, - }, - }, - resourceInformer: &testinformer.FakeManager{IsClusterScopedResource: true}, - wantErr: true, - wantErrMsg: "the resource is not found in schema (please retry) or it is not a cluster scoped resource", - }, - "nil resource informer": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - Group: "rbac.authorization.k8s.io", - Version: "v1", - Kind: "ClusterRole", - Name: "test-cluster-role", - }, - }, - }, - }, - resourceInformer: nil, - wantErr: true, - wantErrMsg: "cannot perform resource scope check for now, please retry", - }, - "invalid placement policy with invalid label selector": { - crp: &fleetv1alpha1.ClusterResourcePlacement{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-crp", - }, - Spec: fleetv1alpha1.ClusterResourcePlacementSpec{ - ResourceSelectors: []fleetv1alpha1.ClusterResourceSelector{ - { - Group: "rbac.authorization.k8s.io", - Version: "v1", - Kind: "ClusterRole", - Name: "test-cluster-role", - }, - }, - Policy: &fleetv1alpha1.PlacementPolicy{ - Affinity: &fleetv1alpha1.Affinity{ - ClusterAffinity: &fleetv1alpha1.ClusterAffinity{ - ClusterSelectorTerms: []fleetv1alpha1.ClusterSelectorTerm{ - { - LabelSelector: metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "test-key", - Operator: metav1.LabelSelectorOpIn, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - resourceInformer: &testinformer.FakeManager{IsClusterScopedResource: false}, - wantErr: true, - wantErrMsg: "for 'in', 'notin' operators, values set can't be empty", - }, - } - for testName, testCase := range tests { - t.Run(testName, func(t *testing.T) { - ResourceInformer = testCase.resourceInformer - gotErr := ValidateClusterResourcePlacementAlpha(testCase.crp) - if (gotErr != nil) != testCase.wantErr { - t.Errorf("ValidateClusterResourcePlacementAlpha() error = %v, wantErr %v", gotErr, testCase.wantErr) - } - if testCase.wantErr && !strings.Contains(gotErr.Error(), testCase.wantErrMsg) { - t.Errorf("ValidateClusterResourcePlacementAlpha() got %v, should contain want %s", gotErr, testCase.wantErrMsg) - } - }) - } -} - func TestValidateClusterResourcePlacement(t *testing.T) { tests := map[string]struct { crp *placementv1beta1.ClusterResourcePlacement diff --git a/pkg/webhook/add_handler.go b/pkg/webhook/add_handler.go index 31b5d936a..e25d92255 100644 --- a/pkg/webhook/add_handler.go +++ b/pkg/webhook/add_handler.go @@ -17,7 +17,6 @@ func init() { AddToManagerFleetResourceValidator = fleetresourcehandler.Add // 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.AddV1Alpha1) AddToManagerFuncs = append(AddToManagerFuncs, clusterresourceplacement.Add) AddToManagerFuncs = append(AddToManagerFuncs, pod.Add) AddToManagerFuncs = append(AddToManagerFuncs, replicaset.Add) diff --git a/pkg/webhook/clusterresourceplacement/v1alpha1_clusterresourceplacement_validating_webhook.go b/pkg/webhook/clusterresourceplacement/v1alpha1_clusterresourceplacement_validating_webhook.go deleted file mode 100644 index bb985f81f..000000000 --- a/pkg/webhook/clusterresourceplacement/v1alpha1_clusterresourceplacement_validating_webhook.go +++ /dev/null @@ -1,52 +0,0 @@ -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" - - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" - "github.com/kubefleet-dev/kubefleet/pkg/utils" - "github.com/kubefleet-dev/kubefleet/pkg/utils/validator" -) - -var ( - // V1Alpha1CRPValidationPath is the webhook service path which admission requests are routed to for v1alpha1 CRP resources. - V1Alpha1CRPValidationPath = fmt.Sprintf(utils.ValidationPathFmt, fleetv1alpha1.GroupVersion.Group, fleetv1alpha1.GroupVersion.Version, "clusterresourceplacement") -) - -type v1alpha1ClusterResourcePlacementValidator struct { - decoder webhook.AdmissionDecoder -} - -// AddV1Alpha1 registers the webhook for K8s bulit-in object types. -func AddV1Alpha1(mgr manager.Manager) error { - hookServer := mgr.GetWebhookServer() - hookServer.Register(V1Alpha1CRPValidationPath, &webhook.Admission{Handler: &v1alpha1ClusterResourcePlacementValidator{admission.NewDecoder(mgr.GetScheme())}}) - return nil -} - -// Handle clusterResourcePlacementValidator handles create, update CRP requests. -func (v *v1alpha1ClusterResourcePlacementValidator) Handle(_ context.Context, req admission.Request) admission.Response { - var crp fleetv1alpha1.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 v1alpha1 CRP object for create/update operation", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups) - return admission.Errored(http.StatusBadRequest, err) - } - if err := validator.ValidateClusterResourcePlacementAlpha(&crp); err != nil { - klog.V(2).InfoS("v1alpha1 cluster resource placement has invalid fields, request is denied", "operation", req.Operation, "namespacedName", types.NamespacedName{Name: crp.Name}) - return admission.Denied(err.Error()) - } - } - klog.V(2).InfoS("user is allowed to modify v1alpha1 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 v1alpha1 CRP") -} diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index b49705776..7b40f986e 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -16,7 +16,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/utils" "github.com/kubefleet-dev/kubefleet/pkg/webhook/validation" ) @@ -35,12 +34,11 @@ const ( ) // Add registers the webhook for K8s built-in object types. -func Add(mgr manager.Manager, whiteListedUsers []string, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool) error { +func Add(mgr manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool) error { hookServer := mgr.GetWebhookServer() handler := &fleetResourceValidator{ client: mgr.GetClient(), whiteListedUsers: whiteListedUsers, - isFleetV1Beta1API: isFleetV1Beta1API, decoder: admission.NewDecoder(mgr.GetScheme()), denyModifyMemberClusterLabels: denyModifyMemberClusterLabels, } @@ -51,7 +49,6 @@ func Add(mgr manager.Manager, whiteListedUsers []string, isFleetV1Beta1API bool, type fleetResourceValidator struct { client client.Client whiteListedUsers []string - isFleetV1Beta1API bool decoder webhook.AdmissionDecoder denyModifyMemberClusterLabels bool } @@ -69,16 +66,13 @@ func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Reque case req.Kind == utils.CRDMetaGVK: klog.V(2).InfoS("handling CRD resource", "name", req.Name, "operation", req.Operation, "subResource", req.SubResource) response = v.handleCRD(req) - case req.Kind == utils.MCV1Alpha1MetaGVK: - klog.V(2).InfoS("handling v1alpha1 member cluster resource", "name", req.Name, "operation", req.Operation, "subResource", req.SubResource) - response = v.handleV1Alpha1MemberCluster(req) case req.Kind == utils.MCMetaGVK: klog.V(2).InfoS("handling member cluster resource", "name", req.Name, "operation", req.Operation, "subResource", req.SubResource) response = v.handleMemberCluster(req) case req.Kind == utils.NamespaceMetaGVK: klog.V(2).InfoS("handling namespace resource", "name", req.Name, "operation", req.Operation, "subResource", req.SubResource) response = v.handleNamespace(req) - case req.Kind == utils.IMCV1Alpha1MetaGVK || req.Kind == utils.WorkV1Alpha1MetaGVK || req.Kind == utils.IMCMetaGVK || req.Kind == utils.WorkMetaGVK || req.Kind == utils.EndpointSliceExportMetaGVK || req.Kind == utils.EndpointSliceImportMetaGVK || req.Kind == utils.InternalServiceExportMetaGVK || req.Kind == utils.InternalServiceImportMetaGVK: + case req.Kind == utils.WorkV1Alpha1MetaGVK || req.Kind == utils.IMCMetaGVK || req.Kind == utils.WorkMetaGVK || req.Kind == utils.EndpointSliceExportMetaGVK || req.Kind == utils.EndpointSliceImportMetaGVK || req.Kind == utils.InternalServiceExportMetaGVK || req.Kind == utils.InternalServiceImportMetaGVK: klog.V(2).InfoS("handling fleet owned namespaced resource in fleet reserved namespaces", "GVK", req.RequestKind, "namespacedName", namespacedName, "operation", req.Operation, "subResource", req.SubResource) response = v.handleFleetReservedNamespacedResource(ctx, req) case req.Kind == utils.EventMetaGVK: @@ -106,22 +100,6 @@ func (v *fleetResourceValidator) handleCRD(req admission.Request) admission.Resp return validation.ValidateUserForFleetCRD(req, v.whiteListedUsers, group) } -// handleV1Alpha1MemberCluster allows/denies the request to modify v1alpha1 member cluster object after validation. -func (v *fleetResourceValidator) handleV1Alpha1MemberCluster(req admission.Request) admission.Response { - var currentMC fleetv1alpha1.MemberCluster - if err := v.decodeRequestObject(req, ¤tMC); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - if req.Operation == admissionv1.Update { - var oldMC fleetv1alpha1.MemberCluster - if err := v.decoder.DecodeRaw(req.OldObject, &oldMC); err != nil { - return admission.Errored(http.StatusBadRequest, err) - } - return validation.ValidateV1Alpha1MemberClusterUpdate(currentMC, oldMC, req, v.whiteListedUsers) - } - return validation.ValidateUserForResource(req, v.whiteListedUsers) -} - // handleMemberCluster allows/denies the request to modify member cluster object after validation. func (v *fleetResourceValidator) handleMemberCluster(req admission.Request) admission.Response { var currentMC clusterv1beta1.MemberCluster @@ -158,7 +136,7 @@ func (v *fleetResourceValidator) handleFleetReservedNamespacedResource(ctx conte if !response.Allowed { // if namespace name is just "fleet-member", mcName variable becomes empty and the request is allowed since that namespaces is not watched by member agents. mcName := parseMemberClusterNameFromNamespace(req.Namespace) - return validation.ValidateMCIdentity(ctx, v.client, req, mcName, v.isFleetV1Beta1API) + return validation.ValidateMCIdentity(ctx, v.client, req, mcName) } return response } else if utils.IsReservedNamespace(req.Namespace) { diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go index 8b48ee258..969580adc 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go @@ -19,7 +19,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/utils" "github.com/kubefleet-dev/kubefleet/pkg/webhook/validation" ) @@ -27,7 +26,6 @@ import ( const ( mcName = "test-mc" testClusterResourceID1 = "test-cluster-resource-id-1" - testClusterResourceID2 = "test-cluster-resource-id-2" testLocation = "test-location" fleetClusterResourceIsAnnotationKey = "fleet.azure.com/cluster-resource-id" @@ -111,294 +109,6 @@ func TestHandleCRD(t *testing.T) { } } -func TestHandleV1Alpha1MemberCluster(t *testing.T) { - MCObjectBytes, err := json.Marshal(&fleetv1alpha1.MemberCluster{ - TypeMeta: metav1.TypeMeta{ - Kind: "MemberCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-mc", - }, - }) - assert.Nil(t, err) - labelUpdatedMCObjectBytes, err := json.Marshal(&fleetv1alpha1.MemberCluster{ - TypeMeta: metav1.TypeMeta{ - Kind: "MemberCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-mc", - Labels: map[string]string{"test-key": "test-value"}, - }, - }) - assert.Nil(t, err) - annotationUpdatedMCObjectBytes, err := json.Marshal(&fleetv1alpha1.MemberCluster{ - TypeMeta: metav1.TypeMeta{ - Kind: "MemberCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-mc", - Annotations: map[string]string{"test-key": "test-value"}, - }, - }) - assert.Nil(t, err) - specUpdatedMCObjectBytes, err := json.Marshal(&fleetv1alpha1.MemberCluster{ - TypeMeta: metav1.TypeMeta{ - Kind: "MemberCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-mc", - }, - Spec: fleetv1alpha1.MemberClusterSpec{ - State: fleetv1alpha1.ClusterStateLeave, - }, - }) - assert.Nil(t, err) - statusUpdatedMCObjectBytes, err := json.Marshal(&fleetv1alpha1.MemberCluster{ - TypeMeta: metav1.TypeMeta{ - Kind: "MemberCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-mc", - }, - Status: fleetv1alpha1.MemberClusterStatus{ - Conditions: []metav1.Condition{ - { - Type: string(fleetv1alpha1.ConditionTypeMemberClusterReadyToJoin), - Status: metav1.ConditionTrue, - }, - }, - }, - }) - assert.Nil(t, err) - - scheme := runtime.NewScheme() - err = fleetv1alpha1.AddToScheme(scheme) - assert.Nil(t, err) - decoder := admission.NewDecoder(scheme) - assert.Nil(t, err) - - testCases := map[string]struct { - req admission.Request - resourceValidator fleetResourceValidator - wantResponse admission.Response - }{ - "allow create MC for user in system:masters group": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: labelUpdatedMCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"system:masters"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Create, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"system:masters"}), admissionv1.Create, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - "allow non system user to modify MC labels": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: labelUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"test-group"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - "allow non system user to modify MC annotations": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: annotationUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"test-group"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - "allow system:masters group user to modify MC spec": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: specUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"system:masters"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"system:masters"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - "allow system:masters group user to modify MC status": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: statusUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"system:masters"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - SubResource: "status", - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"system:masters"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "status", types.NamespacedName{Name: "test-mc"})), - }, - "allow whitelisted user to modify MC status": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: statusUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"test-group"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - SubResource: "status", - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - whiteListedUsers: []string{"test-user"}, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "status", types.NamespacedName{Name: "test-mc"})), - }, - "deny update of member cluster spec by non system user": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: specUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"test-group"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Denied(fmt.Sprintf(validation.ResourceDeniedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - "deny update of member cluster spec by non whitelisted user ": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Object: runtime.RawExtension{ - Raw: specUpdatedMCObjectBytes, - }, - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "test-user", - Groups: []string{"test-group"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - whiteListedUsers: []string{"test-user1"}, - }, - wantResponse: admission.Denied(fmt.Sprintf(validation.ResourceDeniedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - // added as UT since testing this case as an E2E requires - // creating a new user called aks-support in our test environment. - "allow delete of member cluster by aks-support user": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - OldObject: runtime.RawExtension{ - Raw: MCObjectBytes, - }, - UserInfo: authenticationv1.UserInfo{ - Username: "aks-support", - Groups: []string{"system:authenticated"}, - }, - RequestKind: &utils.MCV1Alpha1MetaGVK, - Operation: admissionv1.Delete, - }, - }, - resourceValidator: fleetResourceValidator{ - decoder: decoder, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "aks-support", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Delete, &utils.MCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc"})), - }, - } - - for testName, testCase := range testCases { - t.Run(testName, func(t *testing.T) { - gotResult := testCase.resourceValidator.handleV1Alpha1MemberCluster(testCase.req) - assert.Equal(t, testCase.wantResponse, gotResult, utils.TestCaseMsg, testName) - }) - } -} - func TestHandleMemberCluster(t *testing.T) { // The UTs for this function are less because most of the cases are covered in E2Es in fleet_guard_rail_test.go. // The E2Es also cover actual behavior changes to the requests received by the webhook. @@ -476,7 +186,7 @@ func TestHandleMemberCluster(t *testing.T) { Status: clusterv1beta1.MemberClusterStatus{ Conditions: []metav1.Condition{ { - Type: string(fleetv1alpha1.ConditionTypeMemberClusterReadyToJoin), + Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin), Status: metav1.ConditionTrue, }, }, @@ -499,7 +209,7 @@ func TestHandleMemberCluster(t *testing.T) { Status: clusterv1beta1.MemberClusterStatus{ Conditions: []metav1.Condition{ { - Type: string(fleetv1alpha1.ConditionTypeMemberClusterReadyToJoin), + Type: string(clusterv1beta1.ConditionTypeMemberClusterReadyToJoin), Status: metav1.ConditionTrue, }, }, @@ -508,7 +218,7 @@ func TestHandleMemberCluster(t *testing.T) { assert.Nil(t, err) scheme := runtime.NewScheme() - err = fleetv1alpha1.AddToScheme(scheme) + err = clusterv1beta1.AddToScheme(scheme) assert.Nil(t, err) decoder := admission.NewDecoder(scheme) assert.Nil(t, err) @@ -828,25 +538,6 @@ func TestHandleMemberCluster(t *testing.T) { } func TestHandleFleetReservedNamespacedResource(t *testing.T) { - v1Alpha1MockClient := &test.MockClient{ - MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { - if key.Name == mcName { - o := obj.(*fleetv1alpha1.MemberCluster) - *o = fleetv1alpha1.MemberCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: mcName, - }, - Spec: fleetv1alpha1.MemberClusterSpec{ - Identity: rbacv1.Subject{ - Name: "test-identity", - }, - }, - } - return nil - } - return errors.New("cannot find member cluster") - }, - } mockClient := &test.MockClient{ MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { if key.Name == mcName { @@ -871,72 +562,6 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { resourceValidator fleetResourceValidator wantResponse admission.Response }{ - "allow user not in system:masters group with create in non-fleet member cluster namespace with v1alpha1 IMC": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Namespace: "test-ns", - RequestKind: &utils.IMCV1Alpha1MetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "testUser", - Groups: []string{"testGroup"}, - }, - Operation: admissionv1.Create, - }, - }, - wantResponse: admission.Allowed(allowedMessageFleetReservedNamespacedResource), - }, - "allow hub-agent-sa in MC identity with create with v1alpha1 IMC": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Namespace: "fleet-member-test-mc", - RequestKind: &utils.IMCV1Alpha1MetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "system:serviceaccount:fleet-system:hub-agent-sa", - Groups: []string{"system:serviceaccounts"}, - }, - Operation: admissionv1.Create, - }, - }, - resourceValidator: fleetResourceValidator{ - client: &test.MockClient{ - MockGet: func(ctx context.Context, key client.ObjectKey, obj client.Object) error { - o := obj.(*fleetv1alpha1.MemberCluster) - *o = fleetv1alpha1.MemberCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: mcName, - }, - Spec: fleetv1alpha1.MemberClusterSpec{ - Identity: rbacv1.Subject{ - Name: "hub-agent-sa", - }, - }, - } - return nil - }, - }, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "system:serviceaccount:fleet-system:hub-agent-sa", utils.GenerateGroupString([]string{"system:serviceaccounts"}), admissionv1.Create, &utils.IMCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), - }, - "allow user in MC identity with create in fleet member cluster namespace with internalServiceExport with v1alpha1 client": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-ise", - Namespace: "fleet-member-test-mc", - RequestKind: &utils.InternalServiceExportMetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "test-identity", - Groups: []string{"system:authenticated"}, - }, - Operation: admissionv1.Create, - }, - }, - resourceValidator: fleetResourceValidator{ - client: v1Alpha1MockClient, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Create, &utils.InternalServiceExportMetaGVK, "", types.NamespacedName{Name: "test-ise", Namespace: "fleet-member-test-mc"})), - }, "allow user in MC identity with create in fleet member cluster namespace with internalServiceExport with v1beta1 client": { req: admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ @@ -951,8 +576,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Create, &utils.InternalServiceExportMetaGVK, "", types.NamespacedName{Name: "test-ise", Namespace: "fleet-member-test-mc"})), }, @@ -971,60 +595,6 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "testUser", utils.GenerateGroupString([]string{"system:masters"}), admissionv1.Update, &utils.WorkV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-work", Namespace: "fleet-member-test-mc"})), }, - "allow user in MC identity with update in fleet member cluster namespace with v1alpha1 IMC": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Namespace: "fleet-member-test-mc", - RequestKind: &utils.IMCV1Alpha1MetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "test-identity", - Groups: []string{"system:authenticated"}, - }, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - client: v1Alpha1MockClient, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Update, &utils.IMCV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), - }, - "allow user in MC identity with update in fleet member cluster namespace with v1alpha1 Work": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-mc", - Namespace: "fleet-member-test-mc", - RequestKind: &utils.WorkV1Alpha1MetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "test-identity", - Groups: []string{"system:authenticated"}, - }, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - client: v1Alpha1MockClient, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Update, &utils.WorkV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), - }, - "allow request if get MC failed with internal server error with v1alpha1 Work": { - req: admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Name: "test-work", - Namespace: "fleet-member-test-mc1", - RequestKind: &utils.WorkV1Alpha1MetaGVK, - UserInfo: authenticationv1.UserInfo{ - Username: "testUser", - Groups: []string{"testGroup"}, - }, - Operation: admissionv1.Update, - }, - }, - resourceValidator: fleetResourceValidator{ - client: v1Alpha1MockClient, - }, - wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedGetMCFailed, "testUser", utils.GenerateGroupString([]string{"testGroup"}), admissionv1.Update, &utils.WorkV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-work", Namespace: "fleet-member-test-mc1"})), - }, "allow user in MC identity with update in fleet member cluster namespace with v1beta1 IMC": { req: admission.Request{ AdmissionRequest: admissionv1.AdmissionRequest{ @@ -1039,8 +609,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Update, &utils.IMCMetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), }, @@ -1058,8 +627,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-identity", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Update, &utils.WorkMetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), }, @@ -1077,8 +645,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Denied(fmt.Sprintf(validation.ResourceDeniedFormat, "test-user", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Update, &utils.IMCMetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), }, @@ -1096,8 +663,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedGetMCFailed, "testUser", utils.GenerateGroupString([]string{"testGroup"}), admissionv1.Update, &utils.WorkV1Alpha1MetaGVK, "", types.NamespacedName{Name: "test-work", Namespace: "fleet-member-test-mc1"})), }, @@ -1115,8 +681,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Denied(fmt.Sprintf(validation.ResourceDeniedFormat, "testUser", utils.GenerateGroupString([]string{"testGroup"}), admissionv1.Create, &utils.EndpointSliceExportMetaGVK, "", types.NamespacedName{Name: "test-net-eps", Namespace: "fleet-system"})), }, @@ -1136,8 +701,7 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) { }, }, resourceValidator: fleetResourceValidator{ - client: mockClient, - isFleetV1Beta1API: true, + client: mockClient, }, wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "aks-support", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Delete, &utils.IMCMetaGVK, "", types.NamespacedName{Name: "test-mc", Namespace: "fleet-member-test-mc"})), }, diff --git a/pkg/webhook/validation/uservalidation.go b/pkg/webhook/validation/uservalidation.go index 2bf7af72f..f2078a188 100644 --- a/pkg/webhook/validation/uservalidation.go +++ b/pkg/webhook/validation/uservalidation.go @@ -19,7 +19,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/pkg/utils" ) @@ -72,28 +71,6 @@ func ValidateUserForResource(req admission.Request, whiteListedUsers []string) a return admission.Denied(fmt.Sprintf(ResourceDeniedFormat, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) } -// ValidateV1Alpha1MemberClusterUpdate checks to see if user had updated the member cluster resource and allows/denies the request. -func ValidateV1Alpha1MemberClusterUpdate(currentMC, oldMC fleetv1alpha1.MemberCluster, req admission.Request, whiteListedUsers []string) admission.Response { - namespacedName := types.NamespacedName{Name: currentMC.GetName()} - userInfo := req.UserInfo - response := admission.Allowed(fmt.Sprintf("user %s in groups %v most likely %s read-only field/fields of member cluster resource %+v/%s, so no field/fields will be updated", userInfo.Username, userInfo.Groups, req.Operation, req.RequestKind, req.SubResource)) - isLabelUpdated := isMapFieldUpdated(currentMC.GetLabels(), oldMC.GetLabels()) - isAnnotationUpdated := isMapFieldUpdated(currentMC.GetAnnotations(), oldMC.GetAnnotations()) - isObjUpdated, err := isMemberClusterUpdated(¤tMC, &oldMC) - if err != nil { - return admission.Denied(err.Error()) - } - if (isLabelUpdated || isAnnotationUpdated) && !isObjUpdated { - // we allow any user to modify v1alpha1 MemberCluster labels & annotations. - klog.V(3).InfoS("user in groups is allowed to modify member cluster labels/annotations", "user", userInfo.Username, "groups", userInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName) - response = admission.Allowed(fmt.Sprintf(ResourceAllowedFormat, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) - } - if isObjUpdated { - response = ValidateUserForResource(req, whiteListedUsers) - } - return response -} - // ValidateFleetMemberClusterUpdate checks to see if user had updated the fleet member cluster resource and allows/denies the request. func ValidateFleetMemberClusterUpdate(currentMC, oldMC clusterv1beta1.MemberCluster, req admission.Request, whiteListedUsers []string, denyModifyMemberClusterLabels bool) admission.Response { namespacedName := types.NamespacedName{Name: currentMC.GetName()} @@ -282,29 +259,18 @@ func checkCRDGroup(group string) bool { } // ValidateMCIdentity returns admission allowed/denied based on the member cluster's identity. -func ValidateMCIdentity(ctx context.Context, client client.Client, req admission.Request, mcName string, isFleetV1Beta1API bool) admission.Response { +func ValidateMCIdentity(ctx context.Context, client client.Client, req admission.Request, mcName string) admission.Response { var identity string namespacedName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace} userInfo := req.UserInfo - if !isFleetV1Beta1API { - var mc fleetv1alpha1.MemberCluster - if err := client.Get(ctx, types.NamespacedName{Name: mcName}, &mc); err != nil { - // fail open, if the webhook cannot get member cluster resources we don't block the request. - klog.ErrorS(err, fmt.Sprintf("failed to get v1alpha1 member cluster resource for request to modify %+v/%s, allowing request to be handled by api server", req.RequestKind, req.SubResource), - "user", userInfo.Username, "groups", userInfo.Groups, "namespacedName", namespacedName) - return admission.Allowed(fmt.Sprintf(ResourceAllowedGetMCFailed, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) - } - identity = mc.Spec.Identity.Name - } else { - var mc clusterv1beta1.MemberCluster - if err := client.Get(ctx, types.NamespacedName{Name: mcName}, &mc); err != nil { - // fail open, if the webhook cannot get member cluster resources we don't block the request. - klog.ErrorS(err, fmt.Sprintf("failed to get member cluster resource for request to modify %+v/%s, allowing request to be handled by api server", req.RequestKind, req.SubResource), - "user", userInfo.Username, "groups", userInfo.Groups, "namespacedName", namespacedName) - return admission.Allowed(fmt.Sprintf(ResourceAllowedGetMCFailed, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) - } - identity = mc.Spec.Identity.Name + var mc clusterv1beta1.MemberCluster + if err := client.Get(ctx, types.NamespacedName{Name: mcName}, &mc); err != nil { + // fail open, if the webhook cannot get member cluster resources we don't block the request. + klog.ErrorS(err, fmt.Sprintf("failed to get member cluster resource for request to modify %+v/%s, allowing request to be handled by api server", req.RequestKind, req.SubResource), + "user", userInfo.Username, "groups", userInfo.Groups, "namespacedName", namespacedName) + return admission.Allowed(fmt.Sprintf(ResourceAllowedGetMCFailed, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) } + identity = mc.Spec.Identity.Name // For the upstream E2E we use hub agent service account's token which allows member agent to modify Work status, hence we use serviceAccountFmt to make the check. if identity == userInfo.Username || fmt.Sprintf(serviceAccountFmt, identity) == userInfo.Username { diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index ba08cd295..474e6863f 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -54,7 +54,6 @@ import ( clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" - fleetv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/v1alpha1" "github.com/kubefleet-dev/kubefleet/cmd/hubagent/options" "github.com/kubefleet-dev/kubefleet/pkg/webhook/clusterresourceoverride" "github.com/kubefleet-dev/kubefleet/pkg/webhook/clusterresourceplacement" @@ -132,16 +131,16 @@ var ( ) var AddToManagerFuncs []func(manager.Manager) error -var AddToManagerFleetResourceValidator func(manager.Manager, []string, bool, bool) error +var AddToManagerFleetResourceValidator func(manager.Manager, []string, bool) error // AddToManager adds all Controllers to the Manager -func AddToManager(m manager.Manager, whiteListedUsers []string, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool) error { +func AddToManager(m manager.Manager, whiteListedUsers []string, denyModifyMemberClusterLabels bool) error { for _, f := range AddToManagerFuncs { if err := f(m); err != nil { return err } } - return AddToManagerFleetResourceValidator(m, whiteListedUsers, isFleetV1Beta1API, denyModifyMemberClusterLabels) + return AddToManagerFleetResourceValidator(m, whiteListedUsers, denyModifyMemberClusterLabels) } type Config struct { @@ -321,23 +320,6 @@ func (w *Config) buildFleetValidatingWebhooks() []admv1.ValidatingWebhook { }, TimeoutSeconds: longWebhookTimeout, }, - { - Name: "fleet.clusterresourceplacementv1alpha1.validating", - ClientConfig: w.createClientConfig(clusterresourceplacement.V1Alpha1CRPValidationPath), - FailurePolicy: &failFailurePolicy, - SideEffects: &sideEffortsNone, - AdmissionReviewVersions: admissionReviewVersions, - Rules: []admv1.RuleWithOperations{ - { - Operations: []admv1.OperationType{ - admv1.Create, - admv1.Update, - }, - Rule: createRule([]string{fleetv1alpha1.GroupVersion.Group}, []string{fleetv1alpha1.GroupVersion.Version}, []string{fleetv1alpha1.ClusterResourcePlacementResource}, &clusterScope), - }, - }, - TimeoutSeconds: longWebhookTimeout, - }, { Name: "fleet.clusterresourceplacementv1beta1.validating", ClientConfig: w.createClientConfig(clusterresourceplacement.ValidationPath), @@ -552,10 +534,6 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho Operations: cuOperations, Rule: createRule([]string{storagev1.SchemeGroupVersion.Group}, []string{storagev1.SchemeGroupVersion.Version}, []string{csiStorageCapacityResourceName}, &namespacedScope), }, - { - Operations: cuOperations, - Rule: createRule([]string{fleetv1alpha1.GroupVersion.Group}, []string{fleetv1alpha1.GroupVersion.Version}, []string{internalMemberClusterResourceName, internalMemberClusterResourceName + "/status"}, &namespacedScope), - }, { Operations: cuOperations, Rule: createRule([]string{clusterv1beta1.GroupVersion.Group}, []string{clusterv1beta1.GroupVersion.Version}, []string{internalMemberClusterResourceName, internalMemberClusterResourceName + "/status"}, &namespacedScope), @@ -602,20 +580,6 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho }, TimeoutSeconds: shortWebhookTimeout, }, - { - Name: "fleet.v1alpha1.membercluster.guardrail.validating", - ClientConfig: w.createClientConfig(fleetresourcehandler.ValidationPath), - FailurePolicy: &ignoreFailurePolicy, - SideEffects: &sideEffortsNone, - AdmissionReviewVersions: admissionReviewVersions, - Rules: []admv1.RuleWithOperations{ - { - Operations: cudOperations, - Rule: createRule([]string{fleetv1alpha1.GroupVersion.Group}, []string{fleetv1alpha1.GroupVersion.Version}, []string{memberClusterResourceName, memberClusterResourceName + "/status"}, &clusterScope), - }, - }, - TimeoutSeconds: shortWebhookTimeout, - }, { Name: "fleet.fleetmembernamespacedresources.guardrail.validating", ClientConfig: w.createClientConfig(fleetresourcehandler.ValidationPath), diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index b44d98e8c..acc976825 100644 --- a/pkg/webhook/webhook_test.go +++ b/pkg/webhook/webhook_test.go @@ -52,7 +52,7 @@ func TestBuildFleetValidatingWebhooks(t *testing.T) { serviceURL: "test-url", clientConnectionType: &url, }, - wantLength: 9, + wantLength: 8, }, } @@ -77,7 +77,7 @@ func TestBuildFleetGuardRailValidatingWebhooks(t *testing.T) { serviceURL: "test-url", clientConnectionType: &url, }, - wantLength: 7, + wantLength: 6, }, } From 0471c75e0069c03b8aadd78bb7ff508ccd6aba54 Mon Sep 17 00:00:00 2001 From: Wei Weng Date: Mon, 27 Oct 2025 21:53:48 +0000 Subject: [PATCH 2/2] add TODO for workv1alpha1 Signed-off-by: Wei Weng --- pkg/utils/apiresources.go | 1 + pkg/utils/common.go | 1 + pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go | 1 + 3 files changed, 3 insertions(+) diff --git a/pkg/utils/apiresources.go b/pkg/utils/apiresources.go index 99cb45c73..b788d2af4 100644 --- a/pkg/utils/apiresources.go +++ b/pkg/utils/apiresources.go @@ -186,6 +186,7 @@ func NewResourceConfig(isAllowList bool) *ResourceConfig { if r.isAllowList { return r } + // TODO (weiweng): remove workv1alpha1 in next PR r.AddGroupVersionKind(WorkV1Alpha1GVK) // disable cluster group by default diff --git a/pkg/utils/common.go b/pkg/utils/common.go index b78b7ef40..e1e9e53ba 100644 --- a/pkg/utils/common.go +++ b/pkg/utils/common.go @@ -345,6 +345,7 @@ var ( Resource: "storageclasses", } + // TODO (weiweng): remove workv1alpha1 in next PR WorkV1Alpha1MetaGVK = metav1.GroupVersionKind{ Group: workv1alpha1.GroupVersion.Group, Version: workv1alpha1.GroupVersion.Version, diff --git a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go index 7b40f986e..e96c0c67a 100644 --- a/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go +++ b/pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go @@ -72,6 +72,7 @@ func (v *fleetResourceValidator) Handle(ctx context.Context, req admission.Reque case req.Kind == utils.NamespaceMetaGVK: klog.V(2).InfoS("handling namespace resource", "name", req.Name, "operation", req.Operation, "subResource", req.SubResource) response = v.handleNamespace(req) + // TODO (weiweng): remove workv1alpha1 in next PR case req.Kind == utils.WorkV1Alpha1MetaGVK || req.Kind == utils.IMCMetaGVK || req.Kind == utils.WorkMetaGVK || req.Kind == utils.EndpointSliceExportMetaGVK || req.Kind == utils.EndpointSliceImportMetaGVK || req.Kind == utils.InternalServiceExportMetaGVK || req.Kind == utils.InternalServiceImportMetaGVK: klog.V(2).InfoS("handling fleet owned namespaced resource in fleet reserved namespaces", "GVK", req.RequestKind, "namespacedName", namespacedName, "operation", req.Operation, "subResource", req.SubResource) response = v.handleFleetReservedNamespacedResource(ctx, req)