fix: replace illegal chars in all condition updates (#175)

* fix: ensure compliant condition type and reason

* feat: release v0.23.3

* update

Author: reshnm

VERSION

@@ -1 +1 @@
-v0.23.2-dev
+v0.23.3

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/onsi/ginkgo/v2 v2.27.2
 	github.com/onsi/gomega v1.38.2
-	github.com/openmcp-project/controller-utils/api v0.23.2
+	github.com/openmcp-project/controller-utils/api v0.23.3
 	github.com/spf13/pflag v1.0.10
 	github.com/stretchr/testify v1.11.1
 	go.uber.org/zap v1.27.0

pkg/conditions/updater.go

@@ -90,8 +90,7 @@ func (c *conditionUpdater) WithEventRecorder(recorder record.EventRecorder, verb
 func (c *conditionUpdater) UpdateCondition(conType string, status metav1.ConditionStatus, observedGeneration int64, reason, message string) *conditionUpdater {
 	if reason == "" {
 		// the metav1.Condition type requires a reason, so let's add a dummy if none is given
-		// the type field allows dots ('.'), while the reason field does not, so replace them with colons (':') (which are not allowed in the type field)
-		reason = strings.ReplaceAll(conType, ".", ":") + "_" + string(status)
+		reason = ReplaceIllegalCharsInConditionReason(conType + "_" + string(status))
 	}
 	con := metav1.Condition{
 		Type:               conType,
@@ -263,3 +262,79 @@ func (c *conditionUpdater) Record(obj runtime.Object) *conditionUpdater {
 
 	return c
 }
+
+// ReplaceIllegalCharsInConditionType replaces all characters in the given string that are not allowed in condition types with underscores.
+func ReplaceIllegalCharsInConditionType(s string) string {
+	if s == "" {
+		return s
+	}
+
+	result := make([]rune, 0, len(s))
+	for _, r := range s {
+		// Valid characters for condition types are:
+		// - lowercase letters (a-z)
+		// - uppercase letters (A-Z)
+		// - digits (0-9)
+		// - underscore (_)
+		// - hyphen (-)
+		// - dot (.)
+		// All other characters are replaced with underscore
+		if (r >= 'a' && r <= 'z') ||
+			(r >= 'A' && r <= 'Z') ||
+			(r >= '0' && r <= '9') ||
+			r == '_' ||
+			r == '-' ||
+			r == '.' {
+			result = append(result, r)
+		} else {
+			result = append(result, '_')
+		}
+	}
+	return ensureStartsAndEndsWithLetter(string(result), 'T', 't')
+}
+
+// ReplaceIllegalCharsInConditionReason replaces all characters in the given string that are not allowed in condition reasons with underscores.
+func ReplaceIllegalCharsInConditionReason(s string) string {
+	if s == "" {
+		return s
+	}
+
+	result := make([]rune, 0, len(s))
+	for _, r := range s {
+		// Valid characters for condition types are:
+		// - lowercase letters (a-z)
+		// - uppercase letters (A-Z)
+		// - digits (0-9)
+		// - underscore (_)
+		// - colon (:)
+		//-  comma (,)
+		// All other characters are replaced with underscore
+		if (r >= 'a' && r <= 'z') ||
+			(r >= 'A' && r <= 'Z') ||
+			(r >= '0' && r <= '9') ||
+			r == '_' ||
+			r == ':' ||
+			r == ',' {
+			result = append(result, r)
+		} else {
+			result = append(result, '_')
+		}
+	}
+	return ensureStartsAndEndsWithLetter(string(result), 'R', 'r')
+}
+
+func ensureStartsAndEndsWithLetter(s string, start, end rune) string {
+	if s == "" {
+		return s
+	}
+	runes := []rune(s)
+	// Ensure starts with letter
+	if (runes[0] < 'a' || runes[0] > 'z') && (runes[0] < 'A' || runes[0] > 'Z') {
+		runes = append([]rune{start}, runes...)
+	}
+	// Ensure ends with letter
+	if (runes[len(runes)-1] < 'a' || runes[len(runes)-1] > 'z') && (runes[len(runes)-1] < 'A' || runes[len(runes)-1] > 'Z') {
+		runes = append(runes, end)
+	}
+	return string(runes)
+}

pkg/conditions/updater_test.go

@@ -238,7 +238,7 @@ var _ = Describe("Conditions", func() {
 				MatchCondition(TestCondition().
 					WithType("TestCondition.Test").
 					WithStatus(metav1.ConditionFalse).
-					WithReason("TestCondition:Test_False").
+					WithReason("TestCondition_Test_False").
 					WithMessage("")),
 			))
 		})

pkg/controller/status_updater.go

@@ -449,71 +449,11 @@ func GenerateCreateConditionFunc[Obj client.Object](rr *ReconcileResult[Obj]) fu
 	}
 	return func(conType string, status metav1.ConditionStatus, reason, message string) {
 		rr.Conditions = append(rr.Conditions, metav1.Condition{
-			Type:               ReplaceIllegalCharsInConditionType(conType),
+			Type:               conditions.ReplaceIllegalCharsInConditionType(conType),
 			Status:             status,
 			ObservedGeneration: gen,
-			Reason:             ReplaceIllegalCharsInConditionReason(reason),
+			Reason:             conditions.ReplaceIllegalCharsInConditionReason(reason),
 			Message:            message,
 		})
 	}
 }
-
-// ReplaceIllegalCharsInConditionType replaces all characters in the given string that are not allowed in condition types with underscores.
-func ReplaceIllegalCharsInConditionType(s string) string {
-	if s == "" {
-		return s
-	}
-
-	result := make([]rune, 0, len(s))
-	for _, r := range s {
-		// Valid characters for condition types are:
-		// - lowercase letters (a-z)
-		// - uppercase letters (A-Z)
-		// - digits (0-9)
-		// - underscore (_)
-		// - hyphen (-)
-		// - dot (.)
-		// All other characters are replaced with underscore
-		if (r >= 'a' && r <= 'z') ||
-			(r >= 'A' && r <= 'Z') ||
-			(r >= '0' && r <= '9') ||
-			r == '_' ||
-			r == '-' ||
-			r == '.' {
-			result = append(result, r)
-		} else {
-			result = append(result, '_')
-		}
-	}
-	return string(result)
-}
-
-// ReplaceIllegalCharsInConditionReason replaces all characters in the given string that are not allowed in condition reasons with underscores.
-func ReplaceIllegalCharsInConditionReason(s string) string {
-	if s == "" {
-		return s
-	}
-
-	result := make([]rune, 0, len(s))
-	for _, r := range s {
-		// Valid characters for condition types are:
-		// - lowercase letters (a-z)
-		// - uppercase letters (A-Z)
-		// - digits (0-9)
-		// - underscore (_)
-		// - colon (:)
-		//-  comma (,)
-		// All other characters are replaced with underscore
-		if (r >= 'a' && r <= 'z') ||
-			(r >= 'A' && r <= 'Z') ||
-			(r >= '0' && r <= '9') ||
-			r == '_' ||
-			r == ':' ||
-			r == ',' {
-			result = append(result, r)
-		} else {
-			result = append(result, '_')
-		}
-	}
-	return string(result)
-}

pkg/controller/status_updater_test.go

@@ -128,10 +128,10 @@ var _ = Describe("Status Updater", func() {
 		}
 		condFunc := controller.GenerateCreateConditionFunc(rr)
 
-		condFunc("CondType :,;-_.Test02@", metav1.ConditionTrue, "Reason -.,:_Test93$", "Message")
+		condFunc("0CondType :,;-_.Test02@", metav1.ConditionTrue, "1Reason -.,:_Test93$", "Message")
 		Expect(rr.Conditions).To(HaveLen(1))
-		Expect(rr.Conditions[0].Type).To(Equal("CondType____-_.Test02_"))
-		Expect(rr.Conditions[0].Reason).To(Equal("Reason___,:_Test93_"))
+		Expect(rr.Conditions[0].Type).To(Equal("T0CondType____-_.Test02_t"))
+		Expect(rr.Conditions[0].Reason).To(Equal("R1Reason___,:_Test93_r"))
 	})
 
 	It("should not update disabled fields", func() {