Organization patch: Add label map if not exists (#121)

If the `{"labels":{}}` key is missing from the object sent to the webhook, applying the returned patch failed.
appuio · Aug 29, 2024 · 0c09073 · 0c09073
1 parent 0e26c3e
commit 0c09073
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 8 deletions.
diff --git a/webhooks/namespace_project_organization_mutator.go b/webhooks/namespace_project_organization_mutator.go
@@ -137,7 +137,7 @@ func (m *NamespaceProjectOrganizationMutator) handleUserRequested(ctx context.Co
 	}
 
 	if defaultOrgAdded {
-		return admission.Patched("added default organization", m.orgLabelPatch(org))
+		return admission.Patched("added default organization", m.orgLabelPatch(rawObject, org))
 	}
 
 	return admission.Allowed("Requester is member of organization")
@@ -164,17 +164,26 @@ func (m *NamespaceProjectOrganizationMutator) handleServiceAccountNamespace(ctx
 	}
 
 	if requestedOrg == "" {
-		return admission.Patched("added organization label", m.orgLabelPatch(nsOrg))
+		return admission.Patched("added organization label", m.orgLabelPatch(rawObject, nsOrg))
 	}
 
 	return admission.Allowed("service account may use the organization of its namespace")
 }
 
 // orgLabelPatch returns a JSON patch operation to add the `OrganizationLabel` with value `org` to an object.
-func (m *NamespaceProjectOrganizationMutator) orgLabelPatch(org string) jsonpatch.Operation {
+func (m *NamespaceProjectOrganizationMutator) orgLabelPatch(objToPatch unstructured.Unstructured, org string) jsonpatch.Operation {
+	_, exists, _ := unstructured.NestedStringMap(objToPatch.Object, "metadata", "labels")
+	if exists {
+		return jsonpatch.Operation{
+			Operation: "add",
+			Path:      "/" + strings.Join([]string{"metadata", "labels", escapeJSONPointerSegment(m.OrganizationLabel)}, "/"),
+			Value:     org,
+		}
+	}
+
 	return jsonpatch.Operation{
 		Operation: "add",
-		Path:      "/" + strings.Join([]string{"metadata", "labels", escapeJSONPointerSegment(m.OrganizationLabel)}, "/"),
-		Value:     org,
+		Path:      "/metadata/labels",
+		Value:     map[string]string{m.OrganizationLabel: org},
 	}
 }
diff --git a/webhooks/namespace_project_organization_mutator_test.go b/webhooks/namespace_project_organization_mutator_test.go
@@ -99,6 +99,22 @@ func Test_NamespaceProjectOrganizationMutator_Handle(t *testing.T) {
 			orgPatch: "some-org",
 		},
 
+		{
+			name: "Namespace: user with default org, no org set on object but other labels exist",
+
+			object: newNamespace("project", map[string]string{"foo": "bar"}, nil),
+			additionalObjects: func(*testing.T) []client.Object {
+				return []client.Object{
+					newUser("user", "some-org"),
+					newGroup("some-org", "user"),
+				}
+			},
+
+			user:     "user",
+			allowed:  true,
+			orgPatch: "some-org",
+		},
+
 		{
 			name: "Project: request with org label set, user not in org",
 
@@ -396,17 +412,23 @@ func Test_NamespaceProjectOrganizationMutator_Handle(t *testing.T) {
 			require.Equal(t, tc.allowed, resp.Allowed)
 
 			if tc.orgPatch != "" {
-				requireOrgPatch(t, tc.orgPatch, resp.Patches)
+				requireOrgPatch(t, tc.object, tc.orgPatch, resp.Patches)
 			} else {
 				require.Empty(t, resp.Patches)
 			}
 		})
 	}
 }
 
-func requireOrgPatch(t *testing.T, org string, ps []jsonpatch.Operation) {
+func requireOrgPatch(t *testing.T, origObject client.Object, org string, ps []jsonpatch.Operation) {
+	t.Helper()
+
 	require.Len(t, ps, 1)
-	require.Equal(t, jsonpatch.NewOperation("add", "/metadata/labels/example.com~1organization", org), ps[0])
+	if origObject.GetLabels() != nil {
+		require.Equal(t, jsonpatch.NewOperation("add", "/metadata/labels/example.com~1organization", org), ps[0])
+		return
+	}
+	require.Equal(t, jsonpatch.NewOperation("add", "/metadata/labels", map[string]string{"example.com/organization": org}), ps[0])
 }
 
 func newGroup(name string, users ...string) *userv1.Group {