Repository: projectcapsule/capsule

Issue:
bug(e2e): Tenant Replications
<!--
Thanks for taking time reporting a Capsule bug!
  
-->

# Bug description

The e2e tests for the tenant replications often fail:

```
Summarizing 1 Failure:
  [FAIL] Creating a GlobalTenantResource object [It] should replicate resources to all Tenants
  /home/runner/work/capsule/capsule/e2e/globaltenantresource_test.go:276
 ```
 
They should be fixed, so we can rely on the status of our pipeline


Discussion:
@prometherion: I witnessed this failure several times, despite the feature working as expected.

This could be a nice issue for newcomers, happy to share more information on how to debug the failure and provide a fix, definitely something's race-ish.@dev-saw99: Hi @prometherion , 

I'd like to work on this issue. Could you please provide more details on how to reproduce the failure and any tips for debugging?

Looking forward to contributing!@prometherion: Sorry for this late answer, @dev-saw99!

Unfortunately, I've no idea why this is happening 😢 

What I could suggest is trying back and for the incriminated test: after setting up the local environment, you can easily do this with the following command: `./bin/ginkgo run -v --tags=e2e --focus 'should replicate resources to all Tenants' ./e2e`

Upon a failure, Capsule logs and current state must be investigated since I guess something raceish is happening 🤔 @dev-saw99: @prometherion Hey, I tried but I was not able to reproduce this issue. 

I will spend more time over this weekend and update here.

Meanwhile, if you have any suggestions please let me know.@prometherion: Unfortunately, it seems it could be related to the whole test-suite.

The single suggestion I could have here is to run it entirely, and wait for failures, and check Capsule logs.@prometherion: @oliverbaehler let's see the progress with upcoming e2e runs, but I think #1264 will solve this issue, and it sounds potentially correct: a race condition, happening from time to time, with a timeout meeting the required condition.

If you agree, we could close this and open it back if we face these kinds of failure in the future.@github-actions: This pull request has been automatically closed because it has been inactive for more than 60 days. Please reopen if you still intend to submit this pull request.

Solution patch:
diff --git a/.github/workflows/check-actions.yml b/.github/workflows/check-actions.yml
index 72c2376..c61ffa6 100644
--- a/.github/workflows/check-actions.yml
+++ b/.github/workflows/check-actions.yml
@@ -16,7 +16,7 @@ jobs:
       - name: Checkout
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
       - name: Ensure SHA pinned actions
-        uses: zgosalvez/github-actions-ensure-sha-pinned-actions@38608ef4fb69adae7f1eac6eeb88e67b7d083bfd # v3.0.16
+        uses: zgosalvez/github-actions-ensure-sha-pinned-actions@5d6ac37a4cef8b8df67f482a8e384987766f0213 # v3.0.17
         with:
           # slsa-github-generator requires using a semver tag for reusable workflows. 
           # See: https://github.com/slsa-framework/slsa-github-generator#referencing-slsa-builders-and-generators
diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml
index 6ac841f..3d0d6cd 100644
--- a/.github/workflows/codecov.yml
+++ b/.github/workflows/codecov.yml
@@ -31,7 +31,7 @@ jobs:
         run: make test
       - name: Upload Report to Codecov
         if: steps.checksecret.outputs.result == 'true'
-        uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5.0.2
+        uses: codecov/codecov-action@015f24e6818733317a2da2edd6290ab26238649a # v5.0.7
         with:
           file: ./coverage.out
           fail_ci_if_error: true
diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml
index 086cb7f..f4f016d 100644
--- a/.github/workflows/docker-publish.yml
+++ b/.github/workflows/docker-publish.yml
@@ -28,7 +28,7 @@ jobs:
         with:
           build-cache-key: publish-images
       - name: Run Trivy vulnerability (Repo)
-        uses: aquasecurity/trivy-action@915b19bbe73b92a6cf82a1bc12b087c9a19a5fe2 # v0.28.0
+        uses: aquasecurity/trivy-action@18f2510ee396bbf400402947b394f2dd8c87dbb0 # v0.29.0
         with:
           scan-type: 'fs'
           ignore-unfixed: true
diff --git a/.github/workflows/releaser.yml b/.github/workflows/releaser.yml
index c8352ae..910301d 100644
--- a/.github/workflows/releaser.yml
+++ b/.github/workflows/releaser.yml
@@ -30,7 +30,7 @@ jobs:
         timeout-minutes: 5
         continue-on-error: true
       - uses: creekorful/goreportcard-action@1f35ced8cdac2cba28c9a2f2288a16aacfd507f9 # v1.0
-      - uses: anchore/sbom-action/download-syft@fc46e51fd3cb168ffb36c6d1915723c47db58abb
+      - uses: anchore/sbom-action/download-syft@55dc4ee22412511ee8c3142cbea40418e6cec693
       - name: Install Cosign
         uses: sigstore/cosign-installer@dc72c7d5c4d10cd6bcb8cf6e3fd625a9e5e537da # v3.7.0
       - name: Run GoReleaser
diff --git a/api/v1beta2/tenant_types.go b/api/v1beta2/tenant_types.go
index bb67c3b..beda86d 100644
--- a/api/v1beta2/tenant_types.go
+++ b/api/v1beta2/tenant_types.go
@@ -56,6 +56,15 @@ type TenantSpec struct {
 	// When enabled, the deletion request will be declined.
 	//+kubebuilder:default:=false
 	PreventDeletion bool `json:"preventDeletion,omitempty"`
+	// Use this if you want to disable/enable the Tenant name prefix to specific Tenants, overriding global forceTenantPrefix in CapsuleConfiguration.
+	// When set to 'true', it enforces Namespaces created for this Tenant to be named with the Tenant name prefix,
+	// separated by a dash (i.e. for Tenant 'foo', namespace names must be prefixed with 'foo-'),
+	// this is useful to avoid Namespace name collision.
+	// When set to 'false', it allows Namespaces created for this Tenant to be named anything.
+	// Overrides CapsuleConfiguration global forceTenantPrefix for the Tenant only.
+	// If unset, Tenant uses CapsuleConfiguration's forceTenantPrefix
+	// Optional
+	ForceTenantPrefix *bool `json:"forceTenantPrefix,omitempty"`
 }
 
 // +kubebuilder:object:root=true
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
index f9feb05..79ab4ab 100644
--- a/api/v1beta2/zz_generated.deepcopy.go
+++ b/api/v1beta2/zz_generated.deepcopy.go
@@ -763,6 +763,11 @@ func (in *TenantSpec) DeepCopyInto(out *TenantSpec) {
 		*out = new(api.DefaultAllowedListSpec)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.ForceTenantPrefix != nil {
+		in, out := &in.ForceTenantPrefix, &out.ForceTenantPrefix
+		*out = new(bool)
+		**out = **in
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TenantSpec.
diff --git a/charts/capsule/crds/capsule.clastix.io_tenants.yaml b/charts/capsule/crds/capsule.clastix.io_tenants.yaml
index c2cade7..dcc820f 100644
--- a/charts/capsule/crds/capsule.clastix.io_tenants.yaml
+++ b/charts/capsule/crds/capsule.clastix.io_tenants.yaml
@@ -1149,6 +1149,17 @@ spec:
                 description: Toggling the Tenant resources cordoning, when enable
                   resources cannot be deleted.
                 type: boolean
+              forceTenantPrefix:
+                description: |-
+                  Use this if you want to disable/enable the Tenant name prefix to specific Tenants, overriding global forceTenantPrefix in CapsuleConfiguration.
+                  When set to 'true', it enforces Namespaces created for this Tenant to be named with the Tenant name prefix,
+                  separated by a dash (i.e. for Tenant 'foo', namespace names must be prefixed with 'foo-'),
+                  this is useful to avoid Namespace name collision.
+                  When set to 'false', it allows Namespaces created for this Tenant to be named anything.
+                  Overrides CapsuleConfiguration global forceTenantPrefix for the Tenant only.
+                  If unset, Tenant uses CapsuleConfiguration's forceTenantPrefix
+                  Optional
+                type: boolean
               imagePullPolicies:
                 description: Specify the allowed values for the imagePullPolicies
                   option in Pod resources. Capsule assures that all Pod resources
diff --git a/go.mod b/go.mod
index 5858344..0b8dc9b 100644
--- a/go.mod
+++ b/go.mod
@@ -20,7 +20,7 @@ require (
 	k8s.io/client-go v0.31.1
 	k8s.io/utils v0.0.0-20240821151609-f90d01438635
 	sigs.k8s.io/cluster-api v1.8.4
-	sigs.k8s.io/controller-runtime v0.19.0
+	sigs.k8s.io/controller-runtime v0.19.2
 )
 
 require (
diff --git a/go.sum b/go.sum
index 90f65ac..3437816 100644
--- a/go.sum
+++ b/go.sum
@@ -279,8 +279,8 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3 h1:2770sDpzrjjsA
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.30.3/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw=
 sigs.k8s.io/cluster-api v1.8.4 h1:jBKQH1H/HUdUFk8T6qDzIxZJfWw1F5ZP0ZpYQJDmTHs=
 sigs.k8s.io/cluster-api v1.8.4/go.mod h1:pXv5LqLxuIbhGIXykyNKiJh+KrLweSBajVHHitPLyoY=
-sigs.k8s.io/controller-runtime v0.19.0 h1:nWVM7aq+Il2ABxwiCizrVDSlmDcshi9llbaFbC0ji/Q=
-sigs.k8s.io/controller-runtime v0.19.0/go.mod h1:iRmWllt8IlaLjvTTDLhRBXIEtkCK6hwVBJJsYS9Ajf4=
+sigs.k8s.io/controller-runtime v0.19.2 h1:3sPrF58XQEPzbE8T81TN6selQIMGbtYwuaJ6eDssDF8=
+sigs.k8s.io/controller-runtime v0.19.2/go.mod h1:iRmWllt8IlaLjvTTDLhRBXIEtkCK6hwVBJJsYS9Ajf4=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.4.1 h1:150L+0vs/8DA78h1u02ooW1/fFq/Lwr+sGiqlzvrtq4=
diff --git a/pkg/webhook/namespace/prefix.go b/pkg/webhook/namespace/prefix.go
index 734081a..1bb9935 100644
--- a/pkg/webhook/namespace/prefix.go
+++ b/pkg/webhook/namespace/prefix.go
@@ -59,6 +59,11 @@ func (r *prefixHandler) OnCreate(clt client.Client, decoder admission.Decoder, r
 					return utils.ErroredResponse(err)
 				}
 
+				// Check for Tenant-level ForceTenantPrefix override
+				if tnt.Spec.ForceTenantPrefix != nil && !*tnt.Spec.ForceTenantPrefix {
+					return nil
+				}
+
 				if e := fmt.Sprintf("%s-%s", tnt.GetName(), ns.GetName()); !strings.HasPrefix(ns.GetName(), fmt.Sprintf("%s-", tnt.GetName())) {
 					recorder.Eventf(tnt, corev1.EventTypeWarning, "InvalidTenantPrefix", "Namespace %s does not match the expected prefix for the current Tenant", ns.GetName())
 
diff --git a/pkg/webhook/ownerreference/patching.go b/pkg/webhook/ownerreference/patching.go
index 8a7ac7c..1163ab1 100644
--- a/pkg/webhook/ownerreference/patching.go
+++ b/pkg/webhook/ownerreference/patching.go
@@ -144,6 +144,7 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 		return &response
 	}
 	// If we already had TenantName label on NS -> assign to it
+
 	if label, ok := ns.ObjectMeta.Labels[ln]; ok {
 		// retrieving the selected Tenant
 		tnt := &capsulev1beta2.Tenant{}
@@ -160,6 +161,10 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 
 			return &response
 		}
+		// Check if namespace needs Tenant name prefix
+		if errResponse := h.validateNamespacePrefix(ns, tnt); errResponse != nil {
+			return errResponse
+		}
 		// Patching the response
 		response := h.patchResponseForOwnerRef(tnt, ns, recorder)
 
@@ -221,6 +226,11 @@ func (h *handler) setOwnerRef(ctx context.Context, req admission.Request, client
 	}
 
 	if len(tenants) == 1 {
+		// Check if namespace needs Tenant name prefix
+		if errResponse := h.validateNamespacePrefix(ns, &tenants[0]); errResponse != nil {
+			return errResponse
+		}
+
 		response := h.patchResponseForOwnerRef(&tenants[0], ns, recorder)
 
 		return &response
@@ -281,6 +291,19 @@ func (h *handler) listTenantsForOwnerKind(ctx context.Context, ownerKind string,
 	return tntList, err
 }
 
+func (h *handler) validateNamespacePrefix(ns *corev1.Namespace, tenant *capsulev1beta2.Tenant) *admission.Response {
+	// Check if ForceTenantPrefix is true
+	if tenant.Spec.ForceTenantPrefix != nil && *tenant.Spec.ForceTenantPrefix {
+		if !strings.HasPrefix(ns.GetName(), fmt.Sprintf("%s-", tenant.GetName())) {
+			response := admission.Denied(fmt.Sprintf("The Namespace name must start with '%s-' when ForceTenantPrefix is enabled in the Tenant.", tenant.GetName()))
+
+			return &response
+		}
+	}
+
+	return nil
+}
+
 type sortedTenants []capsulev1beta2.Tenant
 
 func (s sortedTenants) Len() int {



Test patch:
diff --git a/e2e/force_tenant_prefix_tenant_scope_test.go b/e2e/force_tenant_prefix_tenant_scope_test.go
new file mode 100644
index 0000000..5cfcc29
--- /dev/null
+++ b/e2e/force_tenant_prefix_tenant_scope_test.go
@@ -0,0 +1,148 @@
+//go:build e2e
+
+// Copyright 2020-2023 Project Capsule Authors.
+// SPDX-License-Identifier: Apache-2.0
+
+package e2e
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2"
+)
+
+var _ = Describe("creating a Namespace with Tenant name prefix enforcement at Tenant scope", func() {
+	t1 := &capsulev1beta2.Tenant{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "awesome",
+		},
+		Spec: capsulev1beta2.TenantSpec{
+			ForceTenantPrefix: &[]bool{true}[0],
+			Owners: capsulev1beta2.OwnerListSpec{
+				{
+					Name: "john",
+					Kind: "User",
+				},
+			},
+		},
+	}
+	t2 := &capsulev1beta2.Tenant{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "awesome-tenant",
+		},
+		Spec: capsulev1beta2.TenantSpec{
+			ForceTenantPrefix: &[]bool{false}[0],
+			Owners: capsulev1beta2.OwnerListSpec{
+				{
+					Name: "john",
+					Kind: "User",
+				},
+			},
+		},
+	}
+
+	JustBeforeEach(func() {
+		EventuallyCreation(func() error {
+			t1.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), t1)
+		}).Should(Succeed())
+		EventuallyCreation(func() error {
+			t2.ResourceVersion = ""
+			return k8sClient.Create(context.TODO(), t2)
+		}).Should(Succeed())
+	})
+	JustAfterEach(func() {
+		Expect(k8sClient.Delete(context.TODO(), t1)).Should(Succeed())
+		Expect(k8sClient.Delete(context.TODO(), t2)).Should(Succeed())
+
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = false
+		})
+	})
+
+	It("should fail when not using prefix, with tenant label for a tenant with ForceTenantPrefix true and global ForceTenantPrefix false", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = false
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, where the user owns more than one Tenant, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix false", func() {
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, where the user owns more than one Tenant, for a tenant with ForceTenantPrefix false and global ForceTenantPrefix true", func() {
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t2.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed and be assigned with prefix and label, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix false", func() {
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome-tenant", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+
+		TenantNamespaceList(t1, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
+	})
+
+	It("should fail when not using prefix, with tenant label for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed and be assigned with prefix and label, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t1.GetName(),
+		}
+		ns := NewNamespace("awesome-tenant", labels)
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+
+		TenantNamespaceList(t1, defaultTimeoutInterval).Should(ContainElement(ns.GetName()))
+	})
+
+	It("should fail using prefix without capsule.clastix.io/tenant label, for a tenant with ForceTenantPrefix true and global ForceTenantPrefix true", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		ns := NewNamespace("awesome-namespace")
+		NamespaceCreation(ns, t1.Spec.Owners[0], defaultTimeoutInterval).ShouldNot(Succeed())
+	})
+
+	It("should succeed when not using prefix, with tenant label for a tenant with ForceTenantPrefix false and global ForceTenantPrefix false", func() {
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t2.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t2.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+	})
+
+	It("should succeed when not using prefix, with tenant label for a tenant with ForceTenantPrefix false and global ForceTenantPrefix true", func() {
+		ModifyCapsuleConfigurationOpts(func(configuration *capsulev1beta2.CapsuleConfiguration) {
+			configuration.Spec.ForceTenantPrefix = true
+		})
+		labels := map[string]string{
+			"capsule.clastix.io/tenant": t2.GetName(),
+		}
+		ns := NewNamespace("awesome", labels)
+		NamespaceCreation(ns, t2.Spec.Owners[0], defaultTimeoutInterval).Should(Succeed())
+	})
+})
diff --git a/e2e/utils_test.go b/e2e/utils_test.go
index 5ab0744..10a4369 100644
--- a/e2e/utils_test.go
+++ b/e2e/utils_test.go
@@ -53,14 +53,20 @@ func ServiceCreation(svc *corev1.Service, owner capsulev1beta2.OwnerSpec, timeou
 	}, timeout, defaultPollInterval)
 }
 
-func NewNamespace(name string) *corev1.Namespace {
+func NewNamespace(name string, labels ...map[string]string) *corev1.Namespace {
 	if len(name) == 0 {
 		name = rand.String(10)
 	}
 
+	var namespaceLabels map[string]string
+	if len(labels) > 0 {
+		namespaceLabels = labels[0]
+	}
+
 	return &corev1.Namespace{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name:   name,
+			Labels: namespaceLabels,
 		},
 	}
 }