From 0be3c62f06e2bf04c162bc577f2cab8f386bbbeb Mon Sep 17 00:00:00 2001 From: Nont <9658731+nwnt@users.noreply.github.com> Date: Mon, 11 Aug 2025 11:42:18 -0500 Subject: [PATCH] Revert "feat: add managed resource webhook" --- pkg/webhook/add_handler.go | 3 - .../managedresource_validating_webhook.go | 99 -------- ...managedresource_validating_webhook_test.go | 228 ------------------ pkg/webhook/validation/uservalidation.go | 8 +- pkg/webhook/webhook.go | 49 +--- pkg/webhook/webhook_test.go | 2 +- test/e2e/resources_test.go | 1 - test/e2e/utils_test.go | 18 -- test/e2e/webhook_test.go | 68 ------ 9 files changed, 11 insertions(+), 465 deletions(-) delete mode 100644 pkg/webhook/managedresource/managedresource_validating_webhook.go delete mode 100644 pkg/webhook/managedresource/managedresource_validating_webhook_test.go diff --git a/pkg/webhook/add_handler.go b/pkg/webhook/add_handler.go index 075885da9..ac0f9bd5b 100644 --- a/pkg/webhook/add_handler.go +++ b/pkg/webhook/add_handler.go @@ -6,7 +6,6 @@ import ( "go.goms.io/fleet/pkg/webhook/clusterresourceplacementdisruptionbudget" "go.goms.io/fleet/pkg/webhook/clusterresourceplacementeviction" "go.goms.io/fleet/pkg/webhook/fleetresourcehandler" - "go.goms.io/fleet/pkg/webhook/managedresource" "go.goms.io/fleet/pkg/webhook/membercluster" "go.goms.io/fleet/pkg/webhook/pod" "go.goms.io/fleet/pkg/webhook/replicaset" @@ -16,8 +15,6 @@ import ( func init() { // AddToManagerFleetResourceValidator is a function to register fleet guard rail resource validator to the webhook server AddToManagerFleetResourceValidator = fleetresourcehandler.Add - // AddtoManagerManagedResource is a function to register managed resource validator to the webhook server - AddtoManagerManagedResource = managedresource.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) diff --git a/pkg/webhook/managedresource/managedresource_validating_webhook.go b/pkg/webhook/managedresource/managedresource_validating_webhook.go deleted file mode 100644 index ef45be3ba..000000000 --- a/pkg/webhook/managedresource/managedresource_validating_webhook.go +++ /dev/null @@ -1,99 +0,0 @@ -/* -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -*/ - -package managedresource - -import ( - "context" - "fmt" - "net/http" - - admissionv1 "k8s.io/api/admission/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "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" - - "go.goms.io/fleet/pkg/utils" - "go.goms.io/fleet/pkg/webhook/validation" -) - -const ( - ManagedByArmKey = "managed-by" - ManagedByArmValue = "arm" - deniedResource = "denied admission for managed resource" - resourceDeniedFormat = "the operation on the managed resource type '%s' name '%s' in namespace '%s' is not allowed" -) - -// ValidationPath is the webhook service path which admission requests are routed to. -var ( - ValidationPath = fmt.Sprintf(utils.ValidationPathFmt, "arm", "managed", "resources") -) - -// Add registers the webhook for K8s bulit-in object types. -func Add(mgr manager.Manager, whiteListedUsers []string) error { - hookServer := mgr.GetWebhookServer() - hookServer.Register(ValidationPath, &webhook.Admission{Handler: &managedResourceValidator{ - whiteListedUsers: whiteListedUsers, - }}) - return nil -} - -type managedResourceValidator struct { - whiteListedUsers []string -} - -// Handle denies the resource admission if the request target object has a label or annotation key "fleet.azure.com". -func (v *managedResourceValidator) Handle(_ context.Context, req admission.Request) admission.Response { - namespacedName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace} - klog.V(1).InfoS("handling resource", "operation", req.Operation, "subResource", req.SubResource, "namespacedName", namespacedName) - - var objs []runtime.RawExtension - switch req.Operation { - case admissionv1.Create: - objs = append(objs, req.Object) - case admissionv1.Update: - objs = append(objs, req.Object, req.OldObject) - case admissionv1.Delete: - objs = append(objs, req.OldObject) - } - for _, obj := range objs { - labels, annotations, err := getLabelsAndAnnotations(obj) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - if (managedByArm(labels) || managedByArm(annotations)) && !validation.IsAdminGroupUserOrWhiteListedUser(v.whiteListedUsers, req.UserInfo) { - klog.V(2).InfoS(deniedResource, "user", req.UserInfo.Username, "groups", req.UserInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName) - return admission.Denied(fmt.Sprintf(resourceDeniedFormat, req.Kind, req.Name, req.Namespace)) - } - } - return admission.Allowed("") -} - -func getLabelsAndAnnotations(raw runtime.RawExtension) (map[string]string, map[string]string, error) { - var obj runtime.Object - if err := runtime.Convert_runtime_RawExtension_To_runtime_Object(&raw, &obj, nil); err != nil { - return nil, nil, err - } - o, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - if err != nil { - return nil, nil, err - } - u := unstructured.Unstructured{Object: o} - return u.GetLabels(), u.GetAnnotations(), nil -} - -func managedByArm(m map[string]string) bool { - if len(m) == 0 { - return false - } - if v, ok := m[ManagedByArmKey]; ok && v == ManagedByArmValue { - return true - } - return false -} diff --git a/pkg/webhook/managedresource/managedresource_validating_webhook_test.go b/pkg/webhook/managedresource/managedresource_validating_webhook_test.go deleted file mode 100644 index 4355d7aac..000000000 --- a/pkg/webhook/managedresource/managedresource_validating_webhook_test.go +++ /dev/null @@ -1,228 +0,0 @@ -/* -Copyright (c) Microsoft Corporation. -Licensed under the MIT license. -*/ - -package managedresource - -import ( - "context" - "fmt" - "net/http" - "testing" - - "github.com/google/go-cmp/cmp" - admissionv1 "k8s.io/api/admission/v1" - authenticationv1 "k8s.io/api/authentication/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission" -) - -func Test_managedResourceValidator_Handle(t *testing.T) { - const fleet1p = "fleet1p" - validator := &managedResourceValidator{ - whiteListedUsers: []string{fleet1p}, - } - - tests := []struct { - name string - username string - operation admissionv1.Operation - oldLabels map[string]string - oldAnnotations map[string]string - newLabels map[string]string - newAnnotations map[string]string - expectedResp admission.Response - modReq func(*admission.Request) - }{ - { - name: "allowed when not managed by arm", - operation: admissionv1.Update, - oldLabels: map[string]string{"foo": "bar"}, - oldAnnotations: map[string]string{"baz": "qux"}, - newLabels: map[string]string{"foo": "bar"}, - newAnnotations: map[string]string{"baz": "qux"}, - expectedResp: admission.Allowed(""), - }, - { - name: "denied - error on getLabelsAndAnnotations failure", - operation: admissionv1.Create, - expectedResp: admission.Errored(http.StatusInternalServerError, fmt.Errorf("error decoding string from json: unexpected trailing data at offset 9")), - modReq: func(req *admission.Request) { - req.Object = runtime.RawExtension{Raw: []byte(`"invalid"}`)} // Invalid object without labels or annotations - }, - }, - { - name: "denied - managed by arm in labels, not whitelisted", - operation: admissionv1.Create, - oldLabels: nil, - oldAnnotations: nil, - newLabels: map[string]string{ManagedByArmKey: ManagedByArmValue}, - newAnnotations: nil, - expectedResp: admission.Denied(fmt.Sprintf(resourceDeniedFormat, metav1.GroupVersionKind{Kind: "TestKind"}, "test-resource", "default")), - }, - { - name: "denied - managed by arm in annotations, not whitelisted", - operation: admissionv1.Update, - oldLabels: nil, - oldAnnotations: nil, - newLabels: nil, - newAnnotations: map[string]string{ManagedByArmKey: ManagedByArmValue}, - expectedResp: admission.Denied(fmt.Sprintf(resourceDeniedFormat, metav1.GroupVersionKind{Kind: "TestKind"}, "test-resource", "default")), - }, - { - name: "allowed for other operations", - operation: admissionv1.Connect, - oldLabels: nil, - oldAnnotations: nil, - newLabels: nil, - newAnnotations: nil, - expectedResp: admission.Allowed(""), - }, - { - name: "allowed for other operations - managed by arm, but user whitelisted", - username: "fleet1p", - operation: admissionv1.Update, - oldLabels: map[string]string{"managedBy": ManagedByArmValue}, - oldAnnotations: nil, - newLabels: nil, - newAnnotations: nil, - expectedResp: admission.Allowed(""), - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - oldObj := makeUnstructured(tt.oldLabels, tt.oldAnnotations) - newObj := makeUnstructured(tt.newLabels, tt.newAnnotations) - req := admission.Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: tt.operation, - Name: "test-resource", - Namespace: "default", - OldObject: runtime.RawExtension{Object: oldObj}, - Object: runtime.RawExtension{Object: newObj}, - Kind: metav1.GroupVersionKind{Kind: "TestKind"}, - RequestKind: &metav1.GroupVersionKind{Kind: "TestKind"}, - }, - } - req.UserInfo = authenticationv1.UserInfo{ - Username: tt.username, - } - if tt.modReq != nil { - tt.modReq(&req) - } - resp := validator.Handle(context.Background(), req) - if diff := cmp.Diff(tt.expectedResp.Result, resp.Result); diff != "" { - t.Errorf("managedResourceValidator Handle response (-want +got):\n%s", diff) - } - }) - } -} - -func Test_getLabelsAndAnnotations(t *testing.T) { - tests := []struct { - name string - obj runtime.RawExtension - wantLabels map[string]string - wantAnnotations map[string]string - expectError bool - }{ - { - name: "object with labels and annotations", - obj: runtime.RawExtension{ - Object: &metav1.PartialObjectMetadata{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"foo": "bar"}, - Annotations: map[string]string{"baz": "qux"}, - }, - }, - }, - wantLabels: map[string]string{"foo": "bar"}, - wantAnnotations: map[string]string{"baz": "qux"}, - expectError: false, - }, - { - name: "object with no labels or annotations", - obj: runtime.RawExtension{ - Object: &metav1.PartialObjectMetadata{ - ObjectMeta: metav1.ObjectMeta{}, - }, - }, - wantLabels: nil, - wantAnnotations: nil, - expectError: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - labels, annotations, err := getLabelsAndAnnotations(tt.obj) - if err != nil && !tt.expectError { - t.Fatalf("unexpected error: %v", err) - } - if diff := cmp.Diff(tt.wantLabels, labels); diff != "" { - t.Errorf("labels mismatch (-want +got):\n%s", diff) - } - if diff := cmp.Diff(tt.wantAnnotations, annotations); diff != "" { - t.Errorf("annotations mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func Test_managedByArm(t *testing.T) { - tests := []struct { - name string - m map[string]string - want bool - }{ - { - name: "nil map", - m: nil, - want: false, - }, - { - name: "empty map", - m: map[string]string{}, - want: false, - }, - { - name: "key missing", - m: map[string]string{"foo": "bar"}, - want: false, - }, - { - name: "key present, not managed key", - m: map[string]string{"managingBy": ManagedByArmValue}, - want: false, - }, - { - name: "key present, not managed value", - m: map[string]string{ManagedByArmKey: "not-arm"}, - want: false, - }, - { - name: "key present, managed key and value", - m: map[string]string{ManagedByArmKey: ManagedByArmValue}, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if diff := cmp.Diff(tt.want, managedByArm(tt.m)); diff != "" { - t.Errorf("managedByArm result (-want +got):\n%s", diff) - } - }) - } -} - -func makeUnstructured(labels, annotations map[string]string) *unstructured.Unstructured { - obj := &unstructured.Unstructured{} - obj.SetLabels(labels) - obj.SetAnnotations(annotations) - return obj -} diff --git a/pkg/webhook/validation/uservalidation.go b/pkg/webhook/validation/uservalidation.go index 3096340d1..535532c13 100644 --- a/pkg/webhook/validation/uservalidation.go +++ b/pkg/webhook/validation/uservalidation.go @@ -51,7 +51,7 @@ var ( func ValidateUserForFleetCRD(req admission.Request, whiteListedUsers []string, group string) admission.Response { namespacedName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace} userInfo := req.UserInfo - if checkCRDGroup(group) && !IsAdminGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) { + if checkCRDGroup(group) && !isAdminGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) { klog.V(2).InfoS(deniedModifyResource, "user", userInfo.Username, "groups", userInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName) return admission.Denied(fmt.Sprintf(ResourceDeniedFormat, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) } @@ -63,7 +63,7 @@ func ValidateUserForFleetCRD(req admission.Request, whiteListedUsers []string, g func ValidateUserForResource(req admission.Request, whiteListedUsers []string) admission.Response { namespacedName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace} userInfo := req.UserInfo - if IsAdminGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) || isUserAuthenticatedServiceAccount(userInfo) || isUserKubeScheduler(userInfo) || isUserKubeControllerManager(userInfo) || isUserInGroup(userInfo, nodeGroup) || isAKSSupportUser(userInfo) { + if isAdminGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) || isUserAuthenticatedServiceAccount(userInfo) || isUserKubeScheduler(userInfo) || isUserKubeControllerManager(userInfo) || isUserInGroup(userInfo, nodeGroup) || isAKSSupportUser(userInfo) { klog.V(3).InfoS(allowedModifyResource, "user", userInfo.Username, "groups", userInfo.Groups, "operation", req.Operation, "GVK", req.RequestKind, "subResource", req.SubResource, "namespacedName", namespacedName) return admission.Allowed(fmt.Sprintf(ResourceAllowedFormat, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) } @@ -144,10 +144,10 @@ func ValidatedUpstreamMemberClusterUpdate(currentMC, oldMC clusterv1beta1.Member return admission.Allowed(fmt.Sprintf(ResourceAllowedFormat, userInfo.Username, utils.GenerateGroupString(userInfo.Groups), req.Operation, req.RequestKind, req.SubResource, namespacedName)) } -// IsAdminGroupUserOrWhiteListedUser returns true is user belongs to white listed users or user belongs to system:masters/kubeadm:cluster-admins group. +// isAdminGroupUserOrWhiteListedUser returns true is user belongs to white listed users or user belongs to system:masters/kubeadm:cluster-admins group. // In clusters using kubeadm, kubernetes-admin belongs to kubeadm:cluster-admins group and kubernetes-super-admin user belongs to system:masters group. // https://kubernetes.io/docs/reference/setup-tools/kubeadm/implementation-details/#generate-kubeconfig-files-for-control-plane-components -func IsAdminGroupUserOrWhiteListedUser(whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { +func isAdminGroupUserOrWhiteListedUser(whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { return slices.Contains(whiteListedUsers, userInfo.Username) || slices.Contains(userInfo.Groups, mastersGroup) || slices.Contains(userInfo.Groups, kubeadmClusterAdminsGroup) } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 2455f75b9..240bfaa23 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -61,7 +61,6 @@ import ( "go.goms.io/fleet/pkg/webhook/clusterresourceplacementdisruptionbudget" "go.goms.io/fleet/pkg/webhook/clusterresourceplacementeviction" "go.goms.io/fleet/pkg/webhook/fleetresourcehandler" - "go.goms.io/fleet/pkg/webhook/managedresource" "go.goms.io/fleet/pkg/webhook/membercluster" "go.goms.io/fleet/pkg/webhook/pod" "go.goms.io/fleet/pkg/webhook/replicaset" @@ -132,11 +131,8 @@ var ( longWebhookTimeout = ptr.To(int32(5)) ) -var ( - AddToManagerFuncs []func(manager.Manager) error - AddtoManagerManagedResource func(manager.Manager, []string) error - AddToManagerFleetResourceValidator func(manager.Manager, []string, bool, bool) error -) +var AddToManagerFuncs []func(manager.Manager) error +var AddToManagerFleetResourceValidator func(manager.Manager, []string, bool, bool) error // AddToManager adds all Controllers to the Manager func AddToManager(m manager.Manager, whiteListedUsers []string, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool) error { @@ -145,9 +141,6 @@ func AddToManager(m manager.Manager, whiteListedUsers []string, isFleetV1Beta1AP return err } } - if err := AddtoManagerManagedResource(m, whiteListedUsers); err != nil { - return err - } return AddToManagerFleetResourceValidator(m, whiteListedUsers, isFleetV1Beta1API, denyModifyMemberClusterLabels) } @@ -462,32 +455,6 @@ func (w *Config) buildFleetValidatingWebhooks() []admv1.ValidatingWebhook { }, TimeoutSeconds: longWebhookTimeout, }, - { - Name: "fleet.managedresource.validating", - ClientConfig: w.createClientConfig(managedresource.ValidationPath), - FailurePolicy: &failFailurePolicy, - SideEffects: &sideEffortsNone, - AdmissionReviewVersions: admissionReviewVersions, - Rules: []admv1.RuleWithOperations{ - { - Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete}, - Rule: createRule([]string{placementv1beta1.GroupVersion.Group}, []string{placementv1beta1.GroupVersion.Version}, []string{placementv1beta1.ClusterResourcePlacementResource}, &clusterScope), - }, - { - Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete}, - Rule: createRule([]string{corev1.SchemeGroupVersion.Group}, []string{corev1.SchemeGroupVersion.Version}, []string{namespaceResourceName}, &clusterScope), - }, - { - Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete}, - Rule: createRule([]string{corev1.SchemeGroupVersion.Group}, []string{corev1.SchemeGroupVersion.Version}, []string{resourceQuotaResourceName}, &namespacedScope), - }, - { - Operations: []admv1.OperationType{admv1.Create, admv1.Update, admv1.Delete}, - Rule: createRule([]string{networkingv1.SchemeGroupVersion.Group}, []string{networkingv1.SchemeGroupVersion.Version}, []string{networkPolicyResourceName}, &namespacedScope), - }, - }, - TimeoutSeconds: longWebhookTimeout, - }, } return webHooks @@ -542,19 +509,15 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho // TODO(ArvindThiru): not handling pods, replicasets as part of the fleet guard rail since they have validating webhooks, need to remove validating webhooks before adding these resources to fleet guard rail. { Operations: cuOperations, - Rule: createRule([]string{corev1.SchemeGroupVersion.Group}, []string{corev1.SchemeGroupVersion.Version}, []string{ - bindingResourceName, configMapResourceName, endPointResourceName, + Rule: createRule([]string{corev1.SchemeGroupVersion.Group}, []string{corev1.SchemeGroupVersion.Version}, []string{bindingResourceName, configMapResourceName, endPointResourceName, limitRangeResourceName, persistentVolumeClaimsName, persistentVolumeClaimsName + "/status", podTemplateResourceName, replicationControllerResourceName, replicationControllerResourceName + "/status", resourceQuotaResourceName, resourceQuotaResourceName + "/status", secretResourceName, - serviceAccountResourceName, servicesResourceName, servicesResourceName + "/status", - }, &namespacedScope), + serviceAccountResourceName, servicesResourceName, servicesResourceName + "/status"}, &namespacedScope), }, { Operations: cuOperations, - Rule: createRule([]string{appsv1.SchemeGroupVersion.Group}, []string{appsv1.SchemeGroupVersion.Version}, []string{ - controllerRevisionResourceName, daemonSetResourceName, daemonSetResourceName + "/status", - deploymentResourceName, deploymentResourceName + "/status", statefulSetResourceName, statefulSetResourceName + "/status", - }, &namespacedScope), + Rule: createRule([]string{appsv1.SchemeGroupVersion.Group}, []string{appsv1.SchemeGroupVersion.Version}, []string{controllerRevisionResourceName, daemonSetResourceName, daemonSetResourceName + "/status", + deploymentResourceName, deploymentResourceName + "/status", statefulSetResourceName, statefulSetResourceName + "/status"}, &namespacedScope), }, { Operations: cuOperations, diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index 386645b2c..e4e733b25 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: 10, + wantLength: 9, }, } diff --git a/test/e2e/resources_test.go b/test/e2e/resources_test.go index eab4ceeef..e41ef55d9 100644 --- a/test/e2e/resources_test.go +++ b/test/e2e/resources_test.go @@ -49,7 +49,6 @@ const ( crpEvictionNameTemplate = "crpe-%d" updateRunStrategyNameTemplate = "curs-%d" updateRunNameWithSubIndexTemplate = "cur-%d-%d" - managedNamespaceTemplate = "managedns-%d" customDeletionBlockerFinalizer = "kubernetes-fleet.io/custom-deletion-blocker-finalizer" workNamespaceLabelName = "process" diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index a3adf4633..a03f77973 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -51,7 +51,6 @@ import ( "go.goms.io/fleet/pkg/propertyprovider/azure/trackers" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/condition" - "go.goms.io/fleet/pkg/webhook/managedresource" testv1alpha1 "go.goms.io/fleet/test/apis/v1alpha1" "go.goms.io/fleet/test/e2e/framework" ) @@ -713,22 +712,6 @@ func cleanupWorkResources() { cleanWorkResourcesOnCluster(hubCluster) } -func managedNamespace() corev1.Namespace { - return corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf(managedNamespaceTemplate, GinkgoParallelProcess()), - Labels: map[string]string{ - managedresource.ManagedByArmKey: managedresource.ManagedByArmValue, - }, - }, - } -} - -func createManagedNamespace() { - ns := managedNamespace() - Expect(hubClient.Create(ctx, &ns)).To(Succeed(), "Failed to create namespace %s", ns.Name) -} - func cleanWorkResourcesOnCluster(cluster *framework.Cluster) { ns := appNamespace() Expect(client.IgnoreNotFound(cluster.KubeClient.Delete(ctx, &ns))).To(Succeed(), "Failed to delete namespace %s", ns.Name) @@ -844,7 +827,6 @@ func checkIfRemovedWorkResourcesFromMemberClustersConsistently(clusters []*frame Consistently(workResourcesRemovedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to remove work resources from member cluster %s consistently", memberCluster.ClusterName) } } - func checkNamespaceExistsWithOwnerRefOnMemberCluster(nsName, crpName string) { Consistently(func() error { ns := &corev1.Namespace{} diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index 739fb5b60..203d29881 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -33,16 +33,13 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" - "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/defaulter" - "go.goms.io/fleet/pkg/webhook/managedresource" testutils "go.goms.io/fleet/test/e2e/v1alpha1/utils" ) var _ = Describe("webhook tests for CRP CREATE operations", func() { crpName := fmt.Sprintf(crpNameTemplate, GinkgoParallelProcess()) Context("validation webhooks tests", func() { - It("should deny create on CRP with invalid label selector", func() { selector := invalidWorkResourceSelector() // Create the CRP. @@ -1680,68 +1677,3 @@ var _ = Describe("webhook tests for ResourceOverride UPDATE operations", Ordered }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed()) }) }) - -var _ = Describe("webhook tests for operations on ARM managed resources", func() { - BeforeEach(func() { - By("creating a managed namespace") - createManagedNamespace() - }) - - AfterEach(func() { - By("deleting the managed namespace") - ns := managedNamespace() - Expect(hubClient.Delete(ctx, &ns)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{}), "Failed to delete the managed namespace") - Eventually(func() error { - if err := hubClient.Get(ctx, types.NamespacedName{Name: ns.Name}, &corev1.Namespace{}); !k8sErrors.IsNotFound(err) { - return fmt.Errorf("The managed namespace still exists or an unexpected error occurred: %w", err) - } - return nil - }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed(), "Failed to remove the managed namespace %s", ns.Name) - }) - - It("should deny create a managed resource namespace", func() { - createNs := managedNamespace() - createNs.Name = "this-will-be-denied" - err := impersonateHubClient.Create(ctx, &createNs) - var statusErr *k8sErrors.StatusError - Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) - Expect(statusErr.ErrStatus.Message).Should(MatchRegexp(".*the operation on the managed resource type .* is not allowed")) - }) - - It("should deny update a managed resource namespace", func() { - Eventually(func(g Gomega) error { - updateNamespace := managedNamespace() - g.Expect(impersonateHubClient.Get(ctx, types.NamespacedName{Name: updateNamespace.Name}, &updateNamespace)).To(BeNil(), "Failed to get the created unmanaged namespace") - updateNamespace.Labels["foo"] = "NotManaged" - err := impersonateHubClient.Update(ctx, &updateNamespace) - if k8sErrors.IsConflict(err) { - return err - } - var statusErr *k8sErrors.StatusError - g.Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Update managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) - Expect(statusErr.ErrStatus.Message).Should(MatchRegexp(".*the operation on the managed resource type .* is not allowed")) - return nil - }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed()) - }) - - It("should deny delete a managed resource namespace", func() { - created := managedNamespace() - err := impersonateHubClient.Delete(ctx, &created) - var statusErr *k8sErrors.StatusError - Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Delete managed namespace call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) - Expect(statusErr.ErrStatus.Message).Should(MatchRegexp(".*the operation on the managed resource type .* is not allowed")) - }) - - It("should allow create/update/delete an unmanaged resource namespace", func() { - unmanaged := managedNamespace() - unmanaged.Name = "this-should-be-allowed" - delete(unmanaged.Labels, managedresource.ManagedByArmKey) - Expect(impersonateHubClient.Create(ctx, &unmanaged)).Should(SatisfyAny(Succeed()), "Failed to create the unmanaged namespace") - Eventually(func(g Gomega) error { - g.Expect(impersonateHubClient.Get(ctx, types.NamespacedName{Name: unmanaged.Name}, &unmanaged)).To(BeNil(), "Failed to get the created unmanaged namespace") - unmanaged.Labels["foo"] = "NotManaged" - return impersonateHubClient.Update(ctx, &unmanaged) - }, testutils.PollTimeout, testutils.PollInterval).Should(Succeed()) - Expect(impersonateHubClient.Delete(ctx, &unmanaged)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{}), "Failed to delete the unmanaged namespace") - }) -})