working E2E

pkg/webhook/validation/uservalidation.go

@@ -15,6 +15,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
+	clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
 	fleetv1alpha1 "go.goms.io/fleet/apis/v1alpha1"
 )
 
@@ -36,6 +37,8 @@ const (
 
 var (
 	fleetCRDGroups = []string{"networking.fleet.azure.com", "fleet.azure.com", "multicluster.x-k8s.io", "cluster.kubernetes-fleet.io", "placement.kubernetes-fleet.io"}
+	imcGVK         = metav1.GroupVersionKind{Group: clusterv1beta1.GroupVersion.Group, Version: clusterv1beta1.GroupVersion.Version, Kind: "InternalMemberCluster"}
+	v1Alpha1IMCGVK = metav1.GroupVersionKind{Group: fleetv1alpha1.GroupVersion.Group, Version: fleetv1alpha1.GroupVersion.Version, Kind: "InternalMemberCluster"}
 )
 
 // ValidateUserForFleetCRD checks to see if user is not allowed to modify fleet CRDs.
@@ -163,17 +166,31 @@ 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) admission.Response {
+	var identity string
 	namespacedName := types.NamespacedName{Name: req.Name, Namespace: req.Namespace}
 	userInfo := req.UserInfo
-	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.V(2).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, userInfo.Groups, req.Operation, req.RequestKind, req.SubResource, namespacedName))
+	if *req.RequestKind == v1Alpha1IMCGVK {
+		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.V(2).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, userInfo.Groups, req.Operation, req.RequestKind, req.SubResource, namespacedName))
+		}
+		identity = mc.Spec.Identity.Name
+	} else if *req.RequestKind == imcGVK {
+		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.V(2).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, 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 mc.Spec.Identity.Name == userInfo.Username || fmt.Sprintf(serviceAccountFmt, mc.Spec.Identity.Name) == userInfo.Username {
+	if identity == userInfo.Username || fmt.Sprintf(serviceAccountFmt, identity) == userInfo.Username {
 		klog.V(2).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, userInfo.Groups, req.Operation, req.RequestKind, req.SubResource, namespacedName))
 	}

test/e2e/fleet_guard_rail_test.go

@@ -12,12 +12,14 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	admissionv1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
 	clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
+	placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
 	"go.goms.io/fleet/pkg/utils"
 )
 
@@ -147,7 +149,26 @@ var _ = Describe("fleet guard rail tests for allow/deny MC UPDATE, DELETE operat
 })
 
 var _ = Describe("fleet guard rail tests for deny IMC CREATE operations", func() {
-	mcName := fmt.Sprintf(mcNameTemplate, GinkgoParallelProcess())
+	mcName := "test-mc-123"
+
+	BeforeEach(func() {
+		ns := corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   fmt.Sprintf(utils.NamespaceNameFormat, mcName),
+				Labels: map[string]string{placementv1beta1.FleetResourceLabelKey: "true"},
+			},
+		}
+		Expect(hubClient.Create(ctx, &ns)).Should(Succeed())
+	})
+
+	AfterEach(func() {
+		ns := corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: fmt.Sprintf(utils.NamespaceNameFormat, mcName),
+			},
+		}
+		Expect(hubClient.Delete(ctx, &ns)).Should(Succeed())
+	})
 
 	It("should deny CREATE operation on internal member cluster CR for user not in MC identity in fleet member namespace", func() {
 		imcNamespace := fmt.Sprintf(utils.NamespaceNameFormat, mcName)
@@ -164,7 +185,7 @@ var _ = Describe("fleet guard rail tests for deny IMC CREATE operations", func()
 
 		By(fmt.Sprintf("expecting denial of operation CREATE of Internal Member Cluster %s/%s", mcName, imcNamespace))
 		err := impersonateHubClient.Create(ctx, &imc)
-		By(fmt.Sprintf(err.Error()))
+		fmt.Println(err.Error())
 		var statusErr *k8sErrors.StatusError
 		Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Create internal member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{})))
 		Expect(string(statusErr.Status().Reason)).Should(Equal(fmt.Sprintf(resourceDeniedFormat, testUser, testGroups, admissionv1.Create, &imcGVK, "", types.NamespacedName{Name: imc.Name, Namespace: imc.Namespace})))