fix: allow aks-support user in fleet guard rail (#957)

Author: Arvindthiru

pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook_test.go

@@ -367,6 +367,28 @@ func TestHandleV1Alpha1MemberCluster(t *testing.T) {
 			},
 			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 {
@@ -570,6 +592,28 @@ func TestHandleMemberCluster(t *testing.T) {
 			},
 			wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Update, &utils.MCMetaGVK, "status", 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 for fleet MC by aks-support user": {
+			req: admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Name: "test-mc",
+					OldObject: runtime.RawExtension{
+						Raw: fleetMCObjectBytes,
+					},
+					UserInfo: authenticationv1.UserInfo{
+						Username: "aks-support",
+						Groups:   []string{"system:authenticated"},
+					},
+					RequestKind: &utils.MCMetaGVK,
+					Operation:   admissionv1.Delete,
+				},
+			},
+			resourceValidator: fleetResourceValidator{
+				decoder: decoder,
+			},
+			wantResponse: admission.Allowed(fmt.Sprintf(validation.ResourceAllowedFormat, "aks-support", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Delete, &utils.MCMetaGVK, "", types.NamespacedName{Name: "test-mc"})),
+		},
 	}
 
 	for testName, testCase := range testCases {
@@ -873,6 +917,27 @@ func TestHandleFleetReservedNamespacedResource(t *testing.T) {
 			},
 			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"})),
 		},
+		// added as UT since testing this case as an E2E requires
+		// creating a new user called aks-support in our test environment.
+		"allow delete on v1beta1 IMC in fleet-member namespace": {
+			req: admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Name:        "test-mc",
+					Namespace:   "fleet-member-test-mc",
+					RequestKind: &utils.IMCMetaGVK,
+					UserInfo: authenticationv1.UserInfo{
+						Username: "aks-support",
+						Groups:   []string{"system:authenticated"},
+					},
+					Operation: admissionv1.Delete,
+				},
+			},
+			resourceValidator: fleetResourceValidator{
+				client:            mockClient,
+				isFleetV1Beta1API: true,
+			},
+			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"})),
+		},
 	}
 	for testName, testCase := range testCases {
 		t.Run(testName, func(t *testing.T) {

pkg/webhook/validation/uservalidation.go

@@ -29,6 +29,7 @@ const (
 	nodeGroup                 = "system:nodes"
 	kubeSchedulerUser         = "system:kube-scheduler"
 	kubeControllerManagerUser = "system:kube-controller-manager"
+	aksSupportUser            = "aks-support"
 	serviceAccountFmt         = "system:serviceaccount:fleet-system:%s"
 
 	allowedModifyResource       = "user in groups is allowed to modify resource"
@@ -61,7 +62,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) || isNodeGroupUser(userInfo) {
+	if isAdminGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) || isUserAuthenticatedServiceAccount(userInfo) || isUserKubeScheduler(userInfo) || isUserKubeControllerManager(userInfo) || isNodeGroupUser(userInfo) || 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))
 	}
@@ -155,6 +156,12 @@ func isUserKubeControllerManager(userInfo authenticationv1.UserInfo) bool {
 	return userInfo.Username == kubeControllerManagerUser
 }
 
+// isUserKubeControllerManager return true if user is aks-support.
+func isAKSSupportUser(userInfo authenticationv1.UserInfo) bool {
+	// aks-support user only belongs to system:authenticated group hence comparing username.
+	return userInfo.Username == aksSupportUser
+}
+
 // isNodeGroupUser returns true if user belongs to system:nodes group.
 func isNodeGroupUser(userInfo authenticationv1.UserInfo) bool {
 	return slices.Contains(userInfo.Groups, nodeGroup)

pkg/webhook/validation/uservalidation_test.go

@@ -159,6 +159,20 @@ func TestValidateUserForResource(t *testing.T) {
 			},
 			wantResponse: admission.Denied(fmt.Sprintf(ResourceDeniedFormat, "test-user", utils.GenerateGroupString([]string{"test-group"}), admissionv1.Delete, &utils.RoleMetaGVK, "", types.NamespacedName{Name: "test-role", Namespace: "test-namespace"})),
 		},
+		"allow aks-support user": {
+			req: admission.Request{
+				AdmissionRequest: admissionv1.AdmissionRequest{
+					Name:        "test-mc",
+					RequestKind: &utils.MCMetaGVK,
+					UserInfo: authenticationv1.UserInfo{
+						Username: "aks-support",
+						Groups:   []string{"system:authenticated"},
+					},
+					Operation: admissionv1.Create,
+				},
+			},
+			wantResponse: admission.Allowed(fmt.Sprintf(ResourceAllowedFormat, "aks-support", utils.GenerateGroupString([]string{"system:authenticated"}), admissionv1.Create, &utils.MCMetaGVK, "", types.NamespacedName{Name: "test-mc"})),
+		},
 	}
 
 	for testName, testCase := range testCases {