feat(cli): include status conditions on XR output of render command

Signed-off-by: Jared Watts <[email protected]>

Author: jbw976

cmd/crank/render/cmd.go

@@ -44,15 +44,14 @@ type Cmd struct {
 	Functions         string `arg:"" help:"A YAML file or directory of YAML files specifying the Composition Functions to use to render the XR." type:"path"`
 
 	// Flags. Keep them in alphabetical order.
-	ContextFiles            map[string]string `help:"Comma-separated context key-value pairs to pass to the Function pipeline. Values must be files containing JSON."                           mapsep:""`
-	ContextValues           map[string]string `help:"Comma-separated context key-value pairs to pass to the Function pipeline. Values must be JSON. Keys take precedence over --context-files." mapsep:""`
-	IncludeFunctionResults  bool              `help:"Include informational and warning messages from Functions in the rendered output as resources of kind: Result."                            short:"r"`
-	IncludeFullXR           bool              `help:"Include a direct copy of the input XR's spec and metadata fields in the rendered output."                                                  short:"x"`
-	IncludeStatusConditions bool              `help:"Include the status conditions in the rendered output as a resource of kind: Condition."                                                    short:"s"`
-	ObservedResources       string            `help:"A YAML file or directory of YAML files specifying the observed state of composed resources."                                               placeholder:"PATH" short:"o"   type:"path"`
-	ExtraResources          string            `help:"A YAML file or directory of YAML files specifying extra resources to pass to the Function pipeline."                                       placeholder:"PATH" short:"e"   type:"path"`
-	IncludeContext          bool              `help:"Include the context in the rendered output as a resource of kind: Context."                                                                short:"c"`
-	FunctionCredentials     string            `help:"A YAML file or directory of YAML files specifying credentials to use for Functions to render the XR."                                      placeholder:"PATH" type:"path"`
+	ContextFiles           map[string]string `help:"Comma-separated context key-value pairs to pass to the Function pipeline. Values must be files containing JSON."                           mapsep:""`
+	ContextValues          map[string]string `help:"Comma-separated context key-value pairs to pass to the Function pipeline. Values must be JSON. Keys take precedence over --context-files." mapsep:""`
+	IncludeFunctionResults bool              `help:"Include informational and warning messages from Functions in the rendered output as resources of kind: Result."                            short:"r"`
+	IncludeFullXR          bool              `help:"Include a direct copy of the input XR's spec and metadata fields in the rendered output."                                                  short:"x"`
+	ObservedResources      string            `help:"A YAML file or directory of YAML files specifying the observed state of composed resources."                                               placeholder:"PATH" short:"o"   type:"path"`
+	ExtraResources         string            `help:"A YAML file or directory of YAML files specifying extra resources to pass to the Function pipeline."                                       placeholder:"PATH" short:"e"   type:"path"`
+	IncludeContext         bool              `help:"Include the context in the rendered output as a resource of kind: Context."                                                                short:"c"`
+	FunctionCredentials    string            `help:"A YAML file or directory of YAML files specifying credentials to use for Functions to render the XR."                                      placeholder:"PATH" type:"path"`
 
 	Timeout time.Duration `default:"1m" help:"How long to run before timing out."`
 
@@ -271,15 +270,6 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger) error { //nolint:gocognit
 		}
 	}
 
-	if c.IncludeStatusConditions {
-		for i := range out.Conditions {
-			_, _ = fmt.Fprintln(k.Stdout, "---")
-			if err := s.Encode(&out.Conditions[i], os.Stdout); err != nil {
-				return errors.Wrap(err, "cannot marshal condition to YAML")
-			}
-		}
-	}
-
 	if c.IncludeContext {
 		_, _ = fmt.Fprintln(k.Stdout, "---")
 		if err := s.Encode(out.Context, k.Stdout); err != nil {

cmd/crank/render/render.go

@@ -85,7 +85,6 @@ type Outputs struct {
 	ComposedResources []composed.Unstructured
 	Results           []unstructured.Unstructured
 	Context           *unstructured.Unstructured
-	Conditions        []unstructured.Unstructured
 
 	// TODO(negz): Allow returning desired XR connection details. Maybe as a
 	// Secret? Should we honor writeConnectionSecretToRef? What if secret stores
@@ -100,15 +99,6 @@ type RuntimeFunctionRunner struct {
 	mx       sync.Mutex
 }
 
-// A CompositionTarget is the target of a composition event or condition.
-type CompositionTarget string
-
-// Composition event and condition targets.
-const (
-	CompositionTargetComposite         CompositionTarget = "Composite"
-	CompositionTargetCompositeAndClaim CompositionTarget = "CompositeAndClaim"
-)
-
 // NewRuntimeFunctionRunner returns a FunctionRunner that runs functions
 // locally, using the runtime configured in their annotations (e.g. Docker). It
 // starts all the functions and creates gRPC connections when called.
@@ -217,7 +207,7 @@ func Render(ctx context.Context, log logging.Logger, in Inputs) (Outputs, error)
 	d := &fnv1.State{}
 
 	results := make([]unstructured.Unstructured, 0)
-	conditions := make([]unstructured.Unstructured, 0)
+	conditions := make([]xpv1.Condition, 0)
 
 	// The Function context starts empty.
 	fctx := &structpb.Struct{Fields: map[string]*structpb.Value{}}
@@ -293,16 +283,13 @@ func Render(ctx context.Context, log logging.Logger, in Inputs) (Outputs, error)
 				status = corev1.ConditionUnknown
 			}
 
-			conditions = append(conditions, unstructured.Unstructured{Object: map[string]any{
-				"apiVersion":         "render.crossplane.io/v1beta1",
-				"kind":               "Condition",
-				"type":               c.GetType(),
-				"status":             status,
-				"lastTransitionTime": metav1.Now(),
-				"reason":             xpv1.ConditionReason(c.GetReason()),
-				"message":            c.GetMessage(),
-				"target":             convertTarget(c.GetTarget()),
-			}})
+			conditions = append(conditions, xpv1.Condition{
+				Type:               xpv1.ConditionType(c.GetType()),
+				Status:             status,
+				LastTransitionTime: conditionTime(),
+				Reason:             xpv1.ConditionReason(c.GetReason()),
+				Message:            c.GetMessage(),
+			})
 		}
 
 		// Results of fatal severity stop the Composition process.
@@ -369,12 +356,21 @@ func Render(ctx context.Context, log logging.Logger, in Inputs) (Outputs, error)
 	if len(unready) > 0 {
 		xrCond = xpv1.Creating().WithMessage(fmt.Sprintf("Unready resources: %s", resource.StableNAndSomeMore(resource.DefaultFirstN, unready)))
 	}
-	// lastTransitionTime would just be noise, but we can't drop it as it's a
-	// required field and null is not allowed, so we set a random time.
-	xrCond.LastTransitionTime = metav1.NewTime(time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC))
+	xrCond.LastTransitionTime = conditionTime()
 	xr.SetConditions(xrCond)
 
-	out := Outputs{CompositeResource: xr, ComposedResources: desired, Results: results, Conditions: conditions}
+	for _, c := range conditions {
+		if xpv1.IsSystemConditionType(c.Type) {
+			// Do not let users update system conditions.
+			continue
+		}
+		// update the XR with the current condition. If it targets the claim, we
+		// could also set it on the claim here, but we don't support Claims in
+		// render yet.
+		xr.SetConditions(c)
+	}
+
+	out := Outputs{CompositeResource: xr, ComposedResources: desired, Results: results}
 	if fctx != nil {
 		out.Context = &unstructured.Unstructured{Object: map[string]any{
 			"apiVersion": "render.crossplane.io/v1beta1",
@@ -449,9 +445,8 @@ func (f *FilteringFetcher) Fetch(_ context.Context, rs *fnv1.ResourceSelector) (
 	return out, nil
 }
 
-func convertTarget(t fnv1.Target) CompositionTarget {
-	if t == fnv1.Target_TARGET_COMPOSITE_AND_CLAIM {
-		return CompositionTargetCompositeAndClaim
-	}
-	return CompositionTargetComposite
+func conditionTime() metav1.Time {
+	// lastTransitionTime for conditions would just be noise, but we can't drop it
+	// as it's a required field and null is not allowed, so we set a random time.
+	return metav1.NewTime(time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC))
 }

cmd/crank/render/render_test.go

@@ -21,7 +21,6 @@ import (
 	"io"
 	"net"
 	"testing"
-	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -35,7 +34,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/ptr"
 
-	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	"github.com/crossplane/crossplane-runtime/pkg/logging"
 	"github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composed"
 	ucomposite "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured/composite"
@@ -292,13 +290,22 @@ func TestRender(t *testing.T) {
 								},
 								"status": {
 									"widgets": 9001,
-									"conditions": [{
-										"lastTransitionTime": "2024-01-01T00:00:00Z",
-										"type": "Ready",
-										"status": "False",
-										"reason": "Creating",
-										"message": "Unready resources: a-cool-resource, b-cool-resource"
-									}]
+									"conditions": [
+										{
+											"lastTransitionTime": "2024-01-01T00:00:00Z",
+											"type": "Ready",
+											"status": "False",
+											"reason": "Creating",
+											"message": "Unready resources: a-cool-resource, b-cool-resource"
+										},
+										{
+											"lastTransitionTime": "2024-01-01T00:00:00Z",
+											"type": "ProvisioningSuccess",
+											"status": "True",
+											"reason": "Provisioned",
+											"message": "Provisioned successfully"
+										}
+									]
 								}
 							}`),
 						},
@@ -361,20 +368,6 @@ func TestRender(t *testing.T) {
 							},
 						},
 					},
-					Conditions: []unstructured.Unstructured{
-						{
-							Object: map[string]any{
-								"apiVersion":         "render.crossplane.io/v1beta1",
-								"kind":               "Condition",
-								"type":               "ProvisioningSuccess",
-								"status":             corev1.ConditionTrue,
-								"reason":             xpv1.ConditionReason("Provisioned"),
-								"message":            "Provisioned successfully",
-								"target":             CompositionTargetCompositeAndClaim,
-								"lastTransitionTime": metav1.Now(),
-							},
-						},
-					},
 				},
 			},
 		},
@@ -771,7 +764,7 @@ func TestRender(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			out, err := Render(tc.args.ctx, logging.NewNopLogger(), tc.args.in)
 
-			if diff := cmp.Diff(tc.want.out, out, cmpopts.EquateEmpty(), cmpopts.EquateApproxTime(time.Second)); diff != "" {
+			if diff := cmp.Diff(tc.want.out, out, cmpopts.EquateEmpty()); diff != "" {
 				t.Errorf("%s\nRender(...): -want, +got:\n%s", tc.reason, diff)
 			}
 			if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" {