Ignore ProjectRequest organization label for NS create validation (#99

) `ProjectRequest` labels are ignored by the API server and should not be used to validate.
appuio · May 29, 2024 · 62921b6 · 62921b6
1 parent ad9b101
commit 62921b6
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 13 deletions.
diff --git a/webhooks/namespace_quota_validator.go b/webhooks/namespace_quota_validator.go
@@ -68,16 +68,22 @@ func (v *NamespaceQuotaValidator) Handle(ctx context.Context, req admission.Requ
 		return admission.Allowed("skipped")
 	}
 
-	// try to get the organization name from the namespace/projectrequest
 	var rawObject unstructured.Unstructured
 	if err := v.Decoder.Decode(req, &rawObject); err != nil {
 		l.Error(err, "failed to decode request")
 		return admission.Errored(http.StatusBadRequest, err)
 	}
-	organizationName, _, err := unstructured.NestedString(rawObject.Object, "metadata", "labels", v.OrganizationLabel)
-	if err != nil {
-		l.Error(err, "error while fetching organization label")
-		return admission.Errored(http.StatusInternalServerError, err)
+
+	// try to get the organization name from a namespace object.
+	// Note: ProjectRequest labels are ignored by the API server so only the user default organization can be used.
+	var organizationName string
+	if rawObject.GetKind() == "Namespace" {
+		on, _, err := unstructured.NestedString(rawObject.Object, "metadata", "labels", v.OrganizationLabel)
+		if err != nil {
+			l.Error(err, "error while fetching organization label")
+			return admission.Errored(http.StatusInternalServerError, err)
+		}
+		organizationName = on
 	}
 
 	// get the organization name from the user if it is not set on the namespace/projectrequest

diff --git a/webhooks/namespace_quota_validator_test.go b/webhooks/namespace_quota_validator_test.go
@@ -48,13 +48,18 @@ func TestNamespaceQuotaValidator_Handle(t *testing.T) {
 			initObjects: []client.Object{
 				newNamespace("a", map[string]string{orgLabel: "other"}, nil), newNamespace("b", map[string]string{orgLabel: "other"}, nil),
 				newNamespace("an", nil, nil), newNamespace("bn", nil, nil),
+				&userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "user",
+						Annotations: map[string]string{
+							userDefaultOrgAnnotation: "testorg",
+						},
+					},
+				},
 			},
 			object: &projectv1.ProjectRequest{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",
-					Labels: map[string]string{
-						orgLabel: "testorg",
-					},
 				},
 			},
 			allowed: true,
@@ -99,7 +104,37 @@ func TestNamespaceQuotaValidator_Handle(t *testing.T) {
 			matchMessage: "You cannot create more than 2 namespaces for organization \"testorg\"",
 		},
 		"Deny ProjectRequest TooMany": {
-			initObjects: []client.Object{newNamespace("a", map[string]string{orgLabel: "testorg"}, nil), newNamespace("b", map[string]string{orgLabel: "testorg"}, nil)},
+			initObjects: []client.Object{
+				newNamespace("a", map[string]string{orgLabel: "testorg"}, nil),
+				newNamespace("b", map[string]string{orgLabel: "testorg"}, nil),
+				&userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "user",
+						Annotations: map[string]string{
+							userDefaultOrgAnnotation: "testorg",
+						},
+					},
+				},
+			},
+			object: &projectv1.ProjectRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test",
+				},
+			},
+			allowed:      false,
+			matchMessage: "You cannot create more than 2 namespaces for organization \"testorg\"",
+		},
+		"Deny ProjectRequest NoDefaultOrg. ProjectRequests labels are ignored by OCP": {
+			initObjects: []client.Object{
+				&userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "user",
+						Annotations: map[string]string{
+							userDefaultOrgAnnotation: "",
+						},
+					},
+				},
+			},
 			object: &projectv1.ProjectRequest{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",
@@ -109,7 +144,7 @@ func TestNamespaceQuotaValidator_Handle(t *testing.T) {
 				},
 			},
 			allowed:      false,
-			matchMessage: "You cannot create more than 2 namespaces for organization \"testorg\"",
+			matchMessage: "There is no organization label and the user has no default organization set.",
 		},
 
 		"Deny Namespace TooMany GetOrganizationFromUser": {
@@ -174,7 +209,34 @@ func TestNamespaceQuotaValidator_Handle(t *testing.T) {
 			allowed:             true,
 		},
 		"SkipQuotaValidation Allow ProjectRequest TooMany": {
-			initObjects: []client.Object{newNamespace("a", map[string]string{orgLabel: "testorg"}, nil), newNamespace("b", map[string]string{orgLabel: "testorg"}, nil)},
+			initObjects: []client.Object{
+				newNamespace("a", map[string]string{orgLabel: "testorg"}, nil),
+				newNamespace("b", map[string]string{orgLabel: "testorg"}, nil),
+				&userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "user",
+						Annotations: map[string]string{
+							userDefaultOrgAnnotation: "testorg",
+						},
+					},
+				},
+			},
+			object: &projectv1.ProjectRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test",
+				},
+			},
+			skipQuotaValidation: true,
+			allowed:             true,
+		},
+		"SkipQuotaValidation Name Deny NoDefaultOrgForUser": {
+			initObjects: []client.Object{
+				&userv1.User{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "user",
+					},
+				},
+			},
 			object: &projectv1.ProjectRequest{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",
@@ -184,9 +246,10 @@ func TestNamespaceQuotaValidator_Handle(t *testing.T) {
 				},
 			},
 			skipQuotaValidation: true,
-			allowed:             true,
+			allowed:             false,
+			matchMessage:        "There is no organization label and the user has no default organization set.",
 		},
-		"SkipQuotaValidation Deny NoOrganizationLabelAndNoUser": {
+		"SkipQuotaValidation Name Deny NoOrganizationLabelAndNoUser": {
 			object: &corev1.Namespace{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test",