CM-869: Add test coverage for configurable TrustNamespace (#391)

arun717 · Arun Maurya · claude · web-flow · commit 64ddff2a0aaa · 2026-04-02T15:53:48.000Z
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * test(trustmanager): fix for missing namespace case. * Handling PR review comments. * Handling remaining PR review comments on trustmanager_test.go * Addressed Review comments iteration 3 * Handling CodeRabbit comment * Addressed new round of pr review comments * Undo comment for test * Addressing the latest review comment of removal of a test. * Removing duplicate method definition. --------- Co-authored-by: Arun Maurya <amaurya@amaurya-thinkpadp1gen7.bengluru.csb> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
diff --git 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"
 
@@ -251,6 +252,86 @@ 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,
+					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,
+					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; 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:
+						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 {
diff --git a/pkg/controller/trustmanager/install_trustmanager_test.go b/pkg/controller/trustmanager/install_trustmanager_test.go
@@ -0,0 +1,105 @@
+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"
+)
+
+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",
+			trustManager: func() *v1alpha1.TrustManager {
+				return testTrustManager().Build()
+			},
+			wantStatusUpdate: 1,
+			wantStatus:       wantStatusSyncedFromDefaultSpec,
+		},
+		{
+			name: "updates all observed fields for custom spec",
+			trustManager: func() *v1alpha1.TrustManager {
+				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",
+			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,
+			wantStatus:       wantStatusSyncedFromDefaultSpec,
+		},
+	}
+
+	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)
+			}
+
+			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
@@ -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,
@@ -97,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
 }
diff --git 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 {
diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go
diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ func getValidatingWebhookConfigObject(resourceLabels, resourceAnnotations map[st`
`52`	`52`	`return webhookConfig`
`53`	`53`	`}`
`54`	`54`
`55`		`-// updateWebhookClientConfig sets the webhook clientConfig service name and namespace`
	`55`	`+// updateWebhookClientConfig sets the webhook clientConfig service name and namespace.`
`56`	`56`	`func updateWebhookClientConfig(webhookConfig *admissionregistrationv1.ValidatingWebhookConfiguration) {`
`57`	`57`	`for i := range webhookConfig.Webhooks {`
`58`	`58`	`if webhookConfig.Webhooks[i].ClientConfig.Service != nil {`