Merge pull request #693 from securesign/tturek/ctlog-server-config

fix(ctlog): cleanup only linked server configs
securesign · Oct 22, 2024 · 6d8d851 · 6d8d851
2 parents 8824601 + 863bc93
commit 6d8d851
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 90 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
 	})