make StatusUpdater work with condition implementations that are not pointers

Author: Diaphteiros

pkg/controller/status_updater.go

@@ -227,10 +227,23 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context,
 		}
 		newConsRaw, _ := cu.Conditions()
 		conType := reflect.TypeOf(s.conConstruct())
+		targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false))
+		depointerize := false
+		if !reflect.SliceOf(conType).AssignableTo(targetType) {
+			// this can happen if the constructor returns a *ConditionImplementation, which creates a []*ConditionImplementation below,
+			// but the condition list field is a []ConditionImplementation
+			if conType.Kind() == reflect.Ptr {
+				depointerize = true
+				conType = conType.Elem()
+			}
+		}
 		newCons := reflect.MakeSlice(reflect.SliceOf(conType), len(newConsRaw), len(newConsRaw)).Interface()
 		for i := range newConsRaw {
-			val := reflect.ValueOf(newConsRaw[i]).Convert(conType)
-			reflect.ValueOf(newCons).Index(i).Set(val)
+			val := reflect.ValueOf(newConsRaw[i])
+			if depointerize {
+				val = val.Elem()
+			}
+			reflect.ValueOf(newCons).Index(i).Set(val.Convert(conType))
 		}
 		SetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], newCons)
 	}

pkg/controller/status_updater_test.go

@@ -202,7 +202,7 @@ func (c *Condition) SetLastTransitionTime(t time.Time) {
 }
 
 // ConditionList is a list of Conditions.
-type ConditionList []*Condition
+type ConditionList []Condition
 
 type CommonStatus struct {
 	ObservedGeneration int64 `json:"observedGeneration"`
@@ -237,11 +237,7 @@ func (in *CommonStatus) DeepCopyInto(out *CommonStatus) {
 		in, out := &in.Conditions, &out.Conditions
 		*out = make(ConditionList, len(*in))
 		for i := range *in {
-			if (*in)[i] != nil {
-				in, out := &(*in)[i], &(*out)[i]
-				*out = new(Condition)
-				(*in).DeepCopyInto(*out)
-			}
+			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
 }
@@ -278,11 +274,7 @@ func (in ConditionList) DeepCopyInto(out *ConditionList) {
 		in := &in
 		*out = make(ConditionList, len(*in))
 		for i := range *in {
-			if (*in)[i] != nil {
-				in, out := &(*in)[i], &(*out)[i]
-				*out = new(Condition)
-				(*in).DeepCopyInto(*out)
-			}
+			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
 }

pkg/testing/matchers/conditions.go

@@ -20,13 +20,32 @@ type conditionMatcher[T comparable] struct {
 	expected *ConditionImpl[T]
 }
 
+func (c *conditionMatcher[T]) GomegaString() string {
+	if c == nil || c.expected == nil {
+		return "<nil>"
+	}
+	return c.expected.String()
+}
+
 var _ types.GomegaMatcher = &conditionMatcher[bool]{}
 
 // Match implements types.GomegaMatcher.
-func (c *conditionMatcher[T]) Match(actualRaw interface{}) (success bool, err error) {
-	actual, ok := actualRaw.(conditions.Condition[T])
-	if !ok {
-		return false, fmt.Errorf("expected actual to be of type Condition[%s], got %T", reflect.TypeFor[T]().Name(), actualRaw)
+func (c *conditionMatcher[T]) Match(actualRaw any) (success bool, err error) {
+	actual, converted := actualRaw.(conditions.Condition[T])
+	if !converted {
+		// actualRaw doesn't implement conditions.Condition[T], check if a pointer to it does
+		ptrValue := reflect.New(reflect.TypeOf(actualRaw))
+		reflect.Indirect(ptrValue).Set(reflect.ValueOf(actualRaw))
+		actual, converted = ptrValue.Interface().(conditions.Condition[T])
+	}
+	if !converted {
+		return false, fmt.Errorf("expected actual (or &actual) to be of type Condition[%s], got %T", reflect.TypeFor[T]().Name(), actualRaw)
+	}
+	if actual == nil && c.expected == nil {
+		return true, nil
+	}
+	if actual == nil || c.expected == nil {
+		return false, nil
 	}
 	if c.expected.HasType() && c.expected.GetType() != actual.GetType() {
 		return false, nil
@@ -86,6 +105,39 @@ type ConditionImpl[T comparable] struct {
 	lastTransitionTime *time.Time
 }
 
+func (c *ConditionImpl[T]) String() string {
+	if c == nil {
+		return "<nil>"
+	}
+	var status, conType, reason, message, lastTransitionTime string
+	if c.status == nil {
+		status = "<arbitrary>"
+	} else {
+		status = fmt.Sprintf("%v", *c.status)
+	}
+	if c.conType == nil {
+		conType = "<arbitrary>"
+	} else {
+		conType = *c.conType
+	}
+	if c.reason == nil {
+		reason = "<arbitrary>"
+	} else {
+		reason = *c.reason
+	}
+	if c.message == nil {
+		message = "<arbitrary>"
+	} else {
+		message = *c.message
+	}
+	if c.lastTransitionTime == nil {
+		lastTransitionTime = "<arbitrary>"
+	} else {
+		lastTransitionTime = c.lastTransitionTime.Format(time.RFC3339)
+	}
+	return fmt.Sprintf("Condition[%s]{\n\tType: %q,\n\tStatus: %s,\n\tReason: %q,\n\tMessage: %q,\n\tLastTransitionTime: %v,\n}", reflect.TypeFor[T]().Name(), conType, status, reason, message, lastTransitionTime)
+}
+
 var _ conditions.Condition[bool] = &ConditionImpl[bool]{}
 
 func (c *ConditionImpl[T]) GetLastTransitionTime() time.Time {