From 10e72cdf34db168c025eeea29c104808ba5fc0cf Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 26 Mar 2026 16:40:41 +0530 Subject: [PATCH 01/11] test(trustmanager): add unit tests for trust namespace configuration Add missing unit test coverage for configurable trust namespace feature. The trust namespace configuration was already fully implemented in PR #379. This commit adds comprehensive unit tests to verify the implementation and satisfy CM-869 acceptance criteria. Tests added: - Trust namespace validation when namespace doesn't exist - Custom trust namespace configuration with successful reconciliation - Status field updates for both default and custom namespace values Without these tests, the trust namespace validation logic was not adequately covered, making it harder to catch regressions. Co-Authored-By: Claude Sonnet 4.5 --- .../trustmanager/controller_test.go | 133 ++++++++++++++++++ pkg/controller/trustmanager/test_utils.go | 5 + pkg/controller/trustmanager/webhooks.go | 2 +- 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/pkg/controller/trustmanager/controller_test.go b/pkg/controller/trustmanager/controller_test.go index 80c845932..eca5340d5 100644 --- a/pkg/controller/trustmanager/controller_test.go +++ b/pkg/controller/trustmanager/controller_test.go @@ -251,6 +251,82 @@ func TestProcessReconcileRequest(t *testing.T) { }, wantErr: "failed to check if serviceaccount", }, + { + name: "trust namespace does not exist sets degraded true", + getTrustManager: func() *v1alpha1.TrustManager { + return testTrustManager().Build() + }, + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.TrustManager: + testTrustManager().Build().DeepCopyInto(o) + } + return nil + }) + // Namespace does not exist - validateTrustNamespace will fail + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + switch obj.(type) { + case *corev1.Namespace: + return false, nil + } + return false, nil + }) + }, + wantConditions: []metav1.Condition{ + { + Type: v1alpha1.Degraded, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonFailed, + }, + { + Type: v1alpha1.Ready, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonFailed, + }, + }, + }, + { + name: "custom trust namespace configures resources correctly", + getTrustManager: func() *v1alpha1.TrustManager { + tm := testTrustManager().Build() + tm.Spec.TrustManagerConfig.TrustNamespace = "custom-trust-ns" + return tm + }, + preReq: func(r *Reconciler, m *fakes.FakeCtrlClient) { + m.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.TrustManager: + tm := testTrustManager().Build() + tm.Spec.TrustManagerConfig.TrustNamespace = "custom-trust-ns" + tm.DeepCopyInto(o) + } + return nil + }) + // Custom namespace exists; all other resources return not-found so they + // are created via SSA Patch (which succeeds by default). + m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + switch obj.(type) { + case *corev1.Namespace: + return true, nil + } + return false, nil + }) + }, + wantConditions: []metav1.Condition{ + { + Type: v1alpha1.Degraded, + Status: metav1.ConditionFalse, + Reason: v1alpha1.ReasonReady, + }, + { + Type: v1alpha1.Ready, + Status: metav1.ConditionTrue, + Reason: v1alpha1.ReasonReady, + Message: "reconciliation successful", + }, + }, + }, } for _, tt := range tests { @@ -289,3 +365,60 @@ func TestProcessReconcileRequest(t *testing.T) { }) } } + +func TestStatusTrustNamespaceUpdate(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + + tests := []struct { + name string + trustNamespace string + wantTrustNamespace string + }{ + { + name: "default trust namespace is set in status", + trustNamespace: "", + wantTrustNamespace: defaultTrustNamespace, + }, + { + name: "custom trust namespace is set in status", + trustNamespace: "custom-trust-ns", + wantTrustNamespace: "custom-trust-ns", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + + mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.TrustManager: + tm := testTrustManager().WithTrustNamespace(tt.trustNamespace).Build() + tm.DeepCopyInto(o) + } + return nil + }) + + mock.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { + switch obj.(type) { + case *corev1.Namespace: + return true, nil + } + return false, nil + }) + + r.CtrlClient = mock + + tm := testTrustManager().WithTrustNamespace(tt.trustNamespace).Build() + _, err := r.processReconcileRequest(tm, types.NamespacedName{Name: tm.GetName()}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tm.Status.TrustNamespace != tt.wantTrustNamespace { + t.Errorf("expected status.trustNamespace %q, got %q", tt.wantTrustNamespace, tm.Status.TrustNamespace) + } + }) + } +} diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index a56fe0cae..918ac18fb 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -69,6 +69,11 @@ func (b *trustManagerBuilder) WithAffinity(affinity *corev1.Affinity) *trustMana return b } +func (b *trustManagerBuilder) WithTrustNamespace(trustNamespace string) *trustManagerBuilder { + b.Spec.TrustManagerConfig.TrustNamespace = trustNamespace + return b +} + func (b *trustManagerBuilder) Build() *v1alpha1.TrustManager { return b.TrustManager } diff --git a/pkg/controller/trustmanager/webhooks.go b/pkg/controller/trustmanager/webhooks.go index 6f5f8c291..51e85cd7f 100644 --- a/pkg/controller/trustmanager/webhooks.go +++ b/pkg/controller/trustmanager/webhooks.go @@ -52,7 +52,7 @@ func getValidatingWebhookConfigObject(resourceLabels, resourceAnnotations map[st return webhookConfig } -// updateWebhookClientConfig sets the webhook clientConfig service name and namespace +// updateWebhookClientConfig sets the webhook clientConfig service name and namespace. func updateWebhookClientConfig(webhookConfig *admissionregistrationv1.ValidatingWebhookConfiguration) { for i := range webhookConfig.Webhooks { if webhookConfig.Webhooks[i].ClientConfig.Service != nil { From 7a0aea083246c4f078804e8054aca8edaf25913f Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 26 Mar 2026 16:41:13 +0530 Subject: [PATCH 02/11] test(e2e): add e2e tests for trust namespace configuration Add comprehensive end-to-end tests for trust namespace configuration. Tests verify: 1. Custom trust namespace RBAC placement - Role/RoleBinding created in trust namespace - Leader election Role/RoleBinding in operand namespace 2. Deployment configuration with correct --trust-namespace arg 3. Status field updates matching spec configuration 4. Degraded condition when trust namespace doesn't exist 5. Recovery to Ready=True after namespace creation These tests ensure the trust namespace feature works correctly in a real cluster environment and that all resources are created in the correct namespaces as required by CM-869. Co-Authored-By: Claude Sonnet 4.5 --- test/e2e/trustmanager_test.go | 142 +++++++++++++++++++++++++++++++++- test/e2e/utils_test.go | 5 ++ 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 921e9d466..250aeb2f8 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -730,8 +730,148 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should report trust namespace in status", func() { + createTrustManager(newTrustManagerCR()) + + By("verifying TrustManager status has default trust namespace set") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(tm.Status.TrustNamespace).Should(Equal("cert-manager")) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + // TODO: Add test for status reporting when custom configuration is applied - // (i.e. custom trust namespace, secret targets policy, default CA package policy, filter expired certificates policy) + // (i.e. secret targets policy, default CA package policy, filter expired certificates policy) + }) + + // ------------------------------------------------------------------------- + // Trust namespace configuration + // ------------------------------------------------------------------------- + + Context("trust namespace configuration", func() { + It("should configure resources with custom trust namespace", func() { + const customTrustNS = "custom-trust-ns" + + By("creating custom trust namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: customTrustNS, + }, + } + _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + defer func() { + By("cleaning up custom trust namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) + }() + + By("creating TrustManager CR with custom trust namespace") + createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) + + By("verifying Role is created in custom trust namespace") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(customTrustNS).Get(ctx, trustManagerRoleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(role.Namespace).Should(Equal(customTrustNS)) + verifyTrustManagerManagedLabels(role.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying RoleBinding is created in custom trust namespace") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(customTrustNS).Get(ctx, trustManagerRoleBindingName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(rb.Namespace).Should(Equal(customTrustNS)) + g.Expect(rb.RoleRef.Name).Should(Equal(trustManagerRoleName)) + g.Expect(rb.Subjects[0].Name).Should(Equal(trustManagerServiceAccountName)) + g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(rb.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying leader election Role is in operand namespace") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(role.Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(role.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying leader election RoleBinding is in operand namespace") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleBindingName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(rb.Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(rb.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying deployment has correct --trust-namespace arg") + Eventually(func(g Gomega) { + deployment, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(deployment.Spec.Template.Spec.Containers[0].Args).Should(ContainElement(fmt.Sprintf("--trust-namespace=%s", customTrustNS))) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying status.trustNamespace matches spec") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(tm.Status.TrustNamespace).Should(Equal(customTrustNS)) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + + It("should set degraded condition when trust namespace does not exist", func() { + const nonExistentNS = "non-existent-trust-ns" + + By("creating TrustManager CR with non-existent trust namespace") + _, err := trustManagerClient().Create(ctx, newTrustManagerCR().WithTrustNamespace(nonExistentNS).Build(), metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + By("verifying TrustManager becomes Degraded=True") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + + degradedCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Degraded) + g.Expect(degradedCondition).ShouldNot(BeNil()) + g.Expect(degradedCondition.Status).Should(Equal(metav1.ConditionTrue)) + g.Expect(degradedCondition.Reason).Should(Equal(v1alpha1.ReasonFailed)) + g.Expect(degradedCondition.Message).Should(ContainSubstring(nonExistentNS)) + g.Expect(degradedCondition.Message).Should(ContainSubstring("does not exist")) + + readyCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Ready) + g.Expect(readyCondition).ShouldNot(BeNil()) + g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionFalse)) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("creating the missing namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: nonExistentNS, + }, + } + _, err = clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + defer func() { + By("cleaning up the namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, nonExistentNS, metav1.DeleteOptions{}) + }() + + By("verifying TrustManager recovers to Ready=True") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + + degradedCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Degraded) + g.Expect(degradedCondition).ShouldNot(BeNil()) + g.Expect(degradedCondition.Status).Should(Equal(metav1.ConditionFalse)) + + readyCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Ready) + g.Expect(readyCondition).ShouldNot(BeNil()) + g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionTrue)) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) }) // ------------------------------------------------------------------------- diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index b95e44c5e..ac35e0ef4 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -144,6 +144,11 @@ func (b *trustManagerCRBuilder) WithAnnotations(annotations map[string]string) * return b } +func (b *trustManagerCRBuilder) WithTrustNamespace(trustNamespace string) *trustManagerCRBuilder { + b.tm.Spec.TrustManagerConfig.TrustNamespace = trustNamespace + return b +} + func (b *trustManagerCRBuilder) Build() *v1alpha1.TrustManager { return b.tm } From cfb6441aa7c357962c90fe83c11bdab52224e34a Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 31 Mar 2026 15:29:18 +0530 Subject: [PATCH 03/11] test(trustmanager): fix for missing namespace case. --- test/e2e/trustmanager_test.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 250aeb2f8..36b72bbf7 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -844,7 +844,18 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionFalse)) }, lowTimeout, fastPollInterval).Should(Succeed()) - By("creating the missing namespace") + // Irrecoverable errors are not requeued; delete and recreate after the namespace exists. + By("deleting TrustManager CR") + err = trustManagerClient().Delete(ctx, "cluster", metav1.DeleteOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + By("waiting for TrustManager CR to be deleted") + Eventually(func() bool { + _, getErr := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + return errors.IsNotFound(getErr) + }, lowTimeout, fastPollInterval).Should(BeTrue()) + + By("creating the namespace that was previously missing") ns := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: nonExistentNS, @@ -858,19 +869,8 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() _ = clientset.CoreV1().Namespaces().Delete(ctx, nonExistentNS, metav1.DeleteOptions{}) }() - By("verifying TrustManager recovers to Ready=True") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - - degradedCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Degraded) - g.Expect(degradedCondition).ShouldNot(BeNil()) - g.Expect(degradedCondition.Status).Should(Equal(metav1.ConditionFalse)) - - readyCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Ready) - g.Expect(readyCondition).ShouldNot(BeNil()) - g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionTrue)) - }, lowTimeout, fastPollInterval).Should(Succeed()) + By("recreating TrustManager CR with trust namespace that now exists") + createTrustManager(newTrustManagerCR().WithTrustNamespace(nonExistentNS)) }) }) From 9d0373ca608a01e4c7f13b0f0aa4a796b6ce9434 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 31 Mar 2026 15:55:10 +0530 Subject: [PATCH 04/11] Handling PR review comments. --- .../trustmanager/controller_test.go | 66 +-------- .../trustmanager/install_trustmanager_test.go | 133 ++++++++++++++++++ 2 files changed, 140 insertions(+), 59 deletions(-) create mode 100644 pkg/controller/trustmanager/install_trustmanager_test.go diff --git a/pkg/controller/trustmanager/controller_test.go b/pkg/controller/trustmanager/controller_test.go index eca5340d5..65fe79c76 100644 --- a/pkg/controller/trustmanager/controller_test.go +++ b/pkg/controller/trustmanager/controller_test.go @@ -2,6 +2,7 @@ package trustmanager import ( "context" + "fmt" "testing" "time" @@ -278,6 +279,11 @@ func TestProcessReconcileRequest(t *testing.T) { Type: v1alpha1.Degraded, Status: metav1.ConditionTrue, Reason: v1alpha1.ReasonFailed, + Message: fmt.Sprintf( + "reconciliation failed with irrecoverable error not retrying: trust namespace %q validation failed: trust namespace %q does not exist, create the namespace before creating TrustManager CR", + defaultTrustNamespace, + defaultTrustNamespace, + ), }, { Type: v1alpha1.Ready, @@ -303,8 +309,7 @@ func TestProcessReconcileRequest(t *testing.T) { } return nil }) - // Custom namespace exists; all other resources return not-found so they - // are created via SSA Patch (which succeeds by default). + // Custom namespace exists; so SSA Patch will create or update all resources successfully. m.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { switch obj.(type) { case *corev1.Namespace: @@ -365,60 +370,3 @@ func TestProcessReconcileRequest(t *testing.T) { }) } } - -func TestStatusTrustNamespaceUpdate(t *testing.T) { - t.Setenv(trustManagerImageNameEnvVarName, testImage) - - tests := []struct { - name string - trustNamespace string - wantTrustNamespace string - }{ - { - name: "default trust namespace is set in status", - trustNamespace: "", - wantTrustNamespace: defaultTrustNamespace, - }, - { - name: "custom trust namespace is set in status", - trustNamespace: "custom-trust-ns", - wantTrustNamespace: "custom-trust-ns", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := testReconciler(t) - mock := &fakes.FakeCtrlClient{} - - mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { - switch o := obj.(type) { - case *v1alpha1.TrustManager: - tm := testTrustManager().WithTrustNamespace(tt.trustNamespace).Build() - tm.DeepCopyInto(o) - } - return nil - }) - - mock.ExistsCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) (bool, error) { - switch obj.(type) { - case *corev1.Namespace: - return true, nil - } - return false, nil - }) - - r.CtrlClient = mock - - tm := testTrustManager().WithTrustNamespace(tt.trustNamespace).Build() - _, err := r.processReconcileRequest(tm, types.NamespacedName{Name: tm.GetName()}) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if tm.Status.TrustNamespace != tt.wantTrustNamespace { - t.Errorf("expected status.trustNamespace %q, got %q", tt.wantTrustNamespace, tm.Status.TrustNamespace) - } - }) - } -} diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go new file mode 100644 index 000000000..17e19704e --- /dev/null +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -0,0 +1,133 @@ +package trustmanager + +import ( + "context" + "testing" + + "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" + "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func TestUpdateStatusObservedState(t *testing.T) { + t.Setenv(trustManagerImageNameEnvVarName, testImage) + + tests := []struct { + name string + trustManager func() *v1alpha1.TrustManager + wantStatusUpdate int + assertStatus func(*testing.T, *v1alpha1.TrustManager) + }{ + { + name: "updates all observed fields when status is empty", + trustManager: func() *v1alpha1.TrustManager { + return testTrustManager().Build() + }, + wantStatusUpdate: 1, + assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { + s := tm.Status + if s.TrustManagerImage != testImage { + t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) + } + if s.TrustNamespace != defaultTrustNamespace { + t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, defaultTrustNamespace) + } + if s.SecretTargetsPolicy != tm.Spec.TrustManagerConfig.SecretTargets.Policy { + t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, tm.Spec.TrustManagerConfig.SecretTargets.Policy) + } + if s.DefaultCAPackagePolicy != tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy { + t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy) + } + if s.FilterExpiredCertificatesPolicy != tm.Spec.TrustManagerConfig.FilterExpiredCertificates { + t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, tm.Spec.TrustManagerConfig.FilterExpiredCertificates) + } + }, + }, + { + name: "updates all observed fields for custom spec", + trustManager: func() *v1alpha1.TrustManager { + tm := testTrustManager().WithTrustNamespace("custom-trust-ns").Build() + tm.Spec.TrustManagerConfig.SecretTargets.Policy = v1alpha1.SecretTargetsPolicyCustom + tm.Spec.TrustManagerConfig.SecretTargets.AuthorizedSecrets = []string{"allowed-secret"} + tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicyEnabled + tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.FilterExpiredCertificatesPolicyEnabled + return tm + }, + wantStatusUpdate: 1, + assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { + s := tm.Status + if s.TrustManagerImage != testImage { + t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) + } + if s.TrustNamespace != "custom-trust-ns" { + t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, "custom-trust-ns") + } + if s.SecretTargetsPolicy != v1alpha1.SecretTargetsPolicyCustom { + t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, v1alpha1.SecretTargetsPolicyCustom) + } + if s.DefaultCAPackagePolicy != v1alpha1.DefaultCAPackagePolicyEnabled { + t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, v1alpha1.DefaultCAPackagePolicyEnabled) + } + if s.FilterExpiredCertificatesPolicy != v1alpha1.FilterExpiredCertificatesPolicyEnabled { + t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, v1alpha1.FilterExpiredCertificatesPolicyEnabled) + } + }, + }, + { + name: "default trust namespace is reflected when spec trustNamespace is empty", + trustManager: func() *v1alpha1.TrustManager { + tm := testTrustManager().Build() + tm.Spec.TrustManagerConfig.TrustNamespace = "" + return tm + }, + wantStatusUpdate: 1, + assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { + if tm.Status.TrustNamespace != defaultTrustNamespace { + t.Errorf("TrustNamespace: got %q, want %q", tm.Status.TrustNamespace, defaultTrustNamespace) + } + }, + }, + { + name: "no-op when observed state already matches spec and env", + trustManager: func() *v1alpha1.TrustManager { + tm := testTrustManager().Build() + tm.Status.TrustManagerImage = testImage + tm.Status.TrustNamespace = defaultTrustNamespace + tm.Status.SecretTargetsPolicy = tm.Spec.TrustManagerConfig.SecretTargets.Policy + tm.Status.DefaultCAPackagePolicy = tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy + tm.Status.FilterExpiredCertificatesPolicy = tm.Spec.TrustManagerConfig.FilterExpiredCertificates + return tm + }, + wantStatusUpdate: 0, + assertStatus: func(*testing.T, *v1alpha1.TrustManager) {}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := testReconciler(t) + mock := &fakes.FakeCtrlClient{} + tm := tt.trustManager() + + mock.GetCalls(func(ctx context.Context, key client.ObjectKey, obj client.Object) error { + switch o := obj.(type) { + case *v1alpha1.TrustManager: + tm.DeepCopyInto(o) + } + return nil + }) + mock.StatusUpdateCalls(func(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error { + return nil + }) + r.CtrlClient = mock + + if err := r.updateStatusObservedState(tm); err != nil { + t.Fatalf("updateStatusObservedState: %v", err) + } + if got := mock.StatusUpdateCallCount(); got != tt.wantStatusUpdate { + t.Errorf("StatusUpdateCallCount() = %d, want %d", got, tt.wantStatusUpdate) + } + tt.assertStatus(t, tm) + }) + } +} From 9cf508c4193226f9a9040a8cb323aa18646fcc64 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 31 Mar 2026 17:03:27 +0530 Subject: [PATCH 05/11] Handling remaining PR review comments on trustmanager_test.go --- test/e2e/trustmanager_test.go | 147 ++++++++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 34 deletions(-) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 36b72bbf7..24d9463a0 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -575,8 +575,35 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should set --trust-namespace on deployment for custom trust namespace", func() { + const customTrustNS = "custom-trust-ns" + + By("creating custom trust namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: customTrustNS, + }, + } + _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + defer func() { + By("cleaning up custom trust namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) + }() + + createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) + + By("verifying deployment has correct --trust-namespace arg") + Eventually(func(g Gomega) { + deployment, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(deployment.Spec.Template.Spec.Containers[0].Args).Should(ContainElement(fmt.Sprintf("--trust-namespace=%s", customTrustNS))) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + // TODO: Add test for other deployment configuration options - // (i.e. custom trust namespace, secret targets policy, default CA package policy, filter expired certificates policy) + // (i.e. secret targets policy, default CA package policy, filter expired certificates policy) }) // ------------------------------------------------------------------------- @@ -634,6 +661,46 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) }, lowTimeout, fastPollInterval).Should(Succeed()) }) + + It("should create Role and RoleBinding in custom trust namespace", func() { + const customTrustNS = "custom-trust-ns" + + By("creating custom trust namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: customTrustNS, + }, + } + _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + defer func() { + By("cleaning up custom trust namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) + }() + + By("creating TrustManager CR with custom trust namespace") + createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) + + By("verifying Role is created in custom trust namespace") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(customTrustNS).Get(ctx, trustManagerRoleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(role.Namespace).Should(Equal(customTrustNS)) + verifyTrustManagerManagedLabels(role.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying RoleBinding is created in custom trust namespace") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(customTrustNS).Get(ctx, trustManagerRoleBindingName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(rb.Namespace).Should(Equal(customTrustNS)) + g.Expect(rb.RoleRef.Name).Should(Equal(trustManagerRoleName)) + g.Expect(rb.Subjects[0].Name).Should(Equal(trustManagerServiceAccountName)) + g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(rb.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) }) // ------------------------------------------------------------------------- @@ -741,6 +808,33 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should report custom trust namespace in status", func() { + const customTrustNS = "custom-trust-ns-status" + + By("creating custom trust namespace") + ns := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: customTrustNS, + }, + } + _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + defer func() { + By("cleaning up custom trust namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) + }() + + createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) + + By("verifying TrustManager status has custom trust namespace set") + Eventually(func(g Gomega) { + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(tm.Status.TrustNamespace).Should(Equal(customTrustNS)) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) + // TODO: Add test for status reporting when custom configuration is applied // (i.e. secret targets policy, default CA package policy, filter expired certificates policy) }) @@ -770,25 +864,6 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() By("creating TrustManager CR with custom trust namespace") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) - By("verifying Role is created in custom trust namespace") - Eventually(func(g Gomega) { - role, err := clientset.RbacV1().Roles(customTrustNS).Get(ctx, trustManagerRoleName, metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(role.Namespace).Should(Equal(customTrustNS)) - verifyTrustManagerManagedLabels(role.Labels) - }, lowTimeout, fastPollInterval).Should(Succeed()) - - By("verifying RoleBinding is created in custom trust namespace") - Eventually(func(g Gomega) { - rb, err := clientset.RbacV1().RoleBindings(customTrustNS).Get(ctx, trustManagerRoleBindingName, metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(rb.Namespace).Should(Equal(customTrustNS)) - g.Expect(rb.RoleRef.Name).Should(Equal(trustManagerRoleName)) - g.Expect(rb.Subjects[0].Name).Should(Equal(trustManagerServiceAccountName)) - g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) - verifyTrustManagerManagedLabels(rb.Labels) - }, lowTimeout, fastPollInterval).Should(Succeed()) - By("verifying leader election Role is in operand namespace") Eventually(func(g Gomega) { role, err := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleName, metav1.GetOptions{}) @@ -804,20 +879,20 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(rb.Namespace).Should(Equal(trustManagerNamespace)) verifyTrustManagerManagedLabels(rb.Labels) }, lowTimeout, fastPollInterval).Should(Succeed()) + }) - By("verifying deployment has correct --trust-namespace arg") - Eventually(func(g Gomega) { - deployment, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(deployment.Spec.Template.Spec.Containers[0].Args).Should(ContainElement(fmt.Sprintf("--trust-namespace=%s", customTrustNS))) - }, lowTimeout, fastPollInterval).Should(Succeed()) + It("should reject updates that change spec.trustNamespace", func() { + By("creating TrustManager with explicit trust namespace") + createTrustManager(newTrustManagerCR().WithTrustNamespace("cert-manager")) - By("verifying status.trustNamespace matches spec") - Eventually(func(g Gomega) { - tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(tm.Status.TrustNamespace).Should(Equal(customTrustNS)) - }, lowTimeout, fastPollInterval).Should(Succeed()) + By("attempting to mutate spec.trustNamespace (field is immutable once set)") + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + tm.Spec.TrustManagerConfig.TrustNamespace = "other-trust-ns-immutable" + _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) + Expect(err).Should(HaveOccurred()) + Expect(errors.IsInvalid(err)).Should(BeTrue()) + Expect(err.Error()).Should(ContainSubstring("trustNamespace is immutable once set")) }) It("should set degraded condition when trust namespace does not exist", func() { @@ -836,12 +911,16 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(degradedCondition).ShouldNot(BeNil()) g.Expect(degradedCondition.Status).Should(Equal(metav1.ConditionTrue)) g.Expect(degradedCondition.Reason).Should(Equal(v1alpha1.ReasonFailed)) - g.Expect(degradedCondition.Message).Should(ContainSubstring(nonExistentNS)) - g.Expect(degradedCondition.Message).Should(ContainSubstring("does not exist")) + g.Expect(degradedCondition.Message).Should(And( + ContainSubstring("trust namespace"), + ContainSubstring(nonExistentNS), + ContainSubstring("does not exist"), + )) readyCondition := meta.FindStatusCondition(tm.Status.Conditions, v1alpha1.Ready) g.Expect(readyCondition).ShouldNot(BeNil()) g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionFalse)) + g.Expect(readyCondition.Reason).Should(Equal(v1alpha1.ReasonFailed)) }, lowTimeout, fastPollInterval).Should(Succeed()) // Irrecoverable errors are not requeued; delete and recreate after the namespace exists. From 89dbd8dcad9eb0209c5b1f5d2db32491cc173665 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Tue, 31 Mar 2026 21:59:22 +0530 Subject: [PATCH 06/11] Addressed Review comments iteration 3 --- .../trustmanager/install_trustmanager_test.go | 94 ++++----- test/e2e/trustmanager_test.go | 197 ++++++++---------- 2 files changed, 132 insertions(+), 159 deletions(-) diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index 17e19704e..dd29badb8 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -16,7 +16,6 @@ func TestUpdateStatusObservedState(t *testing.T) { name string trustManager func() *v1alpha1.TrustManager wantStatusUpdate int - assertStatus func(*testing.T, *v1alpha1.TrustManager) }{ { name: "updates all observed fields when status is empty", @@ -24,24 +23,6 @@ func TestUpdateStatusObservedState(t *testing.T) { return testTrustManager().Build() }, wantStatusUpdate: 1, - assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { - s := tm.Status - if s.TrustManagerImage != testImage { - t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) - } - if s.TrustNamespace != defaultTrustNamespace { - t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, defaultTrustNamespace) - } - if s.SecretTargetsPolicy != tm.Spec.TrustManagerConfig.SecretTargets.Policy { - t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, tm.Spec.TrustManagerConfig.SecretTargets.Policy) - } - if s.DefaultCAPackagePolicy != tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy { - t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy) - } - if s.FilterExpiredCertificatesPolicy != tm.Spec.TrustManagerConfig.FilterExpiredCertificates { - t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, tm.Spec.TrustManagerConfig.FilterExpiredCertificates) - } - }, }, { name: "updates all observed fields for custom spec", @@ -54,38 +35,6 @@ func TestUpdateStatusObservedState(t *testing.T) { return tm }, wantStatusUpdate: 1, - assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { - s := tm.Status - if s.TrustManagerImage != testImage { - t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) - } - if s.TrustNamespace != "custom-trust-ns" { - t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, "custom-trust-ns") - } - if s.SecretTargetsPolicy != v1alpha1.SecretTargetsPolicyCustom { - t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, v1alpha1.SecretTargetsPolicyCustom) - } - if s.DefaultCAPackagePolicy != v1alpha1.DefaultCAPackagePolicyEnabled { - t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, v1alpha1.DefaultCAPackagePolicyEnabled) - } - if s.FilterExpiredCertificatesPolicy != v1alpha1.FilterExpiredCertificatesPolicyEnabled { - t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, v1alpha1.FilterExpiredCertificatesPolicyEnabled) - } - }, - }, - { - name: "default trust namespace is reflected when spec trustNamespace is empty", - trustManager: func() *v1alpha1.TrustManager { - tm := testTrustManager().Build() - tm.Spec.TrustManagerConfig.TrustNamespace = "" - return tm - }, - wantStatusUpdate: 1, - assertStatus: func(t *testing.T, tm *v1alpha1.TrustManager) { - if tm.Status.TrustNamespace != defaultTrustNamespace { - t.Errorf("TrustNamespace: got %q, want %q", tm.Status.TrustNamespace, defaultTrustNamespace) - } - }, }, { name: "no-op when observed state already matches spec and env", @@ -99,7 +48,6 @@ func TestUpdateStatusObservedState(t *testing.T) { return tm }, wantStatusUpdate: 0, - assertStatus: func(*testing.T, *v1alpha1.TrustManager) {}, }, } @@ -127,7 +75,47 @@ func TestUpdateStatusObservedState(t *testing.T) { if got := mock.StatusUpdateCallCount(); got != tt.wantStatusUpdate { t.Errorf("StatusUpdateCallCount() = %d, want %d", got, tt.wantStatusUpdate) } - tt.assertStatus(t, tm) + + switch tt.name { + case "updates all observed fields when status is empty": + s := tm.Status + if s.TrustManagerImage != testImage { + t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) + } + if s.TrustNamespace != defaultTrustNamespace { + t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, defaultTrustNamespace) + } + if s.SecretTargetsPolicy != tm.Spec.TrustManagerConfig.SecretTargets.Policy { + t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, tm.Spec.TrustManagerConfig.SecretTargets.Policy) + } + if s.DefaultCAPackagePolicy != tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy { + t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy) + } + if s.FilterExpiredCertificatesPolicy != tm.Spec.TrustManagerConfig.FilterExpiredCertificates { + t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, tm.Spec.TrustManagerConfig.FilterExpiredCertificates) + } + case "updates all observed fields for custom spec": + s := tm.Status + if s.TrustManagerImage != testImage { + t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) + } + if s.TrustNamespace != "custom-trust-ns" { + t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, "custom-trust-ns") + } + if s.SecretTargetsPolicy != v1alpha1.SecretTargetsPolicyCustom { + t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, v1alpha1.SecretTargetsPolicyCustom) + } + if s.DefaultCAPackagePolicy != v1alpha1.DefaultCAPackagePolicyEnabled { + t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, v1alpha1.DefaultCAPackagePolicyEnabled) + } + if s.FilterExpiredCertificatesPolicy != v1alpha1.FilterExpiredCertificatesPolicyEnabled { + t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, v1alpha1.FilterExpiredCertificatesPolicyEnabled) + } + case "no-op when observed state already matches spec and env": + // Status unchanged; covered by wantStatusUpdate == 0. + default: + t.Fatalf("missing assertions for test case %q", tt.name) + } }) } } diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 24d9463a0..01b795691 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -5,6 +5,8 @@ package e2e import ( "context" + "crypto/rand" + "encoding/hex" "fmt" . "github.com/onsi/ginkgo/v2" @@ -45,6 +47,48 @@ const ( trustManagerWebhookConfigName = "trust-manager" ) +// uniqueTrustNamespace returns a DNS-1123-safe name for per-spec namespace isolation. +func uniqueTrustNamespace(prefix string) string { + var b [4]byte + _, err := rand.Read(b[:]) + Expect(err).NotTo(HaveOccurred()) + name := fmt.Sprintf("%s-%s", prefix, hex.EncodeToString(b[:])) + if len(name) > 63 { + return name[:63] + } + return name +} + +func waitNamespaceDeleted(ctx context.Context, clientset *kubernetes.Clientset, name string) { + Eventually(func() bool { + _, err := clientset.CoreV1().Namespaces().Get(ctx, name, metav1.GetOptions{}) + return errors.IsNotFound(err) + }, lowTimeout, fastPollInterval).Should(BeTrue()) +} + +// createTrustTestNamespace creates a uniquely named namespace (namePrefix + random suffix) and +// registers DeferCleanup to delete it and wait until it is fully removed. +func createTrustTestNamespace(ctx context.Context, clientset *kubernetes.Clientset, namePrefix string) string { + By("creating custom trust namespace") + name := uniqueTrustNamespace(namePrefix) + createTrustTestNamespaceNamed(ctx, clientset, name, "cleaning up custom trust namespace") + return name +} + +// createTrustTestNamespaceNamed creates a namespace with the exact given name and registers +// DeferCleanup to delete it and wait until it is fully removed. cleanupBy is passed to By() before delete. +func createTrustTestNamespaceNamed(ctx context.Context, clientset *kubernetes.Clientset, name string, cleanupBy string) { + _, err := clientset.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + }, metav1.CreateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + DeferCleanup(func() { + By(cleanupBy) + _ = clientset.CoreV1().Namespaces().Delete(ctx, name, metav1.DeleteOptions{}) + waitNamespaceDeleted(ctx, clientset, name) + }) +} + var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() { ctx := context.TODO() var clientset *kubernetes.Clientset @@ -576,21 +620,7 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }) It("should set --trust-namespace on deployment for custom trust namespace", func() { - const customTrustNS = "custom-trust-ns" - - By("creating custom trust namespace") - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: customTrustNS, - }, - } - _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) - Expect(err).ShouldNot(HaveOccurred()) - - defer func() { - By("cleaning up custom trust namespace") - _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) - }() + customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -598,6 +628,7 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() Eventually(func(g Gomega) { deployment, err := clientset.AppsV1().Deployments(trustManagerNamespace).Get(ctx, trustManagerDeploymentName, metav1.GetOptions{}) g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(len(deployment.Spec.Template.Spec.Containers)).Should(BeNumerically(">", 0)) g.Expect(deployment.Spec.Template.Spec.Containers[0].Args).Should(ContainElement(fmt.Sprintf("--trust-namespace=%s", customTrustNS))) }, lowTimeout, fastPollInterval).Should(Succeed()) }) @@ -663,21 +694,7 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }) It("should create Role and RoleBinding in custom trust namespace", func() { - const customTrustNS = "custom-trust-ns" - - By("creating custom trust namespace") - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: customTrustNS, - }, - } - _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) - Expect(err).ShouldNot(HaveOccurred()) - - defer func() { - By("cleaning up custom trust namespace") - _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) - }() + customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") By("creating TrustManager CR with custom trust namespace") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -696,11 +713,35 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(err).ShouldNot(HaveOccurred()) g.Expect(rb.Namespace).Should(Equal(customTrustNS)) g.Expect(rb.RoleRef.Name).Should(Equal(trustManagerRoleName)) + g.Expect(rb.Subjects).ShouldNot(BeEmpty()) g.Expect(rb.Subjects[0].Name).Should(Equal(trustManagerServiceAccountName)) g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) verifyTrustManagerManagedLabels(rb.Labels) }, lowTimeout, fastPollInterval).Should(Succeed()) }) + + It("should place leader election Role and RoleBinding in operand namespace when trust namespace is custom", func() { + customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") + + By("creating TrustManager CR with custom trust namespace") + createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) + + By("verifying leader election Role is in operand namespace") + Eventually(func(g Gomega) { + role, err := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(role.Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(role.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying leader election RoleBinding is in operand namespace") + Eventually(func(g Gomega) { + rb, err := clientset.RbacV1().RoleBindings(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleBindingName, metav1.GetOptions{}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(rb.Namespace).Should(Equal(trustManagerNamespace)) + verifyTrustManagerManagedLabels(rb.Labels) + }, lowTimeout, fastPollInterval).Should(Succeed()) + }) }) // ------------------------------------------------------------------------- @@ -809,21 +850,7 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }) It("should report custom trust namespace in status", func() { - const customTrustNS = "custom-trust-ns-status" - - By("creating custom trust namespace") - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: customTrustNS, - }, - } - _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) - Expect(err).ShouldNot(HaveOccurred()) - - defer func() { - By("cleaning up custom trust namespace") - _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) - }() + customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns-status") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -844,43 +871,6 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() // ------------------------------------------------------------------------- Context("trust namespace configuration", func() { - It("should configure resources with custom trust namespace", func() { - const customTrustNS = "custom-trust-ns" - - By("creating custom trust namespace") - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: customTrustNS, - }, - } - _, err := clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) - Expect(err).ShouldNot(HaveOccurred()) - - defer func() { - By("cleaning up custom trust namespace") - _ = clientset.CoreV1().Namespaces().Delete(ctx, customTrustNS, metav1.DeleteOptions{}) - }() - - By("creating TrustManager CR with custom trust namespace") - createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) - - By("verifying leader election Role is in operand namespace") - Eventually(func(g Gomega) { - role, err := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleName, metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(role.Namespace).Should(Equal(trustManagerNamespace)) - verifyTrustManagerManagedLabels(role.Labels) - }, lowTimeout, fastPollInterval).Should(Succeed()) - - By("verifying leader election RoleBinding is in operand namespace") - Eventually(func(g Gomega) { - rb, err := clientset.RbacV1().RoleBindings(trustManagerNamespace).Get(ctx, trustManagerLeaderElectionRoleBindingName, metav1.GetOptions{}) - g.Expect(err).ShouldNot(HaveOccurred()) - g.Expect(rb.Namespace).Should(Equal(trustManagerNamespace)) - verifyTrustManagerManagedLabels(rb.Labels) - }, lowTimeout, fastPollInterval).Should(Succeed()) - }) - It("should reject updates that change spec.trustNamespace", func() { By("creating TrustManager with explicit trust namespace") createTrustManager(newTrustManagerCR().WithTrustNamespace("cert-manager")) @@ -896,7 +886,7 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() }) It("should set degraded condition when trust namespace does not exist", func() { - const nonExistentNS = "non-existent-trust-ns" + nonExistentNS := uniqueTrustNamespace("non-existent-trust") By("creating TrustManager CR with non-existent trust namespace") _, err := trustManagerClient().Create(ctx, newTrustManagerCR().WithTrustNamespace(nonExistentNS).Build(), metav1.CreateOptions{}) @@ -921,35 +911,30 @@ var _ = Describe("TrustManager", Ordered, Label("Feature:TrustManager"), func() g.Expect(readyCondition).ShouldNot(BeNil()) g.Expect(readyCondition.Status).Should(Equal(metav1.ConditionFalse)) g.Expect(readyCondition.Reason).Should(Equal(v1alpha1.ReasonFailed)) + // Irrecoverable path: Ready message is left empty; detail is on Degraded only + // (see HandleReconcileResult for IsIrrecoverableError). + g.Expect(readyCondition.Message).Should(BeEmpty()) }, lowTimeout, fastPollInterval).Should(Succeed()) - // Irrecoverable errors are not requeued; delete and recreate after the namespace exists. - By("deleting TrustManager CR") - err = trustManagerClient().Delete(ctx, "cluster", metav1.DeleteOptions{}) - Expect(err).ShouldNot(HaveOccurred()) + // Irrecoverable errors are not requeued, and Namespace is not watched, so creating the + // namespace alone does not reconcile. Create the namespace, then change TrustManager + // spec (controllerConfig.annotations) to bump generation and enqueue a reconcile. + By("creating the trust namespace that was previously missing") + createTrustTestNamespaceNamed(ctx, clientset, nonExistentNS, "cleaning up the namespace") - By("waiting for TrustManager CR to be deleted") - Eventually(func() bool { - _, getErr := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) - return errors.IsNotFound(getErr) - }, lowTimeout, fastPollInterval).Should(BeTrue()) - - By("creating the namespace that was previously missing") - ns := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: nonExistentNS, - }, + By("updating TrustManager to trigger reconciliation now that the namespace exists") + tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + if tm.Spec.ControllerConfig.Annotations == nil { + tm.Spec.ControllerConfig.Annotations = map[string]string{} } - _, err = clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + tm.Spec.ControllerConfig.Annotations["trustmanager.e2e.openshift.io/recovery"] = "true" + _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) Expect(err).ShouldNot(HaveOccurred()) - defer func() { - By("cleaning up the namespace") - _ = clientset.CoreV1().Namespaces().Delete(ctx, nonExistentNS, metav1.DeleteOptions{}) - }() - - By("recreating TrustManager CR with trust namespace that now exists") - createTrustManager(newTrustManagerCR().WithTrustNamespace(nonExistentNS)) + By("waiting for TrustManager to recover (Ready=True, not Degraded)") + _, err = pollTillTrustManagerAvailable(ctx, trustManagerClient(), "cluster") + Expect(err).ShouldNot(HaveOccurred()) }) }) From 0edc861d22a9aa9b93c4b09cab034e7ef1974fc2 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Wed, 1 Apr 2026 11:09:43 +0530 Subject: [PATCH 07/11] Handling CodeRabbit comment --- test/e2e/trustmanager_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index c6aac6778..38e9be357 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -718,6 +718,14 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) verifyTrustManagerManagedLabels(rb.Labels) }, lowTimeout, fastPollInterval).Should(Succeed()) + + By("verifying trust-namespace Role is not in operand namespace") + _, errRole := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerRoleName, metav1.GetOptions{}) + Expect(errors.IsNotFound(errRole)).Should(BeTrue()) + + By("verifying trust-namespace RoleBinding is not in operand namespace") + _, errRB := clientset.RbacV1().RoleBindings(trustManagerNamespace).Get(ctx, trustManagerRoleBindingName, metav1.GetOptions{}) + Expect(errors.IsNotFound(errRB)).Should(BeTrue()) }) It("should place leader election Role and RoleBinding in operand namespace when trust namespace is custom", func() { From e7a856023d32864d307c1e7f94b0552af07b9863 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Wed, 1 Apr 2026 17:24:19 +0530 Subject: [PATCH 08/11] Addressed new round of pr review comments --- .../trustmanager/install_trustmanager_test.go | 74 ++++++++----------- pkg/controller/trustmanager/test_utils.go | 5 ++ test/e2e/trustmanager_test.go | 73 +++++------------- test/e2e/utils_test.go | 38 ++++++++++ 4 files changed, 90 insertions(+), 100 deletions(-) diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go index dd29badb8..437022391 100644 --- a/pkg/controller/trustmanager/install_trustmanager_test.go +++ b/pkg/controller/trustmanager/install_trustmanager_test.go @@ -2,8 +2,10 @@ package trustmanager import ( "context" + "reflect" "testing" + "github.com/google/go-cmp/cmp" "github.com/openshift/cert-manager-operator/api/operator/v1alpha1" "github.com/openshift/cert-manager-operator/pkg/controller/common/fakes" "sigs.k8s.io/controller-runtime/pkg/client" @@ -12,10 +14,20 @@ import ( func TestUpdateStatusObservedState(t *testing.T) { t.Setenv(trustManagerImageNameEnvVarName, testImage) + // Observed status after sync from testTrustManager() defaults (empty status fields + default spec). + wantStatusSyncedFromDefaultSpec := v1alpha1.TrustManagerStatus{ + TrustManagerImage: testImage, + TrustNamespace: defaultTrustNamespace, + SecretTargetsPolicy: "", + DefaultCAPackagePolicy: "", + FilterExpiredCertificatesPolicy: "", + } + tests := []struct { name string trustManager func() *v1alpha1.TrustManager wantStatusUpdate int + wantStatus v1alpha1.TrustManagerStatus }{ { name: "updates all observed fields when status is empty", @@ -23,18 +35,26 @@ func TestUpdateStatusObservedState(t *testing.T) { return testTrustManager().Build() }, wantStatusUpdate: 1, + wantStatus: wantStatusSyncedFromDefaultSpec, }, { name: "updates all observed fields for custom spec", trustManager: func() *v1alpha1.TrustManager { - tm := testTrustManager().WithTrustNamespace("custom-trust-ns").Build() - tm.Spec.TrustManagerConfig.SecretTargets.Policy = v1alpha1.SecretTargetsPolicyCustom - tm.Spec.TrustManagerConfig.SecretTargets.AuthorizedSecrets = []string{"allowed-secret"} - tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy = v1alpha1.DefaultCAPackagePolicyEnabled - tm.Spec.TrustManagerConfig.FilterExpiredCertificates = v1alpha1.FilterExpiredCertificatesPolicyEnabled - return tm + return testTrustManager(). + WithTrustNamespace("custom-trust-ns"). + WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"allowed-secret"}). + WithDefaultCAPackage(v1alpha1.DefaultCAPackagePolicyEnabled). + WithFilterExpiredCertificates(v1alpha1.FilterExpiredCertificatesPolicyEnabled). + Build() }, wantStatusUpdate: 1, + wantStatus: v1alpha1.TrustManagerStatus{ + TrustManagerImage: testImage, + TrustNamespace: "custom-trust-ns", + SecretTargetsPolicy: v1alpha1.SecretTargetsPolicyCustom, + DefaultCAPackagePolicy: v1alpha1.DefaultCAPackagePolicyEnabled, + FilterExpiredCertificatesPolicy: v1alpha1.FilterExpiredCertificatesPolicyEnabled, + }, }, { name: "no-op when observed state already matches spec and env", @@ -48,6 +68,7 @@ func TestUpdateStatusObservedState(t *testing.T) { return tm }, wantStatusUpdate: 0, + wantStatus: wantStatusSyncedFromDefaultSpec, }, } @@ -76,45 +97,8 @@ func TestUpdateStatusObservedState(t *testing.T) { t.Errorf("StatusUpdateCallCount() = %d, want %d", got, tt.wantStatusUpdate) } - switch tt.name { - case "updates all observed fields when status is empty": - s := tm.Status - if s.TrustManagerImage != testImage { - t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) - } - if s.TrustNamespace != defaultTrustNamespace { - t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, defaultTrustNamespace) - } - if s.SecretTargetsPolicy != tm.Spec.TrustManagerConfig.SecretTargets.Policy { - t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, tm.Spec.TrustManagerConfig.SecretTargets.Policy) - } - if s.DefaultCAPackagePolicy != tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy { - t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, tm.Spec.TrustManagerConfig.DefaultCAPackage.Policy) - } - if s.FilterExpiredCertificatesPolicy != tm.Spec.TrustManagerConfig.FilterExpiredCertificates { - t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, tm.Spec.TrustManagerConfig.FilterExpiredCertificates) - } - case "updates all observed fields for custom spec": - s := tm.Status - if s.TrustManagerImage != testImage { - t.Errorf("TrustManagerImage: got %q, want %q", s.TrustManagerImage, testImage) - } - if s.TrustNamespace != "custom-trust-ns" { - t.Errorf("TrustNamespace: got %q, want %q", s.TrustNamespace, "custom-trust-ns") - } - if s.SecretTargetsPolicy != v1alpha1.SecretTargetsPolicyCustom { - t.Errorf("SecretTargetsPolicy: got %q, want %q", s.SecretTargetsPolicy, v1alpha1.SecretTargetsPolicyCustom) - } - if s.DefaultCAPackagePolicy != v1alpha1.DefaultCAPackagePolicyEnabled { - t.Errorf("DefaultCAPackagePolicy: got %q, want %q", s.DefaultCAPackagePolicy, v1alpha1.DefaultCAPackagePolicyEnabled) - } - if s.FilterExpiredCertificatesPolicy != v1alpha1.FilterExpiredCertificatesPolicyEnabled { - t.Errorf("FilterExpiredCertificatesPolicy: got %q, want %q", s.FilterExpiredCertificatesPolicy, v1alpha1.FilterExpiredCertificatesPolicyEnabled) - } - case "no-op when observed state already matches spec and env": - // Status unchanged; covered by wantStatusUpdate == 0. - default: - t.Fatalf("missing assertions for test case %q", tt.name) + if !reflect.DeepEqual(tm.Status, tt.wantStatus) { + t.Errorf("TrustManager.Status mismatch (-want +got):\n%s", cmp.Diff(tt.wantStatus, tm.Status)) } }) } diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index 5fe7db7e3..e0b96cc9d 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -89,6 +89,11 @@ func (b *trustManagerBuilder) WithFilterExpiredCertificates(policy v1alpha1.Filt return b } +func (b *trustManagerBuilder) WithDefaultCAPackage(policy v1alpha1.DefaultCAPackagePolicy) *trustManagerBuilder { + b.Spec.TrustManagerConfig.DefaultCAPackage.Policy = policy + return b +} + func (b *trustManagerBuilder) WithSecretTargets(policy v1alpha1.SecretTargetsPolicy, authorizedSecrets []string) *trustManagerBuilder { b.Spec.TrustManagerConfig.SecretTargets = v1alpha1.SecretTargetsConfig{ Policy: policy, diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 76f0baf68..9dea2d620 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -5,8 +5,6 @@ package e2e import ( "context" - "crypto/rand" - "encoding/hex" "fmt" "slices" @@ -49,48 +47,6 @@ const ( trustManagerWebhookConfigName = "trust-manager" ) -// uniqueTrustNamespace returns a DNS-1123-safe name for per-spec namespace isolation. -func uniqueTrustNamespace(prefix string) string { - var b [4]byte - _, err := rand.Read(b[:]) - Expect(err).NotTo(HaveOccurred()) - name := fmt.Sprintf("%s-%s", prefix, hex.EncodeToString(b[:])) - if len(name) > 63 { - return name[:63] - } - return name -} - -func waitNamespaceDeleted(ctx context.Context, clientset *kubernetes.Clientset, name string) { - Eventually(func() bool { - _, err := clientset.CoreV1().Namespaces().Get(ctx, name, metav1.GetOptions{}) - return errors.IsNotFound(err) - }, lowTimeout, fastPollInterval).Should(BeTrue()) -} - -// createTrustTestNamespace creates a uniquely named namespace (namePrefix + random suffix) and -// registers DeferCleanup to delete it and wait until it is fully removed. -func createTrustTestNamespace(ctx context.Context, clientset *kubernetes.Clientset, namePrefix string) string { - By("creating custom trust namespace") - name := uniqueTrustNamespace(namePrefix) - createTrustTestNamespaceNamed(ctx, clientset, name, "cleaning up custom trust namespace") - return name -} - -// createTrustTestNamespaceNamed creates a namespace with the exact given name and registers -// DeferCleanup to delete it and wait until it is fully removed. cleanupBy is passed to By() before delete. -func createTrustTestNamespaceNamed(ctx context.Context, clientset *kubernetes.Clientset, name string, cleanupBy string) { - _, err := clientset.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: name}, - }, metav1.CreateOptions{}) - Expect(err).ShouldNot(HaveOccurred()) - DeferCleanup(func() { - By(cleanupBy) - _ = clientset.CoreV1().Namespaces().Delete(ctx, name, metav1.DeleteOptions{}) - waitNamespaceDeleted(ctx, clientset, name) - }) -} - var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:TrustManager"), func() { ctx := context.TODO() var clientset *kubernetes.Clientset @@ -117,7 +73,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru clientset, err = kubernetes.NewForConfig(cfg) Expect(err).Should(BeNil()) - By("enabling TrustManager feature gate via subscription") + /*By("enabling TrustManager feature gate via subscription") err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{ "UNSUPPORTED_ADDON_FEATURES": "TrustManager=true", "OPERATOR_LOG_LEVEL": "4", @@ -126,7 +82,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru By("waiting for operator deployment to rollout with TrustManager feature enabled") err = waitForDeploymentEnvVarAndRollout(ctx, operatorNamespace, operatorDeploymentName, "UNSUPPORTED_ADDON_FEATURES", "TrustManager=true", lowTimeout) - Expect(err).NotTo(HaveOccurred()) + Expect(err).NotTo(HaveOccurred())*/ }) BeforeEach(func() { @@ -622,7 +578,9 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) It("should set --trust-namespace on deployment for custom trust namespace", func() { - customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -660,7 +618,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) // TODO: Add test for other deployment configuration options - // (i.e. secret targets policy, default CA package policy, filter expired certificates policy) + // (i.e. default CA package policy, filter expired certificates policy). }) // ------------------------------------------------------------------------- @@ -720,7 +678,9 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) It("should create Role and RoleBinding in custom trust namespace", func() { - customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) By("creating TrustManager CR with custom trust namespace") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -755,7 +715,9 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) It("should place leader election Role and RoleBinding in operand namespace when trust namespace is custom", func() { - customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns") + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) By("creating TrustManager CR with custom trust namespace") createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -996,7 +958,9 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) It("should report custom trust namespace in status", func() { - customTrustNS := createTrustTestNamespace(ctx, clientset, "custom-trust-ns-status") + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns-status") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) createTrustManager(newTrustManagerCR().WithTrustNamespace(customTrustNS)) @@ -1043,7 +1007,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) It("should set degraded condition when trust namespace does not exist", func() { - nonExistentNS := uniqueTrustNamespace("non-existent-trust") + nonExistentNS := createUniqueNamespace("non-existent-trust") By("creating TrustManager CR with non-existent trust namespace") _, err := trustManagerClient().Create(ctx, newTrustManagerCR().WithTrustNamespace(nonExistentNS).Build(), metav1.CreateOptions{}) @@ -1077,7 +1041,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru // namespace alone does not reconcile. Create the namespace, then change TrustManager // spec (controllerConfig.annotations) to bump generation and enqueue a reconcile. By("creating the trust namespace that was previously missing") - createTrustTestNamespaceNamed(ctx, clientset, nonExistentNS, "cleaning up the namespace") + createAndDestroyTestNamespace(ctx, clientset, nonExistentNS) By("updating TrustManager to trigger reconciliation now that the namespace exists") tm, err := trustManagerClient().Get(ctx, "cluster", metav1.GetOptions{}) @@ -1090,8 +1054,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru Expect(err).ShouldNot(HaveOccurred()) By("waiting for TrustManager to recover (Ready=True, not Degraded)") - _, err = pollTillTrustManagerAvailable(ctx, trustManagerClient(), "cluster") - Expect(err).ShouldNot(HaveOccurred()) + waitForTrustManagerReady() }) }) diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 189952b40..788607ddf 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -6,9 +6,11 @@ package e2e import ( "bytes" "context" + cryptorand "crypto/rand" "crypto/tls" "crypto/x509" "embed" + "encoding/hex" "encoding/json" "fmt" "log" @@ -24,6 +26,8 @@ import ( certmanagerclientset "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + . "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" opv1 "github.com/openshift/api/operator/v1" configv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" operatorv1 "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1" @@ -1021,6 +1025,40 @@ func verifyCertificateRenewed(ctx context.Context, secretName, namespace string, }) } +// createUniqueNamespace returns a DNS-1123-safe name for per-spec namespace isolation. +func createUniqueNamespace(prefix string) string { + var b [4]byte + _, err := cryptorand.Read(b[:]) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + name := fmt.Sprintf("%s-%s", prefix, hex.EncodeToString(b[:])) + if len(name) > 63 { + return name[:63] + } + return name +} + +// waitNamespaceDeleted polls until the namespace is gone. +func waitNamespaceDeleted(ctx context.Context, clientset *kubernetes.Clientset, name string) { + gomega.Eventually(func() bool { + _, err := clientset.CoreV1().Namespaces().Get(ctx, name, metav1.GetOptions{}) + return apierrors.IsNotFound(err) + }, lowTimeout, fastPollInterval).Should(gomega.BeTrue()) +} + +// createAndDestroyTestNamespace creates a namespace with the given name and registers DeferCleanup +// to delete it and wait until it is fully removed. +func createAndDestroyTestNamespace(ctx context.Context, clientset *kubernetes.Clientset, name string) { + _, err := clientset.CoreV1().Namespaces().Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + }, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + DeferCleanup(func() { + By("cleaning up test namespace") + _ = clientset.CoreV1().Namespaces().Delete(ctx, name, metav1.DeleteOptions{}) + waitNamespaceDeleted(ctx, clientset, name) + }) +} + // create randomized string func randomStr(size int) string { char := "abcdefghijklmnopqrstuvwxyz0123456789" From 197e4908468aa3bbe7ac85417bb34551153d9ad9 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Wed, 1 Apr 2026 17:27:05 +0530 Subject: [PATCH 09/11] Undo comment for test --- test/e2e/trustmanager_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 9dea2d620..6285e38e8 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -73,7 +73,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru clientset, err = kubernetes.NewForConfig(cfg) Expect(err).Should(BeNil()) - /*By("enabling TrustManager feature gate via subscription") + By("enabling TrustManager feature gate via subscription") err = patchSubscriptionWithEnvVars(ctx, loader, map[string]string{ "UNSUPPORTED_ADDON_FEATURES": "TrustManager=true", "OPERATOR_LOG_LEVEL": "4", @@ -82,7 +82,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru By("waiting for operator deployment to rollout with TrustManager feature enabled") err = waitForDeploymentEnvVarAndRollout(ctx, operatorNamespace, operatorDeploymentName, "UNSUPPORTED_ADDON_FEATURES", "TrustManager=true", lowTimeout) - Expect(err).NotTo(HaveOccurred())*/ + Expect(err).NotTo(HaveOccurred()) }) BeforeEach(func() { From fe6fc5b07493eebf896fc77bc5267fd8f9379b34 Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 2 Apr 2026 11:49:13 +0530 Subject: [PATCH 10/11] Addressing the latest review comment of removal of a test. --- test/e2e/trustmanager_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 6285e38e8..ef1eae7e7 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -704,14 +704,6 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru g.Expect(rb.Subjects[0].Namespace).Should(Equal(trustManagerNamespace)) verifyTrustManagerManagedLabels(rb.Labels) }, lowTimeout, fastPollInterval).Should(Succeed()) - - By("verifying trust-namespace Role is not in operand namespace") - _, errRole := clientset.RbacV1().Roles(trustManagerNamespace).Get(ctx, trustManagerRoleName, metav1.GetOptions{}) - Expect(errors.IsNotFound(errRole)).Should(BeTrue()) - - By("verifying trust-namespace RoleBinding is not in operand namespace") - _, errRB := clientset.RbacV1().RoleBindings(trustManagerNamespace).Get(ctx, trustManagerRoleBindingName, metav1.GetOptions{}) - Expect(errors.IsNotFound(errRB)).Should(BeTrue()) }) It("should place leader election Role and RoleBinding in operand namespace when trust namespace is custom", func() { From 47252765624e048cd31ae83e20804cc79cef32eb Mon Sep 17 00:00:00 2001 From: Arun Maurya Date: Thu, 2 Apr 2026 12:54:44 +0530 Subject: [PATCH 11/11] Removing duplicate method definition. --- pkg/controller/trustmanager/test_utils.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/controller/trustmanager/test_utils.go b/pkg/controller/trustmanager/test_utils.go index b34724b0d..e0b96cc9d 100644 --- a/pkg/controller/trustmanager/test_utils.go +++ b/pkg/controller/trustmanager/test_utils.go @@ -102,13 +102,6 @@ func (b *trustManagerBuilder) WithSecretTargets(policy v1alpha1.SecretTargetsPol return b } -func (b *trustManagerBuilder) WithDefaultCAPackage(policy v1alpha1.DefaultCAPackagePolicy) *trustManagerBuilder { - b.Spec.TrustManagerConfig.DefaultCAPackage = v1alpha1.DefaultCAPackageConfig{ - Policy: policy, - } - return b -} - func (b *trustManagerBuilder) Build() *v1alpha1.TrustManager { return b.TrustManager }