diff --git a/internal/controller/ctlog/actions/constants.go b/internal/controller/ctlog/actions/constants.go index 35b1cf522..5ea3c557e 100644 --- a/internal/controller/ctlog/actions/constants.go +++ b/internal/controller/ctlog/actions/constants.go @@ -8,7 +8,13 @@ const ( RBACName = "ctlog" MonitoringRoleName = "prometheus-k8s-ctlog" - CertCondition = "FulcioCertAvailable" + CertCondition = "FulcioCertAvailable" + + ConfigCondition = "ServerConfigAvailable" + TrillianTreeReason = "TrillianTree" + SignerKeyReason = "SignerKey" + FulcioReason = "FulcioCertificate" + ServerPortName = "http" ServerPort = 80 ServerTargetPort = 6962 diff --git a/internal/controller/ctlog/actions/handle_fulcio_root.go b/internal/controller/ctlog/actions/handle_fulcio_root.go index d6052ec6f..cedaccc79 100644 --- a/internal/controller/ctlog/actions/handle_fulcio_root.go +++ b/internal/controller/ctlog/actions/handle_fulcio_root.go @@ -96,9 +96,12 @@ func (g handleFulcioCert) Handle(ctx context.Context, instance *v1alpha1.CTlog) } // invalidate server config - if instance.Status.ServerConfigRef != nil { - instance.Status.ServerConfigRef = nil - } + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: FulcioReason, + Message: "Fulcio certificate changed", + }) meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ Type: CertCondition, diff --git a/internal/controller/ctlog/actions/handle_fulcio_root_test.go b/internal/controller/ctlog/actions/handle_fulcio_root_test.go index 401c1688e..14f80932e 100644 --- a/internal/controller/ctlog/actions/handle_fulcio_root_test.go +++ b/internal/controller/ctlog/actions/handle_fulcio_root_test.go @@ -336,13 +336,14 @@ func TestCert_Handle(t *testing.T) { want: want{ result: testAction.StatusUpdate(), verify: func(g Gomega, status v1alpha1.CTlogStatus, cli client.WithWatch, configWatch <-chan watch.Event) { - g.Expect(status.ServerConfigRef).Should(BeNil()) - g.Expect(status.RootCertificates).Should(HaveLen(1)) g.Expect(status.RootCertificates[0].Key).Should(Equal("key")) g.Expect(status.RootCertificates[0].Name).Should(Equal("my-secret")) g.Expect(meta.IsStatusConditionTrue(status.Conditions, CertCondition)).To(BeTrue()) + + // Config condition should be invalidated + g.Expect(meta.IsStatusConditionFalse(status.Conditions, ConfigCondition)).Should(BeTrue()) }, }, }, diff --git a/internal/controller/ctlog/actions/handle_keys.go b/internal/controller/ctlog/actions/handle_keys.go index 1df63e9d8..ef7b2e86f 100644 --- a/internal/controller/ctlog/actions/handle_keys.go +++ b/internal/controller/ctlog/actions/handle_keys.go @@ -94,9 +94,12 @@ func (g handleKeys) Handle(ctx context.Context, instance *v1alpha1.CTlog) *actio instance.Status = *newKeyStatus // invalidate server config - if instance.Status.ServerConfigRef != nil { - instance.Status.ServerConfigRef = nil - } + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: SignerKeyReason, + Message: "New signer key", + }) meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ Type: constants.Ready, diff --git a/internal/controller/ctlog/actions/resolve_tree.go b/internal/controller/ctlog/actions/resolve_tree.go index 43cdd89ee..004d4a226 100644 --- a/internal/controller/ctlog/actions/resolve_tree.go +++ b/internal/controller/ctlog/actions/resolve_tree.go @@ -58,6 +58,13 @@ func (i resolveTreeAction) CanHandle(_ context.Context, instance *rhtasv1alpha1. func (i resolveTreeAction) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) *action.Result { if instance.Spec.TreeID != nil && *instance.Spec.TreeID != int64(0) { instance.Status.TreeID = instance.Spec.TreeID + // invalidate server config + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: TrillianTreeReason, + Message: "Trillian tree changed", + }) return i.StatusUpdate(ctx, instance) } var err error @@ -97,5 +104,13 @@ func (i resolveTreeAction) Handle(ctx context.Context, instance *rhtasv1alpha1.C i.Recorder.Eventf(instance, v1.EventTypeNormal, "TrillianTreeCreated", "New Trillian tree created: %d", tree.TreeId) instance.Status.TreeID = &tree.TreeId + // invalidate server config + meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ + Type: ConfigCondition, + Status: metav1.ConditionFalse, + Reason: TrillianTreeReason, + Message: "Trillian tree changed", + }) + return i.StatusUpdate(ctx, instance) } diff --git a/internal/controller/ctlog/actions/server_config.go b/internal/controller/ctlog/actions/server_config.go index 1b152f2d3..6a0c98b34 100644 --- a/internal/controller/ctlog/actions/server_config.go +++ b/internal/controller/ctlog/actions/server_config.go @@ -35,21 +35,19 @@ func (i serverConfig) Name() string { } func (i serverConfig) CanHandle(_ context.Context, instance *rhtasv1alpha1.CTlog) bool { - c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) + c := meta.FindStatusCondition(instance.Status.Conditions, ConfigCondition) switch { case c == nil: return false - case c.Reason != constants.Creating && c.Reason != constants.Ready: - 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.Reason == constants.Ready: - return instance.Generation != c.ObservedGeneration default: - return false + return instance.Generation != c.ObservedGeneration } } @@ -62,10 +60,10 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) instance.Status.ServerConfigRef = instance.Spec.ServerConfigRef i.Recorder.Event(instance, corev1.EventTypeNormal, "CTLogConfigUpdated", "CTLog config updated") meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, - Status: metav1.ConditionFalse, - Reason: constants.Creating, - Message: "CTLog config updated", + Type: ConfigCondition, + Status: metav1.ConditionTrue, + Reason: constants.Ready, + Message: "Using custom server config", ObservedGeneration: instance.Generation, }) return i.StatusUpdate(ctx, instance) @@ -87,24 +85,12 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) labels := constants.LabelsFor(ComponentName, DeploymentName, instance.Name) labels[constants.LabelResource] = serverConfigResourceName - // try to discover existing config and clear them out - partialConfigs, err := utils.ListSecrets(ctx, i.Client, instance.Namespace, labels2.SelectorFromSet(labels).String()) - if err != nil { - i.Logger.Error(err, "problem with listing configmaps", "namespace", instance.Namespace) - } - for _, partialConfig := range partialConfigs.Items { - 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.Client.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: partialConfig.Name, Namespace: partialConfig.Namespace}}) - - } - rootCerts, err := i.handleRootCertificates(instance) if err != nil { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, + Type: ConfigCondition, Status: metav1.ConditionFalse, - Reason: constants.Creating, + Reason: FulcioReason, Message: fmt.Sprintf("Waiting for Fulcio root certificate: %v", err.Error()), ObservedGeneration: instance.Generation, }) @@ -115,9 +101,9 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) certConfig, err := i.handlePrivateKey(instance) if err != nil { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, + Type: ConfigCondition, Status: metav1.ConditionFalse, - Reason: constants.Creating, + Reason: SignerKeyReason, Message: "Waiting for Ctlog private key secret", ObservedGeneration: instance.Generation, }) @@ -128,7 +114,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) var cfg map[string][]byte if cfg, err = ctlogUtils.CreateCtlogConfig(trillianUrl, *instance.Status.TreeID, rootCerts, certConfig); err != nil { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, + Type: ConfigCondition, Status: metav1.ConditionFalse, Reason: constants.Failure, Message: err.Error(), @@ -145,7 +131,7 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) _, err = i.Ensure(ctx, newConfig) if err != nil { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, + Type: ConfigCondition, Status: metav1.ConditionFalse, Reason: constants.Failure, Message: err.Error(), @@ -154,13 +140,33 @@ func (i serverConfig) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) return i.FailedWithStatusUpdate(ctx, err, instance) } + // try to discover existing config and clear them out + partialConfigs, err := utils.ListSecrets(ctx, i.Client, instance.Namespace, labels2.SelectorFromSet(labels).String()) + if err != nil { + i.Logger.Error(err, "problem with listing configmaps", "namespace", instance.Namespace) + } + for _, partialConfig := range partialConfigs.Items { + if partialConfig.Name == newConfig.Name { + continue + } + + 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) + 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) + } + instance.Status.ServerConfigRef = &rhtasv1alpha1.LocalObjectReference{Name: newConfig.Name} - i.Recorder.Event(instance, corev1.EventTypeNormal, "CTLogConfigUpdated", "CTLog config updated") + i.Recorder.Eventf(instance, corev1.EventTypeNormal, "CTLogConfigCreated", "Secret with ctlog configuration created: %s", newConfig.Name) meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, - Status: metav1.ConditionFalse, - Reason: constants.Creating, + Type: ConfigCondition, + Status: metav1.ConditionTrue, + Reason: constants.Ready, Message: "Server config created", ObservedGeneration: instance.Generation, }) diff --git a/internal/controller/ctlog/actions/server_config_test.go b/internal/controller/ctlog/actions/server_config_test.go index 9b97f1195..fd2edce72 100644 --- a/internal/controller/ctlog/actions/server_config_test.go +++ b/internal/controller/ctlog/actions/server_config_test.go @@ -39,7 +39,7 @@ var ( func TestServerConfig_CanHandle(t *testing.T) { tests := []struct { name string - phase string + status metav1.ConditionStatus canHandle bool serverConfigRef *rhtasv1alpha1.LocalObjectReference statusServerConfigRef *rhtasv1alpha1.LocalObjectReference @@ -47,93 +47,79 @@ func TestServerConfig_CanHandle(t *testing.T) { generation int64 }{ { - name: "spec.serverConfigRef is not nil and status.serverConfigRef is nil", - phase: constants.Creating, + name: "ConditionTrue: spec.serverConfigRef is not nil and status.serverConfigRef is nil", + status: metav1.ConditionTrue, canHandle: true, serverConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, statusServerConfigRef: nil, }, { - name: "spec.serverConfigRef is nil and status.serverConfigRef is not nil", - phase: constants.Creating, + name: "ConditionTrue: spec.serverConfigRef is nil and status.serverConfigRef is not nil", + status: metav1.ConditionTrue, canHandle: false, serverConfigRef: nil, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, }, { - name: "spec.serverConfigRef is nil and status.serverConfigRef is nil", - phase: constants.Creating, + name: "ConditionTrue: spec.serverConfigRef is nil and status.serverConfigRef is nil", + status: metav1.ConditionTrue, canHandle: true, serverConfigRef: nil, statusServerConfigRef: nil, }, { - name: "spec.serverConfigRef != status.serverConfigRef", - phase: constants.Creating, + name: "ConditionTrue: spec.serverConfigRef != status.serverConfigRef", + status: metav1.ConditionTrue, canHandle: true, serverConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "new_config"}, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "old_config"}, }, { - name: "spec.serverConfigRef == status.serverConfigRef", - phase: constants.Creating, + name: "ConditionTrue: spec.serverConfigRef == status.serverConfigRef", + status: metav1.ConditionTrue, canHandle: false, serverConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, }, { - name: "Ready: observedGeneration == generation", - phase: constants.Ready, + name: "ConditionTrue: observedGeneration == generation", + status: metav1.ConditionTrue, canHandle: false, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, observedGeneration: 1, generation: 1, }, { - name: "Ready: observedGeneration != generation", - phase: constants.Ready, + name: "ConditionTrue: observedGeneration != generation", + status: metav1.ConditionTrue, canHandle: true, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, observedGeneration: 1, generation: 2, }, { - name: "Creating: observedGeneration != generation", - phase: constants.Creating, + name: "empty condition", + status: "", canHandle: false, statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, observedGeneration: 1, - generation: 2, - }, - { - name: "no phase condition", - phase: "", - canHandle: false, - }, - { - name: constants.Ready, - phase: constants.Ready, - canHandle: true, - }, - { - name: constants.Pending, - phase: constants.Pending, - canHandle: false, - }, - { - name: constants.Creating, - phase: constants.Creating, - canHandle: true, + generation: 1, }, { - name: constants.Initialize, - phase: constants.Initialize, - canHandle: false, + name: "ConditionUnknown", + status: metav1.ConditionUnknown, + canHandle: true, + statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 1, }, { - name: constants.Failure, - phase: constants.Failure, - canHandle: false, + name: "ConditionFalse", + status: metav1.ConditionFalse, + canHandle: true, + statusServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, + observedGeneration: 1, + generation: 1, }, } for _, tt := range tests { @@ -153,10 +139,10 @@ func TestServerConfig_CanHandle(t *testing.T) { ServerConfigRef: tt.statusServerConfigRef, }, } - if tt.phase != "" { + if tt.status != "" { meta.SetStatusCondition(&instance.Status.Conditions, metav1.Condition{ - Type: constants.Ready, - Reason: tt.phase, + Type: ConfigCondition, + Status: tt.status, ObservedGeneration: tt.observedGeneration, }) } @@ -286,6 +272,8 @@ func TestServerConfig_Handle(t *testing.T) { g.Expect(instance.Status.ServerConfigRef).Should(BeNil()) g.Expect(instance.Status.Conditions).To(ContainElement(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Message": ContainSubstring("Waiting for Fulcio root certificate: not-existing/cert"), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(FulcioReason), }))) }, }, @@ -320,6 +308,8 @@ func TestServerConfig_Handle(t *testing.T) { g.Expect(instance.Status.ServerConfigRef).Should(BeNil()) g.Expect(instance.Status.Conditions).To(ContainElement(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{ "Message": ContainSubstring("Waiting for Ctlog private key secret"), + "Status": Equal(metav1.ConditionFalse), + "Reason": Equal(SignerKeyReason), }))) }, }, @@ -332,7 +322,7 @@ func TestServerConfig_Handle(t *testing.T) { Trillian: rhtasv1alpha1.TrillianService{Port: ptr.To(int32(80))}, }, status: rhtasv1alpha1.CTlogStatus{ - ServerConfigRef: nil, + ServerConfigRef: &rhtasv1alpha1.LocalObjectReference{Name: "config"}, TreeID: ptr.To(int64(123456)), RootCertificates: []rhtasv1alpha1.SecretKeySelector{ {LocalObjectReference: rhtasv1alpha1.LocalObjectReference{Name: "secret"}, Key: "cert"}, @@ -364,6 +354,9 @@ func TestServerConfig_Handle(t *testing.T) { g.Expect(k8sErrors.IsNotFound(cli.Get(context.TODO(), client.ObjectKey{Name: "config", Namespace: "default"}, &v1.Secret{}))).To(BeTrue()) + secret, err := kubernetes.GetSecret(cli, "default", instance.Status.ServerConfigRef.Name) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(secret.Data).To(HaveKey("config")) }, }, }, @@ -412,6 +405,10 @@ func TestServerConfig_Handle(t *testing.T) { _, err := kubernetes.GetSecret(cli, "default", "config") g.Expect(k8sErrors.IsNotFound(err)).To(BeTrue()) + + secret, err := kubernetes.GetSecret(cli, "default", instance.Status.ServerConfigRef.Name) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(secret.Data).To(HaveKey("config")) }, }, }, diff --git a/internal/controller/ctlog/ctlog_controller.go b/internal/controller/ctlog/ctlog_controller.go index fcbc27172..50ef47041 100644 --- a/internal/controller/ctlog/ctlog_controller.go +++ b/internal/controller/ctlog/ctlog_controller.go @@ -91,7 +91,7 @@ func (r *CTlogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl target := instance.DeepCopy() acs := []action.Action[*rhtasv1alpha1.CTlog]{ transitions.NewToPendingPhaseAction[*rhtasv1alpha1.CTlog](func(_ *rhtasv1alpha1.CTlog) []string { - return []string{actions.CertCondition} + return []string{actions.CertCondition, actions.ConfigCondition} }), transitions.NewToCreatePhaseAction[*rhtasv1alpha1.CTlog](),