Merge pull request #738 from dalton-hill-0/fn-conditions

Composite Receiver Functions for Conditions

apis/common/v1/condition.go

@@ -34,6 +34,17 @@ const (
 	// TypeSynced resources are believed to be in sync with the
 	// Kubernetes resources that manage their lifecycle.
 	TypeSynced ConditionType = "Synced"
+
+	// TypeHealthy resources are believed to be in a healthy state and to have all
+	// of their child resources in a healthy state. For example, a claim is
+	// healthy when the claim is synced and the underlying composite resource is
+	// both synced and healthy. A composite resource is healthy when the composite
+	// resource is synced and all composed resources are synced and, if
+	// applicable, healthy (e.g., the composed resource is a composite resource).
+	// TODO: This condition is not yet implemented. It is currently just reserved
+	// as a system condition. See the tracking issue for more details
+	// https://github.com/crossplane/crossplane/issues/5643.
+	TypeHealthy ConditionType = "Healthy"
 )
 
 // A ConditionReason represents the reason a resource is in a condition.
@@ -107,6 +118,16 @@ func (c Condition) WithObservedGeneration(gen int64) Condition {
 	return c
 }
 
+// IsSystemConditionType returns true if the condition is owned by the
+// Crossplane system (e.g, Ready, Synced, Healthy).
+func IsSystemConditionType(t ConditionType) bool {
+	switch t {
+	case TypeReady, TypeSynced, TypeHealthy:
+		return true
+	}
+	return false
+}
+
 // NOTE(negz): Conditions are implemented as a slice rather than a map to comply
 // with Kubernetes API conventions. Ideally we'd comply by using a map that
 // marshalled to a JSON array, but doing so confuses the CRD schema generator.

apis/common/v1/condition_test.go

@@ -257,3 +257,35 @@ func TestConditionWithObservedGeneration(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSystemConditionType(t *testing.T) {
+	cases := map[string]struct {
+		c    Condition
+		want bool
+	}{
+		"SystemReady": {
+			c:    Condition{Type: ConditionType("Ready")},
+			want: true,
+		},
+		"SystemSynced": {
+			c:    Condition{Type: ConditionType("Synced")},
+			want: true,
+		},
+		"SystemHealthy": {
+			c:    Condition{Type: ConditionType("Healthy")},
+			want: true,
+		},
+		"Custom": {
+			c:    Condition{Type: ConditionType("Custom")},
+			want: false,
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			if diff := cmp.Diff(tc.want, IsSystemConditionType(tc.c.Type)); diff != "" {
+				t.Errorf("IsSystemConditionType(tc.c.Type): -want, +got:\n%s", diff)
+			}
+		})
+	}
+}

pkg/resource/unstructured/composite/composite.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+	"github.com/crossplane/crossplane-runtime/pkg/errors"
 	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
 	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
 )
@@ -223,6 +224,14 @@ func (c *Unstructured) SetConditions(conditions ...xpv1.Condition) {
 	_ = fieldpath.Pave(c.Object).SetValue("status.conditions", conditioned.Conditions)
 }
 
+// GetConditions of this Composite resource.
+func (c *Unstructured) GetConditions() []xpv1.Condition {
+	conditioned := xpv1.ConditionedStatus{}
+	// The path is directly `status` because conditions are inline.
+	_ = fieldpath.Pave(c.Object).GetValueInto("status", &conditioned)
+	return conditioned.Conditions
+}
+
 // GetConnectionDetailsLastPublishedTime of this Composite resource.
 func (c *Unstructured) GetConnectionDetailsLastPublishedTime() *metav1.Time {
 	out := &metav1.Time{}
@@ -273,3 +282,33 @@ func (c *Unstructured) GetObservedGeneration() int64 {
 	_ = fieldpath.Pave(c.Object).GetValueInto("status", status)
 	return status.GetObservedGeneration()
 }
+
+// SetClaimConditionTypes of this Composite resource. You cannot set system
+// condition types such as Ready, Synced or Healthy as claim conditions.
+func (c *Unstructured) SetClaimConditionTypes(in ...xpv1.ConditionType) error {
+	ts := c.GetClaimConditionTypes()
+	m := make(map[xpv1.ConditionType]bool, len(ts))
+	for _, t := range ts {
+		m[t] = true
+	}
+
+	for _, t := range in {
+		if xpv1.IsSystemConditionType(t) {
+			return errors.Errorf("cannot set system condition %s as a claim condition", t)
+		}
+		if m[t] {
+			continue
+		}
+		m[t] = true
+		ts = append(ts, t)
+	}
+	_ = fieldpath.Pave(c.Object).SetValue("status.claimConditionTypes", ts)
+	return nil
+}
+
+// GetClaimConditionTypes of this Composite resource.
+func (c *Unstructured) GetClaimConditionTypes() []xpv1.ConditionType {
+	cs := []xpv1.ConditionType{}
+	_ = fieldpath.Pave(c.Object).GetValueInto("status.claimConditionTypes", &cs)
+	return cs
+}

pkg/resource/unstructured/composite/composite_test.go

@@ -18,6 +18,7 @@ package composite
 
 import (
 	"encoding/json"
+	"errors"
 	"testing"
 	"time"
 
@@ -30,6 +31,7 @@ import (
 
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/claim"
+	"github.com/crossplane/crossplane-runtime/pkg/test"
 )
 
 var _ client.Object = &Unstructured{}
@@ -69,34 +71,38 @@ func TestWithGroupVersionKind(t *testing.T) {
 
 func TestConditions(t *testing.T) {
 	cases := map[string]struct {
-		reason string
-		u      *Unstructured
-		set    []xpv1.Condition
-		get    xpv1.ConditionType
-		want   xpv1.Condition
+		reason  string
+		u       *Unstructured
+		set     []xpv1.Condition
+		get     xpv1.ConditionType
+		want    xpv1.Condition
+		wantAll []xpv1.Condition
 	}{
 		"NewCondition": {
-			reason: "It should be possible to set a condition of an empty Unstructured.",
-			u:      New(),
-			set:    []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
-			get:    xpv1.TypeReady,
-			want:   xpv1.Available(),
+			reason:  "It should be possible to set a condition of an empty Unstructured.",
+			u:       New(),
+			set:     []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
+			get:     xpv1.TypeReady,
+			want:    xpv1.Available(),
+			wantAll: []xpv1.Condition{xpv1.Available(), xpv1.ReconcileSuccess()},
 		},
 		"ExistingCondition": {
-			reason: "It should be possible to overwrite a condition that is already set.",
-			u:      New(WithConditions(xpv1.Creating())),
-			set:    []xpv1.Condition{xpv1.Available()},
-			get:    xpv1.TypeReady,
-			want:   xpv1.Available(),
+			reason:  "It should be possible to overwrite a condition that is already set.",
+			u:       New(WithConditions(xpv1.Creating())),
+			set:     []xpv1.Condition{xpv1.Available()},
+			get:     xpv1.TypeReady,
+			want:    xpv1.Available(),
+			wantAll: []xpv1.Condition{xpv1.Available()},
 		},
 		"WeirdStatus": {
 			reason: "It should not be possible to set a condition when status is not an object.",
 			u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
 				"status": "wat",
 			}}},
-			set:  []xpv1.Condition{xpv1.Available()},
-			get:  xpv1.TypeReady,
-			want: xpv1.Condition{},
+			set:     []xpv1.Condition{xpv1.Available()},
+			get:     xpv1.TypeReady,
+			want:    xpv1.Condition{},
+			wantAll: nil,
 		},
 		"WeirdStatusConditions": {
 			reason: "Conditions should be overwritten if they are not an object.",
@@ -105,19 +111,99 @@ func TestConditions(t *testing.T) {
 					"conditions": "wat",
 				},
 			}}},
-			set:  []xpv1.Condition{xpv1.Available()},
-			get:  xpv1.TypeReady,
-			want: xpv1.Available(),
+			set:     []xpv1.Condition{xpv1.Available()},
+			get:     xpv1.TypeReady,
+			want:    xpv1.Available(),
+			wantAll: []xpv1.Condition{xpv1.Available()},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			tc.u.SetConditions(tc.set...)
+
 			got := tc.u.GetCondition(tc.get)
 			if diff := cmp.Diff(tc.want, got); diff != "" {
 				t.Errorf("\n%s\nu.GetCondition(%s): -want, +got:\n%s", tc.reason, tc.get, diff)
 			}
+
+			gotAll := tc.u.GetConditions()
+			if diff := cmp.Diff(tc.wantAll, gotAll); diff != "" {
+				t.Errorf("\n%s\nu.GetConditions(): -want, +got:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}
+
+func TestClaimConditionTypes(t *testing.T) {
+	cases := map[string]struct {
+		reason  string
+		u       *Unstructured
+		set     []xpv1.ConditionType
+		want    []xpv1.ConditionType
+		wantErr error
+	}{
+		"CannotSetSystemConditionTypes": {
+			reason: "Claim conditions API should fail to set conditions if a system condition is detected.",
+			u:      New(),
+			set: []xpv1.ConditionType{
+				xpv1.ConditionType("DatabaseReady"),
+				xpv1.ConditionType("NetworkReady"),
+				// system condition
+				xpv1.ConditionType("Ready"),
+			},
+			want:    []xpv1.ConditionType{},
+			wantErr: errors.New("cannot set system condition Ready as a claim condition"),
+		},
+		"SetSingleCustomConditionType": {
+			reason: "Claim condition API should work with a single custom condition type.",
+			u:      New(),
+			set:    []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+			want:   []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+		},
+		"SetMultipleCustomConditionTypes": {
+			reason: "Claim condition API should work with multiple custom condition types.",
+			u:      New(),
+			set:    []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("NetworkReady")},
+			want:   []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("NetworkReady")},
+		},
+		"SetMultipleOfTheSameCustomConditionTypes": {
+			reason: "Claim condition API not add more than one of the same condition.",
+			u:      New(),
+			set:    []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady"), xpv1.ConditionType("DatabaseReady")},
+			want:   []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+		},
+		"WeirdStatus": {
+			reason: "It should not be possible to set a condition when status is not an object.",
+			u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
+				"status": "wat",
+			}}},
+			set:  []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+			want: []xpv1.ConditionType{},
+		},
+		"WeirdStatusClaimConditionTypes": {
+			reason: "Claim conditions should be overwritten if they are not an object.",
+			u: &Unstructured{unstructured.Unstructured{Object: map[string]any{
+				"status": map[string]any{
+					"claimConditionTypes": "wat",
+				},
+			}}},
+			set:  []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+			want: []xpv1.ConditionType{xpv1.ConditionType("DatabaseReady")},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			gotErr := tc.u.SetClaimConditionTypes(tc.set...)
+			if diff := cmp.Diff(tc.wantErr, gotErr, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nu.SetClaimConditionTypes(): -want, +got:\n%s", tc.reason, diff)
+			}
+
+			got := tc.u.GetClaimConditionTypes()
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("\n%s\nu.GetClaimConditionTypes(): -want, +got:\n%s", tc.reason, diff)
+			}
 		})
 	}
 }