From 4e85d48965dcabffbbb1062b23d3f3f07f9b26a0 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 24 Oct 2025 23:07:36 +0200 Subject: [PATCH 01/15] Missing ctlog config secret test added --- test/e2e/ctlog_recovery_test.go | 255 ++++++++++++++++++++++++++++ test/e2e/support/tas/ctlog/ctlog.go | 30 ++++ 2 files changed, 285 insertions(+) create mode 100644 test/e2e/ctlog_recovery_test.go diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go new file mode 100644 index 000000000..cd2ec285a --- /dev/null +++ b/test/e2e/ctlog_recovery_test.go @@ -0,0 +1,255 @@ +//go:build integration + +package e2e + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/securesign/operator/api/v1alpha1" + "github.com/securesign/operator/internal/constants" + ctlogActions "github.com/securesign/operator/internal/controller/ctlog/actions" + "github.com/securesign/operator/test/e2e/support" + "github.com/securesign/operator/test/e2e/support/condition" + "github.com/securesign/operator/test/e2e/support/steps" + "github.com/securesign/operator/test/e2e/support/tas/ctlog" + "github.com/securesign/operator/test/e2e/support/tas/trillian" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" +) + +var _ = Describe("CTlog recovery and validation", Ordered, func() { + cli, _ := support.CreateClient() + + var namespace *v1.Namespace + var trillianCR *v1alpha1.Trillian + var ctlogCR *v1alpha1.CTlog + + // Shared setup - create namespace + BeforeAll(steps.CreateNamespace(cli, func(new *v1.Namespace) { + namespace = new + })) + + // Shared setup - deploy Trillian (needed by CTLog) + BeforeAll(func(ctx SpecContext) { + trillianCR = &v1alpha1.Trillian{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-trillian", + Namespace: namespace.Name, + }, + Spec: v1alpha1.TrillianSpec{ + Db: v1alpha1.TrillianDB{Create: ptr.To(true)}, + }, + } + Expect(cli.Create(ctx, trillianCR)).To(Succeed()) + + By("Waiting for Trillian to be ready") + trillian.Verify(ctx, cli, namespace.Name, trillianCR.Name, true) + }) + + Describe("Test 1: Secret validation - missing secret (Issue 2586)", func() { + var originalSecretName string + + BeforeAll(func(ctx SpecContext) { + // Create keys secret for CTLog + By("Creating CTLog keys secret") + keysSecret := ctlog.CreateSecret(namespace.Name, "test-ctlog-keys") + Expect(cli.Create(ctx, keysSecret)).To(Succeed()) + + // Create a proper root certificate secret (CTLog needs root certs from Fulcio) + By("Creating root certificate") + _, _, rootCert, err := support.CreateCertificates(false) + Expect(err).NotTo(HaveOccurred()) + rootCertSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-root-cert", + Namespace: namespace.Name, + }, + Data: map[string][]byte{ + "cert": rootCert, + }, + } + Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) + + // Create CTLog CR with keys and root cert + ctlogCR = &v1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: namespace.Name, + }, + Spec: v1alpha1.CTlogSpec{ + Trillian: v1alpha1.TrillianService{ + Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), + Port: ptr.To(int32(8091)), + }, + PrivateKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys", + }, + Key: "private", + }, + PublicKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys", + }, + Key: "public", + }, + RootCertificates: []v1alpha1.SecretKeySelector{ + { + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-root-cert", + }, + Key: "cert", + }, + }, + }, + } + Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) + + By("Waiting for CTLog to be ready initially") + // Initial CTLog setup uses default 3 minute timeout for tree creation + ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) + }) + + It("should have a config secret reference in status", func(ctx SpecContext) { + Eventually(func(g Gomega) string { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + g.Expect(c.Status.ServerConfigRef).NotTo(BeNil()) + g.Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) + return c.Status.ServerConfigRef.Name + }).Should(Not(BeEmpty())) + + // Store the original secret name + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + originalSecretName = c.Status.ServerConfigRef.Name + GinkgoWriter.Printf("Original config secret name: %s\n", originalSecretName) + }) + + It("should have the config secret exist in the cluster", func(ctx SpecContext) { + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) + Expect(err).NotTo(HaveOccurred()) + Expect(secret).NotTo(BeNil()) + Expect(secret.Data).To(HaveKey("config")) + GinkgoWriter.Printf("Config secret exists with %d bytes of config data\n", len(secret.Data["config"])) + }) + + It("should delete the config secret to simulate cluster recreation", func(ctx SpecContext) { + By("Deleting config secret: " + originalSecretName) + err := ctlog.DeleteConfigSecret(ctx, cli, namespace.Name, originalSecretName) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying secret is actually gone") + Eventually(func() bool { + _, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) + return errors.IsNotFound(err) + }).WithTimeout(30*time.Second).Should(BeTrue(), "Secret should be deleted") + }) + + It("should trigger reconciliation by updating CTLog annotation", func(ctx SpecContext) { + Eventually(func(g Gomega) error { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + if c.Annotations == nil { + c.Annotations = make(map[string]string) + } + c.Annotations["test.trigger/reconcile"] = time.Now().Format(time.RFC3339) + return cli.Update(ctx, c) + }).WithTimeout(30 * time.Second).Should(Succeed()) + }) + + It("should detect missing secret and recreate it (Phase 1 validation)", func(ctx SpecContext) { + By("Waiting for operator to detect missing secret") + + // The CTLog should go into a non-Ready state first (detecting the issue) + // Then it should recover by recreating the secret + // This needs extra time as it waits for reconciliation loop + Eventually(func(g Gomega) bool { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + + // Check if status has a new/recreated secret reference + if c.Status.ServerConfigRef != nil { + // Try to get the secret + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, c.Status.ServerConfigRef.Name) + if err == nil && secret != nil { + GinkgoWriter.Printf("Config secret recreated: %s\n", c.Status.ServerConfigRef.Name) + return true + } + } + return false + }).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), + "Operator should detect missing secret and recreate it") + }) + + It("should have a valid config secret after recreation", func(ctx SpecContext) { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + Expect(c).NotTo(BeNil()) + Expect(c.Status.ServerConfigRef).NotTo(BeNil()) + + newSecretName := c.Status.ServerConfigRef.Name + GinkgoWriter.Printf("New config secret name: %s\n", newSecretName) + + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, newSecretName) + Expect(err).NotTo(HaveOccurred()) + Expect(secret).NotTo(BeNil()) + Expect(secret.Data).To(HaveKey("config")) + + // Verify config contains correct Trillian address + configData := secret.Data["config"] + expectedTrillianAddr := fmt.Sprintf("trillian-logserver.%s.svc.cluster.local:8091", namespace.Name) + Expect(string(configData)).To(ContainSubstring(expectedTrillianAddr), + "Config should contain correct Trillian address") + }) + + It("should have CTLog deployment updated", func(ctx SpecContext) { + Eventually(condition.DeploymentIsRunning). + WithContext(ctx). + WithArguments(cli, namespace.Name, ctlogActions.ComponentName). + Should(BeTrue(), "CTLog deployment should be running") + }) + + It("should have CTLog pod running (not stuck)", func(ctx SpecContext) { + Eventually(func(g Gomega) bool { + pod := ctlog.GetServerPod(ctx, cli, namespace.Name) + g.Expect(pod).NotTo(BeNil()) + return pod.Status.Phase == v1.PodRunning + }).Should(BeTrue(), + "CTLog pod should be Running, not stuck in ContainerCreating") + }) + + It("should have CTLog status Ready", func(ctx SpecContext) { + Eventually(func(g Gomega) string { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + readyCond := meta.FindStatusCondition(c.Status.Conditions, constants.Ready) + if readyCond == nil { + return "" + } + return readyCond.Reason + }).Should(Equal(constants.Ready), + "CTLog should be Ready after secret recreation") + }) + + It("should preserve TreeID after secret recreation", func(ctx SpecContext) { + // Note: In Phase 1, TreeID is preserved because it's in the CR status + // and the CR was not deleted. This test verifies TreeID remains stable. + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + Expect(c).NotTo(BeNil()) + Expect(c.Status.TreeID).NotTo(BeNil()) + GinkgoWriter.Printf("TreeID after recovery: %d\n", *c.Status.TreeID) + }) + + // Cleanup + AfterAll(func(ctx SpecContext) { + if ctlogCR != nil { + _ = cli.Delete(ctx, ctlogCR) + } + }) + }) +}) diff --git a/test/e2e/support/tas/ctlog/ctlog.go b/test/e2e/support/tas/ctlog/ctlog.go index 53673f4e2..54c0354bf 100644 --- a/test/e2e/support/tas/ctlog/ctlog.go +++ b/test/e2e/support/tas/ctlog/ctlog.go @@ -66,3 +66,33 @@ func CreateSecret(ns string, name string) *v1.Secret { }, } } + +// GetConfigSecret retrieves the ctlog-config secret by name +func GetConfigSecret(ctx context.Context, cli client.Client, namespace string, secretName string) (*v1.Secret, error) { + secret := &v1.Secret{} + err := cli.Get(ctx, client.ObjectKey{ + Namespace: namespace, + Name: secretName, + }, secret) + return secret, err +} + +// DeleteConfigSecret deletes a config secret +func DeleteConfigSecret(ctx context.Context, cli client.Client, namespace string, secretName string) error { + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: namespace, + }, + } + return cli.Delete(ctx, secret) +} + +// GetTreeIDFromStatus retrieves TreeID from CTLog status +func GetTreeIDFromStatus(ctx context.Context, cli client.Client, namespace string, name string) *int64 { + ctlog := Get(ctx, cli, namespace, name) + if ctlog == nil { + return nil + } + return ctlog.Status.TreeID +} From 324a518ce9bbf491c5032154b6663b62bdac81ce Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Sat, 25 Oct 2025 00:11:30 +0200 Subject: [PATCH 02/15] Checking correct trillian addres in secret config --- test/e2e/ctlog_recovery_test.go | 48 ++++++++++++++++------------- test/e2e/support/tas/ctlog/ctlog.go | 16 ++++++++++ 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go index cd2ec285a..4c802cd4c 100644 --- a/test/e2e/ctlog_recovery_test.go +++ b/test/e2e/ctlog_recovery_test.go @@ -4,6 +4,7 @@ package e2e import ( "fmt" + "strings" "time" . "github.com/onsi/ginkgo/v2" @@ -52,8 +53,9 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { trillian.Verify(ctx, cli, namespace.Name, trillianCR.Name, true) }) - Describe("Test 1: Secret validation - missing secret (Issue 2586)", func() { + Describe("Test 1: Secret validation - missing/invalid config (Issues 2586 & 3114)", func() { var originalSecretName string + var correctTrillianAddr string BeforeAll(func(ctx SpecContext) { // Create keys secret for CTLog @@ -76,6 +78,8 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { } Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) + correctTrillianAddr = fmt.Sprintf("trillian-logserver.%s.svc.cluster.local:8091", namespace.Name) + // Create CTLog CR with keys and root cert ctlogCR = &v1alpha1.CTlog{ ObjectMeta: metav1.ObjectMeta{ @@ -116,27 +120,25 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) }) - It("should have a config secret reference in status", func(ctx SpecContext) { - Eventually(func(g Gomega) string { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - g.Expect(c).NotTo(BeNil()) - g.Expect(c.Status.ServerConfigRef).NotTo(BeNil()) - g.Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) - return c.Status.ServerConfigRef.Name - }).Should(Not(BeEmpty())) - - // Store the original secret name + It("should have a config secret with correct Trillian address", func(ctx SpecContext) { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + Expect(c).NotTo(BeNil()) + Expect(c.Status.ServerConfigRef).NotTo(BeNil()) + Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) + originalSecretName = c.Status.ServerConfigRef.Name GinkgoWriter.Printf("Original config secret name: %s\n", originalSecretName) - }) - It("should have the config secret exist in the cluster", func(ctx SpecContext) { + // Verify secret exists with correct Trillian configuration secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) Expect(err).NotTo(HaveOccurred()) Expect(secret).NotTo(BeNil()) Expect(secret.Data).To(HaveKey("config")) - GinkgoWriter.Printf("Config secret exists with %d bytes of config data\n", len(secret.Data["config"])) + + configContent := ctlog.GetTrillianAddressFromSecret(secret) + Expect(configContent).To(ContainSubstring(correctTrillianAddr), + "Config should contain correct Trillian address") + GinkgoWriter.Printf("Config secret has correct Trillian address: %s\n", correctTrillianAddr) }) It("should delete the config secret to simulate cluster recreation", func(ctx SpecContext) { @@ -163,12 +165,12 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { }).WithTimeout(30 * time.Second).Should(Succeed()) }) - It("should detect missing secret and recreate it (Phase 1 validation)", func(ctx SpecContext) { - By("Waiting for operator to detect missing secret") + It("should detect missing/invalid config and recreate it (Phase 1 validation)", func(ctx SpecContext) { + By("Waiting for operator to detect missing/invalid config") // The CTLog should go into a non-Ready state first (detecting the issue) - // Then it should recover by recreating the secret - // This needs extra time as it waits for reconciliation loop + // Then it should recover by recreating the secret with correct Trillian config + // This tests both Issue 2586 (missing secret) and Issue 3114 (wrong namespace) Eventually(func(g Gomega) bool { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) g.Expect(c).NotTo(BeNil()) @@ -178,13 +180,17 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { // Try to get the secret secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, c.Status.ServerConfigRef.Name) if err == nil && secret != nil { - GinkgoWriter.Printf("Config secret recreated: %s\n", c.Status.ServerConfigRef.Name) - return true + // Verify it has correct Trillian address + configContent := ctlog.GetTrillianAddressFromSecret(secret) + if strings.Contains(configContent, correctTrillianAddr) { + GinkgoWriter.Printf("Config secret recreated with correct Trillian address: %s\n", c.Status.ServerConfigRef.Name) + return true + } } } return false }).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), - "Operator should detect missing secret and recreate it") + "Operator should detect missing/invalid config and recreate it with correct Trillian address") }) It("should have a valid config secret after recreation", func(ctx SpecContext) { diff --git a/test/e2e/support/tas/ctlog/ctlog.go b/test/e2e/support/tas/ctlog/ctlog.go index 54c0354bf..1103c073f 100644 --- a/test/e2e/support/tas/ctlog/ctlog.go +++ b/test/e2e/support/tas/ctlog/ctlog.go @@ -96,3 +96,19 @@ func GetTreeIDFromStatus(ctx context.Context, cli client.Client, namespace strin } return ctlog.Status.TreeID } + +// GetTrillianAddressFromSecret extracts Trillian address from config secret +func GetTrillianAddressFromSecret(secret *v1.Secret) string { + if secret == nil { + return "" + } + configData, ok := secret.Data["config"] + if !ok { + return "" + } + // Simple extraction - look for backend_spec pattern + // In protobuf text format: backend_spec: "address:port" + config := string(configData) + // Return config for substring matching in tests + return config +} From beeb63131277e57c2eaa2c7048b05d2ea87e4583 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 29 Oct 2025 20:59:24 +0100 Subject: [PATCH 03/15] Added treeID recovery test --- test/e2e/ctlog_recovery_test.go | 256 ++++++++++++++++++++++++++++ test/e2e/support/tas/ctlog/ctlog.go | 66 +++++++ 2 files changed, 322 insertions(+) diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go index 4c802cd4c..90e7a1e06 100644 --- a/test/e2e/ctlog_recovery_test.go +++ b/test/e2e/ctlog_recovery_test.go @@ -258,4 +258,260 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { } }) }) + + Describe("Test 2: TreeID recovery after CR deletion (Phase 2)", func() { + var originalTreeID *int64 + var originalSecretName string + var keysSecret *v1.Secret + var rootCertSecret *v1.Secret + + BeforeAll(func(ctx SpecContext) { + // Create keys secret for CTLog + By("Creating CTLog keys secret") + keysSecret = ctlog.CreateSecret(namespace.Name, "test-ctlog-keys-phase2") + Expect(cli.Create(ctx, keysSecret)).To(Succeed()) + + // Create a proper root certificate secret + By("Creating root certificate") + _, _, rootCert, err := support.CreateCertificates(false) + Expect(err).NotTo(HaveOccurred()) + rootCertSecret = &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-root-cert-phase2", + Namespace: namespace.Name, + }, + Data: map[string][]byte{ + "cert": rootCert, + }, + } + Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) + + // Create CTLog CR (Phase 2: without owner reference on config secret) + ctlogCR = &v1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-phase2", + Namespace: namespace.Name, + }, + Spec: v1alpha1.CTlogSpec{ + Trillian: v1alpha1.TrillianService{ + Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), + Port: ptr.To(int32(8091)), + }, + PrivateKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys-phase2", + }, + Key: "private", + }, + PublicKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys-phase2", + }, + Key: "public", + }, + RootCertificates: []v1alpha1.SecretKeySelector{ + { + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-root-cert-phase2", + }, + Key: "cert", + }, + }, + }, + } + Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) + + By("Waiting for CTLog to be ready initially") + ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) + }) + + It("should record original TreeID and config secret", func(ctx SpecContext) { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + Expect(c).NotTo(BeNil()) + Expect(c.Status.TreeID).NotTo(BeNil()) + Expect(c.Status.ServerConfigRef).NotTo(BeNil()) + Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) + + originalTreeID = c.Status.TreeID + originalSecretName = c.Status.ServerConfigRef.Name + + GinkgoWriter.Printf("Original TreeID: %d\n", *originalTreeID) + GinkgoWriter.Printf("Original config secret: %s\n", originalSecretName) + }) + + It("should have config secret with NO owner reference (Phase 2)", func(ctx SpecContext) { + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) + Expect(err).NotTo(HaveOccurred()) + Expect(secret).NotTo(BeNil()) + + // Phase 2 Fix: Secret should have NO owner reference + // This allows it to survive CR deletion + hasNoOwnerRef := ctlog.VerifySecretHasNoOwnerReference(secret) + Expect(hasNoOwnerRef).To(BeTrue(), + "Config secret should have NO owner reference in Phase 2 (allows state recovery)") + + GinkgoWriter.Printf("✓ Config secret has NO owner reference (Phase 2 fix)\n") + }) + + It("should have TreeID embedded in config secret", func(ctx SpecContext) { + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) + Expect(err).NotTo(HaveOccurred()) + Expect(secret).NotTo(BeNil()) + + treeIDFromSecret := ctlog.GetTreeIDFromConfigSecret(secret) + Expect(treeIDFromSecret).NotTo(BeNil(), "TreeID should be in config secret") + Expect(*treeIDFromSecret).To(Equal(*originalTreeID), + "TreeID in secret should match TreeID in status") + + GinkgoWriter.Printf("✓ TreeID in config secret: %d (matches status)\n", *treeIDFromSecret) + }) + + It("should delete CTLog CR (simulating disaster recovery)", func(ctx SpecContext) { + By("Deleting CTLog CR") + Expect(cli.Delete(ctx, ctlogCR)).To(Succeed()) + + By("Waiting for CTLog CR to be gone") + Eventually(func(g Gomega) bool { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + return c == nil + }).WithTimeout(2 * time.Minute).Should(BeTrue()) + + GinkgoWriter.Printf("✓ CTLog CR deleted\n") + }) + + It("should verify config secret STILL EXISTS (Phase 2 - no garbage collection)", func(ctx SpecContext) { + // This is the key Phase 2 behavior: + // Without owner reference, K8s does NOT garbage collect the secret + secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) + Expect(err).NotTo(HaveOccurred(), "Secret should still exist after CR deletion") + Expect(secret).NotTo(BeNil()) + Expect(secret.Name).To(Equal(originalSecretName)) + + GinkgoWriter.Printf("✓ Config secret survived CR deletion: %s\n", originalSecretName) + }) + + It("should recreate CTLog CR with same name", func(ctx SpecContext) { + By("Recreating CTLog CR") + ctlogCR = &v1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-phase2", + Namespace: namespace.Name, + }, + Spec: v1alpha1.CTlogSpec{ + Trillian: v1alpha1.TrillianService{ + Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), + Port: ptr.To(int32(8091)), + }, + PrivateKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys-phase2", + }, + Key: "private", + }, + PublicKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys-phase2", + }, + Key: "public", + }, + RootCertificates: []v1alpha1.SecretKeySelector{ + { + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-root-cert-phase2", + }, + Key: "cert", + }, + }, + }, + } + Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) + + GinkgoWriter.Printf("✓ CTLog CR recreated\n") + }) + + It("should discover existing config secret (Phase 2)", func(ctx SpecContext) { + // Operator should discover the existing secret either: + // 1. Via Status.ServerConfigRef (empty on new CR) + // 2. Via label-based discovery + + Eventually(func(g Gomega) bool { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + + // Check if operator has discovered and referenced the secret + if c.Status.ServerConfigRef != nil { + // Should reference the original secret (discovered) + return c.Status.ServerConfigRef.Name == originalSecretName + } + return false + }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), + "Operator should discover existing config secret") + + GinkgoWriter.Printf("✓ Operator discovered existing secret: %s\n", originalSecretName) + }) + + It("should restore TreeID from config secret (Phase 2)", func(ctx SpecContext) { + // This is the critical Phase 2 behavior: + // TreeID should be extracted from the discovered secret, not regenerated + + Eventually(func(g Gomega) bool { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + + if c.Status.TreeID != nil { + return *c.Status.TreeID == *originalTreeID + } + return false + }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), + "TreeID should be restored from config secret, not regenerated") + + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + GinkgoWriter.Printf("✓ TreeID restored: %d (original: %d)\n", *c.Status.TreeID, *originalTreeID) + }) + + It("should have CTLog Ready with restored state", func(ctx SpecContext) { + Eventually(func(g Gomega) string { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + g.Expect(c).NotTo(BeNil()) + readyCond := meta.FindStatusCondition(c.Status.Conditions, constants.Ready) + if readyCond == nil { + return "" + } + return readyCond.Reason + }).WithTimeout(3*time.Minute).Should(Equal(constants.Ready), + "CTLog should be Ready after state recovery") + + GinkgoWriter.Printf("✓ CTLog Ready with original TreeID (full state recovery)\n") + }) + + It("should verify no new TreeID was generated", func(ctx SpecContext) { + c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) + Expect(c).NotTo(BeNil()) + Expect(c.Status.TreeID).NotTo(BeNil()) + Expect(*c.Status.TreeID).To(Equal(*originalTreeID), + "TreeID should be the same as original (no regeneration)") + + GinkgoWriter.Printf("✓ Verified: Same TreeID preserved across CR deletion/recreation\n") + GinkgoWriter.Printf(" Original TreeID: %d\n", *originalTreeID) + GinkgoWriter.Printf(" Restored TreeID: %d\n", *c.Status.TreeID) + GinkgoWriter.Printf(" Result: Old signatures remain verifiable! ✓\n") + }) + + // Cleanup + AfterAll(func(ctx SpecContext) { + if ctlogCR != nil { + _ = cli.Delete(ctx, ctlogCR) + } + if keysSecret != nil { + _ = cli.Delete(ctx, keysSecret) + } + if rootCertSecret != nil { + _ = cli.Delete(ctx, rootCertSecret) + } + // Clean up the config secret (not garbage collected in Phase 2) + if originalSecretName != "" { + _ = ctlog.DeleteConfigSecret(ctx, cli, namespace.Name, originalSecretName) + } + }) + }) }) diff --git a/test/e2e/support/tas/ctlog/ctlog.go b/test/e2e/support/tas/ctlog/ctlog.go index 1103c073f..9e1aaafc7 100644 --- a/test/e2e/support/tas/ctlog/ctlog.go +++ b/test/e2e/support/tas/ctlog/ctlog.go @@ -2,6 +2,7 @@ package ctlog import ( "context" + "fmt" . "github.com/onsi/gomega" "github.com/securesign/operator/api/v1alpha1" @@ -112,3 +113,68 @@ func GetTrillianAddressFromSecret(secret *v1.Secret) string { // Return config for substring matching in tests return config } + +// GetTreeIDFromConfigSecret extracts TreeID from config secret +// TreeID is embedded in the protobuf config as "log_id: " +func GetTreeIDFromConfigSecret(secret *v1.Secret) *int64 { + if secret == nil { + return nil + } + configData, ok := secret.Data["config"] + if !ok { + return nil + } + // Parse protobuf text format to extract log_id + // Format: log_id: 12345 (can be indented) + config := string(configData) + + // Split by lines and look for "log_id:" pattern + for _, line := range splitLines(config) { + line = trimSpace(line) + if hasPrefix(line, "log_id:") { + var treeID int64 + if n, _ := fmt.Sscanf(line, "log_id: %d", &treeID); n == 1 { + return &treeID + } + } + } + return nil +} + +// Helper functions for string parsing +func splitLines(s string) []string { + var lines []string + start := 0 + for i, c := range s { + if c == '\n' { + lines = append(lines, s[start:i]) + start = i + 1 + } + } + if start < len(s) { + lines = append(lines, s[start:]) + } + return lines +} + +func trimSpace(s string) string { + // Simple trim implementation + start := 0 + for start < len(s) && (s[start] == ' ' || s[start] == '\t') { + start++ + } + end := len(s) + for end > start && (s[end-1] == ' ' || s[end-1] == '\t' || s[end-1] == '\r') { + end-- + } + return s[start:end] +} + +func hasPrefix(s, prefix string) bool { + return len(s) >= len(prefix) && s[:len(prefix)] == prefix +} + +// VerifySecretHasNoOwnerReference checks if secret has no owner references +func VerifySecretHasNoOwnerReference(secret *v1.Secret) bool { + return secret != nil && len(secret.OwnerReferences) == 0 +} From 456b7f3f31d0c4d3841b9f7dcf34b49094d36041 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 31 Oct 2025 16:03:45 +0100 Subject: [PATCH 04/15] Recover from missing or invalid ctlog secret --- .../controller/ctlog/actions/server_config.go | 64 +++++++++++++++++- .../controller/ctlog/utils/ctlog_config.go | 66 +++++++++++++++++++ test/e2e/ctlog_recovery_test.go | 32 +-------- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 5c0fb35d4..63afd70b9 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -17,6 +17,7 @@ import ( "github.com/securesign/operator/internal/utils/kubernetes/ensure" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" labels2 "k8s.io/apimachinery/pkg/labels" @@ -52,7 +53,8 @@ func (i serverConfig) CanHandle(_ context.Context, instance *rhtasv1alpha1.CTlog case instance.Spec.ServerConfigRef != nil: return !equality.Semantic.DeepEqual(instance.Spec.ServerConfigRef, instance.Status.ServerConfigRef) default: - return instance.Generation != c.ObservedGeneration + // Always run Handle() to validate the secret: exists and is valid + return true } } @@ -62,6 +64,29 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) ) if instance.Spec.ServerConfigRef != nil { + // Validate that the custom secret actually exists + secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Spec.ServerConfigRef.Name) + if err != nil { + return i.Error(ctx, fmt.Errorf("custom server config secret not found: %w", err), instance, + metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: constants.Failure, + Message: fmt.Sprintf("Custom server config secret not found: %s", instance.Spec.ServerConfigRef.Name), + ObservedGeneration: instance.Generation, + }) + } + if secret.Data == nil || secret.Data["config"] == nil { + return i.Error(ctx, fmt.Errorf("custom server config secret is invalid"), instance, + metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: constants.Failure, + Message: fmt.Sprintf("Custom server config secret is missing 'config' key: %s", instance.Spec.ServerConfigRef.Name), + ObservedGeneration: instance.Generation, + }) + } + instance.Status.ServerConfigRef = instance.Spec.ServerConfigRef i.Recorder.Event(instance, corev1.EventTypeNormal, "CTLogConfigUpdated", "CTLog config updated") meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ @@ -74,6 +99,43 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) return i.StatusUpdate(ctx, instance) } + // Validate existing secret before attempting recreation + if instance.Status.ServerConfigRef != nil && instance.Status.ServerConfigRef.Name != "" { + secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) + + if err != nil { + if apierrors.IsNotFound(err) { + i.Logger.Info("Server config secret is missing, will recreate", + "secret", instance.Status.ServerConfigRef.Name) + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigMissing", + fmt.Sprintf("Config secret is missing, will recreate: %s", instance.Status.ServerConfigRef.Name)) + } else { + i.Logger.Error(err, "Error accessing server config secret, will attempt to recreate", + "secret", instance.Status.ServerConfigRef.Name) + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigError", + fmt.Sprintf("Error accessing config secret, will recreate: %s", instance.Status.ServerConfigRef.Name)) + } + } else if secret == nil { + // Edge case: no error but secret is nil + i.Logger.Info("Server config secret is nil, will recreate", + "secret", instance.Status.ServerConfigRef.Name) + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigMissing", + fmt.Sprintf("Config secret is nil, will recreate: %s", instance.Status.ServerConfigRef.Name)) + } else { + // Secret exists and is accessible - validate it + expectedTrillianAddr := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) + if ctlogUtils.IsSecretDataValid(secret.Data, expectedTrillianAddr) { + return i.Continue() // nothing to do + } + // Secret is invalid, will recreate + i.Logger.Info("Server config secret is invalid, will recreate", + "secret", secret.Name, + "reason", "Trillian configuration mismatch") + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigInvalid", + fmt.Sprintf("Config secret has invalid configuration, will recreate: %s", secret.Name)) + } + } + switch { case instance.Status.TreeID == nil: return i.Error(ctx, fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrTreeNotSpecified), instance) diff --git a/internal/controller/ctlog/utils/ctlog_config.go b/internal/controller/ctlog/utils/ctlog_config.go index 998729a82..fa4938e05 100644 --- a/internal/controller/ctlog/utils/ctlog_config.go +++ b/internal/controller/ctlog/utils/ctlog_config.go @@ -200,3 +200,69 @@ func CreateCtlogConfig(trillianUrl string, treeID int64, rootCerts []RootCertifi } return data, nil } + +// IsSecretDataValid validates that the config secret data contains the correct Trillian configuration +func IsSecretDataValid(secretData map[string][]byte, expectedTrillianAddr string) bool { + if secretData == nil { + return false + } + + configData, ok := secretData["config"] + if !ok { + return false + } + + configString := string(configData) + if len(configString) == 0 { + return false + } + + for _, line := range splitConfigLines(configString) { + if containsSubstring(line, "backend_spec") && containsSubstring(line, expectedTrillianAddr) { + return true + } + } + + return false +} + +func splitConfigLines(config string) []string { + lines := make([]string, 0) + current := "" + for i := 0; i < len(config); i++ { + if config[i] == '\n' { + if len(current) > 0 { + lines = append(lines, current) + } + current = "" + } else { + current += string(config[i]) + } + } + if len(current) > 0 { + lines = append(lines, current) + } + return lines +} + +func containsSubstring(haystack, needle string) bool { + if len(needle) == 0 { + return true + } + if len(needle) > len(haystack) { + return false + } + for i := 0; i <= len(haystack)-len(needle); i++ { + match := true + for j := 0; j < len(needle); j++ { + if haystack[i+j] != needle[j] { + match = false + break + } + } + if match { + return true + } + } + return false +} diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go index 90e7a1e06..697faa521 100644 --- a/test/e2e/ctlog_recovery_test.go +++ b/test/e2e/ctlog_recovery_test.go @@ -127,7 +127,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) originalSecretName = c.Status.ServerConfigRef.Name - GinkgoWriter.Printf("Original config secret name: %s\n", originalSecretName) // Verify secret exists with correct Trillian configuration secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) @@ -138,7 +137,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { configContent := ctlog.GetTrillianAddressFromSecret(secret) Expect(configContent).To(ContainSubstring(correctTrillianAddr), "Config should contain correct Trillian address") - GinkgoWriter.Printf("Config secret has correct Trillian address: %s\n", correctTrillianAddr) }) It("should delete the config secret to simulate cluster recreation", func(ctx SpecContext) { @@ -183,13 +181,12 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { // Verify it has correct Trillian address configContent := ctlog.GetTrillianAddressFromSecret(secret) if strings.Contains(configContent, correctTrillianAddr) { - GinkgoWriter.Printf("Config secret recreated with correct Trillian address: %s\n", c.Status.ServerConfigRef.Name) return true } } } return false - }).WithTimeout(5*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), + }).WithPolling(5*time.Second).Should(BeTrue(), "Operator should detect missing/invalid config and recreate it with correct Trillian address") }) @@ -199,7 +196,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { Expect(c.Status.ServerConfigRef).NotTo(BeNil()) newSecretName := c.Status.ServerConfigRef.Name - GinkgoWriter.Printf("New config secret name: %s\n", newSecretName) secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, newSecretName) Expect(err).NotTo(HaveOccurred()) @@ -248,7 +244,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) Expect(c).NotTo(BeNil()) Expect(c.Status.TreeID).NotTo(BeNil()) - GinkgoWriter.Printf("TreeID after recovery: %d\n", *c.Status.TreeID) }) // Cleanup @@ -334,9 +329,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { originalTreeID = c.Status.TreeID originalSecretName = c.Status.ServerConfigRef.Name - - GinkgoWriter.Printf("Original TreeID: %d\n", *originalTreeID) - GinkgoWriter.Printf("Original config secret: %s\n", originalSecretName) }) It("should have config secret with NO owner reference (Phase 2)", func(ctx SpecContext) { @@ -349,8 +341,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { hasNoOwnerRef := ctlog.VerifySecretHasNoOwnerReference(secret) Expect(hasNoOwnerRef).To(BeTrue(), "Config secret should have NO owner reference in Phase 2 (allows state recovery)") - - GinkgoWriter.Printf("✓ Config secret has NO owner reference (Phase 2 fix)\n") }) It("should have TreeID embedded in config secret", func(ctx SpecContext) { @@ -362,8 +352,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { Expect(treeIDFromSecret).NotTo(BeNil(), "TreeID should be in config secret") Expect(*treeIDFromSecret).To(Equal(*originalTreeID), "TreeID in secret should match TreeID in status") - - GinkgoWriter.Printf("✓ TreeID in config secret: %d (matches status)\n", *treeIDFromSecret) }) It("should delete CTLog CR (simulating disaster recovery)", func(ctx SpecContext) { @@ -375,8 +363,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) return c == nil }).WithTimeout(2 * time.Minute).Should(BeTrue()) - - GinkgoWriter.Printf("✓ CTLog CR deleted\n") }) It("should verify config secret STILL EXISTS (Phase 2 - no garbage collection)", func(ctx SpecContext) { @@ -386,8 +372,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { Expect(err).NotTo(HaveOccurred(), "Secret should still exist after CR deletion") Expect(secret).NotTo(BeNil()) Expect(secret.Name).To(Equal(originalSecretName)) - - GinkgoWriter.Printf("✓ Config secret survived CR deletion: %s\n", originalSecretName) }) It("should recreate CTLog CR with same name", func(ctx SpecContext) { @@ -425,8 +409,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { }, } Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) - - GinkgoWriter.Printf("✓ CTLog CR recreated\n") }) It("should discover existing config secret (Phase 2)", func(ctx SpecContext) { @@ -446,8 +428,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { return false }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), "Operator should discover existing config secret") - - GinkgoWriter.Printf("✓ Operator discovered existing secret: %s\n", originalSecretName) }) It("should restore TreeID from config secret (Phase 2)", func(ctx SpecContext) { @@ -464,9 +444,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { return false }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), "TreeID should be restored from config secret, not regenerated") - - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - GinkgoWriter.Printf("✓ TreeID restored: %d (original: %d)\n", *c.Status.TreeID, *originalTreeID) }) It("should have CTLog Ready with restored state", func(ctx SpecContext) { @@ -480,8 +457,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { return readyCond.Reason }).WithTimeout(3*time.Minute).Should(Equal(constants.Ready), "CTLog should be Ready after state recovery") - - GinkgoWriter.Printf("✓ CTLog Ready with original TreeID (full state recovery)\n") }) It("should verify no new TreeID was generated", func(ctx SpecContext) { @@ -490,11 +465,6 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { Expect(c.Status.TreeID).NotTo(BeNil()) Expect(*c.Status.TreeID).To(Equal(*originalTreeID), "TreeID should be the same as original (no regeneration)") - - GinkgoWriter.Printf("✓ Verified: Same TreeID preserved across CR deletion/recreation\n") - GinkgoWriter.Printf(" Original TreeID: %d\n", *originalTreeID) - GinkgoWriter.Printf(" Restored TreeID: %d\n", *c.Status.TreeID) - GinkgoWriter.Printf(" Result: Old signatures remain verifiable! ✓\n") }) // Cleanup From 0520fcda23907613bab7d9624273049b718c461f Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 31 Oct 2025 19:03:17 +0100 Subject: [PATCH 05/15] Remove unnecessary phase 2 tests - ctlog CR is only source of truth --- test/e2e/ctlog_recovery_test.go | 231 ---------------------------- test/e2e/support/tas/ctlog/ctlog.go | 75 --------- 2 files changed, 306 deletions(-) diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go index 697faa521..5c439f948 100644 --- a/test/e2e/ctlog_recovery_test.go +++ b/test/e2e/ctlog_recovery_test.go @@ -253,235 +253,4 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { } }) }) - - Describe("Test 2: TreeID recovery after CR deletion (Phase 2)", func() { - var originalTreeID *int64 - var originalSecretName string - var keysSecret *v1.Secret - var rootCertSecret *v1.Secret - - BeforeAll(func(ctx SpecContext) { - // Create keys secret for CTLog - By("Creating CTLog keys secret") - keysSecret = ctlog.CreateSecret(namespace.Name, "test-ctlog-keys-phase2") - Expect(cli.Create(ctx, keysSecret)).To(Succeed()) - - // Create a proper root certificate secret - By("Creating root certificate") - _, _, rootCert, err := support.CreateCertificates(false) - Expect(err).NotTo(HaveOccurred()) - rootCertSecret = &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-root-cert-phase2", - Namespace: namespace.Name, - }, - Data: map[string][]byte{ - "cert": rootCert, - }, - } - Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) - - // Create CTLog CR (Phase 2: without owner reference on config secret) - ctlogCR = &v1alpha1.CTlog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-phase2", - Namespace: namespace.Name, - }, - Spec: v1alpha1.CTlogSpec{ - Trillian: v1alpha1.TrillianService{ - Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), - Port: ptr.To(int32(8091)), - }, - PrivateKeyRef: &v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys-phase2", - }, - Key: "private", - }, - PublicKeyRef: &v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys-phase2", - }, - Key: "public", - }, - RootCertificates: []v1alpha1.SecretKeySelector{ - { - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-root-cert-phase2", - }, - Key: "cert", - }, - }, - }, - } - Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) - - By("Waiting for CTLog to be ready initially") - ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) - }) - - It("should record original TreeID and config secret", func(ctx SpecContext) { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - Expect(c).NotTo(BeNil()) - Expect(c.Status.TreeID).NotTo(BeNil()) - Expect(c.Status.ServerConfigRef).NotTo(BeNil()) - Expect(c.Status.ServerConfigRef.Name).NotTo(BeEmpty()) - - originalTreeID = c.Status.TreeID - originalSecretName = c.Status.ServerConfigRef.Name - }) - - It("should have config secret with NO owner reference (Phase 2)", func(ctx SpecContext) { - secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) - Expect(err).NotTo(HaveOccurred()) - Expect(secret).NotTo(BeNil()) - - // Phase 2 Fix: Secret should have NO owner reference - // This allows it to survive CR deletion - hasNoOwnerRef := ctlog.VerifySecretHasNoOwnerReference(secret) - Expect(hasNoOwnerRef).To(BeTrue(), - "Config secret should have NO owner reference in Phase 2 (allows state recovery)") - }) - - It("should have TreeID embedded in config secret", func(ctx SpecContext) { - secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) - Expect(err).NotTo(HaveOccurred()) - Expect(secret).NotTo(BeNil()) - - treeIDFromSecret := ctlog.GetTreeIDFromConfigSecret(secret) - Expect(treeIDFromSecret).NotTo(BeNil(), "TreeID should be in config secret") - Expect(*treeIDFromSecret).To(Equal(*originalTreeID), - "TreeID in secret should match TreeID in status") - }) - - It("should delete CTLog CR (simulating disaster recovery)", func(ctx SpecContext) { - By("Deleting CTLog CR") - Expect(cli.Delete(ctx, ctlogCR)).To(Succeed()) - - By("Waiting for CTLog CR to be gone") - Eventually(func(g Gomega) bool { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - return c == nil - }).WithTimeout(2 * time.Minute).Should(BeTrue()) - }) - - It("should verify config secret STILL EXISTS (Phase 2 - no garbage collection)", func(ctx SpecContext) { - // This is the key Phase 2 behavior: - // Without owner reference, K8s does NOT garbage collect the secret - secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) - Expect(err).NotTo(HaveOccurred(), "Secret should still exist after CR deletion") - Expect(secret).NotTo(BeNil()) - Expect(secret.Name).To(Equal(originalSecretName)) - }) - - It("should recreate CTLog CR with same name", func(ctx SpecContext) { - By("Recreating CTLog CR") - ctlogCR = &v1alpha1.CTlog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-phase2", - Namespace: namespace.Name, - }, - Spec: v1alpha1.CTlogSpec{ - Trillian: v1alpha1.TrillianService{ - Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), - Port: ptr.To(int32(8091)), - }, - PrivateKeyRef: &v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys-phase2", - }, - Key: "private", - }, - PublicKeyRef: &v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys-phase2", - }, - Key: "public", - }, - RootCertificates: []v1alpha1.SecretKeySelector{ - { - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-root-cert-phase2", - }, - Key: "cert", - }, - }, - }, - } - Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) - }) - - It("should discover existing config secret (Phase 2)", func(ctx SpecContext) { - // Operator should discover the existing secret either: - // 1. Via Status.ServerConfigRef (empty on new CR) - // 2. Via label-based discovery - - Eventually(func(g Gomega) bool { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - g.Expect(c).NotTo(BeNil()) - - // Check if operator has discovered and referenced the secret - if c.Status.ServerConfigRef != nil { - // Should reference the original secret (discovered) - return c.Status.ServerConfigRef.Name == originalSecretName - } - return false - }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), - "Operator should discover existing config secret") - }) - - It("should restore TreeID from config secret (Phase 2)", func(ctx SpecContext) { - // This is the critical Phase 2 behavior: - // TreeID should be extracted from the discovered secret, not regenerated - - Eventually(func(g Gomega) bool { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - g.Expect(c).NotTo(BeNil()) - - if c.Status.TreeID != nil { - return *c.Status.TreeID == *originalTreeID - } - return false - }).WithTimeout(3*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), - "TreeID should be restored from config secret, not regenerated") - }) - - It("should have CTLog Ready with restored state", func(ctx SpecContext) { - Eventually(func(g Gomega) string { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - g.Expect(c).NotTo(BeNil()) - readyCond := meta.FindStatusCondition(c.Status.Conditions, constants.Ready) - if readyCond == nil { - return "" - } - return readyCond.Reason - }).WithTimeout(3*time.Minute).Should(Equal(constants.Ready), - "CTLog should be Ready after state recovery") - }) - - It("should verify no new TreeID was generated", func(ctx SpecContext) { - c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) - Expect(c).NotTo(BeNil()) - Expect(c.Status.TreeID).NotTo(BeNil()) - Expect(*c.Status.TreeID).To(Equal(*originalTreeID), - "TreeID should be the same as original (no regeneration)") - }) - - // Cleanup - AfterAll(func(ctx SpecContext) { - if ctlogCR != nil { - _ = cli.Delete(ctx, ctlogCR) - } - if keysSecret != nil { - _ = cli.Delete(ctx, keysSecret) - } - if rootCertSecret != nil { - _ = cli.Delete(ctx, rootCertSecret) - } - // Clean up the config secret (not garbage collected in Phase 2) - if originalSecretName != "" { - _ = ctlog.DeleteConfigSecret(ctx, cli, namespace.Name, originalSecretName) - } - }) - }) }) diff --git a/test/e2e/support/tas/ctlog/ctlog.go b/test/e2e/support/tas/ctlog/ctlog.go index 9e1aaafc7..8c131d6c2 100644 --- a/test/e2e/support/tas/ctlog/ctlog.go +++ b/test/e2e/support/tas/ctlog/ctlog.go @@ -2,7 +2,6 @@ package ctlog import ( "context" - "fmt" . "github.com/onsi/gomega" "github.com/securesign/operator/api/v1alpha1" @@ -89,15 +88,6 @@ func DeleteConfigSecret(ctx context.Context, cli client.Client, namespace string return cli.Delete(ctx, secret) } -// GetTreeIDFromStatus retrieves TreeID from CTLog status -func GetTreeIDFromStatus(ctx context.Context, cli client.Client, namespace string, name string) *int64 { - ctlog := Get(ctx, cli, namespace, name) - if ctlog == nil { - return nil - } - return ctlog.Status.TreeID -} - // GetTrillianAddressFromSecret extracts Trillian address from config secret func GetTrillianAddressFromSecret(secret *v1.Secret) string { if secret == nil { @@ -113,68 +103,3 @@ func GetTrillianAddressFromSecret(secret *v1.Secret) string { // Return config for substring matching in tests return config } - -// GetTreeIDFromConfigSecret extracts TreeID from config secret -// TreeID is embedded in the protobuf config as "log_id: " -func GetTreeIDFromConfigSecret(secret *v1.Secret) *int64 { - if secret == nil { - return nil - } - configData, ok := secret.Data["config"] - if !ok { - return nil - } - // Parse protobuf text format to extract log_id - // Format: log_id: 12345 (can be indented) - config := string(configData) - - // Split by lines and look for "log_id:" pattern - for _, line := range splitLines(config) { - line = trimSpace(line) - if hasPrefix(line, "log_id:") { - var treeID int64 - if n, _ := fmt.Sscanf(line, "log_id: %d", &treeID); n == 1 { - return &treeID - } - } - } - return nil -} - -// Helper functions for string parsing -func splitLines(s string) []string { - var lines []string - start := 0 - for i, c := range s { - if c == '\n' { - lines = append(lines, s[start:i]) - start = i + 1 - } - } - if start < len(s) { - lines = append(lines, s[start:]) - } - return lines -} - -func trimSpace(s string) string { - // Simple trim implementation - start := 0 - for start < len(s) && (s[start] == ' ' || s[start] == '\t') { - start++ - } - end := len(s) - for end > start && (s[end-1] == ' ' || s[end-1] == '\t' || s[end-1] == '\r') { - end-- - } - return s[start:end] -} - -func hasPrefix(s, prefix string) bool { - return len(s) >= len(prefix) && s[:len(prefix)] == prefix -} - -// VerifySecretHasNoOwnerReference checks if secret has no owner references -func VerifySecretHasNoOwnerReference(secret *v1.Secret) bool { - return secret != nil && len(secret.OwnerReferences) == 0 -} From 3e1cde2fa59a7ffff8634abccb6673e5ab70eca2 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 31 Oct 2025 22:14:57 +0100 Subject: [PATCH 06/15] Code readibility improved --- .../controller/ctlog/actions/server_config.go | 16 +- .../controller/ctlog/utils/ctlog_config.go | 59 ++----- test/e2e/ctlog_recovery_test.go | 151 ++++++++---------- 3 files changed, 90 insertions(+), 136 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 63afd70b9..6f840ef53 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -64,25 +64,25 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) ) if instance.Spec.ServerConfigRef != nil { - // Validate that the custom secret actually exists + // Validate that the custom secret is accessible secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Spec.ServerConfigRef.Name) if err != nil { - return i.Error(ctx, fmt.Errorf("custom server config secret not found: %w", err), instance, + return i.Error(ctx, fmt.Errorf("error accessing custom server config secret: %w", err), instance, metav1.Condition{ Type: ConfigCondition, Status: metav1.ConditionFalse, Reason: constants.Failure, - Message: fmt.Sprintf("Custom server config secret not found: %s", instance.Spec.ServerConfigRef.Name), + Message: fmt.Sprintf("Error accessing custom server config secret: %s", instance.Spec.ServerConfigRef.Name), ObservedGeneration: instance.Generation, }) } - if secret.Data == nil || secret.Data["config"] == nil { + if secret.Data == nil || secret.Data[ctlogUtils.ConfigKey] == nil { return i.Error(ctx, fmt.Errorf("custom server config secret is invalid"), instance, metav1.Condition{ Type: ConfigCondition, Status: metav1.ConditionFalse, Reason: constants.Failure, - Message: fmt.Sprintf("Custom server config secret is missing 'config' key: %s", instance.Spec.ServerConfigRef.Name), + Message: fmt.Sprintf("Custom server config secret is missing '%s' key: %s", ctlogUtils.ConfigKey, instance.Spec.ServerConfigRef.Name), ObservedGeneration: instance.Generation, }) } @@ -115,12 +115,6 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigError", fmt.Sprintf("Error accessing config secret, will recreate: %s", instance.Status.ServerConfigRef.Name)) } - } else if secret == nil { - // Edge case: no error but secret is nil - i.Logger.Info("Server config secret is nil, will recreate", - "secret", instance.Status.ServerConfigRef.Name) - i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigMissing", - fmt.Sprintf("Config secret is nil, will recreate: %s", instance.Status.ServerConfigRef.Name)) } else { // Secret exists and is accessible - validate it expectedTrillianAddr := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) diff --git a/internal/controller/ctlog/utils/ctlog_config.go b/internal/controller/ctlog/utils/ctlog_config.go index fa4938e05..3e35a86e5 100644 --- a/internal/controller/ctlog/utils/ctlog_config.go +++ b/internal/controller/ctlog/utils/ctlog_config.go @@ -7,6 +7,7 @@ import ( "crypto/x509" "encoding/pem" "fmt" + "strings" "github.com/google/certificate-transparency-go/trillian/ctfe/configpb" "github.com/google/trillian/crypto/keyspb" @@ -201,13 +202,22 @@ func CreateCtlogConfig(trillianUrl string, treeID int64, rootCerts []RootCertifi return data, nil } -// IsSecretDataValid validates that the config secret data contains the correct Trillian configuration +// IsSecretDataValid validates that a CTLog config secret contains valid configuration +// with the correct Trillian backend address. +// +// Parameters: +// - secretData: The Data field from a Kubernetes Secret containing CTLog configuration +// - expectedTrillianAddr: The Trillian address to validate against (e.g., "trillian-logserver.namespace.svc:8091") +// +// Returns true if the secret contains valid configuration with the correct Trillian address, +// false otherwise. Used by the operator for self-healing to detect missing or invalid +// configuration secrets that need to be recreated. func IsSecretDataValid(secretData map[string][]byte, expectedTrillianAddr string) bool { if secretData == nil { return false } - configData, ok := secretData["config"] + configData, ok := secretData[ConfigKey] if !ok { return false } @@ -217,52 +227,11 @@ func IsSecretDataValid(secretData map[string][]byte, expectedTrillianAddr string return false } - for _, line := range splitConfigLines(configString) { - if containsSubstring(line, "backend_spec") && containsSubstring(line, expectedTrillianAddr) { + for _, line := range strings.Split(configString, "\n") { + if strings.Contains(line, "backend_spec") && strings.Contains(line, expectedTrillianAddr) { return true } } return false } - -func splitConfigLines(config string) []string { - lines := make([]string, 0) - current := "" - for i := 0; i < len(config); i++ { - if config[i] == '\n' { - if len(current) > 0 { - lines = append(lines, current) - } - current = "" - } else { - current += string(config[i]) - } - } - if len(current) > 0 { - lines = append(lines, current) - } - return lines -} - -func containsSubstring(haystack, needle string) bool { - if len(needle) == 0 { - return true - } - if len(needle) > len(haystack) { - return false - } - for i := 0; i <= len(haystack)-len(needle); i++ { - match := true - for j := 0; j < len(needle); j++ { - if haystack[i+j] != needle[j] { - match = false - break - } - } - if match { - return true - } - } - return false -} diff --git a/test/e2e/ctlog_recovery_test.go b/test/e2e/ctlog_recovery_test.go index 5c439f948..8ae261613 100644 --- a/test/e2e/ctlog_recovery_test.go +++ b/test/e2e/ctlog_recovery_test.go @@ -12,6 +12,7 @@ import ( "github.com/securesign/operator/api/v1alpha1" "github.com/securesign/operator/internal/constants" ctlogActions "github.com/securesign/operator/internal/controller/ctlog/actions" + ctlogUtils "github.com/securesign/operator/internal/controller/ctlog/utils" "github.com/securesign/operator/test/e2e/support" "github.com/securesign/operator/test/e2e/support/condition" "github.com/securesign/operator/test/e2e/support/steps" @@ -30,13 +31,13 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { var namespace *v1.Namespace var trillianCR *v1alpha1.Trillian var ctlogCR *v1alpha1.CTlog + var originalSecretName string + var correctTrillianAddr string - // Shared setup - create namespace BeforeAll(steps.CreateNamespace(cli, func(new *v1.Namespace) { namespace = new })) - // Shared setup - deploy Trillian (needed by CTLog) BeforeAll(func(ctx SpecContext) { trillianCR = &v1alpha1.Trillian{ ObjectMeta: metav1.ObjectMeta{ @@ -53,72 +54,67 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { trillian.Verify(ctx, cli, namespace.Name, trillianCR.Name, true) }) - Describe("Test 1: Secret validation - missing/invalid config (Issues 2586 & 3114)", func() { - var originalSecretName string - var correctTrillianAddr string + BeforeAll(func(ctx SpecContext) { + By("Setting up CTLog prerequisites") - BeforeAll(func(ctx SpecContext) { - // Create keys secret for CTLog - By("Creating CTLog keys secret") - keysSecret := ctlog.CreateSecret(namespace.Name, "test-ctlog-keys") - Expect(cli.Create(ctx, keysSecret)).To(Succeed()) + keysSecret := ctlog.CreateSecret(namespace.Name, "test-ctlog-keys") + Expect(cli.Create(ctx, keysSecret)).To(Succeed()) - // Create a proper root certificate secret (CTLog needs root certs from Fulcio) - By("Creating root certificate") - _, _, rootCert, err := support.CreateCertificates(false) - Expect(err).NotTo(HaveOccurred()) - rootCertSecret := &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-root-cert", - Namespace: namespace.Name, - }, - Data: map[string][]byte{ - "cert": rootCert, - }, - } - Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) + _, _, rootCert, err := support.CreateCertificates(false) + Expect(err).NotTo(HaveOccurred()) + rootCertSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-root-cert", + Namespace: namespace.Name, + }, + Data: map[string][]byte{ + "cert": rootCert, + }, + } + Expect(cli.Create(ctx, rootCertSecret)).To(Succeed()) - correctTrillianAddr = fmt.Sprintf("trillian-logserver.%s.svc.cluster.local:8091", namespace.Name) + correctTrillianAddr = fmt.Sprintf("trillian-logserver.%s.svc.cluster.local:8091", namespace.Name) - // Create CTLog CR with keys and root cert - ctlogCR = &v1alpha1.CTlog{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - Namespace: namespace.Name, + By("Creating CTLog instance") + ctlogCR = &v1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: namespace.Name, + }, + Spec: v1alpha1.CTlogSpec{ + Trillian: v1alpha1.TrillianService{ + Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), + Port: ptr.To(int32(8091)), }, - Spec: v1alpha1.CTlogSpec{ - Trillian: v1alpha1.TrillianService{ - Address: fmt.Sprintf("trillian-logserver.%s.svc.cluster.local", namespace.Name), - Port: ptr.To(int32(8091)), + PrivateKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys", }, - PrivateKeyRef: &v1alpha1.SecretKeySelector{ - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys", - }, - Key: "private", + Key: "private", + }, + PublicKeyRef: &v1alpha1.SecretKeySelector{ + LocalObjectReference: v1alpha1.LocalObjectReference{ + Name: "test-ctlog-keys", }, - PublicKeyRef: &v1alpha1.SecretKeySelector{ + Key: "public", + }, + RootCertificates: []v1alpha1.SecretKeySelector{ + { LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-ctlog-keys", - }, - Key: "public", - }, - RootCertificates: []v1alpha1.SecretKeySelector{ - { - LocalObjectReference: v1alpha1.LocalObjectReference{ - Name: "test-root-cert", - }, - Key: "cert", + Name: "test-root-cert", }, + Key: "cert", }, }, - } - Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) + }, + } + Expect(cli.Create(ctx, ctlogCR)).To(Succeed()) - By("Waiting for CTLog to be ready initially") - // Initial CTLog setup uses default 3 minute timeout for tree creation - ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) - }) + By("Waiting for initial CTLog deployment") + ctlog.Verify(ctx, cli, namespace.Name, ctlogCR.Name) + }) + + Describe("CTLog self-healing when config secret is missing or invalid", func() { It("should have a config secret with correct Trillian address", func(ctx SpecContext) { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) @@ -132,19 +128,19 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) Expect(err).NotTo(HaveOccurred()) Expect(secret).NotTo(BeNil()) - Expect(secret.Data).To(HaveKey("config")) + Expect(secret.Data).To(HaveKey(ctlogUtils.ConfigKey)) configContent := ctlog.GetTrillianAddressFromSecret(secret) Expect(configContent).To(ContainSubstring(correctTrillianAddr), "Config should contain correct Trillian address") }) - It("should delete the config secret to simulate cluster recreation", func(ctx SpecContext) { - By("Deleting config secret: " + originalSecretName) + It("should simulate cluster recreation by deleting the config secret", func(ctx SpecContext) { + By("Deleting config secret to simulate disaster scenario") err := ctlog.DeleteConfigSecret(ctx, cli, namespace.Name, originalSecretName) Expect(err).NotTo(HaveOccurred()) - By("Verifying secret is actually gone") + By("Verifying secret deletion") Eventually(func() bool { _, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, originalSecretName) return errors.IsNotFound(err) @@ -163,22 +159,18 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { }).WithTimeout(30 * time.Second).Should(Succeed()) }) - It("should detect missing/invalid config and recreate it (Phase 1 validation)", func(ctx SpecContext) { - By("Waiting for operator to detect missing/invalid config") + It("should automatically detect and recreate the missing config secret", func(ctx SpecContext) { + By("Waiting for operator to detect and fix the missing config") - // The CTLog should go into a non-Ready state first (detecting the issue) - // Then it should recover by recreating the secret with correct Trillian config - // This tests both Issue 2586 (missing secret) and Issue 3114 (wrong namespace) + // The operator should detect the missing secret and recreate it + // with the correct Trillian configuration from the CTLog spec Eventually(func(g Gomega) bool { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) g.Expect(c).NotTo(BeNil()) - // Check if status has a new/recreated secret reference if c.Status.ServerConfigRef != nil { - // Try to get the secret secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, c.Status.ServerConfigRef.Name) if err == nil && secret != nil { - // Verify it has correct Trillian address configContent := ctlog.GetTrillianAddressFromSecret(secret) if strings.Contains(configContent, correctTrillianAddr) { return true @@ -200,32 +192,32 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { secret, err := ctlog.GetConfigSecret(ctx, cli, namespace.Name, newSecretName) Expect(err).NotTo(HaveOccurred()) Expect(secret).NotTo(BeNil()) - Expect(secret.Data).To(HaveKey("config")) + Expect(secret.Data).To(HaveKey(ctlogUtils.ConfigKey)) // Verify config contains correct Trillian address - configData := secret.Data["config"] + configData := secret.Data[ctlogUtils.ConfigKey] expectedTrillianAddr := fmt.Sprintf("trillian-logserver.%s.svc.cluster.local:8091", namespace.Name) Expect(string(configData)).To(ContainSubstring(expectedTrillianAddr), "Config should contain correct Trillian address") }) - It("should have CTLog deployment updated", func(ctx SpecContext) { + It("should have CTLog deployment running with the new config", func(ctx SpecContext) { Eventually(condition.DeploymentIsRunning). WithContext(ctx). WithArguments(cli, namespace.Name, ctlogActions.ComponentName). - Should(BeTrue(), "CTLog deployment should be running") + Should(BeTrue(), "CTLog deployment should be running after config recreation") }) - It("should have CTLog pod running (not stuck)", func(ctx SpecContext) { + It("should have CTLog pod running without being stuck waiting for secret", func(ctx SpecContext) { Eventually(func(g Gomega) bool { pod := ctlog.GetServerPod(ctx, cli, namespace.Name) g.Expect(pod).NotTo(BeNil()) return pod.Status.Phase == v1.PodRunning }).Should(BeTrue(), - "CTLog pod should be Running, not stuck in ContainerCreating") + "CTLog pod should reach Running phase, proving it's not stuck waiting for a missing secret") }) - It("should have CTLog status Ready", func(ctx SpecContext) { + It("should report Ready status after successful recovery", func(ctx SpecContext) { Eventually(func(g Gomega) string { c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) g.Expect(c).NotTo(BeNil()) @@ -235,18 +227,17 @@ var _ = Describe("CTlog recovery and validation", Ordered, func() { } return readyCond.Reason }).Should(Equal(constants.Ready), - "CTLog should be Ready after secret recreation") + "CTLog should transition to Ready status after config secret recreation") }) It("should preserve TreeID after secret recreation", func(ctx SpecContext) { - // Note: In Phase 1, TreeID is preserved because it's in the CR status - // and the CR was not deleted. This test verifies TreeID remains stable. + // TreeID is preserved because it's stored in the CR status, + // and the CR itself was not deleted during this recovery scenario c := ctlog.Get(ctx, cli, namespace.Name, ctlogCR.Name) Expect(c).NotTo(BeNil()) - Expect(c.Status.TreeID).NotTo(BeNil()) + Expect(c.Status.TreeID).NotTo(BeNil(), "TreeID should remain stable across secret recreation") }) - // Cleanup AfterAll(func(ctx SpecContext) { if ctlogCR != nil { _ = cli.Delete(ctx, ctlogCR) From 018dfb5663b677035e1b30da815f281908b93e97 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 31 Oct 2025 23:08:27 +0100 Subject: [PATCH 07/15] Improvements based on AI code review --- .../controller/ctlog/actions/server_config.go | 12 +++++----- .../controller/ctlog/utils/ctlog_config.go | 24 ++++++++++++++----- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 6f840ef53..285d8837b 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -108,12 +108,12 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) i.Logger.Info("Server config secret is missing, will recreate", "secret", instance.Status.ServerConfigRef.Name) i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigMissing", - fmt.Sprintf("Config secret is missing, will recreate: %s", instance.Status.ServerConfigRef.Name)) + "Config secret is missing, will recreate") } else { i.Logger.Error(err, "Error accessing server config secret, will attempt to recreate", "secret", instance.Status.ServerConfigRef.Name) i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigError", - fmt.Sprintf("Error accessing config secret, will recreate: %s", instance.Status.ServerConfigRef.Name)) + "Error accessing config secret, will recreate") } } else { // Secret exists and is accessible - validate it @@ -126,7 +126,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) "secret", secret.Name, "reason", "Trillian configuration mismatch") i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigInvalid", - fmt.Sprintf("Config secret has invalid configuration, will recreate: %s", secret.Name)) + "Config secret has invalid configuration, will recreate") } } @@ -207,7 +207,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) instance.Status.ServerConfigRef = &rhtasv1alpha1.LocalObjectReference{Name: newConfig.Name} - i.Recorder.Eventf(instance, corev1.EventTypeNormal, "CTLogConfigCreated", "Secret with ctlog configuration created: %s", newConfig.Name) + i.Recorder.Event(instance, corev1.EventTypeNormal, "CTLogConfigCreated", "Config secret created successfully") meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ Type: ConfigCondition, Status: metav1.ConditionTrue, @@ -242,11 +242,11 @@ func (i serverConfig) cleanup(ctx context.Context, instance *rhtasv1alpha1.CTlog err = i.Client.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: partialConfig.Name, Namespace: partialConfig.Namespace}}) if err != nil { i.Logger.Error(err, "unable to delete secret", "namespace", instance.Namespace, "name", partialConfig.Name) - i.Recorder.Eventf(instance, corev1.EventTypeWarning, "CTLogConfigDeleted", "Unable to delete secret: %s", partialConfig.Name) + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigCleanupFailed", "Unable to delete old config secret") continue } i.Logger.Info("Remove invalid Secret with ctlog configuration", "Name", partialConfig.Name) - i.Recorder.Eventf(instance, corev1.EventTypeNormal, "CTLogConfigDeleted", "Secret with ctlog configuration deleted: %s", partialConfig.Name) + i.Recorder.Event(instance, corev1.EventTypeNormal, "CTLogConfigCleanedUp", "Old config secret deleted successfully") } } diff --git a/internal/controller/ctlog/utils/ctlog_config.go b/internal/controller/ctlog/utils/ctlog_config.go index 3e35a86e5..f57a6fcb7 100644 --- a/internal/controller/ctlog/utils/ctlog_config.go +++ b/internal/controller/ctlog/utils/ctlog_config.go @@ -7,7 +7,6 @@ import ( "crypto/x509" "encoding/pem" "fmt" - "strings" "github.com/google/certificate-transparency-go/trillian/ctfe/configpb" "github.com/google/trillian/crypto/keyspb" @@ -205,6 +204,11 @@ func CreateCtlogConfig(trillianUrl string, treeID int64, rootCerts []RootCertifi // IsSecretDataValid validates that a CTLog config secret contains valid configuration // with the correct Trillian backend address. // +// This function parses the protobuf text configuration and validates: +// 1. The configuration can be unmarshalled into the expected structure +// 2. At least one backend exists +// 3. The backend's BackendSpec matches the expected Trillian address +// // Parameters: // - secretData: The Data field from a Kubernetes Secret containing CTLog configuration // - expectedTrillianAddr: The Trillian address to validate against (e.g., "trillian-logserver.namespace.svc:8091") @@ -218,17 +222,25 @@ func IsSecretDataValid(secretData map[string][]byte, expectedTrillianAddr string } configData, ok := secretData[ConfigKey] - if !ok { + if !ok || len(configData) == 0 { + return false + } + + // Parse the protobuf text format configuration + var multiConfig configpb.LogMultiConfig + if err := prototext.Unmarshal(configData, &multiConfig); err != nil { + // Failed to parse - invalid configuration return false } - configString := string(configData) - if len(configString) == 0 { + // Validate that at least one backend exists + if multiConfig.Backends == nil || multiConfig.Backends.Backend == nil || len(multiConfig.Backends.Backend) == 0 { return false } - for _, line := range strings.Split(configString, "\n") { - if strings.Contains(line, "backend_spec") && strings.Contains(line, expectedTrillianAddr) { + // Check if any backend matches the expected Trillian address + for _, backend := range multiConfig.Backends.Backend { + if backend.BackendSpec == expectedTrillianAddr { return true } } From e37c9084008b2fe9062ace2586009f7268ae7b67 Mon Sep 17 00:00:00 2001 From: Petr Pinkas <66317936+petrpinkas@users.noreply.github.com> Date: Mon, 3 Nov 2025 16:07:14 +0100 Subject: [PATCH 08/15] Update trigger-konflux-builds.txt --- trigger-konflux-builds.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trigger-konflux-builds.txt b/trigger-konflux-builds.txt index a6a4267d4..908b72b90 100644 --- a/trigger-konflux-builds.txt +++ b/trigger-konflux-builds.txt @@ -1 +1 @@ -2025-09-23,21-51-00 +2025-11-03,16-00-00 From 0ab0b7f6841b51176dfa34e25863a9f66f947fe0 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 12 Nov 2025 17:34:26 +0100 Subject: [PATCH 09/15] Trillian address processed before validation --- .../controller/ctlog/actions/server_config.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 285d8837b..aaa83dc88 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -99,6 +99,20 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) return i.StatusUpdate(ctx, instance) } + // Validate prerequisites and normalize Trillian address before validation + switch { + case instance.Status.TreeID == nil: + return i.Error(ctx, fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrTreeNotSpecified), instance) + case instance.Status.PrivateKeyRef == nil: + return i.Error(ctx, fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrPrivateKeyNotSpecified), instance) + case instance.Spec.Trillian.Port == nil: + return i.Error(ctx, reconcile.TerminalError(fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrTrillianPortNotSpecified)), instance) + case instance.Spec.Trillian.Address == "": + instance.Spec.Trillian.Address = fmt.Sprintf("%s.%s.svc", trillian.LogserverDeploymentName, instance.Namespace) + } + + trillianUrl := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) + // Validate existing secret before attempting recreation if instance.Status.ServerConfigRef != nil && instance.Status.ServerConfigRef.Name != "" { secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) @@ -117,8 +131,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) } } else { // Secret exists and is accessible - validate it - expectedTrillianAddr := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) - if ctlogUtils.IsSecretDataValid(secret.Data, expectedTrillianAddr) { + if ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { return i.Continue() // nothing to do } // Secret is invalid, will recreate @@ -130,19 +143,6 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) } } - switch { - case instance.Status.TreeID == nil: - return i.Error(ctx, fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrTreeNotSpecified), instance) - case instance.Status.PrivateKeyRef == nil: - return i.Error(ctx, fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrPrivateKeyNotSpecified), instance) - case instance.Spec.Trillian.Port == nil: - return i.Error(ctx, reconcile.TerminalError(fmt.Errorf("%s: %v", i.Name(), ctlogUtils.ErrTrillianPortNotSpecified)), instance) - case instance.Spec.Trillian.Address == "": - instance.Spec.Trillian.Address = fmt.Sprintf("%s.%s.svc", trillian.LogserverDeploymentName, instance.Namespace) - } - - trillianUrl := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) - configLabels := labels.ForResource(ComponentName, DeploymentName, instance.Name, serverConfigResourceName) rootCerts, err := i.handleRootCertificates(instance) From e17aa9c66201091a95b190f27a9261067a7e805c Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 12 Nov 2025 18:20:18 +0100 Subject: [PATCH 10/15] ctlog test repaired to work with new canHandle behaviour --- .../controller/ctlog/actions/server_config.go | 2 ++ .../ctlog/actions/server_config_test.go | 17 +++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index aaa83dc88..c85087993 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -52,6 +52,8 @@ func (i serverConfig) CanHandle(_ context.Context, instance *rhtasv1alpha1.CTlog return true case instance.Spec.ServerConfigRef != nil: return !equality.Semantic.DeepEqual(instance.Spec.ServerConfigRef, instance.Status.ServerConfigRef) + case c.ObservedGeneration != instance.Generation: + return true default: // Always run Handle() to validate the secret: exists and is valid return true diff --git a/internal/controller/ctlog/actions/server_config_test.go b/internal/controller/ctlog/actions/server_config_test.go index 11d5e7af7..00d2b3ce8 100644 --- a/internal/controller/ctlog/actions/server_config_test.go +++ b/internal/controller/ctlog/actions/server_config_test.go @@ -57,9 +57,11 @@ func TestServerConfig_CanHandle(t *testing.T) { { name: "ConditionTrue: spec.serverConfigRef is nil and status.serverConfigRef is not nil", status: metav1.ConditionTrue, - canHandle: false, + canHandle: true, serverConfigRef: nil, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 1, }, { name: "ConditionTrue: spec.serverConfigRef is nil and status.serverConfigRef is nil", @@ -85,7 +87,7 @@ func TestServerConfig_CanHandle(t *testing.T) { { name: "ConditionTrue: observedGeneration == generation", status: metav1.ConditionTrue, - canHandle: false, + canHandle: true, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, observedGeneration: 1, generation: 1, @@ -182,6 +184,17 @@ func TestServerConfig_Handle(t *testing.T) { status: rhtasv1alpha1.CTlogStatus{ ServerConfigRef: nil, }, + objects: []client.Object{ + &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + Namespace: "default", + }, + Data: map[string][]byte{ + ctlogUtils.ConfigKey: []byte("test-config"), + }, + }, + }, }, want: want{ result: testAction.StatusUpdate(), From 14b3b9c2fe7eccd114dce4581118e5d11728562a Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 12 Nov 2025 18:38:59 +0100 Subject: [PATCH 11/15] ctlog test repaired to work with new canHandle behaviour --- .../controller/ctlog/actions/server_config.go | 33 ++++++++++++++----- .../ctlog/actions/server_config_test.go | 11 +++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index c85087993..ac97027af 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -133,15 +133,32 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) } } else { // Secret exists and is accessible - validate it - if ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { - return i.Continue() // nothing to do + if !ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { + // Secret has wrong Trillian configuration, will recreate + i.Logger.Info("Server config secret is invalid, will recreate", + "secret", secret.Name, + "reason", "Trillian configuration mismatch") + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigInvalid", + "Config secret has invalid Trillian configuration, will recreate") + } else { + // Check if root certificates match (for hot updates) + expectedRootCertCount := len(instance.Status.RootCertificates) + actualRootCertCount := 0 + for key := range secret.Data { + if len(key) >= 6 && key[:6] == "fulcio" { + actualRootCertCount++ + } + } + if actualRootCertCount == expectedRootCertCount { + // Everything matches - no need to recreate + return i.Continue() + } + // Root certificates changed - need to recreate for hot update + i.Logger.Info("Server config secret needs update for root certificate change", + "secret", secret.Name, + "expected_certs", expectedRootCertCount, + "actual_certs", actualRootCertCount) } - // Secret is invalid, will recreate - i.Logger.Info("Server config secret is invalid, will recreate", - "secret", secret.Name, - "reason", "Trillian configuration mismatch") - i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigInvalid", - "Config secret has invalid configuration, will recreate") } } diff --git a/internal/controller/ctlog/actions/server_config_test.go b/internal/controller/ctlog/actions/server_config_test.go index 00d2b3ce8..99926488f 100644 --- a/internal/controller/ctlog/actions/server_config_test.go +++ b/internal/controller/ctlog/actions/server_config_test.go @@ -251,6 +251,17 @@ func TestServerConfig_Handle(t *testing.T) { status: rhtasv1alpha1.CTlogStatus{ ServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "old_config"}, }, + objects: []client.Object{ + &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new_config", + Namespace: "default", + }, + Data: map[string][]byte{ + ctlogUtils.ConfigKey: []byte("new-test-config"), + }, + }, + }, }, want: want{ result: testAction.StatusUpdate(), From 4453c05196e950b7fb56f3cf948484555040eb07 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 12 Nov 2025 18:57:13 +0100 Subject: [PATCH 12/15] Improved check if root certificates match --- internal/controller/ctlog/actions/server_config.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index ac97027af..55f7ceb2f 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -6,6 +6,7 @@ import ( "fmt" "maps" "slices" + "strings" rhtasv1alpha1 "github.com/securesign/operator/api/v1alpha1" "github.com/securesign/operator/internal/action" @@ -142,14 +143,17 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) "Config secret has invalid Trillian configuration, will recreate") } else { // Check if root certificates match (for hot updates) - expectedRootCertCount := len(instance.Status.RootCertificates) + // Count fulcio-* keys in the secret actualRootCertCount := 0 for key := range secret.Data { - if len(key) >= 6 && key[:6] == "fulcio" { + if strings.HasPrefix(key, "fulcio-") { actualRootCertCount++ } } - if actualRootCertCount == expectedRootCertCount { + + // Compare with expected count from status + expectedRootCertCount := len(instance.Status.RootCertificates) + if actualRootCertCount == expectedRootCertCount && expectedRootCertCount > 0 { // Everything matches - no need to recreate return i.Continue() } From c021042fee144aacf0ffc40c66db3323644af368 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Wed, 19 Nov 2025 22:28:46 +0100 Subject: [PATCH 13/15] Improved handling of hot updates --- internal/controller/ctlog/actions/server_config.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 55f7ceb2f..ef348b37c 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -116,8 +116,11 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) trillianUrl := fmt.Sprintf("%s:%d", instance.Spec.Trillian.Address, *instance.Spec.Trillian.Port) - // Validate existing secret before attempting recreation - if instance.Status.ServerConfigRef != nil && instance.Status.ServerConfigRef.Name != "" { + c := meta.FindStatusCondition(instance.Status.Conditions, ConfigCondition) + isSpecChange := c != nil && c.ObservedGeneration != instance.Generation + + // Validate existing secret before attempting recreation (only for hot updates, not spec changes) + if !isSpecChange && instance.Status.ServerConfigRef != nil && instance.Status.ServerConfigRef.Name != "" { secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) if err != nil { @@ -133,7 +136,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) "Error accessing config secret, will recreate") } } else { - // Secret exists and is accessible - validate it + // Secret exists and is accessible - validate it (for hot updates only) if !ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { // Secret has wrong Trillian configuration, will recreate i.Logger.Info("Server config secret is invalid, will recreate", @@ -150,7 +153,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) actualRootCertCount++ } } - + // Compare with expected count from status expectedRootCertCount := len(instance.Status.RootCertificates) if actualRootCertCount == expectedRootCertCount && expectedRootCertCount > 0 { From 77cd7feb9352a6b6b6afc7754fdff57456588e29 Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 12 Dec 2025 20:50:27 +0100 Subject: [PATCH 14/15] Simplified CanHandle method --- .../controller/ctlog/actions/server_config.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index ef348b37c..c44eb7fc1 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -17,7 +17,6 @@ import ( "github.com/securesign/operator/internal/utils/kubernetes" "github.com/securesign/operator/internal/utils/kubernetes/ensure" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -43,22 +42,8 @@ func (i serverConfig) Name() string { func (i serverConfig) CanHandle(_ context.Context, instance *rhtasv1alpha1.CTlog) bool { c := meta.FindStatusCondition(instance.Status.Conditions, ConfigCondition) - - switch { - case c == nil: - return false - case !meta.IsStatusConditionTrue(instance.Status.Conditions, ConfigCondition): - return true - case instance.Status.ServerConfigRef == nil: - return true - case instance.Spec.ServerConfigRef != nil: - return !equality.Semantic.DeepEqual(instance.Spec.ServerConfigRef, instance.Status.ServerConfigRef) - case c.ObservedGeneration != instance.Generation: - return true - default: - // Always run Handle() to validate the secret: exists and is valid - return true - } + // Always run Handle() to validate the config secret exists and is valid + return c != nil } func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) *action.Result { From 66ed4ff605243942b5e50783320cf81f8fec85be Mon Sep 17 00:00:00 2001 From: Petr Pinkas Date: Fri, 12 Dec 2025 21:22:31 +0100 Subject: [PATCH 15/15] Function to validate existing secret added --- .../controller/ctlog/actions/server_config.go | 80 +++++++++---------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index c44eb7fc1..2f0bc6cb5 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -106,51 +106,11 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) // Validate existing secret before attempting recreation (only for hot updates, not spec changes) if !isSpecChange && instance.Status.ServerConfigRef != nil && instance.Status.ServerConfigRef.Name != "" { - secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) - - if err != nil { - if apierrors.IsNotFound(err) { - i.Logger.Info("Server config secret is missing, will recreate", - "secret", instance.Status.ServerConfigRef.Name) - i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigMissing", - "Config secret is missing, will recreate") - } else { - i.Logger.Error(err, "Error accessing server config secret, will attempt to recreate", - "secret", instance.Status.ServerConfigRef.Name) - i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigError", - "Error accessing config secret, will recreate") - } + if err := i.validateExistingSecret(instance, trillianUrl); err != nil { + i.Logger.Info(err.Error(), "secret", instance.Status.ServerConfigRef.Name) + i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigRecreate", err.Error()) } else { - // Secret exists and is accessible - validate it (for hot updates only) - if !ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { - // Secret has wrong Trillian configuration, will recreate - i.Logger.Info("Server config secret is invalid, will recreate", - "secret", secret.Name, - "reason", "Trillian configuration mismatch") - i.Recorder.Event(instance, corev1.EventTypeWarning, "CTLogConfigInvalid", - "Config secret has invalid Trillian configuration, will recreate") - } else { - // Check if root certificates match (for hot updates) - // Count fulcio-* keys in the secret - actualRootCertCount := 0 - for key := range secret.Data { - if strings.HasPrefix(key, "fulcio-") { - actualRootCertCount++ - } - } - - // Compare with expected count from status - expectedRootCertCount := len(instance.Status.RootCertificates) - if actualRootCertCount == expectedRootCertCount && expectedRootCertCount > 0 { - // Everything matches - no need to recreate - return i.Continue() - } - // Root certificates changed - need to recreate for hot update - i.Logger.Info("Server config secret needs update for root certificate change", - "secret", secret.Name, - "expected_certs", expectedRootCertCount, - "actual_certs", actualRootCertCount) - } + return i.Continue() } } @@ -298,3 +258,35 @@ func (i serverConfig) handleRootCertificates(instance *rhtasv1alpha1.CTlog) ([]c return certs, nil } + +func (i serverConfig) validateExistingSecret(instance *rhtasv1alpha1.CTlog, trillianUrl string) error { + secret, err := kubernetes.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) + if err != nil { + if apierrors.IsNotFound(err) { + return errors.New("config secret is missing, will recreate") + } + return fmt.Errorf("error accessing config secret, will recreate: %w", err) + } + + // Secret exists and is accessible - validate its content + if !ctlogUtils.IsSecretDataValid(secret.Data, trillianUrl) { + return errors.New("config secret has invalid Trillian configuration, will recreate") + } + + // Check if root certificates match + actualRootCertCount := 0 + for key := range secret.Data { + if strings.HasPrefix(key, "fulcio-") { + actualRootCertCount++ + } + } + + expectedRootCertCount := len(instance.Status.RootCertificates) + if actualRootCertCount != expectedRootCertCount || expectedRootCertCount == 0 { + return fmt.Errorf("root certificate count mismatch (expected: %d, actual: %d), will recreate", + expectedRootCertCount, actualRootCertCount) + } + + // Secret is valid + return nil +}