From 8aa164a87d7e3b6eb4d1606aba4cbc5424630e08 Mon Sep 17 00:00:00 2001 From: Adam Wolfe Gordon Date: Fri, 5 Jun 2026 14:14:02 -0600 Subject: [PATCH 1/2] render: Replace condition lastTransitionTime timestamps in output The old render command used a static timestamp for status condition `lastTransitionTime`s so that output was stable across runs with the same inputs. The new render engine calls the real reconciler, and therefore produces real timestamps. Restore stability by replacing the timestamps in any status conditions returned by the render engine before outputting them. We use the same static timestamp that was present in the old CLI for consistency with its output. Fixes #50 Signed-off-by: Adam Wolfe Gordon --- cmd/crossplane/render/op/cmd.go | 11 ++ cmd/crossplane/render/output.go | 69 +++++++ cmd/crossplane/render/output_test.go | 257 +++++++++++++++++++++++++++ cmd/crossplane/render/xr/cmd.go | 11 ++ 4 files changed, 348 insertions(+) create mode 100644 cmd/crossplane/render/output.go create mode 100644 cmd/crossplane/render/output_test.go diff --git a/cmd/crossplane/render/op/cmd.go b/cmd/crossplane/render/op/cmd.go index 36bb60b..04e6bc9 100644 --- a/cmd/crossplane/render/op/cmd.go +++ b/cmd/crossplane/render/op/cmd.go @@ -267,6 +267,17 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte out.Operation.Spec = *op.Spec.DeepCopy() } + // Replace condition timestamps in the operation and any applied resources with a + // stable value. + if err := render.ReplaceConditionTimestamps(out.Operation); err != nil { + return errors.Wrap(err, "cannot replace condition timestamps in operation") + } + for i, ar := range out.AppliedResources { + if err := render.ReplaceConditionTimestamps(&out.AppliedResources[i]); err != nil { + return errors.Wrapf(err, "cannot replace condition timestamps in applied resource %s", ar.GetName()) + } + } + // Always output the Operation (with metadata and status, optionally with spec) if out.Operation != nil { _, _ = fmt.Fprintln(k.Stdout, "---") diff --git a/cmd/crossplane/render/output.go b/cmd/crossplane/render/output.go new file mode 100644 index 0000000..0f95ca3 --- /dev/null +++ b/cmd/crossplane/render/output.go @@ -0,0 +1,69 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/fieldpath" + + xpv2 "github.com/crossplane/crossplane/apis/v2/core/v2" +) + +func conditionTime() metav1.Time { + return metav1.NewTime(time.Date(2024, time.January, 1, 0, 0, 0, 0, time.UTC)) +} + +// ReplaceConditionTimestamps replaces the lastTransitionTime timestamps in any +// of the argument's conditions with a static timestamp (the same one used by +// the old CLI renderer). This makes render output stable across runs despite +// the render engine producing real timestamps. It is a no-op if the argument +// does not have a status or has a status without conditions. +func ReplaceConditionTimestamps(o runtime.Object) error { + u, ok := o.(*unstructured.Unstructured) + if !ok { + data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(o) + if err != nil { + return errors.Wrap(err, "cannot convert object to unstructured") + } + u = &unstructured.Unstructured{Object: data} + } + + fp := fieldpath.Pave(u.Object) + + // This function is a no-op for objects with no status, or whose status does + // not have conditions. + var cs xpv2.ConditionedStatus + if err := fp.GetValueInto("status", &cs); err != nil { + return nil //nolint:nilerr // See comment above. + } + + for i := range cs.Conditions { + cs.Conditions[i].LastTransitionTime = conditionTime() + } + + if err := fp.SetValue("status.conditions", cs.Conditions); err != nil { + return errors.Wrap(err, "cannot set condition timestamps") + } + + return nil +} diff --git a/cmd/crossplane/render/output_test.go b/cmd/crossplane/render/output_test.go new file mode 100644 index 0000000..b4c4faf --- /dev/null +++ b/cmd/crossplane/render/output_test.go @@ -0,0 +1,257 @@ +/* +Copyright 2026 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package render + +import ( + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "github.com/crossplane/crossplane-runtime/v2/pkg/fieldpath" + + xpv2 "github.com/crossplane/crossplane/apis/v2/core/v2" +) + +// realTime is an arbitrary non-fixed timestamp, distinct from conditionTime(), +// used to verify that ReplaceConditionTimestamps actually rewrites it. +func realTime() metav1.Time { + return metav1.NewTime(time.Date(2023, time.June, 15, 12, 30, 0, 0, time.UTC)) +} + +// conditionsOf reads status.conditions back out of an unstructured object. It +// returns nil if the object has no status or no conditions. +func conditionsOf(t *testing.T, u *unstructured.Unstructured) []xpv2.Condition { + t.Helper() + + var cs xpv2.ConditionedStatus + if err := fieldpath.Pave(u.Object).GetValueInto("status", &cs); err != nil { + return nil + } + + return cs.Conditions +} + +func TestReplaceConditionTimestamps(t *testing.T) { + type args struct { + o runtime.Object + } + type want struct { + // conditions is the expected status.conditions after the call. It is + // only asserted when checkConditions is true. + conditions []xpv2.Condition + checkConditions bool + err error + } + + cases := map[string]struct { + reason string + args args + want want + }{ + "NoStatus": { + reason: "An object with no status should be a no-op and return nil.", + args: args{ + o: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "example.org/v1", + "kind": "Thing", + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"coolField": "cool"}, + }, + }, + }, + want: want{ + conditions: nil, + checkConditions: true, + err: nil, + }, + }, + "StatusNoConditions": { + reason: "An object with a status but no conditions should be a no-op and return nil.", + args: args{ + o: &unstructured.Unstructured{Object: MustLoadJSON(`{ + "apiVersion": "example.org/v1", + "kind": "Thing", + "metadata": {"name": "test"}, + "status": {"phase": "Ready"} + }`)}, + }, + want: want{ + conditions: nil, + checkConditions: true, + err: nil, + }, + }, + "SingleCondition": { + reason: "A single condition's timestamp should be replaced, preserving its other fields.", + args: args{ + o: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "example.org/v1", + "kind": "Thing", + "metadata": map[string]any{"name": "test"}, + "status": map[string]any{ + "conditions": []xpv2.Condition{{ + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + Message: "all good", + LastTransitionTime: realTime(), + }}, + }, + }, + }, + }, + want: want{ + conditions: []xpv2.Condition{{ + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + Message: "all good", + LastTransitionTime: conditionTime(), + }}, + checkConditions: true, + err: nil, + }, + }, + "MultipleConditions": { + reason: "Every condition's timestamp should be replaced, preserving order and other fields.", + args: args{ + o: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "example.org/v1", + "kind": "Thing", + "metadata": map[string]any{"name": "test"}, + "status": map[string]any{ + "conditions": []xpv2.Condition{ + { + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + LastTransitionTime: realTime(), + }, + { + Type: "Synced", + Status: corev1.ConditionFalse, + Reason: "ReconcileError", + Message: "boom", + LastTransitionTime: metav1.NewTime(time.Date(2025, time.March, 9, 8, 0, 0, 0, time.UTC)), + }, + }, + }, + }, + }, + }, + want: want{ + conditions: []xpv2.Condition{ + { + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + LastTransitionTime: conditionTime(), + }, + { + Type: "Synced", + Status: corev1.ConditionFalse, + Reason: "ReconcileError", + Message: "boom", + LastTransitionTime: conditionTime(), + }, + }, + checkConditions: true, + err: nil, + }, + }, + "AlreadyFixedTimestamp": { + reason: "A condition already at the fixed timestamp should be unchanged (idempotent).", + args: args{ + o: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "example.org/v1", + "kind": "Thing", + "metadata": map[string]any{"name": "test"}, + "status": map[string]any{ + "conditions": []xpv2.Condition{{ + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + LastTransitionTime: conditionTime(), + }}, + }, + }, + }, + }, + want: want{ + conditions: []xpv2.Condition{{ + Type: "Ready", + Status: corev1.ConditionTrue, + Reason: "Available", + LastTransitionTime: conditionTime(), + }}, + checkConditions: true, + err: nil, + }, + }, + "TypedObjectConversion": { + // A non-*unstructured.Unstructured runtime.Object exercises the + // ToUnstructured conversion branch. The function converts into a + // local copy and does not write back to the argument, so only the + // returned error is observable here. + reason: "A typed (non-unstructured) object should be converted without error.", + args: args{ + o: &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "ConfigMap"}, + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Data: map[string]string{"key": "value"}, + }, + }, + want: want{ + checkConditions: false, + err: nil, + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ReplaceConditionTimestamps(tc.args.o) + + if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { + t.Errorf("%s\nReplaceConditionTimestamps(): -want err, +got err:\n%s", tc.reason, diff) + } + + if !tc.want.checkConditions { + return + } + + u, ok := tc.args.o.(*unstructured.Unstructured) + if !ok { + t.Fatalf("%s\ncheckConditions requires an *unstructured.Unstructured argument", tc.reason) + } + + if diff := cmp.Diff(tc.want.conditions, conditionsOf(t, u)); diff != "" { + t.Errorf("%s\nReplaceConditionTimestamps(): -want conditions, +got conditions:\n%s", tc.reason, diff) + } + }) + } +} diff --git a/cmd/crossplane/render/xr/cmd.go b/cmd/crossplane/render/xr/cmd.go index 5f4a317..20a684f 100644 --- a/cmd/crossplane/render/xr/cmd.go +++ b/cmd/crossplane/render/xr/cmd.go @@ -329,6 +329,17 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte out.CompositeResource = updatedXR } + // Replace condition timestamps in the XR and any composed resources with a + // stable value. + if err := render.ReplaceConditionTimestamps(&out.CompositeResource.Unstructured); err != nil { + return errors.Wrap(err, "cannot replace condition timestamps in xr") + } + for i, cr := range out.ComposedResources { + if err := render.ReplaceConditionTimestamps(&out.ComposedResources[i].Unstructured); err != nil { + return errors.Wrapf(err, "cannot replace condition timestamps in composed resource %s", cr.GetName()) + } + } + _, _ = fmt.Fprintln(k.Stdout, "---") if err := s.Encode(out.CompositeResource, k.Stdout); err != nil { return errors.Wrapf(err, "cannot marshal composite resource %q to YAML", xr.GetName()) From ddc2f67c2968b44dbbffc40357781f596e05627d Mon Sep 17 00:00:00 2001 From: Adam Wolfe Gordon Date: Fri, 5 Jun 2026 14:15:45 -0600 Subject: [PATCH 2/2] render: Sort composed resources by `composition-resource-name` annotation The old CLI's `render` command sorted composed resources by the `composition-resource-name` annotation to ensure output consistency across runs with the same inputs. The real reconciler code used by the new render engine does not sort the results, so we need to sort them client-side to ensure stability. Fixes #53 Signed-off-by: Adam Wolfe Gordon --- cmd/crossplane/render/xr/cmd.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cmd/crossplane/render/xr/cmd.go b/cmd/crossplane/render/xr/cmd.go index 20a684f..4621acc 100644 --- a/cmd/crossplane/render/xr/cmd.go +++ b/cmd/crossplane/render/xr/cmd.go @@ -22,6 +22,8 @@ import ( "fmt" "os" "path/filepath" + "slices" + "strings" "time" "dario.cat/mergo" @@ -340,6 +342,20 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, sp terminal.SpinnerPrinte } } + // Sort composite resources by composition-resource-name to ensure stable + // output across runs. + slices.SortStableFunc(out.ComposedResources, func(a, b composed.Unstructured) int { + nameA, nameB := "", "" + if anns := a.GetAnnotations(); anns != nil { + nameA = anns[xcrd.AnnotationKeyCompositionResourceName] + } + if anns := b.GetAnnotations(); anns != nil { + nameB = anns[xcrd.AnnotationKeyCompositionResourceName] + } + + return strings.Compare(nameA, nameB) + }) + _, _ = fmt.Fprintln(k.Stdout, "---") if err := s.Encode(out.CompositeResource, k.Stdout); err != nil { return errors.Wrapf(err, "cannot marshal composite resource %q to YAML", xr.GetName())