From 00758181ecbbe6486757b9a7cd9c44be4f737911 Mon Sep 17 00:00:00 2001 From: Tomas Turek Date: Mon, 30 Sep 2024 11:25:54 +0200 Subject: [PATCH] fix: create new ctlog config when it's content changed --- .../controller/ctlog/actions/server_config.go | 11 + .../ctlog/actions/server_config_test.go | 202 +++++++++++++++++- 2 files changed, 211 insertions(+), 2 deletions(-) diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 4ddae8c3..2ec66dba 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -119,6 +120,16 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) return i.FailedWithStatusUpdate(ctx, fmt.Errorf("could not create CTLog configuration: %w", err), instance) } + if instance.Status.ServerConfigRef != nil { + currentSecret, err := utils.GetSecret(i.Client, instance.Namespace, instance.Status.ServerConfigRef.Name) + if client.IgnoreNotFound(err) != nil { + return i.Failed(err) + } + if currentSecret != nil && equality.Semantic.DeepEqual(cfg, currentSecret.Data) { + return i.Continue() + } + } + newConfig := utils.CreateImmutableSecret(fmt.Sprintf("ctlog-config-%s", instance.Name), instance.Namespace, cfg, labels) if err = controllerutil.SetControllerReference(instance, newConfig, i.Client.Scheme()); err != nil { diff --git a/internal/controller/ctlog/actions/server_config_test.go b/internal/controller/ctlog/actions/server_config_test.go index a9a30c10..9bf5e4db 100644 --- a/internal/controller/ctlog/actions/server_config_test.go +++ b/internal/controller/ctlog/actions/server_config_test.go @@ -6,6 +6,9 @@ import ( "reflect" "testing" + ctlogUtils "github.com/securesign/operator/internal/controller/ctlog/utils" + "github.com/securesign/operator/internal/testing/errors" + "github.com/onsi/gomega/gstruct" "github.com/securesign/operator/internal/controller/common/utils/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,6 +40,8 @@ func TestServerConfig_CanHandle(t *testing.T) { canHandle bool serverConfigRef *rhtasv1alpha1.LocalObjectReference statusServerConfigRef *rhtasv1alpha1.LocalObjectReference + observedGeneration int64 + generation int64 }{ { name: "spec.serverConfigRef is not nil and status.serverConfigRef is nil", @@ -73,6 +78,30 @@ func TestServerConfig_CanHandle(t *testing.T) { serverConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, }, + { + name: "Ready: observedGeneration == generation", + phase: constants.Ready, + canHandle: false, + statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 1, + }, + { + name: "Ready: observedGeneration != generation", + phase: constants.Ready, + canHandle: true, + statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 2, + }, + { + name: "Creating: observedGeneration != generation", + phase: constants.Creating, + canHandle: false, + statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 2, + }, { name: "no phase condition", phase: "", @@ -109,6 +138,11 @@ func TestServerConfig_CanHandle(t *testing.T) { c := testAction.FakeClientBuilder().Build() a := testAction.PrepareAction(c, NewServerConfigAction()) instance := rhtasv1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: tt.generation, + }, Spec: rhtasv1alpha1.CTlogSpec{ ServerConfigRef: tt.serverConfigRef, }, @@ -118,8 +152,9 @@ func TestServerConfig_CanHandle(t *testing.T) { } if tt.phase != "" { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, - Reason: tt.phase, + Type: constants.Ready, + Reason: tt.phase, + ObservedGeneration: tt.observedGeneration, }) } @@ -318,3 +353,166 @@ func TestServerConfig_Handle(t *testing.T) { }) } } + +func TestServerConfig_Update(t *testing.T) { + g := NewWithT(t) + type env struct { + instance rhtasv1alpha1.CTlog + objects []client.Object + } + type want struct { + result *action.Result + verify func(Gomega, client.Client, *rhtasv1alpha1.CTlog) + } + tests := []struct { + name string + env env + want want + }{ + { + name: "new secret with config created", + env: env{ + instance: rhtasv1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 2, + }, + Spec: rhtasv1alpha1.CTlogSpec{ + Trillian: rhtasv1alpha1.TrillianService{Port: ptr.To(int32(443))}, + TreeID: ptr.To(int64(123456)), + }, + Status: rhtasv1alpha1.CTlogStatus{ + ServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "old_secret"}, + TreeID: ptr.To(int64(123456)), + RootCertificates: []rhtasv1alpha1.SecretKeySelector{ + {LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "cert"}, + }, + PrivateKeyRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "private"}, + PrivateKeyPasswordRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "password"}, + PublicKeyRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "public"}, + Conditions: []metav1.Condition{ + { + Type: constants.Ready, + Reason: constants.Ready, + ObservedGeneration: 1, + }, + }, + }, + }, + objects: []client.Object{ + kubernetes.CreateSecret("secret", "default", map[string][]byte{ + "cert": cert, + "private": privateKey, + "public": publicKey, + "password": []byte("secure"), + }, map[string]string{}), + kubernetes.CreateSecret("old_secret", "default", + errors.IgnoreError(ctlogUtils.CreateCtlogConfig( + "trillian-logserver.default.svc:80", + 654321, + []ctlogUtils.RootCertificate{cert}, + &ctlogUtils.PrivateKeyConfig{ + PrivateKey: privateKey, + PublicKey: publicKey, + PrivateKeyPass: []byte("secure"), + })), + map[string]string{}), + }, + }, + want: want{ + result: testAction.StatusUpdate(), + verify: func(g Gomega, cli client.Client, current *rhtasv1alpha1.CTlog) { + g.Expect(current.Status.ServerConfigRef).ShouldNot(BeNil()) + g.Expect(current.Status.ServerConfigRef.Name).Should(ContainSubstring("ctlog-config-")) + + data, err := kubernetes.GetSecretData(cli, "default", &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: *current.Status.ServerConfigRef, Key: "config"}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(data).To(And(ContainSubstring("trillian-logserver.default.svc:443"), ContainSubstring("123456"))) + }, + }, + }, + { + name: "use current secret when it's content has not change", + env: env{ + instance: rhtasv1alpha1.CTlog{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 2, + }, + Spec: rhtasv1alpha1.CTlogSpec{ + Trillian: rhtasv1alpha1.TrillianService{Port: ptr.To(int32(80))}, + TreeID: ptr.To(int64(123456)), + }, + Status: rhtasv1alpha1.CTlogStatus{ + ServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "old_secret"}, + TreeID: ptr.To(int64(123456)), + RootCertificates: []rhtasv1alpha1.SecretKeySelector{ + {LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "cert"}, + }, + PrivateKeyRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "private"}, + PrivateKeyPasswordRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "password"}, + PublicKeyRef: &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "public"}, + Conditions: []metav1.Condition{ + { + Type: constants.Ready, + Reason: constants.Ready, + ObservedGeneration: 1, + }, + }, + }, + }, + objects: []client.Object{ + kubernetes.CreateSecret("secret", "default", map[string][]byte{ + "cert": cert, + "private": privateKey, + "public": publicKey, + "password": []byte("secure"), + }, map[string]string{}), + kubernetes.CreateSecret("old_secret", "default", + errors.IgnoreError(ctlogUtils.CreateCtlogConfig( + "trillian-logserver.default.svc:80", + 123456, + []ctlogUtils.RootCertificate{cert}, + &ctlogUtils.PrivateKeyConfig{ + PrivateKey: privateKey, + PublicKey: publicKey, + PrivateKeyPass: []byte("secure"), + })), + map[string]string{}), + }, + }, + want: want{ + result: testAction.Continue(), + verify: func(g Gomega, cli client.Client, current *rhtasv1alpha1.CTlog) { + g.Expect(current.Status.ServerConfigRef).ShouldNot(BeNil()) + g.Expect(current.Status.ServerConfigRef.Name).Should(Equal("old_secret")) + + data, err := kubernetes.GetSecretData(cli, "default", &rhtasv1alpha1.SecretKeySelector{LocalObjectReference: *current.Status.ServerConfigRef, Key: "config"}) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(data).To(And(ContainSubstring("trillian-logserver.default.svc:80"), ContainSubstring("123456"))) + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.TODO() + c := testAction.FakeClientBuilder(). + WithObjects(&tt.env.instance). + WithStatusSubresource(&tt.env.instance). + WithObjects(tt.env.objects...). + Build() + + a := testAction.PrepareAction(c, NewServerConfigAction()) + + if got := a.Handle(ctx, &tt.env.instance); !reflect.DeepEqual(got, tt.want.result) { + t.Errorf("CanHandle() = %v, want %v", got, tt.want.result) + } + if tt.want.verify != nil { + tt.want.verify(g, c, &tt.env.instance) + } + }) + } +}