From 62921b6f65ae3810be66e619b32e40ef8b7f5783 Mon Sep 17 00:00:00 2001 From: Sebastian Widmer Date: Wed, 29 May 2024 15:53:11 +0200 Subject: [PATCH] Ignore `ProjectRequest` organization label for NS create validation (#99) `ProjectRequest` labels are ignored by the API server and should not be used to validate. --- webhooks/namespace_quota_validator.go | 16 +++-- webhooks/namespace_quota_validator_test.go | 79 +++++++++++++++++++--- 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/webhooks/namespace_quota_validator.go b/webhooks/namespace_quota_validator.go index 70378e4..e595352 100644 --- 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 index 0e68477..c2ce8ce 100644 --- 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",