diff --git a/pkg/controller/trustmanager/controller_test.go b/pkg/controller/trustmanager/controller_test.go index 80c845932..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" @@ -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 new file mode 100644 index 000000000..437022391 --- /dev/null +++ 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 index be841d73e..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, @@ -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 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 { diff --git a/test/e2e/trustmanager_test.go b/test/e2e/trustmanager_test.go index 80056d834..bd26442f7 100644 --- a/test/e2e/trustmanager_test.go +++ b/test/e2e/trustmanager_test.go @@ -587,6 +587,22 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should set --trust-namespace on deployment for custom trust namespace", func() { + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) + + 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(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()) + }) + It("should add secret-targets-enabled arg when secretTargets policy is Custom", func() { createTrustManager(newTrustManagerCR().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"test-secret"})) @@ -612,7 +628,7 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) // TODO: Add test for other deployment configuration options - // (i.e. custom trust namespace, filter expired certificates policy) + // (e.g. filter expired certificates policy; custom trust namespace is covered above.) }) // ------------------------------------------------------------------------- @@ -877,6 +893,60 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }, lowTimeout, fastPollInterval).Should(Succeed()) }) + It("should create Role and RoleBinding in custom trust namespace", func() { + 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)) + + 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).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() { + 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)) + + 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 have no secret rules on ClusterRole when secretTargets is Disabled", func() { createTrustManager(newTrustManagerCR()) @@ -1084,6 +1154,32 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }, 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()) + }) + + It("should report custom trust namespace in status", func() { + By("creating custom trust namespace") + customTrustNS := createUniqueNamespace("custom-trust-ns-status") + createAndDestroyTestNamespace(ctx, clientset, customTrustNS) + + 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()) + }) + It("should report secretTargets policy in status", func() { createTrustManager(newTrustManagerCR().WithSecretTargets(v1alpha1.SecretTargetsPolicyCustom, []string{"status-test-secret"})) @@ -1125,7 +1221,78 @@ var _ = Describe("TrustManager", Ordered, Label("Platform:Generic", "Feature:Tru }) // TODO: Add test for status reporting when custom configuration is applied - // (i.e. custom trust namespace, filter expired certificates policy) + // (e.g. filter expired certificates policy; secret targets, default CA package, and trust namespace are covered above.) + }) + + // ------------------------------------------------------------------------- + // Trust namespace configuration + // ------------------------------------------------------------------------- + + Context("trust namespace configuration", func() { + It("should reject updates that change spec.trustNamespace", func() { + By("creating TrustManager with explicit trust namespace") + createTrustManager(newTrustManagerCR().WithTrustNamespace("cert-manager")) + + 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() { + nonExistentNS := createUniqueNamespace("non-existent-trust") + + 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(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)) + // 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, 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") + createAndDestroyTestNamespace(ctx, clientset, 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{} + } + tm.Spec.ControllerConfig.Annotations["trustmanager.e2e.openshift.io/recovery"] = "true" + _, err = trustManagerClient().Update(ctx, tm, metav1.UpdateOptions{}) + Expect(err).ShouldNot(HaveOccurred()) + + By("waiting for TrustManager to recover (Ready=True, not Degraded)") + waitForTrustManagerReady() + }) }) // ------------------------------------------------------------------------- diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 5dad5d4f6..b8999ba8b 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" @@ -144,6 +148,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) WithSecretTargets(policy v1alpha1.SecretTargetsPolicy, authorizedSecrets []string) *trustManagerCRBuilder { b.tm.Spec.TrustManagerConfig.SecretTargets = v1alpha1.SecretTargetsConfig{ Policy: policy, @@ -1021,6 +1030,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"