From 0ef84fff04d0a1910e2121c19340276b38c5bc66 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 12:29:31 -0400 Subject: [PATCH 1/8] Initial pass of composition diffing Signed-off-by: Jonathan Ogilvie --- .../client/crossplane/composition_client.go | 78 ++ cmd/diff/cmd_utils.go | 82 ++ cmd/diff/comp.go | 139 ++++ cmd/diff/diff_integration_test.go | 742 ++++++++++++++---- cmd/diff/diff_test.go | 34 +- cmd/diff/diffprocessor/comp_processor.go | 422 ++++++++++ cmd/diff/diffprocessor/comp_processor_test.go | 564 +++++++++++++ cmd/diff/diffprocessor/diff_processor.go | 37 +- cmd/diff/diffprocessor/diff_processor_test.go | 14 +- cmd/diff/main.go | 4 +- cmd/diff/testdata/comp/crds/nopclaim-crd.yaml | 91 +++ .../comp/crds/xdefaultresource-ns-crd.yaml | 79 ++ .../comp/crds/xdownstreamenvresource-crd.yaml | 55 ++ .../crds/xdownstreamresource-cluster-crd.yaml | 82 ++ ...xdownstreamresource-legacycluster-crd.yaml | 79 ++ .../comp/crds/xdownstreamresource-ns-crd.yaml | 82 ++ .../testdata/comp/crds/xenvresource-crd.yaml | 71 ++ .../comp/crds/xnopresource-cluster-crd.yaml | 88 +++ .../crds/xnopresource-legacycluster-crd.yaml | 85 ++ .../comp/crds/xnopresource-ns-crd.yaml | 89 +++ .../testdata/comp/net-new-composition.yaml | 37 + .../comp/resources/custom-namespace.yaml | 4 + .../existing-custom-ns-downstream.yaml | 13 + .../comp/resources/existing-custom-ns-xr.yaml | 10 + .../comp/resources/existing-downstream-1.yaml | 13 + .../comp/resources/existing-downstream-2.yaml | 13 + .../comp/resources/existing-xr-1.yaml | 10 + .../comp/resources/existing-xr-2.yaml | 10 + .../testdata/comp/resources/functions.yaml | 29 + .../resources/original-composition-2.yaml | 33 + .../comp/resources/original-composition.yaml | 33 + cmd/diff/testdata/comp/resources/xrd.yaml | 51 ++ .../testdata/comp/updated-composition-2.yaml | 33 + .../testdata/comp/updated-composition.yaml | 33 + cmd/diff/testutils/mock_builder.go | 36 +- cmd/diff/testutils/mocks.go | 27 +- cmd/diff/types/types.go | 29 + cmd/diff/{diff.go => xr.go} | 127 +-- 38 files changed, 3186 insertions(+), 272 deletions(-) create mode 100644 cmd/diff/cmd_utils.go create mode 100644 cmd/diff/comp.go create mode 100644 cmd/diff/diffprocessor/comp_processor.go create mode 100644 cmd/diff/diffprocessor/comp_processor_test.go create mode 100644 cmd/diff/testdata/comp/crds/nopclaim-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xdefaultresource-ns-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xdownstreamenvresource-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xdownstreamresource-cluster-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xdownstreamresource-legacycluster-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xdownstreamresource-ns-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xenvresource-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xnopresource-cluster-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xnopresource-legacycluster-crd.yaml create mode 100644 cmd/diff/testdata/comp/crds/xnopresource-ns-crd.yaml create mode 100644 cmd/diff/testdata/comp/net-new-composition.yaml create mode 100644 cmd/diff/testdata/comp/resources/custom-namespace.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-custom-ns-downstream.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-custom-ns-xr.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-downstream-1.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-downstream-2.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-xr-1.yaml create mode 100644 cmd/diff/testdata/comp/resources/existing-xr-2.yaml create mode 100644 cmd/diff/testdata/comp/resources/functions.yaml create mode 100644 cmd/diff/testdata/comp/resources/original-composition-2.yaml create mode 100644 cmd/diff/testdata/comp/resources/original-composition.yaml create mode 100644 cmd/diff/testdata/comp/resources/xrd.yaml create mode 100644 cmd/diff/testdata/comp/updated-composition-2.yaml create mode 100644 cmd/diff/testdata/comp/updated-composition.yaml create mode 100644 cmd/diff/types/types.go rename cmd/diff/{diff.go => xr.go} (61%) diff --git a/cmd/diff/client/crossplane/composition_client.go b/cmd/diff/client/crossplane/composition_client.go index c26f672..c992049 100644 --- a/cmd/diff/client/crossplane/composition_client.go +++ b/cmd/diff/client/crossplane/composition_client.go @@ -28,6 +28,9 @@ type CompositionClient interface { // GetComposition gets a composition by name GetComposition(ctx context.Context, name string) (*apiextensionsv1.Composition, error) + + // FindXRsUsingComposition finds all XRs that use the specified composition + FindXRsUsingComposition(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) } // DefaultCompositionClient implements CompositionClient. @@ -424,3 +427,78 @@ func (c *DefaultCompositionClient) findByTypeReference(ctx context.Context, _ *u return compatibleCompositions[0], nil } + +// FindXRsUsingComposition finds all XRs that use the specified composition. +func (c *DefaultCompositionClient) FindXRsUsingComposition(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) { + c.logger.Debug("Finding XRs using composition", + "compositionName", compositionName, + "namespace", namespace) + + // First, get the composition to understand what XR type it targets + comp, err := c.GetComposition(ctx, compositionName) + if err != nil { + return nil, errors.Wrapf(err, "cannot get composition %s", compositionName) + } + + // Get the XR type this composition targets + xrAPIVersion := comp.Spec.CompositeTypeRef.APIVersion + xrKind := comp.Spec.CompositeTypeRef.Kind + + // Parse the GVK + gv, err := schema.ParseGroupVersion(xrAPIVersion) + if err != nil { + return nil, errors.Wrapf(err, "cannot parse API version %s", xrAPIVersion) + } + + xrGVK := schema.GroupVersionKind{ + Group: gv.Group, + Version: gv.Version, + Kind: xrKind, + } + + c.logger.Debug("Composition targets XR type", + "gvk", xrGVK.String()) + + // List all resources of this XR type in the specified namespace + xrs, err := c.resourceClient.ListResources(ctx, xrGVK, namespace) + if err != nil { + return nil, errors.Wrapf(err, "cannot list XRs of type %s in namespace %s", xrGVK.String(), namespace) + } + + c.logger.Debug("Found XRs of target type", "count", len(xrs)) + + // Filter XRs that use this specific composition + var matchingXRs []*un.Unstructured + + for _, xr := range xrs { + if c.xrUsesComposition(xr, compositionName) { + matchingXRs = append(matchingXRs, xr) + } + } + + c.logger.Debug("Found XRs using composition", + "compositionName", compositionName, + "count", len(matchingXRs)) + + return matchingXRs, nil +} + +// xrUsesComposition checks if an XR uses the specified composition. +func (c *DefaultCompositionClient) xrUsesComposition(xr *un.Unstructured, compositionName string) bool { + // Check direct composition reference in spec.compositionRef.name or spec.crossplane.compositionRef.name + apiVersion := xr.GetAPIVersion() + + // Try both v1 and v2 paths + paths := [][]string{ + makeCrossplaneRefPath(apiVersion, "compositionRef", "name"), + {"spec", "compositionRef", "name"}, // fallback for v1 + } + + for _, path := range paths { + if refName, found, _ := un.NestedString(xr.Object, path...); found && refName == compositionName { + return true + } + } + + return false +} diff --git a/cmd/diff/cmd_utils.go b/cmd/diff/cmd_utils.go new file mode 100644 index 0000000..bb875d4 --- /dev/null +++ b/cmd/diff/cmd_utils.go @@ -0,0 +1,82 @@ +/* +Copyright 2025 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 main contains shared command utilities. +package main + +import ( + "time" + + "github.com/alecthomas/kong" + "k8s.io/client-go/rest" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" +) + +// CommonCmdFields contains common fields shared by both XR and Comp commands. +type CommonCmdFields struct { + // Configuration options + NoColor bool `help:"Disable colorized output." name:"no-color"` + Compact bool `help:"Show compact diffs with minimal context." name:"compact"` + Timeout time.Duration `default:"1m" help:"How long to run before timing out."` + QPS float32 `default:"0" help:"Maximum QPS to the API server."` + Burst int `default:"0" help:"Maximum burst for throttle."` +} + +// initializeSharedDependencies handles the common initialization logic for both commands. +func initializeSharedDependencies(ctx *kong.Context, log logging.Logger, config *rest.Config, fields CommonCmdFields) (*AppContext, error) { + config = initRestConfig(config, log, fields) + + appCtx, err := NewAppContext(config, log) + if err != nil { + return nil, errors.Wrap(err, "cannot create app context") + } + + ctx.Bind(appCtx) + + return appCtx, nil +} + +// initRestConfig configures REST client rate limits for both commands. +func initRestConfig(config *rest.Config, logger logging.Logger, fields CommonCmdFields) *rest.Config { + // Set default QPS and Burst if they are not set in the config + // or override with values from options if provided + originalQPS := config.QPS + originalBurst := config.Burst + + if fields.QPS > 0 { + config.QPS = fields.QPS + } else if config.QPS == 0 { + config.QPS = 20 + } + + if fields.Burst > 0 { + config.Burst = fields.Burst + } else if config.Burst == 0 { + config.Burst = 30 + } + + logger.Debug("Configured REST client rate limits", + "original_qps", originalQPS, + "original_burst", originalBurst, + "options_qps", fields.QPS, + "options_burst", fields.Burst, + "final_qps", config.QPS, + "final_burst", config.Burst) + + return config +} diff --git a/cmd/diff/comp.go b/cmd/diff/comp.go new file mode 100644 index 0000000..d9742eb --- /dev/null +++ b/cmd/diff/comp.go @@ -0,0 +1,139 @@ +/* +Copyright 2025 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 main contains the composition diff command. +package main + +import ( + "context" + + "github.com/alecthomas/kong" + dp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/diffprocessor" + "k8s.io/client-go/rest" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + ld "github.com/crossplane/crossplane/v2/cmd/crank/common/load" +) + +// CompDiffProcessor is imported from the diffprocessor package + +// CompCmd represents the composition diff command. +type CompCmd struct { + // Embed common fields + CommonCmdFields + + Files []string `arg:"" help:"YAML files containing updated Composition(s)." optional:""` + + // Configuration options + Namespace string `default:"" help:"Namespace to find XRs (empty = all namespaces)." name:"namespace" short:"n"` +} + +// Help returns help instructions for the composition diff command. +func (c *CompCmd) Help() string { + return ` +This command shows the impact of composition changes on existing XRs in the cluster. + +It finds all XRs that use the specified composition(s) and shows what would change +if they were rendered with the updated composition(s) from the file(s). + +Examples: + # Show impact of updated composition on all XRs using it + crossplane-diff comp updated-composition.yaml + + # Show impact of multiple composition changes + crossplane-diff comp comp1.yaml comp2.yaml comp3.yaml + + # Show impact only on XRs in a specific namespace + crossplane-diff comp updated-composition.yaml -n production + + # Show compact diffs with minimal context + crossplane-diff comp updated-composition.yaml --compact +` +} + +// AfterApply implements kong's AfterApply method to bind our dependencies. +func (c *CompCmd) AfterApply(ctx *kong.Context, log logging.Logger, config *rest.Config) error { + return c.initializeDependencies(ctx, log, config) +} + +func (c *CompCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, config *rest.Config) error { + appCtx, err := initializeSharedDependencies(ctx, log, config, c.CommonCmdFields) + if err != nil { + return err + } + + proc := makeDefaultCompProc(c, appCtx, log) + + loader, err := makeDefaultCompLoader(c) + if err != nil { + return errors.Wrap(err, "cannot create composition loader") + } + + ctx.BindTo(proc, (*dp.CompDiffProcessor)(nil)) + ctx.BindTo(loader, (*ld.Loader)(nil)) + + return nil +} + +func makeDefaultCompProc(c *CompCmd, ctx *AppContext, log logging.Logger) dp.CompDiffProcessor { + // Create the options for the processor using the unified ProcessorOption + options := []dp.ProcessorOption{ + dp.WithNamespace(c.Namespace), + dp.WithLogger(log), + dp.WithColorize(!c.NoColor), + dp.WithCompact(c.Compact), + } + + // Use explicit type references to make sure imports are used + var ( + _ = ctx.K8sClients + _ = ctx.XpClients + ) + + return dp.NewCompDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) +} + +func makeDefaultCompLoader(c *CompCmd) (ld.Loader, error) { + return ld.NewCompositeLoader(c.Files) +} + +// Run executes the composition diff command. +func (c *CompCmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc dp.CompDiffProcessor, loader ld.Loader) error { + ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) + defer cancel() + + if err := appCtx.Initialize(ctx, log); err != nil { + return errors.Wrap(err, "cannot initialize client") + } + + err := proc.Initialize(ctx) + if err != nil { + return errors.Wrap(err, "cannot initialize composition diff processor") + } + + compositions, err := loader.Load() + if err != nil { + return errors.Wrap(err, "cannot load compositions") + } + + if err := proc.DiffComposition(ctx, k.Stdout, compositions, c.Namespace); err != nil { + return errors.Wrap(err, "unable to process composition diff") + } + + return nil +} diff --git a/cmd/diff/diff_integration_test.go b/cmd/diff/diff_integration_test.go index cad28a7..fb6c6f6 100644 --- a/cmd/diff/diff_integration_test.go +++ b/cmd/diff/diff_integration_test.go @@ -31,6 +31,29 @@ const ( timeout = 60 * time.Second ) +// DiffTestType represents the type of diff test to run. +type DiffTestType int + +const ( + XRDiffTest DiffTestType = iota + CompositionDiffTest +) + +// IntegrationTestCase represents a common test case structure for both XR and composition diff tests. +type IntegrationTestCase struct { + setupFiles []string + setupFilesWithOwnerRefs []HierarchicalOwnershipRelation + inputFiles []string // Input files to diff (XR YAML files or Composition YAML files) + expectedOutput string + expectedError bool + expectedErrorContains string + noColor bool + namespace string // For composition tests (optional) + xrdAPIVersion XrdAPIVersion // For XR tests (optional) + skip bool + skipReason string +} + type XrdAPIVersion int const ( @@ -47,8 +70,9 @@ func (s XrdAPIVersion) String() string { return versionNames[s] } -// TestDiffIntegration runs an integration test for the diff command. -func TestDiffIntegration(t *testing.T) { +// runIntegrationTest runs a common integration test for both XR and composition diff commands. +func runIntegrationTest(t *testing.T, testType DiffTestType, tests map[string]IntegrationTestCase) { + t.Helper() // Create a scheme with both Kubernetes and Crossplane types scheme := runtime.NewScheme() @@ -61,19 +85,200 @@ func TestDiffIntegration(t *testing.T) { _ = pkgv1.AddToScheme(scheme) _ = extv1.AddToScheme(scheme) - // Test cases - tests := map[string]struct { - setupFiles []string - setupFilesWithOwnerRefs []HierarchicalOwnershipRelation - inputFiles []string - expectedOutput string - expectedError bool - expectedErrorContains string - noColor bool - xrdAPIVersion XrdAPIVersion - skip bool - skipReason string - }{ + tu.SetupKubeTestLogger(t) + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Skip test if requested + if tt.skip { + t.Skip(tt.skipReason) + return + } + + // Setup a brand new test environment for each test case + _, thisFile, _, _ := run.Caller(0) + thisDir := filepath.Dir(thisFile) + + var crdPaths []string + if testType == CompositionDiffTest { + crdPaths = []string{ + filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), + filepath.Join(thisDir, "testdata", "comp", "crds"), + } + } else { + crdPaths = []string{ + filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), + filepath.Join(thisDir, "testdata", "diff", "crds"), + } + } + + testEnv := &envtest.Environment{ + CRDDirectoryPaths: crdPaths, + ErrorIfCRDPathMissing: true, + Scheme: scheme, + } + + // Start the test environment + cfg, err := testEnv.Start() + if err != nil { + t.Fatalf("failed to start test environment: %v", err) + } + + // Ensure we clean up at the end of the test + defer func() { + err := testEnv.Stop() + if err != nil { + t.Logf("failed to stop test environment: %v", err) + } + }() + + // Create a controller-runtime client for setup operations + k8sClient, err := client.New(cfg, client.Options{Scheme: scheme}) + if err != nil { + t.Fatalf("failed to create client: %v", err) + } + + ctx, cancel := context.WithTimeout(t.Context(), timeout) + defer cancel() + + // Apply the setup resources + if err := applyResourcesFromFiles(ctx, k8sClient, tt.setupFiles); err != nil { + t.Fatalf("failed to setup resources: %v", err) + } + + // Default to v2 API version for XR resources unless otherwise specified + xrdAPIVersion := V2 + if tt.xrdAPIVersion != V2 { + xrdAPIVersion = tt.xrdAPIVersion + } + + // Apply resources with owner references + if len(tt.setupFilesWithOwnerRefs) > 0 { + err := applyHierarchicalOwnership(ctx, tu.TestLogger(t, false), k8sClient, xrdAPIVersion, tt.setupFilesWithOwnerRefs) + if err != nil { + t.Fatalf("failed to setup owner references: %v", err) + } + } + + // Set up the test files + tempDir := t.TempDir() + + var testFiles []string + + // Handle any additional input files + for i, inputFile := range tt.inputFiles { + testFile := filepath.Join(tempDir, fmt.Sprintf("test_%d.yaml", i)) + + content, err := os.ReadFile(inputFile) + if err != nil { + t.Fatalf("failed to read input file: %v", err) + } + + err = os.WriteFile(testFile, content, 0o644) + if err != nil { + t.Fatalf("failed to write test file: %v", err) + } + + testFiles = append(testFiles, testFile) + } + + // Create a buffer to capture the output + var stdout bytes.Buffer + + // Create command line args that match your pre-populated struct + args := []string{ + fmt.Sprintf("--timeout=%s", timeout.String()), + } + + // Add namespace if specified (for composition tests) + if tt.namespace != "" { + args = append(args, fmt.Sprintf("--namespace=%s", tt.namespace)) + } else if testType == XRDiffTest { + // For XR tests, always add default namespace + args = append(args, "--namespace=default") + } + + // Add no-color flag if true + if tt.noColor { + args = append(args, "--no-color") + } + + // Add files as positional arguments + args = append(args, testFiles...) + + // Set up the appropriate command based on test type + var cmd interface{} + if testType == CompositionDiffTest { + cmd = &CompCmd{} + } else { + cmd = &XRCmd{} + } + + logger := tu.TestLogger(t, true) + // Create a Kong context with stdout + parser, err := kong.New(cmd, + kong.Writers(&stdout, &stdout), + kong.Bind(cfg), + kong.BindTo(logger, (*logging.Logger)(nil)), + ) + if err != nil { + t.Fatalf("failed to create kong parser: %v", err) + } + + kongCtx, err := parser.Parse(args) + if err != nil { + t.Fatalf("failed to parse kong context: %v", err) + } + + err = kongCtx.Run(cfg) + + if tt.expectedError && err == nil { + t.Fatal("expected error but got none") + } + + if !tt.expectedError && err != nil { + t.Fatalf("expected no error but got: %v", err) + } + + // Check for specific error message if expected + if err != nil { + if tt.expectedErrorContains != "" && strings.Contains(err.Error(), tt.expectedErrorContains) { + // This is an expected error with the expected message + t.Logf("Got expected error containing: %s", tt.expectedErrorContains) + } else { + t.Errorf("Expected no error or specific error message, got: %v", err) + } + } + + // For expected errors with specific messages, we've already checked above + if tt.expectedError && tt.expectedErrorContains != "" { + // Skip output check for expected error cases + return + } + + // Check the output + outputStr := stdout.String() + // Using TrimSpace because the output might have trailing newlines + if !strings.Contains(strings.TrimSpace(outputStr), strings.TrimSpace(tt.expectedOutput)) { + // Strings aren't equal, *including* ansi. but we can compare ignoring ansi to determine what output to + // show for the failure. if the difference is only in color codes, we'll show escaped ansi codes. + out := outputStr + + expect := tt.expectedOutput + if tu.CompareIgnoringAnsi(strings.TrimSpace(outputStr), strings.TrimSpace(tt.expectedOutput)) { + out = strconv.QuoteToASCII(outputStr) + expect = strconv.QuoteToASCII(tt.expectedOutput) + } + + t.Fatalf("expected output to contain:\n%s\n\nbut got:\n%s", expect, out) + } + }) + } +} + +// TestDiffIntegration runs an integration test for the diff command. +func TestDiffIntegration(t *testing.T) { + tests := map[string]IntegrationTestCase{ "New resource shows color diff": { inputFiles: []string{"testdata/diff/new-xr.yaml"}, setupFiles: []string{ @@ -1109,181 +1314,400 @@ Summary: 1 added`, }, } - tu.SetupKubeTestLogger(t) + runIntegrationTest(t, XRDiffTest, tests) +} - // version := V2 +// TestCompDiffIntegration runs an integration test for the composition diff command. +func TestCompDiffIntegration(t *testing.T) { + tests := map[string]IntegrationTestCase{ + "Composition change impacts existing XRs": { + // Set up existing XRs that use the original composition + setupFiles: []string{ + "testdata/comp/resources/xrd.yaml", + "testdata/comp/resources/original-composition.yaml", + "testdata/comp/resources/functions.yaml", + // Add existing XRs that use the composition + "testdata/comp/resources/existing-xr-1.yaml", + "testdata/comp/resources/existing-downstream-1.yaml", + "testdata/comp/resources/existing-xr-2.yaml", + "testdata/comp/resources/existing-downstream-2.yaml", + }, + // New composition files that will be diffed + inputFiles: []string{"testdata/comp/updated-composition.yaml"}, + namespace: "default", + expectedOutput: ` +=== Composition Changes === - for name, tt := range tests { - // if tt.crdVersions == nil || len(tt.crdVersions) == 0 { - // // Default to testing against v2 CRDs if not specified. claims still exist in v2, though deprecated. - // // old style XRDs are still supported, too. - // tt.crdVersions = []XrdApiVersion{V2} - //} - - // for _, version := range tt.crdVersions { - t.Run(name /*fmt.Sprintf("%s (%s)", name, version.String()) */, func(t *testing.T) { - // Skip test if requested - if tt.skip { - t.Skip(tt.skipReason) - return - } +~~~ Composition/xnopresources.diff.example.org + apiVersion: apiextensions.crossplane.io/v1 + kind: Composition + metadata: + name: xnopresources.diff.example.org + spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: +- configData: {{ .observed.composite.resource.spec.coolField }} +- resourceTier: basic ++ configData: updated-{{ .observed.composite.resource.spec.coolField }} ++ resourceTier: premium + kind: GoTemplate + source: Inline + step: generate-resources + - functionRef: + name: function-auto-ready + step: automatically-detect-ready-composed-resources - // Setup a brand new test environment for each test case - _, thisFile, _, _ := run.Caller(0) - thisDir := filepath.Dir(thisFile) +--- - testEnv := &envtest.Environment{ - CRDDirectoryPaths: []string{ - filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), - filepath.Join(thisDir, "testdata", "diff", "crds"), - }, - ErrorIfCRDPathMissing: true, - Scheme: scheme, - } +Summary: 1 modified - // Start the test environment - cfg, err := testEnv.Start() - if err != nil { - t.Fatalf("failed to start test environment: %v", err) - } +=== Affected Composite Resources === - // Ensure we clean up at the end of the test - defer func() { - err := testEnv.Stop() - if err != nil { - t.Logf("failed to stop test environment: %v", err) - } - }() +- XNopResource/another-resource (namespace: default) +- XNopResource/test-resource (namespace: default) - // Create a controller-runtime client for setup operations - k8sClient, err := client.New(cfg, client.Options{Scheme: scheme}) - if err != nil { - t.Fatalf("failed to create client: %v", err) - } +=== Impact Analysis === - ctx, cancel := context.WithTimeout(t.Context(), timeout) - defer cancel() +~~~ XDownstreamResource/another-resource + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: ++ crossplane.io/composition-resource-name: nop-resource + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: another-resource + name: another-resource + namespace: default + spec: + forProvider: +- configData: another-existing-value +- resourceTier: basic ++ configData: updated-another-existing-value ++ resourceTier: premium - // Apply the setup resources - if err := applyResourcesFromFiles(ctx, k8sClient, tt.setupFiles); err != nil { - t.Fatalf("failed to setup resources: %v", err) - } +--- +~~~ XDownstreamResource/test-resource + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: ++ crossplane.io/composition-resource-name: nop-resource + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: test-resource + name: test-resource + namespace: default + spec: + forProvider: +- configData: existing-value +- resourceTier: basic ++ configData: updated-existing-value ++ resourceTier: premium - // Default to v2 API version for XR resources unless otherwise specified - xrdAPIVersion := V2 - if tt.xrdAPIVersion != V2 { - xrdAPIVersion = tt.xrdAPIVersion - } +--- - // Apply resources with owner references - if len(tt.setupFilesWithOwnerRefs) > 0 { - err := applyHierarchicalOwnership(ctx, tu.TestLogger(t, false), k8sClient, xrdAPIVersion, tt.setupFilesWithOwnerRefs) - if err != nil { - t.Fatalf("failed to setup owner references: %v", err) - } - } +Summary: 2 modified`, + expectedError: false, + noColor: true, + }, + "Composition diff with custom namespace": { + // Set up existing XRs in both custom and default namespaces to validate filtering + setupFiles: []string{ + "testdata/comp/resources/xrd.yaml", + "testdata/comp/resources/original-composition.yaml", + "testdata/comp/resources/functions.yaml", + // Create the custom namespace first + "testdata/comp/resources/custom-namespace.yaml", + // Add existing XRs in custom namespace (should be included in output) + "testdata/comp/resources/existing-custom-ns-xr.yaml", + "testdata/comp/resources/existing-custom-ns-downstream.yaml", + // Add existing XRs in default namespace (should be filtered out) + "testdata/comp/resources/existing-xr-1.yaml", + "testdata/comp/resources/existing-downstream-1.yaml", + }, + // New composition files that will be diffed + inputFiles: []string{"testdata/comp/updated-composition.yaml"}, + namespace: "custom-namespace", + // Expected output should only show custom-namespace resources, NOT default namespace resources + // This validates that namespace filtering works correctly + expectedOutput: ` +=== Composition Changes === - // Set up the test file - tempDir := t.TempDir() +~~~ Composition/xnopresources.diff.example.org + apiVersion: apiextensions.crossplane.io/v1 + kind: Composition + metadata: + name: xnopresources.diff.example.org + spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: +- configData: {{ .observed.composite.resource.spec.coolField }} +- resourceTier: basic ++ configData: updated-{{ .observed.composite.resource.spec.coolField }} ++ resourceTier: premium + kind: GoTemplate + source: Inline + step: generate-resources + - functionRef: + name: function-auto-ready + step: automatically-detect-ready-composed-resources - var testFiles []string +--- - // Handle any additional input files - for i, inputFile := range tt.inputFiles { - testFile := filepath.Join(tempDir, fmt.Sprintf("test_%d.yaml", i)) +Summary: 1 modified - content, err := os.ReadFile(inputFile) - if err != nil { - t.Fatalf("failed to read input file: %v", err) - } +=== Affected Composite Resources === - err = os.WriteFile(testFile, content, 0o644) - if err != nil { - t.Fatalf("failed to write test file: %v", err) - } +- XNopResource/custom-namespace-resource (namespace: custom-namespace) - testFiles = append(testFiles, testFile) - } +=== Impact Analysis === - // Create a buffer to capture the output - var stdout bytes.Buffer +~~~ XDownstreamResource/custom-namespace-resource + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: ++ crossplane.io/composition-resource-name: nop-resource + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: custom-namespace-resource + name: custom-namespace-resource + namespace: custom-namespace + spec: + forProvider: +- configData: custom-ns-existing-value +- resourceTier: basic ++ configData: updated-custom-ns-existing-value ++ resourceTier: premium - // Create command line args that match your pre-populated struct - args := []string{ - "--namespace=default", - fmt.Sprintf("--timeout=%s", timeout.String()), - } +--- - // Add no-color flag if true - if tt.noColor { - args = append(args, "--no-color") - } +Summary: 1 modified`, + expectedError: false, + noColor: true, + }, + "Multiple composition diff shows impact on existing XRs": { + // Set up existing XRs that use both compositions + setupFiles: []string{ + "testdata/comp/resources/xrd.yaml", + "testdata/comp/resources/original-composition.yaml", + "testdata/comp/resources/original-composition-2.yaml", + "testdata/comp/resources/functions.yaml", + // Add existing XRs that use the compositions + "testdata/comp/resources/existing-xr-1.yaml", + "testdata/comp/resources/existing-downstream-1.yaml", + "testdata/comp/resources/existing-xr-2.yaml", + "testdata/comp/resources/existing-downstream-2.yaml", + }, + // Multiple composition files that will be diffed + inputFiles: []string{ + "testdata/comp/updated-composition.yaml", + "testdata/comp/updated-composition-2.yaml", + }, + namespace: "default", + expectedOutput: ` +=== Composition Changes === - // Add files as positional arguments - args = append(args, testFiles...) +~~~ Composition/xnopresources.diff.example.org + apiVersion: apiextensions.crossplane.io/v1 + kind: Composition + metadata: + name: xnopresources.diff.example.org + spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: +- configData: {{ .observed.composite.resource.spec.coolField }} +- resourceTier: basic ++ configData: updated-{{ .observed.composite.resource.spec.coolField }} ++ resourceTier: premium + kind: GoTemplate + source: Inline + step: generate-resources + - functionRef: + name: function-auto-ready + step: automatically-detect-ready-composed-resources - // Set up the diff command - cmd := &Cmd{} +--- - logger := tu.TestLogger(t, true) - // Create a Kong context with stdout - parser, err := kong.New(cmd, - kong.Writers(&stdout, &stdout), - kong.Bind(cfg), - kong.BindTo(logger, (*logging.Logger)(nil)), - ) - if err != nil { - t.Fatalf("failed to create kong parser: %v", err) - } +Summary: 1 modified - kongCtx, err := parser.Parse(args) - if err != nil { - t.Fatalf("failed to parse kong context: %v", err) - } +=== Affected Composite Resources === - err = kongCtx.Run(cfg) +- XNopResource/another-resource (namespace: default) +- XNopResource/test-resource (namespace: default) - if tt.expectedError && err == nil { - t.Fatal("expected error but got none") - } +=== Impact Analysis === - if !tt.expectedError && err != nil { - t.Fatalf("expected no error but got: %v", err) - } +~~~ XDownstreamResource/another-resource + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: ++ crossplane.io/composition-resource-name: nop-resource + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: another-resource + name: another-resource + namespace: default + spec: + forProvider: +- configData: another-existing-value +- resourceTier: basic ++ configData: updated-another-existing-value ++ resourceTier: premium - // Check for specific error message if expected - if err != nil { - if tt.expectedErrorContains != "" && strings.Contains(err.Error(), tt.expectedErrorContains) { - // This is an expected error with the expected message - t.Logf("Got expected error containing: %s", tt.expectedErrorContains) - } else { - t.Errorf("Expected no error or specific error message, got: %v", err) - } - } +--- +~~~ XDownstreamResource/test-resource + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + annotations: ++ crossplane.io/composition-resource-name: nop-resource + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: test-resource + name: test-resource + namespace: default + spec: + forProvider: +- configData: existing-value +- resourceTier: basic ++ configData: updated-existing-value ++ resourceTier: premium - // For expected errors with specific messages, we've already checked above - if tt.expectedError && tt.expectedErrorContains != "" { - // Skip output check for expected error cases - return - } +--- - // Check the output - outputStr := stdout.String() - // Using TrimSpace because the output might have trailing newlines - if !strings.Contains(strings.TrimSpace(outputStr), strings.TrimSpace(tt.expectedOutput)) { - // Strings aren't equal, *including* ansi. but we can compare ignoring ansi to determine what output to - // show for the failure. if the difference is only in color codes, we'll show escaped ansi codes. - out := outputStr +Summary: 2 modified - expect := tt.expectedOutput - if tu.CompareIgnoringAnsi(strings.TrimSpace(outputStr), strings.TrimSpace(tt.expectedOutput)) { - out = strconv.QuoteToASCII(outputStr) - expect = strconv.QuoteToASCII(tt.expectedOutput) - } +================================================================================ - t.Fatalf("expected output to contain:\n%s\n\nbut got:\n%s", expect, out) - } - }) +=== Composition Changes === + +No changes detected in composition xnopresources-v2.diff.example.org + +No XRs found using composition xnopresources-v2.diff.example.org`, + expectedError: false, + noColor: true, + }, + "Net-new composition with no downstream impact": { + // Set up cluster with existing resources but no composition that matches the new one + setupFiles: []string{ + "testdata/comp/resources/xrd.yaml", + "testdata/comp/resources/original-composition.yaml", + "testdata/comp/resources/functions.yaml", + // Add existing XRs that use different compositions (won't be affected) + "testdata/comp/resources/existing-xr-1.yaml", + "testdata/comp/resources/existing-downstream-1.yaml", + }, + // Net-new composition file that doesn't exist in cluster and targets different XR type + inputFiles: []string{"testdata/comp/net-new-composition.yaml"}, + namespace: "default", + expectedOutput: ` +=== Composition Changes === + ++++ Composition/xnewresources.diff.example.org ++ apiVersion: apiextensions.crossplane.io/v1 ++ kind: Composition ++ metadata: ++ labels: ++ environment: staging ++ provider: aws ++ name: xnewresources.diff.example.org ++ spec: ++ compositeTypeRef: ++ apiVersion: staging.diff.example.org/v1alpha1 ++ kind: XNewResource ++ mode: Pipeline ++ pipeline: ++ - functionRef: ++ name: function-go-templating ++ input: ++ apiVersion: template.fn.crossplane.io/v1beta1 ++ inline: ++ template: | ++ apiVersion: staging.nop.example.org/v1alpha1 ++ kind: XStagingResource ++ metadata: ++ name: {{ .observed.composite.resource.metadata.name }} ++ namespace: {{ .observed.composite.resource.metadata.namespace }} ++ annotations: ++ gotemplating.fn.crossplane.io/composition-resource-name: staging-resource ++ spec: ++ forProvider: ++ environment: staging ++ configData: new-{{ .observed.composite.resource.spec.newField }} ++ resourceTier: premium ++ kind: GoTemplate ++ source: Inline ++ step: generate-new-resources ++ - functionRef: ++ name: function-auto-ready ++ step: automatically-detect-ready-composed-resources + +--- + +Summary: 1 added + +No XRs found using composition xnewresources.diff.example.org`, + expectedError: false, + noColor: true, + }, } - //} + + runIntegrationTest(t, CompositionDiffTest, tests) } diff --git a/cmd/diff/diff_test.go b/cmd/diff/diff_test.go index 7f695e4..40e8806 100644 --- a/cmd/diff/diff_test.go +++ b/cmd/diff/diff_test.go @@ -32,6 +32,7 @@ import ( k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" dp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/diffprocessor" tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" "github.com/google/go-cmp/cmp" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -66,10 +67,10 @@ func TestCmd_Run(t *testing.T) { // Create a buffer to capture output type fields struct { + CommonCmdFields + Namespace string Files []string - NoColor bool - Compact bool } type args struct { @@ -109,8 +110,10 @@ func TestCmd_Run(t *testing.T) { fields: fields{ Namespace: "default", Files: []string{}, - NoColor: false, - Compact: false, + CommonCmdFields: CommonCmdFields{ + NoColor: false, + Compact: false, + }, }, args: args{ appContext: appCtx, @@ -254,11 +257,10 @@ metadata: // Setup test files if needed files := tc.setupFiles() - c := &Cmd{ - Namespace: tc.fields.Namespace, - Files: files, - NoColor: tc.fields.NoColor, - Compact: tc.fields.Compact, + c := &XRCmd{ + Namespace: tc.fields.Namespace, + Files: files, + CommonCmdFields: tc.fields.CommonCmdFields, } err := c.Run( @@ -426,7 +428,7 @@ func TestDiffCommand(t *testing.T) { setupProcessor: func() dp.DiffProcessor { return tu.NewMockDiffProcessor(). WithSuccessfulInitialize(). - WithPerformDiff(func(w io.Writer, _ context.Context, _ []*un.Unstructured) error { + WithPerformDiff(func(_ context.Context, w io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { // Generate a mock diff for our test _, _ = fmt.Fprintf(w, `~ ComposedResource/test-xr-composed-resource { @@ -513,7 +515,7 @@ spec: setupProcessor: func() dp.DiffProcessor { return tu.NewMockDiffProcessor(). WithSuccessfulInitialize(). - WithPerformDiff(func(io.Writer, context.Context, []*un.Unstructured) error { + WithPerformDiff(func(_ context.Context, _ io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { return errors.New("processing error") }). Build() @@ -616,7 +618,7 @@ spec: setupProcessor: func() dp.DiffProcessor { return tu.NewMockDiffProcessor(). WithSuccessfulInitialize(). - WithPerformDiff(func(io.Writer, context.Context, []*un.Unstructured) error { + WithPerformDiff(func(_ context.Context, _ io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { // For matching resources, we don't produce any output return nil }). @@ -716,7 +718,7 @@ spec: setupProcessor: func() dp.DiffProcessor { return tu.NewMockDiffProcessor(). WithSuccessfulInitialize(). - WithPerformDiff(func(w io.Writer, _ context.Context, _ []*un.Unstructured) error { + WithPerformDiff(func(_ context.Context, w io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { // Generate output for a new resource _, _ = fmt.Fprintf(w, `+++ ComposedResource/test-xr-composed-resource { @@ -881,9 +883,11 @@ spec: var buf bytes.Buffer // Create our command - cmd := &Cmd{ + cmd := &XRCmd{ Namespace: "default", - Timeout: time.Second * 30, + CommonCmdFields: CommonCmdFields{ + Timeout: time.Second * 30, + }, } // Create a Kong context diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go new file mode 100644 index 0000000..d287fe3 --- /dev/null +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -0,0 +1,422 @@ +/* +Copyright 2025 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 diffprocessor contains the composition diff processor. +package diffprocessor + +import ( + "context" + "fmt" + "io" + "strings" + + xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" + k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer" + dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + un "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/logging" + + apiextensionsv1 "github.com/crossplane/crossplane/v2/apis/apiextensions/v1" + "github.com/crossplane/crossplane/v2/cmd/crank/render" +) + +// CompDiffProcessor defines the interface for composition diffing. +type CompDiffProcessor interface { + DiffComposition(ctx context.Context, stdout io.Writer, compositions []*un.Unstructured, namespace string) error + Initialize(ctx context.Context) error +} + +// DefaultCompDiffProcessor implements CompDiffProcessor. +type DefaultCompDiffProcessor struct { + k8sClients k8.Clients + xpClients xp.Clients + config ProcessorConfig + xrProc DiffProcessor +} + +// NewCompDiffProcessor creates a new DefaultCompDiffProcessor. +func NewCompDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) CompDiffProcessor { + // Create default configuration + config := ProcessorConfig{ + Namespace: "", + Colorize: true, + Compact: false, + Logger: logging.NewNopLogger(), + RenderFunc: render.Render, + } + + // Apply all provided options + for _, option := range opts { + option(&config) + } + + return &DefaultCompDiffProcessor{ + k8sClients: k8cs, + xpClients: xpcs, + config: config, + } +} + +// Initialize loads required resources. +func (p *DefaultCompDiffProcessor) Initialize(ctx context.Context) error { + p.config.Logger.Debug("Initializing composition diff processor") + + // Create an XR diff processor that we'll reuse + xrOptions := []ProcessorOption{ + WithNamespace(p.config.Namespace), + WithLogger(p.config.Logger), + WithRenderFunc(render.Render), + WithColorize(p.config.Colorize), + WithCompact(p.config.Compact), + } + + p.xrProc = NewDiffProcessor(p.k8sClients, p.xpClients, xrOptions...) + + // Initialize the XR processor + if err := p.xrProc.Initialize(ctx); err != nil { + return errors.Wrap(err, "cannot initialize XR diff processor") + } + + p.config.Logger.Debug("Composition diff processor initialized") + + return nil +} + +// DiffComposition processes composition changes and shows impact on existing XRs. +func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, stdout io.Writer, compositions []*un.Unstructured, namespace string) error { + p.config.Logger.Debug("Processing composition diff", "compositionCount", len(compositions), "namespace", namespace) + + if len(compositions) == 0 { + return errors.New("no compositions provided") + } + + var errs []error + + // Process each composition, filtering out non-Composition objects + for i, comp := range compositions { + compositionID := fmt.Sprintf("composition %d", i+1) + + // Skip non-Composition objects (e.g., GoTemplate objects extracted from pipeline steps) + if comp.GetKind() != "Composition" { + p.config.Logger.Debug("Skipping non-Composition object", "kind", comp.GetKind(), "apiVersion", comp.GetAPIVersion()) + continue + } + + // Convert unstructured to typed composition + newComp, err := p.unstructuredToComposition(comp) + if err != nil { + p.config.Logger.Debug("Failed to convert composition", "composition", compositionID, "error", err) + errs = append(errs, errors.Wrapf(err, "cannot convert %s from unstructured", compositionID)) + + continue + } + + compositionID = newComp.GetName() // Use actual name once we have it + p.config.Logger.Debug("Processing composition", "name", compositionID) + + // Process this single composition + if err := p.processSingleComposition(ctx, stdout, newComp, namespace); err != nil { + p.config.Logger.Debug("Failed to process composition", "composition", compositionID, "error", err) + errs = append(errs, errors.Wrapf(err, "cannot process composition %s", compositionID)) + + continue + } + + // Add separator between compositions if processing multiple + if len(compositions) > 1 && i < len(compositions)-1 { + separator := "\n" + strings.Repeat("=", 80) + "\n\n" + if _, err := fmt.Fprint(stdout, separator); err != nil { + return errors.Wrap(err, "cannot write composition separator") + } + } + } + + // Handle any errors that occurred during processing + if len(errs) > 0 { + if len(errs) == len(compositions) { + // All compositions failed - this is a complete failure + return errors.New("failed to process all compositions") + } + // Some compositions failed - log the errors but don't fail completely + p.config.Logger.Info("Some compositions failed to process", "failedCount", len(errs), "totalCount", len(compositions)) + + for _, err := range errs { + p.config.Logger.Debug("Composition processing error", "error", err) + } + } + + return nil +} + +// processSingleComposition processes a single composition and shows its impact on existing XRs. +func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition, namespace string) error { + // First, show the composition diff itself + if err := p.displayCompositionDiff(ctx, stdout, newComp); err != nil { + return errors.Wrap(err, "cannot display composition diff") + } + + // Find all XRs that use this composition + affectedXRs, err := p.findXRsUsingComposition(ctx, newComp.GetName(), namespace) + if err != nil { + // For net-new compositions, the composition won't exist in the cluster + // so findXRsUsingComposition will fail. This is expected behavior. + p.config.Logger.Debug("Cannot find XRs using composition (likely net-new composition)", + "composition", newComp.GetName(), "error", err) + + // Display the "no XRs found" message for net-new compositions + if _, err := fmt.Fprintf(stdout, "No XRs found using composition %s\n", newComp.GetName()); err != nil { + return errors.Wrap(err, "cannot write no XRs message") + } + + return nil + } + + p.config.Logger.Debug("Found affected XRs", "composition", newComp.GetName(), "count", len(affectedXRs)) + + if len(affectedXRs) == 0 { + p.config.Logger.Info("No XRs found using composition", "composition", newComp.GetName()) + + if _, err := fmt.Fprintf(stdout, "No XRs found using composition %s\n", newComp.GetName()); err != nil { + return errors.Wrap(err, "cannot write no XRs message") + } + + return nil + } + + // Process affected XRs using the existing XR processor with composition override + // List the affected XRs so users can understand the scope of impact + if _, err := fmt.Fprintf(stdout, "=== Affected Composite Resources ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write affected XRs header") + } + + for _, xr := range affectedXRs { + if _, err := fmt.Fprintf(stdout, "- %s/%s (namespace: %s)\n", xr.GetKind(), xr.GetName(), xr.GetNamespace()); err != nil { + return errors.Wrap(err, "cannot write affected XR info") + } + } + + if _, err := fmt.Fprintf(stdout, "\n=== Impact Analysis ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write impact analysis header") + } + + if err := p.xrProc.PerformDiff(ctx, stdout, affectedXRs, func(context.Context, *un.Unstructured) (*apiextensionsv1.Composition, error) { + return newComp, nil + }); err != nil { + return errors.Wrap(err, "cannot process XRs with composition override") + } + + return nil +} + +// findXRsUsingComposition finds all XRs that use the specified composition. +func (p *DefaultCompDiffProcessor) findXRsUsingComposition(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) { + // Use the composition client to find XRs that reference this composition + return p.xpClients.Composition.FindXRsUsingComposition(ctx, compositionName, namespace) +} + +// displayCompositionDiff shows the diff between the cluster composition and the file composition. +func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition) error { + p.config.Logger.Debug("Displaying composition diff", "composition", newComp.GetName()) + + // Get the original composition from the cluster + originalComp, err := p.xpClients.Composition.GetComposition(ctx, newComp.GetName()) + if err != nil { + p.config.Logger.Debug("Original composition not found in cluster, treating as new composition", + "composition", newComp.GetName(), "error", err) + + // Handle case where composition doesn't exist in cluster (net-new composition) + return p.displayNewComposition(ctx, stdout, newComp) + } + + p.config.Logger.Debug("Retrieved original composition from cluster", "name", originalComp.GetName(), "composition", originalComp) + + // Convert both compositions to unstructured for comparison + originalCompUnstructured, err := p.compositionToUnstructured(originalComp) + if err != nil { + return errors.Wrap(err, "cannot convert original composition to unstructured") + } + + newCompUnstructured, err := p.compositionToUnstructured(newComp) + if err != nil { + return errors.Wrap(err, "cannot convert new composition to unstructured") + } + + // Clean up managed fields and other cluster metadata before diff calculation + originalCompUnstructured.SetManagedFields(nil) + originalCompUnstructured.SetResourceVersion("") + originalCompUnstructured.SetUID("") + originalCompUnstructured.SetGeneration(0) + originalCompUnstructured.SetCreationTimestamp(metav1.Time{}) + + newCompUnstructured.SetManagedFields(nil) + newCompUnstructured.SetResourceVersion("") + newCompUnstructured.SetUID("") + newCompUnstructured.SetGeneration(0) + newCompUnstructured.SetCreationTimestamp(metav1.Time{}) + + // Calculate the composition diff directly without dry-run apply + // (compositions are static YAML documents that don't need server-side processing) + diffOptions := renderer.DefaultDiffOptions() + diffOptions.UseColors = p.config.Colorize + diffOptions.Compact = p.config.Compact + + compDiff, err := renderer.GenerateDiffWithOptions(ctx, originalCompUnstructured, newCompUnstructured, p.config.Logger, diffOptions) + if err != nil { + return errors.Wrap(err, "cannot calculate composition diff") + } + + p.config.Logger.Debug("Calculated composition diff", + "composition", newComp.GetName(), + "hasChanges", compDiff != nil) + + // Display the composition diff if there are changes + if compDiff != nil && compDiff.DiffType != dt.DiffTypeEqual { + // Create a diff renderer with proper options + rendererOptions := renderer.DefaultDiffOptions() + rendererOptions.UseColors = p.config.Colorize + rendererOptions.Compact = p.config.Compact + diffRenderer := renderer.NewDiffRenderer(p.config.Logger, rendererOptions) + + // Create a map with the single composition diff + diffs := map[string]*dt.ResourceDiff{ + fmt.Sprintf("Composition/%s", newComp.GetName()): compDiff, + } + + // Add a header to distinguish composition diff from XR diffs + if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write composition changes header") + } + + if err := diffRenderer.RenderDiffs(stdout, diffs); err != nil { + return errors.Wrap(err, "cannot render composition diff") + } + + if _, err := fmt.Fprintf(stdout, "\n"); err != nil { + return errors.Wrap(err, "cannot write separator") + } + } else { + p.config.Logger.Info("No changes detected in composition", "composition", newComp.GetName()) + + if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write composition changes header") + } + + if _, err := fmt.Fprintf(stdout, "No changes detected in composition %s\n\n", newComp.GetName()); err != nil { + return errors.Wrap(err, "cannot write no changes message") + } + } + + return nil +} + +// compositionToUnstructured converts a typed Composition to unstructured for diff calculation. +func (p *DefaultCompDiffProcessor) compositionToUnstructured(comp *apiextensionsv1.Composition) (*un.Unstructured, error) { + // Convert composition to unstructured using runtime conversion + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(comp) + if err != nil { + return nil, errors.Wrap(err, "cannot convert composition to unstructured map") + } + + return &un.Unstructured{Object: unstructuredObj}, nil +} + +// unstructuredToComposition converts an unstructured object to a typed Composition. +func (p *DefaultCompDiffProcessor) unstructuredToComposition(u *un.Unstructured) (*apiextensionsv1.Composition, error) { + comp := &apiextensionsv1.Composition{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, comp); err != nil { + return nil, errors.Wrap(err, "cannot convert unstructured to Composition") + } + + return comp, nil +} + +// displayNewComposition shows a new composition that doesn't exist in the cluster. +func (p *DefaultCompDiffProcessor) displayNewComposition(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition) error { + p.config.Logger.Debug("Displaying new composition", "composition", newComp.GetName()) + + // Convert new composition to unstructured for rendering + newCompUnstructured, err := p.compositionToUnstructured(newComp) + if err != nil { + return errors.Wrap(err, "cannot convert new composition to unstructured") + } + + // Clean up managed fields and other cluster metadata + newCompUnstructured.SetManagedFields(nil) + newCompUnstructured.SetResourceVersion("") + newCompUnstructured.SetUID("") + newCompUnstructured.SetGeneration(0) + newCompUnstructured.SetCreationTimestamp(metav1.Time{}) + + // Generate a diff showing the entire composition as new (all additions) + diffOptions := renderer.DefaultDiffOptions() + diffOptions.UseColors = p.config.Colorize + diffOptions.Compact = p.config.Compact + + // Create a diff with empty original (nil) to show everything as additions + compDiff, err := renderer.GenerateDiffWithOptions(ctx, nil, newCompUnstructured, p.config.Logger, diffOptions) + if err != nil { + return errors.Wrap(err, "cannot calculate new composition diff") + } + + p.config.Logger.Debug("Calculated new composition diff", + "composition", newComp.GetName(), + "hasChanges", compDiff != nil) + + // Display the new composition diff + if compDiff != nil { + // Create a diff renderer with proper options + rendererOptions := renderer.DefaultDiffOptions() + rendererOptions.UseColors = p.config.Colorize + rendererOptions.Compact = p.config.Compact + diffRenderer := renderer.NewDiffRenderer(p.config.Logger, rendererOptions) + + // Create a map with the single composition diff + diffs := map[string]*dt.ResourceDiff{ + fmt.Sprintf("Composition/%s", newComp.GetName()): compDiff, + } + + // Add a header to distinguish composition diff from XR diffs + if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write composition changes header") + } + + if err := diffRenderer.RenderDiffs(stdout, diffs); err != nil { + return errors.Wrap(err, "cannot render new composition diff") + } + + if _, err := fmt.Fprintf(stdout, "\n"); err != nil { + return errors.Wrap(err, "cannot write separator") + } + } else { + // This shouldn't happen for a new composition, but handle it gracefully + p.config.Logger.Debug("No diff generated for new composition", "composition", newComp.GetName()) + + if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write composition changes header") + } + + if _, err := fmt.Fprintf(stdout, "New composition %s\n\n", newComp.GetName()); err != nil { + return errors.Wrap(err, "cannot write new composition message") + } + } + + return nil +} diff --git a/cmd/diff/diffprocessor/comp_processor_test.go b/cmd/diff/diffprocessor/comp_processor_test.go new file mode 100644 index 0000000..94656b4 --- /dev/null +++ b/cmd/diff/diffprocessor/comp_processor_test.go @@ -0,0 +1,564 @@ +/* +Copyright 2025 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 diffprocessor + +import ( + "bytes" + "context" + "io" + "strings" + "testing" + + xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" + k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" + tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" + gcmp "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/crossplane/crossplane-runtime/v2/pkg/errors" + "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + apiextensionsv1 "github.com/crossplane/crossplane/v2/apis/apiextensions/v1" + "github.com/crossplane/crossplane/v2/cmd/crank/render" +) + +func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { + ctx := context.Background() + + // Create test XR data + xr1 := tu.NewResource("example.org/v1", "XResource", "xr-1"). + WithNamespace("default"). + Build() + xr2 := tu.NewResource("example.org/v1", "XResource", "xr-2"). + WithNamespace("custom-ns"). + Build() + + tests := map[string]struct { + compositionName string + namespace string + setupMocks func() xp.Clients + want []*un.Unstructured + wantErr bool + }{ + "SuccessfulFind": { + compositionName: "test-composition", + namespace: "default", + setupMocks: func() xp.Clients { + return xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithFindXRsUsingComposition(func(_ context.Context, compositionName, namespace string) ([]*un.Unstructured, error) { + if compositionName == "test-composition" && namespace == "default" { + return []*un.Unstructured{xr1}, nil + } + return nil, errors.New("not found") + }). + Build(), + } + }, + want: []*un.Unstructured{xr1}, + wantErr: false, + }, + "DifferentNamespace": { + compositionName: "test-composition", + namespace: "custom-ns", + setupMocks: func() xp.Clients { + return xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithFindXRsUsingComposition(func(_ context.Context, compositionName, namespace string) ([]*un.Unstructured, error) { + if compositionName == "test-composition" && namespace == "custom-ns" { + return []*un.Unstructured{xr2}, nil + } + return nil, errors.New("not found") + }). + Build(), + } + }, + want: []*un.Unstructured{xr2}, + wantErr: false, + }, + "ClientError": { + compositionName: "test-composition", + namespace: "default", + setupMocks: func() xp.Clients { + return xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { + return nil, errors.New("client error") + }). + Build(), + } + }, + want: nil, + wantErr: true, + }, + "NoXRsFound": { + compositionName: "test-composition", + namespace: "default", + setupMocks: func() xp.Clients { + return xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { + return []*un.Unstructured{}, nil + }). + Build(), + } + }, + want: []*un.Unstructured{}, + wantErr: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Create processor + processor := &DefaultCompDiffProcessor{ + xpClients: tt.setupMocks(), + config: ProcessorConfig{ + Logger: tu.TestLogger(t, false), + }, + } + + got, err := processor.findXRsUsingComposition(ctx, tt.compositionName, tt.namespace) + + if (err != nil) != tt.wantErr { + t.Errorf("findXRsUsingComposition() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if diff := gcmp.Diff(tt.want, got); diff != "" { + t.Errorf("findXRsUsingComposition() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestDefaultCompDiffProcessor_unstructuredToComposition(t *testing.T) { + tests := map[string]struct { + unstructured *un.Unstructured + want *apiextensionsv1.Composition + wantErr bool + }{ + "ValidUnstructured": { + unstructured: &un.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apiextensions.crossplane.io/v1", + "kind": "Composition", + "metadata": map[string]interface{}{ + "name": "test-composition", + }, + "spec": map[string]interface{}{ + "compositeTypeRef": map[string]interface{}{ + "apiVersion": "example.org/v1", + "kind": "XResource", + }, + "mode": "Pipeline", + }, + }, + }, + want: &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.crossplane.io/v1", + Kind: "Composition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-composition", + }, + Spec: apiextensionsv1.CompositionSpec{ + CompositeTypeRef: apiextensionsv1.TypeReference{ + APIVersion: "example.org/v1", + Kind: "XResource", + }, + Mode: apiextensionsv1.CompositionModePipeline, + }, + }, + wantErr: false, + }, + "InvalidUnstructured": { + unstructured: &un.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Pod", // Wrong kind + }, + }, + want: &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Pod", + }, + }, + wantErr: false, // Runtime converter doesn't validate, it just converts + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Create processor + processor := &DefaultCompDiffProcessor{ + config: ProcessorConfig{ + Logger: tu.TestLogger(t, false), + }, + } + + got, err := processor.unstructuredToComposition(tt.unstructured) + + if (err != nil) != tt.wantErr { + t.Errorf("unstructuredToComposition() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + if diff := gcmp.Diff(tt.want, got); diff != "" { + t.Errorf("unstructuredToComposition() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestDefaultCompDiffProcessor_compositionToUnstructured(t *testing.T) { + tests := map[string]struct { + composition *apiextensionsv1.Composition + wantErr bool + }{ + "ValidComposition": { + composition: &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.crossplane.io/v1", + Kind: "Composition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-composition", + }, + Spec: apiextensionsv1.CompositionSpec{ + CompositeTypeRef: apiextensionsv1.TypeReference{ + APIVersion: "example.org/v1", + Kind: "XResource", + }, + Mode: apiextensionsv1.CompositionModePipeline, + }, + }, + wantErr: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + // Create processor + processor := &DefaultCompDiffProcessor{ + config: ProcessorConfig{ + Logger: tu.TestLogger(t, false), + }, + } + + got, err := processor.compositionToUnstructured(tt.composition) + + if (err != nil) != tt.wantErr { + t.Errorf("compositionToUnstructured() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + // Basic checks + if got == nil { + t.Errorf("compositionToUnstructured() returned nil") + return + } + + if got.GetKind() != "Composition" { + t.Errorf("compositionToUnstructured() kind = %v, want Composition", got.GetKind()) + } + + if got.GetName() != tt.composition.GetName() { + t.Errorf("compositionToUnstructured() name = %v, want %v", got.GetName(), tt.composition.GetName()) + } + }) + } +} + +func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { + ctx := context.Background() + + // Create test composition + testComp := &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.crossplane.io/v1", + Kind: "Composition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-composition", + }, + Spec: apiextensionsv1.CompositionSpec{ + CompositeTypeRef: apiextensionsv1.TypeReference{ + APIVersion: "example.org/v1", + Kind: "XResource", + }, + Mode: apiextensionsv1.CompositionModePipeline, + }, + } + + // Create test XR + testXR := tu.NewResource("example.org/v1", "XResource", "test-xr"). + WithNamespace("default"). + Build() + + tests := map[string]struct { + compositions []*un.Unstructured + namespace string + setupMocks func() (k8.Clients, xp.Clients) + verifyOutput func(t *testing.T, output string) + wantErr bool + }{ + "SuccessfulDiff": { + namespace: "default", + compositions: []*un.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apiextensions.crossplane.io/v1", + "kind": "Composition", + "metadata": map[string]interface{}{ + "name": "test-composition", + }, + "spec": map[string]interface{}{ + "compositeTypeRef": map[string]interface{}{ + "apiVersion": "example.org/v1", + "kind": "XResource", + }, + "mode": "Pipeline", + }, + }, + }, + }, + setupMocks: func() (k8.Clients, xp.Clients) { + k8sClients := k8.Clients{ + Apply: tu.NewMockApplyClient().Build(), + Resource: tu.NewMockResourceClient().Build(), + Schema: tu.NewMockSchemaClient().Build(), + Type: tu.NewMockTypeConverter().Build(), + } + + xpClients := xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithSuccessfulCompositionFetch(testComp). + WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { + return []*un.Unstructured{testXR}, nil + }). + Build(), + Definition: tu.NewMockDefinitionClient().Build(), + Environment: tu.NewMockEnvironmentClient().Build(), + Function: tu.NewMockFunctionClient().Build(), + ResourceTree: tu.NewMockResourceTreeClient().Build(), + } + + return k8sClients, xpClients + }, + verifyOutput: func(t *testing.T, output string) { + t.Helper() + // Should contain composition changes section + if !strings.Contains(output, "=== Composition Changes ===") { + t.Errorf("Expected output to contain composition changes section") + } + // Should contain affected XRs section + if !strings.Contains(output, "=== Affected Composite Resources ===") { + t.Errorf("Expected output to contain affected XRs section") + } + }, + wantErr: false, + }, + "NoCompositions": { + namespace: "default", + compositions: []*un.Unstructured{}, + setupMocks: func() (k8.Clients, xp.Clients) { + return k8.Clients{}, xp.Clients{} + }, + wantErr: true, + }, + "MultipleCompositions": { + namespace: "default", + compositions: []*un.Unstructured{ + { + Object: map[string]interface{}{ + "apiVersion": "apiextensions.crossplane.io/v1", + "kind": "Composition", + "metadata": map[string]interface{}{ + "name": "test-composition-1", + }, + "spec": map[string]interface{}{ + "compositeTypeRef": map[string]interface{}{ + "apiVersion": "example.org/v1", + "kind": "XResource", + }, + "mode": "Pipeline", + }, + }, + }, + { + Object: map[string]interface{}{ + "apiVersion": "apiextensions.crossplane.io/v1", + "kind": "Composition", + "metadata": map[string]interface{}{ + "name": "test-composition-2", + }, + "spec": map[string]interface{}{ + "compositeTypeRef": map[string]interface{}{ + "apiVersion": "example.org/v1", + "kind": "XResource", + }, + "mode": "Pipeline", + }, + }, + }, + }, + setupMocks: func() (k8.Clients, xp.Clients) { + k8sClients := k8.Clients{ + Apply: tu.NewMockApplyClient().Build(), + Resource: tu.NewMockResourceClient().Build(), + Schema: tu.NewMockSchemaClient().Build(), + Type: tu.NewMockTypeConverter().Build(), + } + + // Create test compositions for the multi-composition test + testComp1 := &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.crossplane.io/v1", + Kind: "Composition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-composition-1", + }, + Spec: apiextensionsv1.CompositionSpec{ + CompositeTypeRef: apiextensionsv1.TypeReference{ + APIVersion: "example.org/v1", + Kind: "XResource", + }, + Mode: apiextensionsv1.CompositionModePipeline, + }, + } + + testComp2 := &apiextensionsv1.Composition{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiextensions.crossplane.io/v1", + Kind: "Composition", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-composition-2", + }, + Spec: apiextensionsv1.CompositionSpec{ + CompositeTypeRef: apiextensionsv1.TypeReference{ + APIVersion: "example.org/v1", + Kind: "XResource", + }, + Mode: apiextensionsv1.CompositionModePipeline, + }, + } + + xpClients := xp.Clients{ + Composition: tu.NewMockCompositionClient(). + WithGetComposition(func(_ context.Context, name string) (*apiextensionsv1.Composition, error) { + switch name { + case "test-composition-1": + return testComp1, nil + case "test-composition-2": + return testComp2, nil + default: + return nil, errors.New("composition not found") + } + }). + WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { + // Return no XRs for simplicity - just testing that multiple compositions are processed + return []*un.Unstructured{}, nil + }). + Build(), + Definition: tu.NewMockDefinitionClient().Build(), + Environment: tu.NewMockEnvironmentClient().Build(), + Function: tu.NewMockFunctionClient().Build(), + ResourceTree: tu.NewMockResourceTreeClient().Build(), + } + + return k8sClients, xpClients + }, + verifyOutput: func(t *testing.T, output string) { + t.Helper() + // Should contain composition changes sections for both compositions + compositionChangesSections := strings.Count(output, "=== Composition Changes ===") + if compositionChangesSections != 2 { + t.Errorf("Expected 2 composition changes sections, got %d", compositionChangesSections) + } + // Should contain separator between compositions + if !strings.Contains(output, strings.Repeat("=", 80)) { + t.Errorf("Expected output to contain composition separator") + } + }, + wantErr: false, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + k8sClients, xpClients := tt.setupMocks() + + // Create mock XR processor + mockXRProc := &tu.MockDiffProcessor{ + PerformDiffFn: func(_ context.Context, stdout io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { + _, err := stdout.Write([]byte("Mock XR diff output")) + return err + }, + } + + // Create processor + processor := &DefaultCompDiffProcessor{ + k8sClients: k8sClients, + xpClients: xpClients, + xrProc: mockXRProc, + config: ProcessorConfig{ + Namespace: tt.namespace, + Colorize: false, + Compact: false, + Logger: tu.TestLogger(t, false), + RenderFunc: func(_ context.Context, _ logging.Logger, in render.Inputs) (render.Outputs, error) { + return render.Outputs{ + CompositeResource: in.CompositeResource, + }, nil + }, + }, + } + + var stdout bytes.Buffer + + err := processor.DiffComposition(ctx, &stdout, tt.compositions, tt.namespace) + + if (err != nil) != tt.wantErr { + t.Errorf("DiffComposition() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + if tt.verifyOutput != nil { + tt.verifyOutput(t, stdout.String()) + } + }) + } +} diff --git a/cmd/diff/diffprocessor/diff_processor.go b/cmd/diff/diffprocessor/diff_processor.go index d282c52..193c6bf 100644 --- a/cmd/diff/diffprocessor/diff_processor.go +++ b/cmd/diff/diffprocessor/diff_processor.go @@ -12,6 +12,7 @@ import ( k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer" dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" @@ -30,8 +31,8 @@ type RenderFunc func(ctx context.Context, log logging.Logger, in render.Inputs) // DiffProcessor interface for processing resources. type DiffProcessor interface { - // PerformDiff processes all resources and produces a diff output - PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured) error + // PerformDiff processes resources using a composition provider function + PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured, compositionProvider types.CompositionProvider) error // Initialize loads required resources like CRDs and environment configs Initialize(ctx context.Context) error @@ -131,9 +132,9 @@ func (p *DefaultDiffProcessor) initializeSchemaValidator(ctx context.Context) er return nil } -// PerformDiff processes all resources and produces a diff output. -func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured) error { - p.config.Logger.Debug("Processing resources", "count", len(resources)) +// PerformDiff processes resources using a composition provider function. +func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured, compositionProvider types.CompositionProvider) error { + p.config.Logger.Debug("Processing resources with composition provider", "count", len(resources)) if len(resources) == 0 { p.config.Logger.Debug("No resources to process") @@ -148,7 +149,7 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer for _, res := range resources { resourceID := fmt.Sprintf("%s/%s", res.GetKind(), res.GetName()) - diffs, err := p.DiffSingleResource(ctx, res) + diffs, err := p.DiffSingleResource(ctx, res, compositionProvider) if err != nil { p.config.Logger.Debug("Failed to process resource", "resource", resourceID, "error", err) errs = append(errs, errors.Wrapf(err, "unable to process resource %s", resourceID)) @@ -183,7 +184,8 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer } // DiffSingleResource handles one resource at a time and returns its diffs. -func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.Unstructured) (map[string]*dt.ResourceDiff, error) { +// The compositionProvider function is called to obtain the composition to use for rendering. +func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.Unstructured, compositionProvider types.CompositionProvider) (map[string]*dt.ResourceDiff, error) { resourceID := fmt.Sprintf("%s/%s", res.GetKind(), res.GetName()) p.config.Logger.Debug("Processing resource", "resource", resourceID) @@ -192,16 +194,14 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U return nil, err } - // Find the matching composition - comp, err := p.compClient.FindMatchingComposition(ctx, res) + // Get the composition using the provided function + comp, err := compositionProvider(ctx, res) if err != nil { - p.config.Logger.Debug("No matching composition found", "resource", resourceID, "error", err) - return nil, errors.Wrap(err, "cannot find matching composition") + p.config.Logger.Debug("Failed to get composition", "resource", resourceID, "error", err) + return nil, errors.Wrap(err, "cannot get composition") } - p.config.Logger.Debug("Resource setup complete", - "resource", resourceID, - "composition", comp.GetName()) + p.config.Logger.Debug("Resource setup complete", "resource", resourceID, "composition", comp.GetName()) // Get functions for rendering fns, err := p.fnClient.GetFunctionsFromPipeline(comp) @@ -254,10 +254,15 @@ func (p *DefaultDiffProcessor) DiffSingleResource(ctx context.Context, res *un.U return nil, errors.Wrap(err, "cannot validate resources") } - // Calculate all diffs + // Calculate diffs p.config.Logger.Debug("Calculating diffs", "resource", resourceID) - diffs, err := p.diffCalculator.CalculateDiffs(ctx, xr, desired) + // Clean the XR for diff calculation - remove managed fields that can cause apply issues + cleanXR := xr.DeepCopy() + cleanXR.SetManagedFields(nil) + cleanXR.SetResourceVersion("") + + diffs, err := p.diffCalculator.CalculateDiffs(ctx, cleanXR, desired) if err != nil { // We don't fail completely if some diffs couldn't be calculated p.config.Logger.Debug("Partial error calculating diffs", "resource", resourceID, "error", err) diff --git a/cmd/diff/diffprocessor/diff_processor_test.go b/cmd/diff/diffprocessor/diff_processor_test.go index 9da3e00..b162008 100644 --- a/cmd/diff/diffprocessor/diff_processor_test.go +++ b/cmd/diff/diffprocessor/diff_processor_test.go @@ -139,7 +139,7 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { processorOpts: []ProcessorOption{ WithLogger(tu.TestLogger(t, false)), }, - want: errors.New("unable to process resource XR1/my-xr-1: cannot find matching composition: composition not found"), + want: errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"), }, "MultipleResourceErrors": { setupMocks: func() (k8.Clients, xp.Clients) { @@ -170,8 +170,8 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { processorOpts: []ProcessorOption{ WithLogger(tu.TestLogger(t, false)), }, - want: errors.New("[unable to process resource XR1/my-xr-1: cannot find matching composition: composition not found, " + - "unable to process resource XR1/my-xr-2: cannot find matching composition: composition not found]"), + want: errors.New("[unable to process resource XR1/my-xr-1: cannot get composition: composition not found, " + + "unable to process resource XR1/my-xr-2: cannot get composition: composition not found]"), }, "CompositionNotFound": { setupMocks: func() (k8.Clients, xp.Clients) { @@ -202,7 +202,7 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { processorOpts: []ProcessorOption{ WithLogger(tu.TestLogger(t, false)), }, - want: errors.New("unable to process resource XR1/my-xr-1: cannot find matching composition: composition not found"), + want: errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"), }, "GetFunctionsError": { setupMocks: func() (k8.Clients, xp.Clients) { @@ -492,7 +492,11 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { // Create a dummy writer for stdout var stdout bytes.Buffer - err := processor.PerformDiff(ctx, &stdout, tt.resources) + // Create a mock composition provider that uses the same mock composition client + compositionProvider := func(ctx context.Context, res *un.Unstructured) (*apiextensionsv1.Composition, error) { + return xpClients.Composition.FindMatchingComposition(ctx, res) + } + err := processor.PerformDiff(ctx, &stdout, tt.resources, compositionProvider) if tt.want != nil { if err == nil { diff --git a/cmd/diff/main.go b/cmd/diff/main.go index 477fa11..23074b7 100644 --- a/cmd/diff/main.go +++ b/cmd/diff/main.go @@ -48,7 +48,9 @@ type cli struct { // order they're specified here. Keep them in alphabetical order. // Subcommands. - Diff Cmd `cmd:"" help:"See what changes will be made against a live cluster when a given Crossplane resource would be applied."` + Comp CompCmd `cmd:"" help:"Show impact of composition changes on existing XRs."` + Diff DeprecatedDiffCmd `cmd:"" help:"(Deprecated) Use 'xr' subcommand instead. See what changes will be made against a live cluster when a given Crossplane resource would be applied."` + XR XRCmd `cmd:"" help:"See what changes will be made against a live cluster when a given Crossplane resource would be applied."` Version version.Cmd `cmd:"" help:"Print the client and server version information for the current context."` diff --git a/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml b/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml new file mode 100644 index 0000000..2b0dec7 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/nopclaim-crd.yaml @@ -0,0 +1,91 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: nopclaims.diff.example.org +spec: + group: diff.example.org + names: + kind: NopClaim + listKind: NopClaimList + plural: nopclaims + singular: nopclaim + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + compositionRef: + type: object + properties: + name: + type: string + compositeDeletePolicy: + type: string + enum: + - Background + - Foreground + default: "Background" + compositionUpdatePolicy: + type: string + enum: + - Automatic + - Manual + default: "Automatic" + compositionSelector: + type: object + properties: + matchLabels: + type: object + additionalProperties: + type: string + publishConnectionDetailsTo: + type: object + properties: + name: + type: string + configRef: + type: object + properties: + name: + type: string + metadata: + type: object + properties: + labels: + type: object + additionalProperties: + type: string + annotations: + type: object + additionalProperties: + type: string + writeConnectionSecretsToNamespace: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + lastTransitionTime: + type: string + format: date-time + message: + type: string + reason: + type: string + status: + type: string + type: + type: string \ No newline at end of file diff --git a/cmd/diff/testdata/comp/crds/xdefaultresource-ns-crd.yaml b/cmd/diff/testdata/comp/crds/xdefaultresource-ns-crd.yaml new file mode 100644 index 0000000..bdc0edd --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xdefaultresource-ns-crd.yaml @@ -0,0 +1,79 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xtestdefaultresources.ns.diff.example.org +spec: + group: ns.diff.example.org + names: + kind: XTestDefaultResource + plural: xtestdefaultresources + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + region: + type: string + default: "us-east-1" + description: "AWS region with default value" + size: + type: string + default: "small" + description: "Instance size with default" + settings: + type: object + default: + enabled: true + timeout: 30 + properties: + enabled: + type: boolean + default: true + timeout: + type: integer + default: 30 + retries: + type: integer + default: 3 + tags: + type: object + default: + environment: "development" + properties: + environment: + type: string + default: "development" + team: + type: string + status: + type: object + properties: + conditions: + description: Conditions of the resource. + type: array + items: + type: object + required: + - lastTransitionTime + - reason + - status + - type + properties: + lastTransitionTime: + type: string + format: date-time + message: + type: string + reason: + type: string + status: + type: string + type: + type: string \ No newline at end of file diff --git a/cmd/diff/testdata/comp/crds/xdownstreamenvresource-crd.yaml b/cmd/diff/testdata/comp/crds/xdownstreamenvresource-crd.yaml new file mode 100644 index 0000000..285d59d --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xdownstreamenvresource-crd.yaml @@ -0,0 +1,55 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xdownstreamenvresources.nop.example.org +spec: + group: nop.example.org + names: + kind: XDownstreamEnvResource + listKind: XDownstreamEnvResourceList + plural: xdownstreamenvresources + singular: xdownstreamenvresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + forProvider: + type: object + properties: + configData: + type: string + region: + type: string + environment: + type: string + serviceLevel: + type: string + compositionUpdatePolicy: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xdownstreamresource-cluster-crd.yaml b/cmd/diff/testdata/comp/crds/xdownstreamresource-cluster-crd.yaml new file mode 100644 index 0000000..97fb39c --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xdownstreamresource-cluster-crd.yaml @@ -0,0 +1,82 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xdownstreamresources.nop.example.org +spec: + group: nop.example.org + names: + kind: XDownstreamResource + listKind: XDownstreamResourceList + plural: xdownstreamresources + singular: xdownstreamresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + forProvider: + type: object + properties: + configData: + type: string + roleName: + type: string + description: "Name of an external cluster role referenced by the resource" + resourceTier: + type: string + description: "Tier of the resource (e.g., default, production, staging)" + crossplane: + type: object + properties: + compositeDeletePolicy: + type: string + resourceRefs: + type: array + description: "References to resources that this resource depends on" + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xdownstreamresource-legacycluster-crd.yaml b/cmd/diff/testdata/comp/crds/xdownstreamresource-legacycluster-crd.yaml new file mode 100644 index 0000000..84e4ee0 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xdownstreamresource-legacycluster-crd.yaml @@ -0,0 +1,79 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xdownstreamresources.legacycluster.nop.example.org +spec: + group: legacycluster.nop.example.org + names: + kind: XDownstreamResource + listKind: XDownstreamResourceList + plural: xdownstreamresources + singular: xdownstreamresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + forProvider: + type: object + properties: + configData: + type: string + roleName: + type: string + description: "Name of an external cluster role referenced by the resource" + resourceTier: + type: string + description: "Tier of the resource (e.g., default, production, staging)" + compositeDeletePolicy: + type: string + resourceRefs: + type: array + description: "References to resources that this resource depends on" + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xdownstreamresource-ns-crd.yaml b/cmd/diff/testdata/comp/crds/xdownstreamresource-ns-crd.yaml new file mode 100644 index 0000000..f7d5747 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xdownstreamresource-ns-crd.yaml @@ -0,0 +1,82 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xdownstreamresources.ns.nop.example.org +spec: + group: ns.nop.example.org + names: + kind: XDownstreamResource + listKind: XDownstreamResourceList + plural: xdownstreamresources + singular: xdownstreamresource + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + forProvider: + type: object + properties: + configData: + type: string + roleName: + type: string + description: "Name of an external cluster role referenced by the resource" + resourceTier: + type: string + description: "Tier of the resource (e.g., default, production, staging)" + crossplane: + type: object + properties: + compositeDeletePolicy: + type: string + resourceRefs: + type: array + description: "References to resources that this resource depends on" + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xenvresource-crd.yaml b/cmd/diff/testdata/comp/crds/xenvresource-crd.yaml new file mode 100644 index 0000000..f4852b7 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xenvresource-crd.yaml @@ -0,0 +1,71 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xenvresources.ns.diff.example.org +spec: + group: ns.diff.example.org + names: + kind: XEnvResource + listKind: XEnvResourceList + plural: xenvresources + singular: xenvresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + configKey: + type: string + compositeDeletePolicy: + type: string + compositionUpdatePolicy: + type: string + resourceRefs: + type: array + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xnopresource-cluster-crd.yaml b/cmd/diff/testdata/comp/crds/xnopresource-cluster-crd.yaml new file mode 100644 index 0000000..8ace739 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xnopresource-cluster-crd.yaml @@ -0,0 +1,88 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xnopresources.diff.example.org +spec: + group: diff.example.org + names: + kind: XNopResource + listKind: XNopResourceList + plural: xnopresources + singular: xnopresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + environment: + type: string + crossplane: + type: object + properties: + compositionRef: + type: object + properties: + name: + type: string + compositionSelector: + type: object + properties: + matchLabels: + type: object + additionalProperties: + type: string + compositeDeletePolicy: + type: string + compositionUpdatePolicy: + type: string + resourceRefs: + type: array + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xnopresource-legacycluster-crd.yaml b/cmd/diff/testdata/comp/crds/xnopresource-legacycluster-crd.yaml new file mode 100644 index 0000000..aea2031 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xnopresource-legacycluster-crd.yaml @@ -0,0 +1,85 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xnopresources.legacycluster.diff.example.org +spec: + group: legacycluster.diff.example.org + names: + kind: XNopResource + listKind: XNopResourceList + plural: xnopresources + singular: xnopresource + scope: Cluster + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + environment: + type: string + compositionRef: + type: object + properties: + name: + type: string + compositionSelector: + type: object + properties: + matchLabels: + type: object + additionalProperties: + type: string + compositeDeletePolicy: + type: string + compositionUpdatePolicy: + type: string + resourceRefs: + type: array + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/crds/xnopresource-ns-crd.yaml b/cmd/diff/testdata/comp/crds/xnopresource-ns-crd.yaml new file mode 100644 index 0000000..a2838a3 --- /dev/null +++ b/cmd/diff/testdata/comp/crds/xnopresource-ns-crd.yaml @@ -0,0 +1,89 @@ +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: xnopresources.ns.diff.example.org +spec: + group: ns.diff.example.org + names: + kind: XNopResource + listKind: XNopResourceList + plural: xnopresources + singular: xnopresource + # This supports a v2 XRD, which makes it namespace scoped. + scope: Namespaced + versions: + - name: v1alpha1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + environment: + type: string + crossplane: + type: object + properties: + compositionRef: + type: object + properties: + name: + type: string + compositionSelector: + type: object + properties: + matchLabels: + type: object + additionalProperties: + type: string + compositeDeletePolicy: + type: string + compositionUpdatePolicy: + type: string + resourceRefs: + type: array + items: + type: object + required: + - apiVersion + - kind + - name + properties: + apiVersion: + type: string + kind: + type: string + name: + type: string + namespace: + type: string + uid: + type: string + resourceVersion: + type: string + fieldPath: + type: string + status: + type: object + properties: + conditions: + type: array + items: + type: object + properties: + type: + type: string + status: + type: string + reason: + type: string + message: + type: string + lastTransitionTime: + type: string + format: date-time diff --git a/cmd/diff/testdata/comp/net-new-composition.yaml b/cmd/diff/testdata/comp/net-new-composition.yaml new file mode 100644 index 0000000..4bfa487 --- /dev/null +++ b/cmd/diff/testdata/comp/net-new-composition.yaml @@ -0,0 +1,37 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xnewresources.diff.example.org + labels: + environment: staging + provider: aws +spec: + compositeTypeRef: + apiVersion: staging.diff.example.org/v1alpha1 + kind: XNewResource + mode: Pipeline + pipeline: + - step: generate-new-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: staging.nop.example.org/v1alpha1 + kind: XStagingResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: staging-resource + spec: + forProvider: + environment: staging + configData: new-{{ .observed.composite.resource.spec.newField }} + resourceTier: premium + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/custom-namespace.yaml b/cmd/diff/testdata/comp/resources/custom-namespace.yaml new file mode 100644 index 0000000..bb71c9a --- /dev/null +++ b/cmd/diff/testdata/comp/resources/custom-namespace.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: custom-namespace \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-custom-ns-downstream.yaml b/cmd/diff/testdata/comp/resources/existing-custom-ns-downstream.yaml new file mode 100644 index 0000000..b8340bd --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-custom-ns-downstream.yaml @@ -0,0 +1,13 @@ +apiVersion: ns.nop.example.org/v1alpha1 +kind: XDownstreamResource +metadata: + name: custom-namespace-resource + namespace: custom-namespace + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: custom-namespace-resource +spec: + forProvider: + configData: custom-ns-existing-value + resourceTier: basic \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-custom-ns-xr.yaml b/cmd/diff/testdata/comp/resources/existing-custom-ns-xr.yaml new file mode 100644 index 0000000..47780a6 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-custom-ns-xr.yaml @@ -0,0 +1,10 @@ +apiVersion: ns.diff.example.org/v1alpha1 +kind: XNopResource +metadata: + name: custom-namespace-resource + namespace: custom-namespace +spec: + coolField: custom-ns-existing-value + crossplane: + compositionRef: + name: xnopresources.diff.example.org \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-downstream-1.yaml b/cmd/diff/testdata/comp/resources/existing-downstream-1.yaml new file mode 100644 index 0000000..926572e --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-downstream-1.yaml @@ -0,0 +1,13 @@ +apiVersion: ns.nop.example.org/v1alpha1 +kind: XDownstreamResource +metadata: + name: test-resource + namespace: default + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: test-resource +spec: + forProvider: + configData: existing-value + resourceTier: basic \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-downstream-2.yaml b/cmd/diff/testdata/comp/resources/existing-downstream-2.yaml new file mode 100644 index 0000000..3607c39 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-downstream-2.yaml @@ -0,0 +1,13 @@ +apiVersion: ns.nop.example.org/v1alpha1 +kind: XDownstreamResource +metadata: + name: another-resource + namespace: default + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + labels: + crossplane.io/composite: another-resource +spec: + forProvider: + configData: another-existing-value + resourceTier: basic \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-xr-1.yaml b/cmd/diff/testdata/comp/resources/existing-xr-1.yaml new file mode 100644 index 0000000..1b1e29d --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-xr-1.yaml @@ -0,0 +1,10 @@ +apiVersion: ns.diff.example.org/v1alpha1 +kind: XNopResource +metadata: + name: test-resource + namespace: default +spec: + coolField: existing-value + crossplane: + compositionRef: + name: xnopresources.diff.example.org \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/existing-xr-2.yaml b/cmd/diff/testdata/comp/resources/existing-xr-2.yaml new file mode 100644 index 0000000..3c2dd68 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/existing-xr-2.yaml @@ -0,0 +1,10 @@ +apiVersion: ns.diff.example.org/v1alpha1 +kind: XNopResource +metadata: + name: another-resource + namespace: default +spec: + coolField: another-existing-value + crossplane: + compositionRef: + name: xnopresources.diff.example.org \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/functions.yaml b/cmd/diff/testdata/comp/resources/functions.yaml new file mode 100644 index 0000000..3c32822 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/functions.yaml @@ -0,0 +1,29 @@ +--- +# TODO: when updating these, ensure you also update the list of pulled images in Earthfile/go-test +apiVersion: pkg.crossplane.io/v1beta1 +kind: Function +metadata: + name: function-go-templating +spec: + package: xpkg.upbound.io/crossplane-contrib/function-go-templating:v0.11.0 +--- +apiVersion: pkg.crossplane.io/v1beta1 +kind: Function +metadata: + name: function-auto-ready +spec: + package: xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.4.2 +--- +apiVersion: pkg.crossplane.io/v1beta1 +kind: Function +metadata: + name: function-environment-configs +spec: + package: xpkg.upbound.io/crossplane-contrib/function-environment-configs:v0.4.0 +--- +apiVersion: pkg.crossplane.io/v1beta1 +kind: Function +metadata: + name: function-extra-resources +spec: + package: xpkg.upbound.io/crossplane-contrib/function-extra-resources:v0.1.0 diff --git a/cmd/diff/testdata/comp/resources/original-composition-2.yaml b/cmd/diff/testdata/comp/resources/original-composition-2.yaml new file mode 100644 index 0000000..a6605ce --- /dev/null +++ b/cmd/diff/testdata/comp/resources/original-composition-2.yaml @@ -0,0 +1,33 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xnopresources-v2.diff.example.org +spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource-v2 + spec: + forProvider: + configData: v2-updated-{{ .observed.composite.resource.spec.coolField }} + resourceTier: enterprise + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/original-composition.yaml b/cmd/diff/testdata/comp/resources/original-composition.yaml new file mode 100644 index 0000000..0a58369 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/original-composition.yaml @@ -0,0 +1,33 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xnopresources.diff.example.org +spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: + configData: {{ .observed.composite.resource.spec.coolField }} + resourceTier: basic + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/cmd/diff/testdata/comp/resources/xrd.yaml b/cmd/diff/testdata/comp/resources/xrd.yaml new file mode 100644 index 0000000..b4b5706 --- /dev/null +++ b/cmd/diff/testdata/comp/resources/xrd.yaml @@ -0,0 +1,51 @@ +# This is a v2 XRD so instances of the XR will be namespace scoped. +apiVersion: apiextensions.crossplane.io/v2 +kind: CompositeResourceDefinition +metadata: + name: xnopresources.ns.diff.example.org +spec: + group: ns.diff.example.org + names: + kind: XNopResource + plural: xnopresources + scope: Namespaced + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + environment: + type: string + status: + type: object + properties: + conditions: + description: Conditions of the resource. + type: array + items: + type: object + required: + - lastTransitionTime + - reason + - status + - type + properties: + lastTransitionTime: + type: string + format: date-time + message: + type: string + reason: + type: string + status: + type: string + type: + type: string diff --git a/cmd/diff/testdata/comp/updated-composition-2.yaml b/cmd/diff/testdata/comp/updated-composition-2.yaml new file mode 100644 index 0000000..a6605ce --- /dev/null +++ b/cmd/diff/testdata/comp/updated-composition-2.yaml @@ -0,0 +1,33 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xnopresources-v2.diff.example.org +spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource-v2 + spec: + forProvider: + configData: v2-updated-{{ .observed.composite.resource.spec.coolField }} + resourceTier: enterprise + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/cmd/diff/testdata/comp/updated-composition.yaml b/cmd/diff/testdata/comp/updated-composition.yaml new file mode 100644 index 0000000..e3e72ed --- /dev/null +++ b/cmd/diff/testdata/comp/updated-composition.yaml @@ -0,0 +1,33 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xnopresources.diff.example.org +spec: + compositeTypeRef: + apiVersion: ns.diff.example.org/v1alpha1 + kind: XNopResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-go-templating + input: + apiVersion: template.fn.crossplane.io/v1beta1 + kind: GoTemplate + source: Inline + inline: + template: | + apiVersion: ns.nop.example.org/v1alpha1 + kind: XDownstreamResource + metadata: + name: {{ .observed.composite.resource.metadata.name }} + namespace: {{ .observed.composite.resource.metadata.namespace }} + annotations: + gotemplating.fn.crossplane.io/composition-resource-name: nop-resource + spec: + forProvider: + configData: updated-{{ .observed.composite.resource.spec.coolField }} + resourceTier: premium + - step: automatically-detect-ready-composed-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/cmd/diff/testutils/mock_builder.go b/cmd/diff/testutils/mock_builder.go index 0904643..8dad4ce 100644 --- a/cmd/diff/testutils/mock_builder.go +++ b/cmd/diff/testutils/mock_builder.go @@ -6,13 +6,14 @@ import ( "io" "strings" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" + k8stypes "k8s.io/apimachinery/pkg/types" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" cpd "github.com/crossplane/crossplane-runtime/v2/pkg/resource/unstructured/composed" @@ -657,6 +658,23 @@ func (b *MockCompositionClientBuilder) WithGetComposition(fn func(context.Contex return b } +// WithSuccessfulCompositionFetch sets up a successful composition fetch. +func (b *MockCompositionClientBuilder) WithSuccessfulCompositionFetch(comp *xpextv1.Composition) *MockCompositionClientBuilder { + return b.WithGetComposition(func(_ context.Context, name string) (*xpextv1.Composition, error) { + if name == comp.GetName() { + return comp, nil + } + + return nil, errors.New("composition not found") + }) +} + +// WithFindXRsUsingComposition sets the FindXRsUsingComposition behavior. +func (b *MockCompositionClientBuilder) WithFindXRsUsingComposition(fn func(context.Context, string, string) ([]*un.Unstructured, error)) *MockCompositionClientBuilder { + b.mock.FindXRsUsingCompositionFn = fn + return b +} + // Build returns the built mock. func (b *MockCompositionClientBuilder) Build() *MockCompositionClient { return b.mock @@ -1049,21 +1067,21 @@ func (b *DiffProcessorBuilder) WithFailedInitialize(errMsg string) *DiffProcesso } // WithPerformDiff adds an implementation for the PerformDiff method. -func (b *DiffProcessorBuilder) WithPerformDiff(fn func(io.Writer, context.Context, []*un.Unstructured) error) *DiffProcessorBuilder { +func (b *DiffProcessorBuilder) WithPerformDiff(fn func(context.Context, io.Writer, []*un.Unstructured, types.CompositionProvider) error) *DiffProcessorBuilder { b.mock.PerformDiffFn = fn return b } // WithSuccessfulPerformDiff sets a successful PerformDiff implementation. func (b *DiffProcessorBuilder) WithSuccessfulPerformDiff() *DiffProcessorBuilder { - return b.WithPerformDiff(func(io.Writer, context.Context, []*un.Unstructured) error { + return b.WithPerformDiff(func(context.Context, io.Writer, []*un.Unstructured, types.CompositionProvider) error { return nil }) } // WithDiffOutput sets a PerformDiff implementation that writes a specific output. func (b *DiffProcessorBuilder) WithDiffOutput(output string) *DiffProcessorBuilder { - return b.WithPerformDiff(func(stdout io.Writer, _ context.Context, _ []*un.Unstructured) error { + return b.WithPerformDiff(func(_ context.Context, stdout io.Writer, _ []*un.Unstructured, _ types.CompositionProvider) error { if stdout != nil { _, _ = io.WriteString(stdout, output) } @@ -1074,7 +1092,7 @@ func (b *DiffProcessorBuilder) WithDiffOutput(output string) *DiffProcessorBuild // WithFailedPerformDiff sets a failing PerformDiff implementation. func (b *DiffProcessorBuilder) WithFailedPerformDiff(errMsg string) *DiffProcessorBuilder { - return b.WithPerformDiff(func(io.Writer, context.Context, []*un.Unstructured) error { + return b.WithPerformDiff(func(context.Context, io.Writer, []*un.Unstructured, types.CompositionProvider) error { return errors.New(errMsg) }) } @@ -1202,7 +1220,7 @@ func (b *ResourceBuilder) WithOwnerReference(kind, name, apiVersion, uid string) APIVersion: apiVersion, Kind: kind, Name: name, - UID: types.UID(uid), + UID: k8stypes.UID(uid), } // Append the new owner reference @@ -1241,6 +1259,12 @@ func (b *ResourceBuilder) WithCompositionResourceName(name string) *ResourceBuil return b } +// WithNamespace sets the namespace for the resource. +func (b *ResourceBuilder) WithNamespace(namespace string) *ResourceBuilder { + b.resource.SetNamespace(namespace) + return b +} + // Build returns the built unstructured resource. func (b *ResourceBuilder) Build() *un.Unstructured { return b.resource.DeepCopy() diff --git a/cmd/diff/testutils/mocks.go b/cmd/diff/testutils/mocks.go index 4869c88..99cdf03 100644 --- a/cmd/diff/testutils/mocks.go +++ b/cmd/diff/testutils/mocks.go @@ -5,11 +5,12 @@ import ( "io" dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" + "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" + k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/dynamic" @@ -56,7 +57,7 @@ type MockNamespaceableResourceInterface struct { CreateFn func(ctx context.Context, obj *un.Unstructured, options metav1.CreateOptions, subresources ...string) (*un.Unstructured, error) UpdateFn func(ctx context.Context, obj *un.Unstructured, options metav1.UpdateOptions, subresources ...string) (*un.Unstructured, error) DeleteFn func(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error - PatchFn func(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) + PatchFn func(ctx context.Context, name string, pt k8stypes.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) } // Namespace implements dynamic.NamespaceableResourceInterface. @@ -136,7 +137,7 @@ func (m *MockNamespaceableResourceInterface) Watch(_ context.Context, _ metav1.L } // Patch implements dynamic.ResourceInterface. -func (m *MockNamespaceableResourceInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) { +func (m *MockNamespaceableResourceInterface) Patch(ctx context.Context, name string, pt k8stypes.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) { if m.PatchFn != nil { return m.PatchFn(ctx, name, pt, data, options, subresources...) } @@ -165,7 +166,7 @@ type MockResourceInterface struct { CreateFn func(ctx context.Context, obj *un.Unstructured, options metav1.CreateOptions, subresources ...string) (*un.Unstructured, error) UpdateFn func(ctx context.Context, obj *un.Unstructured, options metav1.UpdateOptions, subresources ...string) (*un.Unstructured, error) DeleteFn func(ctx context.Context, name string, options metav1.DeleteOptions, subresources ...string) error - PatchFn func(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) + PatchFn func(ctx context.Context, name string, pt k8stypes.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) } // Create implements dynamic.ResourceInterface. @@ -229,7 +230,7 @@ func (m *MockResourceInterface) Watch(_ context.Context, _ metav1.ListOptions) ( } // Patch implements dynamic.ResourceInterface. -func (m *MockResourceInterface) Patch(ctx context.Context, name string, pt types.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) { +func (m *MockResourceInterface) Patch(ctx context.Context, name string, pt k8stypes.PatchType, data []byte, options metav1.PatchOptions, subresources ...string) (*un.Unstructured, error) { if m.PatchFn != nil { return m.PatchFn(ctx, name, pt, data, options, subresources...) } @@ -255,7 +256,7 @@ func (m *MockResourceInterface) ApplyStatus(_ context.Context, _ string, _ *un.U type MockDiffProcessor struct { // Function fields for mocking behavior InitializeFn func(ctx context.Context) error - PerformDiffFn func(stdout io.Writer, ctx context.Context, resources []*un.Unstructured) error + PerformDiffFn func(ctx context.Context, stdout io.Writer, resources []*un.Unstructured, compositionProvider types.CompositionProvider) error } // Initialize implements the DiffProcessor interface. @@ -268,9 +269,9 @@ func (m *MockDiffProcessor) Initialize(ctx context.Context) error { } // PerformDiff implements the DiffProcessor.PerformDiff method. -func (m *MockDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured) error { +func (m *MockDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer, resources []*un.Unstructured, compositionProvider types.CompositionProvider) error { if m.PerformDiffFn != nil { - return m.PerformDiffFn(stdout, ctx, resources) + return m.PerformDiffFn(ctx, stdout, resources, compositionProvider) } return nil @@ -537,6 +538,7 @@ type MockCompositionClient struct { FindMatchingCompositionFn func(ctx context.Context, res *un.Unstructured) (*xpextv1.Composition, error) ListCompositionsFn func(ctx context.Context) ([]*xpextv1.Composition, error) GetCompositionFn func(ctx context.Context, name string) (*xpextv1.Composition, error) + FindXRsUsingCompositionFn func(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) } // Initialize implements crossplane.CompositionClient. @@ -575,6 +577,15 @@ func (m *MockCompositionClient) GetComposition(ctx context.Context, name string) return nil, errors.New("GetComposition not implemented") } +// FindXRsUsingComposition implements crossplane.CompositionClient. +func (m *MockCompositionClient) FindXRsUsingComposition(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) { + if m.FindXRsUsingCompositionFn != nil { + return m.FindXRsUsingCompositionFn(ctx, compositionName, namespace) + } + + return nil, errors.New("FindXRsUsingComposition not implemented") +} + // MockFunctionClient implements the crossplane.FunctionClient interface. type MockFunctionClient struct { InitializeFn func(ctx context.Context) error diff --git a/cmd/diff/types/types.go b/cmd/diff/types/types.go new file mode 100644 index 0000000..428e199 --- /dev/null +++ b/cmd/diff/types/types.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 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 types defines shared type definitions and interfaces used across the crossplane-diff application. +package types //nolint:revive // types is an appropriate name for a package containing shared type definitions + +import ( + "context" + + un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + apiextensionsv1 "github.com/crossplane/crossplane/v2/apis/apiextensions/v1" +) + +// CompositionProvider is a function that provides a composition for a given resource. +type CompositionProvider func(ctx context.Context, res *un.Unstructured) (*apiextensionsv1.Composition, error) diff --git a/cmd/diff/diff.go b/cmd/diff/xr.go similarity index 61% rename from cmd/diff/diff.go rename to cmd/diff/xr.go index 1eb3225..e0d6d14 100644 --- a/cmd/diff/diff.go +++ b/cmd/diff/xr.go @@ -14,12 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package diff contains the diff command. +// Package main contains the XR diff command. package main import ( "context" - "time" + "fmt" "github.com/alecthomas/kong" dp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/diffprocessor" @@ -32,22 +32,17 @@ import ( "github.com/crossplane/crossplane/v2/cmd/crank/render" ) -// Cmd represents the diff command. -// Cmd represents the diff command. -type Cmd struct { +// XRCmd represents the XR diff command. +type XRCmd struct { + // Embed common fields + CommonCmdFields + Namespace string `default:"crossplane-system" help:"Namespace to compare resources against." name:"namespace" short:"n"` Files []string `arg:"" help:"YAML files containing Crossplane resources to diff." optional:""` - - // Configuration options - NoColor bool `help:"Disable colorized output." name:"no-color"` - Compact bool `help:"Show compact diffs with minimal context." name:"compact"` - Timeout time.Duration `default:"1m" help:"How long to run before timing out."` - QPS float32 `default:"0" help:"Maximum QPS to the API server."` - Burst int `default:"0" help:"Maximum burst for throttle."` } -// Help returns help instructions for the diff command. -func (c *Cmd) Help() string { +// Help returns help instructions for the XR diff command. +func (c *XRCmd) Help() string { return ` This command returns a diff of the in-cluster resources that would be modified if the provided Crossplane resources were applied. @@ -55,79 +50,47 @@ Similar to kubectl diff, it requires Crossplane to be operating in the live clus Examples: # Show the changes that would result from applying xr.yaml (via file) in the default 'crossplane-system' namespace. - crossplane diff xr.yaml + crossplane-diff xr xr.yaml # Show the changes that would result from applying xr.yaml (via stdin) in the default 'crossplane-system' namespace. - cat xr.yaml | crossplane diff -- + cat xr.yaml | crossplane-diff xr -- # Show the changes that would result from applying xr.yaml, xr1.yaml, and xr2.yaml in the default 'crossplane-system' namespace. - cat xr.yaml | crossplane diff xr1.yaml xr2.yaml -- + cat xr.yaml | crossplane-diff xr xr1.yaml xr2.yaml -- # Show the changes that would result from applying xr.yaml (via file) in the 'foobar' namespace with no color output. - crossplane diff xr.yaml -n foobar --no-color + crossplane-diff xr xr.yaml -n foobar --no-color # Show the changes in a compact format with minimal context. - crossplane diff xr.yaml --compact + crossplane-diff xr xr.yaml --compact ` } // AfterApply implements kong's AfterApply method to bind our dependencies. -func (c *Cmd) AfterApply(ctx *kong.Context, log logging.Logger, config *rest.Config) error { +func (c *XRCmd) AfterApply(ctx *kong.Context, log logging.Logger, config *rest.Config) error { return c.initializeDependencies(ctx, log, config) } -func (c *Cmd) initializeDependencies(ctx *kong.Context, log logging.Logger, config *rest.Config) error { - config = c.initRestConfig(config, log) - - appCtx, err := NewAppContext(config, log) +func (c *XRCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, config *rest.Config) error { + appCtx, err := initializeSharedDependencies(ctx, log, config, c.CommonCmdFields) if err != nil { - return errors.Wrap(err, "cannot create app context") + return err } - proc := makeDefaultProc(c, appCtx, log) + proc := makeDefaultXRProc(c, appCtx, log) - loader, err := makeDefaultLoader(c) + loader, err := makeDefaultXRLoader(c) if err != nil { return errors.Wrap(err, "cannot create resource loader") } - ctx.Bind(appCtx) ctx.BindTo(proc, (*dp.DiffProcessor)(nil)) ctx.BindTo(loader, (*ld.Loader)(nil)) return nil } -func (c *Cmd) initRestConfig(config *rest.Config, logger logging.Logger) *rest.Config { - // Set default QPS and Burst if they are not set in the config - // or override with values from options if provided - originalQPS := config.QPS - originalBurst := config.Burst - - if c.QPS > 0 { - config.QPS = c.QPS - } else if config.QPS == 0 { - config.QPS = 20 - } - - if c.Burst > 0 { - config.Burst = c.Burst - } else if config.Burst == 0 { - config.Burst = 30 - } - - logger.Debug("Configured REST client rate limits", - "original_qps", originalQPS, - "original_burst", originalBurst, - "options_qps", c.QPS, - "options_burst", c.Burst, - "final_qps", config.QPS, - "final_burst", config.Burst) - - return config -} - -func makeDefaultProc(c *Cmd, ctx *AppContext, log logging.Logger) dp.DiffProcessor { +func makeDefaultXRProc(c *XRCmd, ctx *AppContext, log logging.Logger) dp.DiffProcessor { // Create the options for the processor options := []dp.ProcessorOption{ dp.WithNamespace(c.Namespace), @@ -140,12 +103,52 @@ func makeDefaultProc(c *Cmd, ctx *AppContext, log logging.Logger) dp.DiffProcess return dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) } -func makeDefaultLoader(c *Cmd) (ld.Loader, error) { +func makeDefaultXRLoader(c *XRCmd) (ld.Loader, error) { return ld.NewCompositeLoader(c.Files) } -// Run executes the diff command. -func (c *Cmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc dp.DiffProcessor, loader ld.Loader) error { +// DeprecatedDiffCmd represents the deprecated diff command for backward compatibility. +type DeprecatedDiffCmd struct { + XRCmd +} + +// Help returns help instructions for the deprecated diff command. +func (c *DeprecatedDiffCmd) Help() string { + return ` +This command is deprecated. Please use 'crossplane-diff xr' instead. + +This command returns a diff of the in-cluster resources that would be modified if the provided Crossplane resources were applied. + +Similar to kubectl diff, it requires Crossplane to be operating in the live cluster found in your kubeconfig. + +Examples: + # Deprecated usage (still works but will show warning) + crossplane-diff diff xr.yaml + + # Preferred usage + crossplane-diff xr xr.yaml +` +} + +// Run executes the deprecated diff command with a deprecation warning. +func (c *DeprecatedDiffCmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc dp.DiffProcessor, loader ld.Loader) error { + if _, err := fmt.Fprintln(k.Stderr, "Warning: The 'diff' subcommand is deprecated. Please use 'xr' instead."); err != nil { + return err + } + + if _, err := fmt.Fprintln(k.Stderr, "Example: crossplane-diff xr your-file.yaml"); err != nil { + return err + } + + if _, err := fmt.Fprintln(k.Stderr); err != nil { + return err + } + + return c.XRCmd.Run(k, log, appCtx, proc, loader) +} + +// Run executes the XR diff command. +func (c *XRCmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc dp.DiffProcessor, loader ld.Loader) error { // the rest config here is provided by a function in main.go that's only invoked for commands that request it // in their arguments. that means we won't get "can't find kubeconfig" errors for cases where the config isn't asked for. @@ -173,7 +176,7 @@ func (c *Cmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc return errors.Wrap(err, "cannot initialize diff processor") } - if err := proc.PerformDiff(ctx, k.Stdout, resources); err != nil { + if err := proc.PerformDiff(ctx, k.Stdout, resources, appCtx.XpClients.Composition.FindMatchingComposition); err != nil { return errors.Wrap(err, "unable to process one or more resources") } From 585cbc63b2a6fff610f217ae5140f9269a4bdbc9 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 13:06:21 -0400 Subject: [PATCH 2/8] Minor refactoring to increase code reuse Signed-off-by: Jonathan Ogilvie --- cmd/diff/cmd_utils.go | 27 +++++++++++++++++++++++++++ cmd/diff/comp.go | 27 ++++++--------------------- cmd/diff/xr.go | 21 +++++---------------- 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/cmd/diff/cmd_utils.go b/cmd/diff/cmd_utils.go index bb875d4..644df31 100644 --- a/cmd/diff/cmd_utils.go +++ b/cmd/diff/cmd_utils.go @@ -18,13 +18,17 @@ limitations under the License. package main import ( + "context" "time" "github.com/alecthomas/kong" + dp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/diffprocessor" "k8s.io/client-go/rest" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" + + "github.com/crossplane/crossplane/v2/cmd/crank/render" ) // CommonCmdFields contains common fields shared by both XR and Comp commands. @@ -80,3 +84,26 @@ func initRestConfig(config *rest.Config, logger logging.Logger, fields CommonCmd return config } + +// createProcessorOptions creates the standard processor options from common command fields. +// Includes all options that may be used by any processor type. +func createProcessorOptions(fields CommonCmdFields, namespace string, log logging.Logger) []dp.ProcessorOption { + return []dp.ProcessorOption{ + dp.WithNamespace(namespace), + dp.WithLogger(log), + dp.WithColorize(!fields.NoColor), + dp.WithCompact(fields.Compact), + dp.WithRenderFunc(render.Render), // Safe to include even if unused + } +} + +// initializeAppContext initializes the application context with timeout and error handling. +func initializeAppContext(timeout time.Duration, appCtx *AppContext, log logging.Logger) (context.Context, context.CancelFunc, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + if err := appCtx.Initialize(ctx, log); err != nil { + cancel() + return nil, nil, errors.Wrap(err, "cannot initialize client") + } + + return ctx, cancel, nil +} diff --git a/cmd/diff/comp.go b/cmd/diff/comp.go index d9742eb..3b9844b 100644 --- a/cmd/diff/comp.go +++ b/cmd/diff/comp.go @@ -18,8 +18,6 @@ limitations under the License. package main import ( - "context" - "github.com/alecthomas/kong" dp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/diffprocessor" "k8s.io/client-go/rest" @@ -91,19 +89,7 @@ func (c *CompCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, } func makeDefaultCompProc(c *CompCmd, ctx *AppContext, log logging.Logger) dp.CompDiffProcessor { - // Create the options for the processor using the unified ProcessorOption - options := []dp.ProcessorOption{ - dp.WithNamespace(c.Namespace), - dp.WithLogger(log), - dp.WithColorize(!c.NoColor), - dp.WithCompact(c.Compact), - } - - // Use explicit type references to make sure imports are used - var ( - _ = ctx.K8sClients - _ = ctx.XpClients - ) + options := createProcessorOptions(c.CommonCmdFields, c.Namespace, log) return dp.NewCompDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) } @@ -114,14 +100,13 @@ func makeDefaultCompLoader(c *CompCmd) (ld.Loader, error) { // Run executes the composition diff command. func (c *CompCmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, proc dp.CompDiffProcessor, loader ld.Loader) error { - ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) - defer cancel() - - if err := appCtx.Initialize(ctx, log); err != nil { - return errors.Wrap(err, "cannot initialize client") + ctx, cancel, err := initializeAppContext(c.Timeout, appCtx, log) + if err != nil { + return err } + defer cancel() - err := proc.Initialize(ctx) + err = proc.Initialize(ctx) if err != nil { return errors.Wrap(err, "cannot initialize composition diff processor") } diff --git a/cmd/diff/xr.go b/cmd/diff/xr.go index e0d6d14..292bcbf 100644 --- a/cmd/diff/xr.go +++ b/cmd/diff/xr.go @@ -18,7 +18,6 @@ limitations under the License. package main import ( - "context" "fmt" "github.com/alecthomas/kong" @@ -29,7 +28,6 @@ import ( "github.com/crossplane/crossplane-runtime/v2/pkg/logging" ld "github.com/crossplane/crossplane/v2/cmd/crank/common/load" - "github.com/crossplane/crossplane/v2/cmd/crank/render" ) // XRCmd represents the XR diff command. @@ -91,15 +89,7 @@ func (c *XRCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, co } func makeDefaultXRProc(c *XRCmd, ctx *AppContext, log logging.Logger) dp.DiffProcessor { - // Create the options for the processor - options := []dp.ProcessorOption{ - dp.WithNamespace(c.Namespace), - dp.WithLogger(log), - dp.WithRenderFunc(render.Render), - dp.WithColorize(!c.NoColor), - dp.WithCompact(c.Compact), - } - + options := createProcessorOptions(c.CommonCmdFields, c.Namespace, log) return dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) } @@ -159,12 +149,11 @@ func (c *XRCmd) Run(k *kong.Context, log logging.Logger, appCtx *AppContext, pro // TODO: diff against upgraded schema that isn't applied yet // TODO: diff against upgraded composition that isn't applied yet // TODO: diff against upgraded composition version that is already available - ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) - defer cancel() - - if err := appCtx.Initialize(ctx, log); err != nil { - return errors.Wrap(err, "cannot initialize client") + ctx, cancel, err := initializeAppContext(c.Timeout, appCtx, log) + if err != nil { + return err } + defer cancel() resources, err := loader.Load() if err != nil { From e1c96e0318d98e691d214f6347537c469d120a6a Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 16:01:46 -0400 Subject: [PATCH 3/8] Minor options cleanup Signed-off-by: Jonathan Ogilvie --- cmd/diff/cmd_utils.go | 22 ++++++++++------------ cmd/diff/comp.go | 17 ++++++++++++++--- cmd/diff/diffprocessor/comp_processor.go | 16 +++------------- cmd/diff/xr.go | 11 +++++++++-- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/cmd/diff/cmd_utils.go b/cmd/diff/cmd_utils.go index 644df31..5918c27 100644 --- a/cmd/diff/cmd_utils.go +++ b/cmd/diff/cmd_utils.go @@ -85,18 +85,6 @@ func initRestConfig(config *rest.Config, logger logging.Logger, fields CommonCmd return config } -// createProcessorOptions creates the standard processor options from common command fields. -// Includes all options that may be used by any processor type. -func createProcessorOptions(fields CommonCmdFields, namespace string, log logging.Logger) []dp.ProcessorOption { - return []dp.ProcessorOption{ - dp.WithNamespace(namespace), - dp.WithLogger(log), - dp.WithColorize(!fields.NoColor), - dp.WithCompact(fields.Compact), - dp.WithRenderFunc(render.Render), // Safe to include even if unused - } -} - // initializeAppContext initializes the application context with timeout and error handling. func initializeAppContext(timeout time.Duration, appCtx *AppContext, log logging.Logger) (context.Context, context.CancelFunc, error) { ctx, cancel := context.WithTimeout(context.Background(), timeout) @@ -107,3 +95,13 @@ func initializeAppContext(timeout time.Duration, appCtx *AppContext, log logging return ctx, cancel, nil } + +// defaultProcessorOptions returns the standard default options used by both XR and composition processors. +// Call sites can append additional options or override these defaults as needed. +func defaultProcessorOptions() []dp.ProcessorOption { + return []dp.ProcessorOption{ + dp.WithColorize(true), + dp.WithCompact(false), + dp.WithRenderFunc(render.Render), + } +} diff --git a/cmd/diff/comp.go b/cmd/diff/comp.go index 3b9844b..ae46018 100644 --- a/cmd/diff/comp.go +++ b/cmd/diff/comp.go @@ -89,9 +89,20 @@ func (c *CompCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, } func makeDefaultCompProc(c *CompCmd, ctx *AppContext, log logging.Logger) dp.CompDiffProcessor { - options := createProcessorOptions(c.CommonCmdFields, c.Namespace, log) - - return dp.NewCompDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) + // Both processors share the same options since they're part of the same command + opts := defaultProcessorOptions() + opts = append(opts, + dp.WithNamespace(c.Namespace), + dp.WithLogger(log), + dp.WithColorize(!c.NoColor), // Override default if NoColor is set + dp.WithCompact(c.Compact), // Override default if Compact is set + ) + + // Create XR processor first (peer processor) + xrProc := dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, opts...) + + // Inject it into composition processor + return dp.NewCompDiffProcessor(xrProc, ctx.K8sClients, ctx.XpClients, opts...) } func makeDefaultCompLoader(c *CompCmd) (ld.Loader, error) { diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index d287fe3..9455e72 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -53,7 +53,7 @@ type DefaultCompDiffProcessor struct { } // NewCompDiffProcessor creates a new DefaultCompDiffProcessor. -func NewCompDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) CompDiffProcessor { +func NewCompDiffProcessor(xrProc DiffProcessor, k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) CompDiffProcessor { // Create default configuration config := ProcessorConfig{ Namespace: "", @@ -72,6 +72,7 @@ func NewCompDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOpt k8sClients: k8cs, xpClients: xpcs, config: config, + xrProc: xrProc, } } @@ -79,18 +80,7 @@ func NewCompDiffProcessor(k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOpt func (p *DefaultCompDiffProcessor) Initialize(ctx context.Context) error { p.config.Logger.Debug("Initializing composition diff processor") - // Create an XR diff processor that we'll reuse - xrOptions := []ProcessorOption{ - WithNamespace(p.config.Namespace), - WithLogger(p.config.Logger), - WithRenderFunc(render.Render), - WithColorize(p.config.Colorize), - WithCompact(p.config.Compact), - } - - p.xrProc = NewDiffProcessor(p.k8sClients, p.xpClients, xrOptions...) - - // Initialize the XR processor + // Initialize the injected XR processor if err := p.xrProc.Initialize(ctx); err != nil { return errors.Wrap(err, "cannot initialize XR diff processor") } diff --git a/cmd/diff/xr.go b/cmd/diff/xr.go index 292bcbf..9ca5906 100644 --- a/cmd/diff/xr.go +++ b/cmd/diff/xr.go @@ -89,8 +89,15 @@ func (c *XRCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, co } func makeDefaultXRProc(c *XRCmd, ctx *AppContext, log logging.Logger) dp.DiffProcessor { - options := createProcessorOptions(c.CommonCmdFields, c.Namespace, log) - return dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, options...) + opts := defaultProcessorOptions() + opts = append(opts, + dp.WithNamespace(c.Namespace), + dp.WithLogger(log), + dp.WithColorize(!c.NoColor), // Override default if NoColor is set + dp.WithCompact(c.Compact), // Override default if Compact is set + ) + + return dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, opts...) } func makeDefaultXRLoader(c *XRCmd) (ld.Loader, error) { From d209b9320ef42f241e4c7b0eedfd795eb96a5110 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 17:06:49 -0400 Subject: [PATCH 4/8] Cleanup of composition processor duplication Signed-off-by: Jonathan Ogilvie --- cmd/diff/comp.go | 8 +- cmd/diff/diff_integration_test.go | 20 +- cmd/diff/diffprocessor/comp_processor.go | 213 +++++------------- cmd/diff/diffprocessor/comp_processor_test.go | 45 ++-- 4 files changed, 81 insertions(+), 205 deletions(-) diff --git a/cmd/diff/comp.go b/cmd/diff/comp.go index ae46018..b37c401 100644 --- a/cmd/diff/comp.go +++ b/cmd/diff/comp.go @@ -77,7 +77,7 @@ func (c *CompCmd) initializeDependencies(ctx *kong.Context, log logging.Logger, proc := makeDefaultCompProc(c, appCtx, log) - loader, err := makeDefaultCompLoader(c) + loader, err := ld.NewCompositeLoader(c.Files) if err != nil { return errors.Wrap(err, "cannot create composition loader") } @@ -102,11 +102,7 @@ func makeDefaultCompProc(c *CompCmd, ctx *AppContext, log logging.Logger) dp.Com xrProc := dp.NewDiffProcessor(ctx.K8sClients, ctx.XpClients, opts...) // Inject it into composition processor - return dp.NewCompDiffProcessor(xrProc, ctx.K8sClients, ctx.XpClients, opts...) -} - -func makeDefaultCompLoader(c *CompCmd) (ld.Loader, error) { - return ld.NewCompositeLoader(c.Files) + return dp.NewCompDiffProcessor(xrProc, ctx.XpClients.Composition, opts...) } // Run executes the composition diff command. diff --git a/cmd/diff/diff_integration_test.go b/cmd/diff/diff_integration_test.go index fb6c6f6..09d7cd1 100644 --- a/cmd/diff/diff_integration_test.go +++ b/cmd/diff/diff_integration_test.go @@ -32,11 +32,11 @@ const ( ) // DiffTestType represents the type of diff test to run. -type DiffTestType int +type DiffTestType string const ( - XRDiffTest DiffTestType = iota - CompositionDiffTest + XRDiffTest DiffTestType = "diff" + CompositionDiffTest DiffTestType = "comp" ) // IntegrationTestCase represents a common test case structure for both XR and composition diff tests. @@ -99,17 +99,9 @@ func runIntegrationTest(t *testing.T, testType DiffTestType, tests map[string]In _, thisFile, _, _ := run.Caller(0) thisDir := filepath.Dir(thisFile) - var crdPaths []string - if testType == CompositionDiffTest { - crdPaths = []string{ - filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), - filepath.Join(thisDir, "testdata", "comp", "crds"), - } - } else { - crdPaths = []string{ - filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), - filepath.Join(thisDir, "testdata", "diff", "crds"), - } + crdPaths := []string{ + filepath.Join(thisDir, "..", "..", "cluster", "main", "crds"), + filepath.Join(thisDir, "testdata", string(testType), "crds"), } testEnv := &envtest.Environment{ diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index 9455e72..a7d49ec 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -24,7 +24,6 @@ import ( "strings" xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" - k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer" dt "github.com/crossplane-contrib/crossplane-diff/cmd/diff/renderer/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -46,14 +45,13 @@ type CompDiffProcessor interface { // DefaultCompDiffProcessor implements CompDiffProcessor. type DefaultCompDiffProcessor struct { - k8sClients k8.Clients - xpClients xp.Clients - config ProcessorConfig - xrProc DiffProcessor + compositionClient xp.CompositionClient + config ProcessorConfig + xrProc DiffProcessor } // NewCompDiffProcessor creates a new DefaultCompDiffProcessor. -func NewCompDiffProcessor(xrProc DiffProcessor, k8cs k8.Clients, xpcs xp.Clients, opts ...ProcessorOption) CompDiffProcessor { +func NewCompDiffProcessor(xrProc DiffProcessor, compositionClient xp.CompositionClient, opts ...ProcessorOption) CompDiffProcessor { // Create default configuration config := ProcessorConfig{ Namespace: "", @@ -69,10 +67,9 @@ func NewCompDiffProcessor(xrProc DiffProcessor, k8cs k8.Clients, xpcs xp.Clients } return &DefaultCompDiffProcessor{ - k8sClients: k8cs, - xpClients: xpcs, - config: config, - xrProc: xrProc, + compositionClient: compositionClient, + config: config, + xrProc: xrProc, } } @@ -110,20 +107,11 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, stdout i continue } - // Convert unstructured to typed composition - newComp, err := p.unstructuredToComposition(comp) - if err != nil { - p.config.Logger.Debug("Failed to convert composition", "composition", compositionID, "error", err) - errs = append(errs, errors.Wrapf(err, "cannot convert %s from unstructured", compositionID)) - - continue - } - - compositionID = newComp.GetName() // Use actual name once we have it + compositionID = comp.GetName() // Use actual name from unstructured p.config.Logger.Debug("Processing composition", "name", compositionID) // Process this single composition - if err := p.processSingleComposition(ctx, stdout, newComp, namespace); err != nil { + if err := p.processSingleComposition(ctx, stdout, comp, namespace); err != nil { p.config.Logger.Debug("Failed to process composition", "composition", compositionID, "error", err) errs = append(errs, errors.Wrapf(err, "cannot process composition %s", compositionID)) @@ -157,14 +145,14 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, stdout i } // processSingleComposition processes a single composition and shows its impact on existing XRs. -func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition, namespace string) error { +func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, stdout io.Writer, newComp *un.Unstructured, namespace string) error { // First, show the composition diff itself if err := p.displayCompositionDiff(ctx, stdout, newComp); err != nil { return errors.Wrap(err, "cannot display composition diff") } // Find all XRs that use this composition - affectedXRs, err := p.findXRsUsingComposition(ctx, newComp.GetName(), namespace) + affectedXRs, err := p.compositionClient.FindXRsUsingComposition(ctx, newComp.GetName(), namespace) if err != nil { // For net-new compositions, the composition won't exist in the cluster // so findXRsUsingComposition will fail. This is expected behavior. @@ -193,22 +181,18 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, // Process affected XRs using the existing XR processor with composition override // List the affected XRs so users can understand the scope of impact - if _, err := fmt.Fprintf(stdout, "=== Affected Composite Resources ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write affected XRs header") - } - + var xrList strings.Builder for _, xr := range affectedXRs { - if _, err := fmt.Fprintf(stdout, "- %s/%s (namespace: %s)\n", xr.GetKind(), xr.GetName(), xr.GetNamespace()); err != nil { - return errors.Wrap(err, "cannot write affected XR info") - } + xrList.WriteString(fmt.Sprintf("- %s/%s (namespace: %s)\n", xr.GetKind(), xr.GetName(), xr.GetNamespace())) } - if _, err := fmt.Fprintf(stdout, "\n=== Impact Analysis ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write impact analysis header") + if _, err := fmt.Fprintf(stdout, "=== Affected Composite Resources ===\n\n%s\n=== Impact Analysis ===\n\n", xrList.String()); err != nil { + return errors.Wrap(err, "cannot write XR list and headers") } if err := p.xrProc.PerformDiff(ctx, stdout, affectedXRs, func(context.Context, *un.Unstructured) (*apiextensionsv1.Composition, error) { - return newComp, nil + // Convert unstructured to structured only when needed by the XR processor + return p.unstructuredToComposition(newComp) }); err != nil { return errors.Wrap(err, "cannot process XRs with composition override") } @@ -216,51 +200,45 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, return nil } -// findXRsUsingComposition finds all XRs that use the specified composition. -func (p *DefaultCompDiffProcessor) findXRsUsingComposition(ctx context.Context, compositionName string, namespace string) ([]*un.Unstructured, error) { - // Use the composition client to find XRs that reference this composition - return p.xpClients.Composition.FindXRsUsingComposition(ctx, compositionName, namespace) -} - // displayCompositionDiff shows the diff between the cluster composition and the file composition. -func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition) error { +// If the composition doesn't exist in the cluster, it shows it as a new composition. +func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, stdout io.Writer, newComp *un.Unstructured) error { p.config.Logger.Debug("Displaying composition diff", "composition", newComp.GetName()) + var originalCompUnstructured *un.Unstructured + // Get the original composition from the cluster - originalComp, err := p.xpClients.Composition.GetComposition(ctx, newComp.GetName()) + originalComp, err := p.compositionClient.GetComposition(ctx, newComp.GetName()) if err != nil { p.config.Logger.Debug("Original composition not found in cluster, treating as new composition", "composition", newComp.GetName(), "error", err) + // originalCompUnstructured remains nil for new compositions + } else { + p.config.Logger.Debug("Retrieved original composition from cluster", "name", originalComp.GetName(), "composition", originalComp) - // Handle case where composition doesn't exist in cluster (net-new composition) - return p.displayNewComposition(ctx, stdout, newComp) + // Convert original composition to unstructured for comparison + originalCompUnstructured, err = p.compositionToUnstructured(originalComp) + if err != nil { + return errors.Wrap(err, "cannot convert original composition to unstructured") + } } - p.config.Logger.Debug("Retrieved original composition from cluster", "name", originalComp.GetName(), "composition", originalComp) - - // Convert both compositions to unstructured for comparison - originalCompUnstructured, err := p.compositionToUnstructured(originalComp) - if err != nil { - return errors.Wrap(err, "cannot convert original composition to unstructured") - } + newCompUnstructured := newComp - newCompUnstructured, err := p.compositionToUnstructured(newComp) - if err != nil { - return errors.Wrap(err, "cannot convert new composition to unstructured") + // Clean up managed fields and other cluster metadata before diff calculation + cleanupClusterMetadata := func(obj *un.Unstructured) { + if obj == nil { + return + } + obj.SetManagedFields(nil) + obj.SetResourceVersion("") + obj.SetUID("") + obj.SetGeneration(0) + obj.SetCreationTimestamp(metav1.Time{}) } - // Clean up managed fields and other cluster metadata before diff calculation - originalCompUnstructured.SetManagedFields(nil) - originalCompUnstructured.SetResourceVersion("") - originalCompUnstructured.SetUID("") - originalCompUnstructured.SetGeneration(0) - originalCompUnstructured.SetCreationTimestamp(metav1.Time{}) - - newCompUnstructured.SetManagedFields(nil) - newCompUnstructured.SetResourceVersion("") - newCompUnstructured.SetUID("") - newCompUnstructured.SetGeneration(0) - newCompUnstructured.SetCreationTimestamp(metav1.Time{}) + cleanupClusterMetadata(originalCompUnstructured) + cleanupClusterMetadata(newCompUnstructured) // Calculate the composition diff directly without dry-run apply // (compositions are static YAML documents that don't need server-side processing) @@ -275,10 +253,25 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s p.config.Logger.Debug("Calculated composition diff", "composition", newComp.GetName(), - "hasChanges", compDiff != nil) + "hasChanges", compDiff != nil, + "isNewComposition", originalCompUnstructured == nil) + + // Add header for composition changes (common to all cases) + if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { + return errors.Wrap(err, "cannot write composition changes header") + } // Display the composition diff if there are changes - if compDiff != nil && compDiff.DiffType != dt.DiffTypeEqual { + switch compDiff.DiffType { + case dt.DiffTypeEqual: + // No changes detected (only possible for existing compositions) + p.config.Logger.Info("No changes detected in composition", "composition", newComp.GetName()) + + if _, err := fmt.Fprintf(stdout, "No changes detected in composition %s\n\n", newComp.GetName()); err != nil { + return errors.Wrap(err, "cannot write no changes message") + } + default: + // Changes detected - show the diff // Create a diff renderer with proper options rendererOptions := renderer.DefaultDiffOptions() rendererOptions.UseColors = p.config.Colorize @@ -290,11 +283,6 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s fmt.Sprintf("Composition/%s", newComp.GetName()): compDiff, } - // Add a header to distinguish composition diff from XR diffs - if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write composition changes header") - } - if err := diffRenderer.RenderDiffs(stdout, diffs); err != nil { return errors.Wrap(err, "cannot render composition diff") } @@ -302,16 +290,6 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s if _, err := fmt.Fprintf(stdout, "\n"); err != nil { return errors.Wrap(err, "cannot write separator") } - } else { - p.config.Logger.Info("No changes detected in composition", "composition", newComp.GetName()) - - if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write composition changes header") - } - - if _, err := fmt.Fprintf(stdout, "No changes detected in composition %s\n\n", newComp.GetName()); err != nil { - return errors.Wrap(err, "cannot write no changes message") - } } return nil @@ -338,75 +316,4 @@ func (p *DefaultCompDiffProcessor) unstructuredToComposition(u *un.Unstructured) return comp, nil } -// displayNewComposition shows a new composition that doesn't exist in the cluster. -func (p *DefaultCompDiffProcessor) displayNewComposition(ctx context.Context, stdout io.Writer, newComp *apiextensionsv1.Composition) error { - p.config.Logger.Debug("Displaying new composition", "composition", newComp.GetName()) - // Convert new composition to unstructured for rendering - newCompUnstructured, err := p.compositionToUnstructured(newComp) - if err != nil { - return errors.Wrap(err, "cannot convert new composition to unstructured") - } - - // Clean up managed fields and other cluster metadata - newCompUnstructured.SetManagedFields(nil) - newCompUnstructured.SetResourceVersion("") - newCompUnstructured.SetUID("") - newCompUnstructured.SetGeneration(0) - newCompUnstructured.SetCreationTimestamp(metav1.Time{}) - - // Generate a diff showing the entire composition as new (all additions) - diffOptions := renderer.DefaultDiffOptions() - diffOptions.UseColors = p.config.Colorize - diffOptions.Compact = p.config.Compact - - // Create a diff with empty original (nil) to show everything as additions - compDiff, err := renderer.GenerateDiffWithOptions(ctx, nil, newCompUnstructured, p.config.Logger, diffOptions) - if err != nil { - return errors.Wrap(err, "cannot calculate new composition diff") - } - - p.config.Logger.Debug("Calculated new composition diff", - "composition", newComp.GetName(), - "hasChanges", compDiff != nil) - - // Display the new composition diff - if compDiff != nil { - // Create a diff renderer with proper options - rendererOptions := renderer.DefaultDiffOptions() - rendererOptions.UseColors = p.config.Colorize - rendererOptions.Compact = p.config.Compact - diffRenderer := renderer.NewDiffRenderer(p.config.Logger, rendererOptions) - - // Create a map with the single composition diff - diffs := map[string]*dt.ResourceDiff{ - fmt.Sprintf("Composition/%s", newComp.GetName()): compDiff, - } - - // Add a header to distinguish composition diff from XR diffs - if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write composition changes header") - } - - if err := diffRenderer.RenderDiffs(stdout, diffs); err != nil { - return errors.Wrap(err, "cannot render new composition diff") - } - - if _, err := fmt.Fprintf(stdout, "\n"); err != nil { - return errors.Wrap(err, "cannot write separator") - } - } else { - // This shouldn't happen for a new composition, but handle it gracefully - p.config.Logger.Debug("No diff generated for new composition", "composition", newComp.GetName()) - - if _, err := fmt.Fprintf(stdout, "=== Composition Changes ===\n\n"); err != nil { - return errors.Wrap(err, "cannot write composition changes header") - } - - if _, err := fmt.Fprintf(stdout, "New composition %s\n\n", newComp.GetName()); err != nil { - return errors.Wrap(err, "cannot write new composition message") - } - } - - return nil -} diff --git a/cmd/diff/diffprocessor/comp_processor_test.go b/cmd/diff/diffprocessor/comp_processor_test.go index 94656b4..2ea8fb8 100644 --- a/cmd/diff/diffprocessor/comp_processor_test.go +++ b/cmd/diff/diffprocessor/comp_processor_test.go @@ -24,7 +24,6 @@ import ( "testing" xp "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/crossplane" - k8 "github.com/crossplane-contrib/crossplane-diff/cmd/diff/client/kubernetes" tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" gcmp "github.com/google/go-cmp/cmp" @@ -127,14 +126,15 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { // Create processor + mocks := tt.setupMocks() processor := &DefaultCompDiffProcessor{ - xpClients: tt.setupMocks(), + compositionClient: mocks.Composition, config: ProcessorConfig{ Logger: tu.TestLogger(t, false), }, } - got, err := processor.findXRsUsingComposition(ctx, tt.compositionName, tt.namespace) + got, err := processor.compositionClient.FindXRsUsingComposition(ctx, tt.compositionName, tt.namespace) if (err != nil) != tt.wantErr { t.Errorf("findXRsUsingComposition() error = %v, wantErr %v", err, tt.wantErr) @@ -325,7 +325,7 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { tests := map[string]struct { compositions []*un.Unstructured namespace string - setupMocks func() (k8.Clients, xp.Clients) + setupMocks func() xp.Clients verifyOutput func(t *testing.T, output string) wantErr bool }{ @@ -349,15 +349,8 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { }, }, }, - setupMocks: func() (k8.Clients, xp.Clients) { - k8sClients := k8.Clients{ - Apply: tu.NewMockApplyClient().Build(), - Resource: tu.NewMockResourceClient().Build(), - Schema: tu.NewMockSchemaClient().Build(), - Type: tu.NewMockTypeConverter().Build(), - } - - xpClients := xp.Clients{ + setupMocks: func() xp.Clients { + return xp.Clients{ Composition: tu.NewMockCompositionClient(). WithSuccessfulCompositionFetch(testComp). WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { @@ -369,8 +362,6 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { Function: tu.NewMockFunctionClient().Build(), ResourceTree: tu.NewMockResourceTreeClient().Build(), } - - return k8sClients, xpClients }, verifyOutput: func(t *testing.T, output string) { t.Helper() @@ -388,8 +379,8 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { "NoCompositions": { namespace: "default", compositions: []*un.Unstructured{}, - setupMocks: func() (k8.Clients, xp.Clients) { - return k8.Clients{}, xp.Clients{} + setupMocks: func() xp.Clients { + return xp.Clients{} }, wantErr: true, }, @@ -429,14 +420,7 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { }, }, }, - setupMocks: func() (k8.Clients, xp.Clients) { - k8sClients := k8.Clients{ - Apply: tu.NewMockApplyClient().Build(), - Resource: tu.NewMockResourceClient().Build(), - Schema: tu.NewMockSchemaClient().Build(), - Type: tu.NewMockTypeConverter().Build(), - } - + setupMocks: func() xp.Clients { // Create test compositions for the multi-composition test testComp1 := &apiextensionsv1.Composition{ TypeMeta: metav1.TypeMeta{ @@ -472,7 +456,7 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { }, } - xpClients := xp.Clients{ + return xp.Clients{ Composition: tu.NewMockCompositionClient(). WithGetComposition(func(_ context.Context, name string) (*apiextensionsv1.Composition, error) { switch name { @@ -494,8 +478,6 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { Function: tu.NewMockFunctionClient().Build(), ResourceTree: tu.NewMockResourceTreeClient().Build(), } - - return k8sClients, xpClients }, verifyOutput: func(t *testing.T, output string) { t.Helper() @@ -515,7 +497,7 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - k8sClients, xpClients := tt.setupMocks() + xpClients := tt.setupMocks() // Create mock XR processor mockXRProc := &tu.MockDiffProcessor{ @@ -527,9 +509,8 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { // Create processor processor := &DefaultCompDiffProcessor{ - k8sClients: k8sClients, - xpClients: xpClients, - xrProc: mockXRProc, + compositionClient: xpClients.Composition, + xrProc: mockXRProc, config: ProcessorConfig{ Namespace: tt.namespace, Colorize: false, From d6960f01d5fd8e3b56e121eed3a6cba82162942e Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 17:27:23 -0400 Subject: [PATCH 5/8] More cleanup around composition unstructured conversion Signed-off-by: Jonathan Ogilvie --- cmd/diff/diffprocessor/comp_processor.go | 37 +--- cmd/diff/diffprocessor/comp_processor_test.go | 178 +----------------- cmd/diff/testutils/mock_builder.go | 17 ++ 3 files changed, 34 insertions(+), 198 deletions(-) diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index a7d49ec..9e2753f 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -99,15 +99,13 @@ func (p *DefaultCompDiffProcessor) DiffComposition(ctx context.Context, stdout i // Process each composition, filtering out non-Composition objects for i, comp := range compositions { - compositionID := fmt.Sprintf("composition %d", i+1) - // Skip non-Composition objects (e.g., GoTemplate objects extracted from pipeline steps) if comp.GetKind() != "Composition" { p.config.Logger.Debug("Skipping non-Composition object", "kind", comp.GetKind(), "apiVersion", comp.GetAPIVersion()) continue } - compositionID = comp.GetName() // Use actual name from unstructured + compositionID := comp.GetName() // Use actual name from unstructured p.config.Logger.Debug("Processing composition", "name", compositionID) // Process this single composition @@ -192,7 +190,11 @@ func (p *DefaultCompDiffProcessor) processSingleComposition(ctx context.Context, if err := p.xrProc.PerformDiff(ctx, stdout, affectedXRs, func(context.Context, *un.Unstructured) (*apiextensionsv1.Composition, error) { // Convert unstructured to structured only when needed by the XR processor - return p.unstructuredToComposition(newComp) + comp := &apiextensionsv1.Composition{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(newComp.Object, comp); err != nil { + return nil, errors.Wrap(err, "cannot convert unstructured to Composition") + } + return comp, nil }); err != nil { return errors.Wrap(err, "cannot process XRs with composition override") } @@ -217,10 +219,11 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s p.config.Logger.Debug("Retrieved original composition from cluster", "name", originalComp.GetName(), "composition", originalComp) // Convert original composition to unstructured for comparison - originalCompUnstructured, err = p.compositionToUnstructured(originalComp) + unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(originalComp) if err != nil { return errors.Wrap(err, "cannot convert original composition to unstructured") } + originalCompUnstructured = &un.Unstructured{Object: unstructuredObj} } newCompUnstructured := newComp @@ -230,6 +233,7 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s if obj == nil { return } + obj.SetManagedFields(nil) obj.SetResourceVersion("") obj.SetUID("") @@ -270,7 +274,7 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s if _, err := fmt.Fprintf(stdout, "No changes detected in composition %s\n\n", newComp.GetName()); err != nil { return errors.Wrap(err, "cannot write no changes message") } - default: + case dt.DiffTypeAdded, dt.DiffTypeRemoved, dt.DiffTypeModified: // Changes detected - show the diff // Create a diff renderer with proper options rendererOptions := renderer.DefaultDiffOptions() @@ -295,25 +299,4 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s return nil } -// compositionToUnstructured converts a typed Composition to unstructured for diff calculation. -func (p *DefaultCompDiffProcessor) compositionToUnstructured(comp *apiextensionsv1.Composition) (*un.Unstructured, error) { - // Convert composition to unstructured using runtime conversion - unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(comp) - if err != nil { - return nil, errors.Wrap(err, "cannot convert composition to unstructured map") - } - - return &un.Unstructured{Object: unstructuredObj}, nil -} - -// unstructuredToComposition converts an unstructured object to a typed Composition. -func (p *DefaultCompDiffProcessor) unstructuredToComposition(u *un.Unstructured) (*apiextensionsv1.Composition, error) { - comp := &apiextensionsv1.Composition{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, comp); err != nil { - return nil, errors.Wrap(err, "cannot convert unstructured to Composition") - } - - return comp, nil -} - diff --git a/cmd/diff/diffprocessor/comp_processor_test.go b/cmd/diff/diffprocessor/comp_processor_test.go index 2ea8fb8..56284d8 100644 --- a/cmd/diff/diffprocessor/comp_processor_test.go +++ b/cmd/diff/diffprocessor/comp_processor_test.go @@ -61,12 +61,7 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { setupMocks: func() xp.Clients { return xp.Clients{ Composition: tu.NewMockCompositionClient(). - WithFindXRsUsingComposition(func(_ context.Context, compositionName, namespace string) ([]*un.Unstructured, error) { - if compositionName == "test-composition" && namespace == "default" { - return []*un.Unstructured{xr1}, nil - } - return nil, errors.New("not found") - }). + WithXRsForComposition("test-composition", "default", []*un.Unstructured{xr1}). Build(), } }, @@ -79,12 +74,7 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { setupMocks: func() xp.Clients { return xp.Clients{ Composition: tu.NewMockCompositionClient(). - WithFindXRsUsingComposition(func(_ context.Context, compositionName, namespace string) ([]*un.Unstructured, error) { - if compositionName == "test-composition" && namespace == "custom-ns" { - return []*un.Unstructured{xr2}, nil - } - return nil, errors.New("not found") - }). + WithXRsForComposition("test-composition", "custom-ns", []*un.Unstructured{xr2}). Build(), } }, @@ -97,9 +87,7 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { setupMocks: func() xp.Clients { return xp.Clients{ Composition: tu.NewMockCompositionClient(). - WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { - return nil, errors.New("client error") - }). + WithFindXRsError("client error"). Build(), } }, @@ -112,9 +100,7 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { setupMocks: func() xp.Clients { return xp.Clients{ Composition: tu.NewMockCompositionClient(). - WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { - return []*un.Unstructured{}, nil - }). + WithXRsForComposition("test-composition", "default", []*un.Unstructured{}). Build(), } }, @@ -148,153 +134,7 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { } } -func TestDefaultCompDiffProcessor_unstructuredToComposition(t *testing.T) { - tests := map[string]struct { - unstructured *un.Unstructured - want *apiextensionsv1.Composition - wantErr bool - }{ - "ValidUnstructured": { - unstructured: &un.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "apiextensions.crossplane.io/v1", - "kind": "Composition", - "metadata": map[string]interface{}{ - "name": "test-composition", - }, - "spec": map[string]interface{}{ - "compositeTypeRef": map[string]interface{}{ - "apiVersion": "example.org/v1", - "kind": "XResource", - }, - "mode": "Pipeline", - }, - }, - }, - want: &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.crossplane.io/v1", - Kind: "Composition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-composition", - }, - Spec: apiextensionsv1.CompositionSpec{ - CompositeTypeRef: apiextensionsv1.TypeReference{ - APIVersion: "example.org/v1", - Kind: "XResource", - }, - Mode: apiextensionsv1.CompositionModePipeline, - }, - }, - wantErr: false, - }, - "InvalidUnstructured": { - unstructured: &un.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "Pod", // Wrong kind - }, - }, - want: &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "v1", - Kind: "Pod", - }, - }, - wantErr: false, // Runtime converter doesn't validate, it just converts - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - // Create processor - processor := &DefaultCompDiffProcessor{ - config: ProcessorConfig{ - Logger: tu.TestLogger(t, false), - }, - } - - got, err := processor.unstructuredToComposition(tt.unstructured) - if (err != nil) != tt.wantErr { - t.Errorf("unstructuredToComposition() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if tt.wantErr { - return - } - - if diff := gcmp.Diff(tt.want, got); diff != "" { - t.Errorf("unstructuredToComposition() mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestDefaultCompDiffProcessor_compositionToUnstructured(t *testing.T) { - tests := map[string]struct { - composition *apiextensionsv1.Composition - wantErr bool - }{ - "ValidComposition": { - composition: &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.crossplane.io/v1", - Kind: "Composition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-composition", - }, - Spec: apiextensionsv1.CompositionSpec{ - CompositeTypeRef: apiextensionsv1.TypeReference{ - APIVersion: "example.org/v1", - Kind: "XResource", - }, - Mode: apiextensionsv1.CompositionModePipeline, - }, - }, - wantErr: false, - }, - } - - for name, tt := range tests { - t.Run(name, func(t *testing.T) { - // Create processor - processor := &DefaultCompDiffProcessor{ - config: ProcessorConfig{ - Logger: tu.TestLogger(t, false), - }, - } - - got, err := processor.compositionToUnstructured(tt.composition) - - if (err != nil) != tt.wantErr { - t.Errorf("compositionToUnstructured() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if tt.wantErr { - return - } - - // Basic checks - if got == nil { - t.Errorf("compositionToUnstructured() returned nil") - return - } - - if got.GetKind() != "Composition" { - t.Errorf("compositionToUnstructured() kind = %v, want Composition", got.GetKind()) - } - - if got.GetName() != tt.composition.GetName() { - t.Errorf("compositionToUnstructured() name = %v, want %v", got.GetName(), tt.composition.GetName()) - } - }) - } -} func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { ctx := context.Background() @@ -353,9 +193,7 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { return xp.Clients{ Composition: tu.NewMockCompositionClient(). WithSuccessfulCompositionFetch(testComp). - WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { - return []*un.Unstructured{testXR}, nil - }). + WithXRsForComposition("test-composition", "default", []*un.Unstructured{testXR}). Build(), Definition: tu.NewMockDefinitionClient().Build(), Environment: tu.NewMockEnvironmentClient().Build(), @@ -468,10 +306,8 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { return nil, errors.New("composition not found") } }). - WithFindXRsUsingComposition(func(_ context.Context, _, _ string) ([]*un.Unstructured, error) { - // Return no XRs for simplicity - just testing that multiple compositions are processed - return []*un.Unstructured{}, nil - }). + WithXRsForComposition("test-composition-1", "default", []*un.Unstructured{}). + WithXRsForComposition("test-composition-2", "default", []*un.Unstructured{}). Build(), Definition: tu.NewMockDefinitionClient().Build(), Environment: tu.NewMockEnvironmentClient().Build(), diff --git a/cmd/diff/testutils/mock_builder.go b/cmd/diff/testutils/mock_builder.go index 8dad4ce..15d9d61 100644 --- a/cmd/diff/testutils/mock_builder.go +++ b/cmd/diff/testutils/mock_builder.go @@ -675,6 +675,23 @@ func (b *MockCompositionClientBuilder) WithFindXRsUsingComposition(fn func(conte return b } +// WithXRsForComposition sets FindXRsUsingComposition to return specific XRs for a given composition name and namespace. +func (b *MockCompositionClientBuilder) WithXRsForComposition(compositionName, namespace string, xrs []*un.Unstructured) *MockCompositionClientBuilder { + return b.WithFindXRsUsingComposition(func(_ context.Context, compName, ns string) ([]*un.Unstructured, error) { + if compName == compositionName && ns == namespace { + return xrs, nil + } + return nil, errors.Errorf("no XRs found for composition %s in namespace %s", compName, ns) + }) +} + +// WithFindXRsError sets FindXRsUsingComposition to return an error. +func (b *MockCompositionClientBuilder) WithFindXRsError(errMsg string) *MockCompositionClientBuilder { + return b.WithFindXRsUsingComposition(func(context.Context, string, string) ([]*un.Unstructured, error) { + return nil, errors.New(errMsg) + }) +} + // Build returns the built mock. func (b *MockCompositionClientBuilder) Build() *MockCompositionClient { return b.mock From b5ba63b3b63c4ddeec514dde572ed9a14acd80d6 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 17:47:14 -0400 Subject: [PATCH 6/8] Test cleanup for compositions Signed-off-by: Jonathan Ogilvie --- cmd/diff/diffprocessor/comp_processor.go | 3 +- cmd/diff/diffprocessor/comp_processor_test.go | 135 ++++-------------- cmd/diff/testutils/mock_builder.go | 30 ++++ 3 files changed, 56 insertions(+), 112 deletions(-) diff --git a/cmd/diff/diffprocessor/comp_processor.go b/cmd/diff/diffprocessor/comp_processor.go index 9e2753f..40b6fd9 100644 --- a/cmd/diff/diffprocessor/comp_processor.go +++ b/cmd/diff/diffprocessor/comp_processor.go @@ -223,6 +223,7 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s if err != nil { return errors.Wrap(err, "cannot convert original composition to unstructured") } + originalCompUnstructured = &un.Unstructured{Object: unstructuredObj} } @@ -298,5 +299,3 @@ func (p *DefaultCompDiffProcessor) displayCompositionDiff(ctx context.Context, s return nil } - - diff --git a/cmd/diff/diffprocessor/comp_processor_test.go b/cmd/diff/diffprocessor/comp_processor_test.go index 56284d8..462c762 100644 --- a/cmd/diff/diffprocessor/comp_processor_test.go +++ b/cmd/diff/diffprocessor/comp_processor_test.go @@ -27,10 +27,8 @@ import ( tu "github.com/crossplane-contrib/crossplane-diff/cmd/diff/testutils" "github.com/crossplane-contrib/crossplane-diff/cmd/diff/types" gcmp "github.com/google/go-cmp/cmp" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" un "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" apiextensionsv1 "github.com/crossplane/crossplane/v2/apis/apiextensions/v1" @@ -134,28 +132,14 @@ func TestDefaultCompDiffProcessor_findXRsUsingComposition(t *testing.T) { } } - - func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { ctx := context.Background() // Create test composition - testComp := &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.crossplane.io/v1", - Kind: "Composition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-composition", - }, - Spec: apiextensionsv1.CompositionSpec{ - CompositeTypeRef: apiextensionsv1.TypeReference{ - APIVersion: "example.org/v1", - Kind: "XResource", - }, - Mode: apiextensionsv1.CompositionModePipeline, - }, - } + testComp := tu.NewComposition("test-composition"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + Build() // Create test XR testXR := tu.NewResource("example.org/v1", "XResource", "test-xr"). @@ -172,22 +156,10 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { "SuccessfulDiff": { namespace: "default", compositions: []*un.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "apiextensions.crossplane.io/v1", - "kind": "Composition", - "metadata": map[string]interface{}{ - "name": "test-composition", - }, - "spec": map[string]interface{}{ - "compositeTypeRef": map[string]interface{}{ - "apiVersion": "example.org/v1", - "kind": "XResource", - }, - "mode": "Pipeline", - }, - }, - }, + tu.NewComposition("test-composition"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + BuildAsUnstructured(), }, setupMocks: func() xp.Clients { return xp.Clients{ @@ -225,87 +197,30 @@ func TestDefaultCompDiffProcessor_DiffComposition(t *testing.T) { "MultipleCompositions": { namespace: "default", compositions: []*un.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "apiextensions.crossplane.io/v1", - "kind": "Composition", - "metadata": map[string]interface{}{ - "name": "test-composition-1", - }, - "spec": map[string]interface{}{ - "compositeTypeRef": map[string]interface{}{ - "apiVersion": "example.org/v1", - "kind": "XResource", - }, - "mode": "Pipeline", - }, - }, - }, - { - Object: map[string]interface{}{ - "apiVersion": "apiextensions.crossplane.io/v1", - "kind": "Composition", - "metadata": map[string]interface{}{ - "name": "test-composition-2", - }, - "spec": map[string]interface{}{ - "compositeTypeRef": map[string]interface{}{ - "apiVersion": "example.org/v1", - "kind": "XResource", - }, - "mode": "Pipeline", - }, - }, - }, + tu.NewComposition("test-composition-1"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + BuildAsUnstructured(), + tu.NewComposition("test-composition-2"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + BuildAsUnstructured(), }, setupMocks: func() xp.Clients { // Create test compositions for the multi-composition test - testComp1 := &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.crossplane.io/v1", - Kind: "Composition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-composition-1", - }, - Spec: apiextensionsv1.CompositionSpec{ - CompositeTypeRef: apiextensionsv1.TypeReference{ - APIVersion: "example.org/v1", - Kind: "XResource", - }, - Mode: apiextensionsv1.CompositionModePipeline, - }, - } + testComp1 := tu.NewComposition("test-composition-1"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + Build() - testComp2 := &apiextensionsv1.Composition{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "apiextensions.crossplane.io/v1", - Kind: "Composition", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-composition-2", - }, - Spec: apiextensionsv1.CompositionSpec{ - CompositeTypeRef: apiextensionsv1.TypeReference{ - APIVersion: "example.org/v1", - Kind: "XResource", - }, - Mode: apiextensionsv1.CompositionModePipeline, - }, - } + testComp2 := tu.NewComposition("test-composition-2"). + WithCompositeTypeRef("example.org/v1", "XResource"). + WithPipelineMode(). + Build() return xp.Clients{ Composition: tu.NewMockCompositionClient(). - WithGetComposition(func(_ context.Context, name string) (*apiextensionsv1.Composition, error) { - switch name { - case "test-composition-1": - return testComp1, nil - case "test-composition-2": - return testComp2, nil - default: - return nil, errors.New("composition not found") - } - }). + WithSuccessfulCompositionFetches([]*apiextensionsv1.Composition{testComp1, testComp2}). WithXRsForComposition("test-composition-1", "default", []*un.Unstructured{}). WithXRsForComposition("test-composition-2", "default", []*un.Unstructured{}). Build(), diff --git a/cmd/diff/testutils/mock_builder.go b/cmd/diff/testutils/mock_builder.go index 15d9d61..936cc44 100644 --- a/cmd/diff/testutils/mock_builder.go +++ b/cmd/diff/testutils/mock_builder.go @@ -669,6 +669,22 @@ func (b *MockCompositionClientBuilder) WithSuccessfulCompositionFetch(comp *xpex }) } +// WithSuccessfulCompositionFetches sets up successful composition fetches for multiple compositions. +func (b *MockCompositionClientBuilder) WithSuccessfulCompositionFetches(comps []*xpextv1.Composition) *MockCompositionClientBuilder { + compositionMap := make(map[string]*xpextv1.Composition) + for _, comp := range comps { + compositionMap[comp.GetName()] = comp + } + + return b.WithGetComposition(func(_ context.Context, name string) (*xpextv1.Composition, error) { + if composition, found := compositionMap[name]; found { + return composition, nil + } + + return nil, errors.New("composition not found") + }) +} + // WithFindXRsUsingComposition sets the FindXRsUsingComposition behavior. func (b *MockCompositionClientBuilder) WithFindXRsUsingComposition(fn func(context.Context, string, string) ([]*un.Unstructured, error)) *MockCompositionClientBuilder { b.mock.FindXRsUsingCompositionFn = fn @@ -681,6 +697,7 @@ func (b *MockCompositionClientBuilder) WithXRsForComposition(compositionName, na if compName == compositionName && ns == namespace { return xrs, nil } + return nil, errors.Errorf("no XRs found for composition %s in namespace %s", compName, ns) }) } @@ -1649,4 +1666,17 @@ func (b *CompositionBuilder) Build() *xpextv1.Composition { return b.composition.DeepCopy() } +// BuildAsUnstructured returns the built Composition as an unstructured object. +func (b *CompositionBuilder) BuildAsUnstructured() *un.Unstructured { + comp := b.Build() + + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(comp) + if err != nil { + // This should not happen in tests, but if it does, we'll return an empty unstructured + return &un.Unstructured{} + } + + return &un.Unstructured{Object: obj} +} + // endregion From c795f8d3a75abf6f58b00ca24955fd756d53e114 Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 22:02:17 -0400 Subject: [PATCH 7/8] Add e2e for comp test Signed-off-by: Jonathan Ogilvie --- test/e2e/diff_test.go | 94 ++++++++++++++++--- .../beta/diff/main/comp/existing-xr.yaml | 9 ++ .../diff/main/comp/setup/composition.yaml | 42 +++++++++ .../beta/diff/main/comp/setup/definition.yaml | 43 +++++++++ .../beta/diff/main/comp/setup/provider.yaml | 22 +++++ .../diff/main/comp/updated-composition.yaml | 47 ++++++++++ 6 files changed, 245 insertions(+), 12 deletions(-) create mode 100644 test/e2e/manifests/beta/diff/main/comp/existing-xr.yaml create mode 100644 test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml create mode 100644 test/e2e/manifests/beta/diff/main/comp/setup/definition.yaml create mode 100644 test/e2e/manifests/beta/diff/main/comp/setup/provider.yaml create mode 100644 test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml diff --git a/test/e2e/diff_test.go b/test/e2e/diff_test.go index 98799b4..c0c4003 100644 --- a/test/e2e/diff_test.go +++ b/test/e2e/diff_test.go @@ -58,15 +58,13 @@ const ( CrossplaneVersionRelease120 = "release-1.20" ) -// RunDiff runs the crossplane diff command on the provided resources. +// runCrossplaneDiff runs the crossplane diff command with the specified subcommand on the provided resources. // It returns the output and any error encountered. -func RunDiff(t *testing.T, c *envconf.Config, binPath string, resourcePaths ...string) (string, string, error) { +func runCrossplaneDiff(t *testing.T, c *envconf.Config, binPath, subcommand string, resourcePaths ...string) (string, string, error) { t.Helper() - var err error - // Prepare the command to run - args := append([]string{"--verbose", "diff", "--timeout=2m", "-n", namespace}, resourcePaths...) + args := append([]string{"--verbose", subcommand, "--timeout=2m"}, resourcePaths...) t.Logf("Running command: %s %s", binPath, strings.Join(args, " ")) cmd := exec.Command(binPath, args...) @@ -80,11 +78,23 @@ func RunDiff(t *testing.T, c *envconf.Config, binPath string, resourcePaths ...s cmd.Stderr = &stderr // Run the command - err = cmd.Run() + err := cmd.Run() return stdout.String(), stderr.String(), err } +// RunXRDiff runs the crossplane xr diff command on the provided resources. +// It returns the output and any error encountered. +func RunXRDiff(t *testing.T, c *envconf.Config, binPath string, resourcePaths ...string) (string, string, error) { + return runCrossplaneDiff(t, c, binPath, "xr", resourcePaths...) +} + +// RunCompDiff runs the crossplane comp diff command on the provided compositions. +// It returns the output and any error encountered. +func RunCompDiff(t *testing.T, c *envconf.Config, binPath string, compositionPaths ...string) (string, string, error) { + return runCrossplaneDiff(t, c, binPath, "comp", compositionPaths...) +} + // TestDiffNewResourceV2Cluster tests the crossplane diff command against net-new resources in v2-cluster variant. func TestDiffNewResourceV2Cluster(t *testing.T) { imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1] @@ -107,7 +117,7 @@ func TestDiffNewResourceV2Cluster(t *testing.T) { Assess("CanDiffNewResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -150,7 +160,7 @@ func TestDiffExistingResourceV2Cluster(t *testing.T) { )). Assess("CanDiffExistingResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -189,7 +199,7 @@ func TestDiffNewResourceV2Namespaced(t *testing.T) { )). Assess("CanDiffNewResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -236,7 +246,7 @@ func TestDiffExistingResourceV2Namespaced(t *testing.T) { )). Assess("CanDiffExistingResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -282,7 +292,7 @@ func TestDiffNewResourceV1(t *testing.T) { )). Assess("CanDiffNewResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "new-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -330,7 +340,7 @@ func TestDiffExistingResourceV1(t *testing.T) { )). Assess("CanDiffExistingResource", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { t.Helper() - output, log, err := RunDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) + output, log, err := RunXRDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "modified-xr.yaml")) if err != nil { t.Fatalf("Error running diff command: %v\nLog output:\n%s", err, log) } @@ -520,3 +530,63 @@ func makeStringReadable(s string) string { return result.String() } + +// TestDiffExistingComposition tests the crossplane comp diff command against existing XRs in the cluster. +func TestDiffExistingComposition(t *testing.T) { + imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1] + manifests := filepath.Join("test/e2e/manifests/beta/diff", imageTag, "comp") + setupPath := filepath.Join(manifests, "setup") + + environment.Test(t, + features.New("DiffExistingComposition"). + WithLabel(e2e.LabelArea, LabelAreaDiff). + WithLabel(e2e.LabelSize, e2e.LabelSizeSmall). + WithLabel(config.LabelTestSuite, config.TestSuiteDefault). + WithLabel(LabelCrossplaneVersion, CrossplaneVersionMain). + WithSetup("CreatePrerequisites", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, setupPath, "*.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, setupPath, "*.yaml"), + )). + WithSetup("PrerequisitesAreReady", funcs.AllOf( + funcs.ResourcesHaveConditionWithin(1*time.Minute, setupPath, "definition.yaml", apiextensionsv1.WatchingComposite()), + funcs.ResourcesHaveConditionWithin(2*time.Minute, setupPath, "provider.yaml", pkgv1.Healthy(), pkgv1.Active()), + )). + WithSetup("CreateExistingXR", funcs.AllOf( + funcs.ApplyResources(e2e.FieldManager, manifests, "existing-xr.yaml"), + funcs.ResourcesCreatedWithin(30*time.Second, manifests, "existing-xr.yaml"), + funcs.ResourcesHaveConditionWithin(2*time.Minute, manifests, "existing-xr.yaml", xpv1.Available()), + )). + Assess("CanDiffComposition", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context { + t.Helper() + output, log, err := RunCompDiff(t, c, "./crossplane-diff", filepath.Join(manifests, "updated-composition.yaml")) + if err != nil { + t.Fatalf("Error running comp diff command: %v\nLog output:\n%s", err, log) + } + + // Basic validation - ensure we get meaningful output + if output == "" { + t.Fatalf("Expected non-empty output from comp diff command") + } + + // Check that output contains references to our test resource + if !strings.Contains(output, "test-comp-resource") { + t.Errorf("Expected output to contain reference to test-comp-resource, got: %s", output) + } + + // Check that output shows the expected changes + if !strings.Contains(output, "resource-tier") || !strings.Contains(output, "config-data") { + t.Logf("Output: %s", output) + t.Errorf("Expected output to show changes to resource-tier and config-data annotations") + } + + t.Logf("Comp diff output:\n%s", output) + return ctx + }). + WithTeardown("DeleteExistingXR", funcs.AllOf( + funcs.DeleteResources(manifests, "existing-xr.yaml"), + funcs.ResourcesDeletedWithin(2*time.Minute, manifests, "existing-xr.yaml"), + )). + WithTeardown("DeletePrerequisites", funcs.ResourcesDeletedAfterListedAreGone(3*time.Minute, setupPath, "*.yaml", clusterNopList)). + Feature(), + ) +} diff --git a/test/e2e/manifests/beta/diff/main/comp/existing-xr.yaml b/test/e2e/manifests/beta/diff/main/comp/existing-xr.yaml new file mode 100644 index 0000000..a5e92fb --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/existing-xr.yaml @@ -0,0 +1,9 @@ +apiVersion: compdiff.example.org/v1alpha1 +kind: XCompDiffResource +metadata: + name: test-comp-resource +spec: + coolField: existing-value + crossplane: + compositionRef: + name: xcompdiffresources.compdiff.example.org \ No newline at end of file diff --git a/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml b/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml new file mode 100644 index 0000000..0037305 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml @@ -0,0 +1,42 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xcompdiffresources.compdiff.example.org +spec: + compositeTypeRef: + apiVersion: compdiff.example.org/v1alpha1 + kind: XCompDiffResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-patch-and-transform + input: + apiVersion: pt.fn.crossplane.io/v1beta1 + kind: Resources + resources: + - name: nop-resource + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "True" + time: 0s + patches: + - type: FromCompositeFieldPath + fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[config-data] + - type: FromCompositeFieldPath + fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[resource-tier] + transforms: + - type: string + string: + type: Format + fmt: "basic" + - step: detect-ready-resources + functionRef: + name: function-auto-ready \ No newline at end of file diff --git a/test/e2e/manifests/beta/diff/main/comp/setup/definition.yaml b/test/e2e/manifests/beta/diff/main/comp/setup/definition.yaml new file mode 100644 index 0000000..4492e76 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/setup/definition.yaml @@ -0,0 +1,43 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: CompositeResourceDefinition +metadata: + name: xcompdiffresources.compdiff.example.org +spec: + group: compdiff.example.org + names: + kind: XCompDiffResource + plural: xcompdiffresources + claimNames: + kind: CompDiffResource + plural: compdiffresources + versions: + - name: v1alpha1 + served: true + referenceable: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + coolField: + type: string + crossplane: + type: object + properties: + compositionRef: + type: object + properties: + name: + type: string + status: + type: object + properties: + downstreamRef: + type: object + properties: + name: + type: string + namespace: + type: string \ No newline at end of file diff --git a/test/e2e/manifests/beta/diff/main/comp/setup/provider.yaml b/test/e2e/manifests/beta/diff/main/comp/setup/provider.yaml new file mode 100644 index 0000000..9a96c14 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/setup/provider.yaml @@ -0,0 +1,22 @@ +--- +apiVersion: pkg.crossplane.io/v1 +kind: Provider +metadata: + name: provider-nop +spec: + package: xpkg.upbound.io/crossplane-contrib/provider-nop:v0.5.0 + ignoreCrossplaneConstraints: true +--- +apiVersion: pkg.crossplane.io/v1 +kind: Function +metadata: + name: function-patch-and-transform +spec: + package: xpkg.upbound.io/crossplane-contrib/function-patch-and-transform:v0.7.0 +--- +apiVersion: pkg.crossplane.io/v1 +kind: Function +metadata: + name: function-auto-ready +spec: + package: xpkg.upbound.io/crossplane-contrib/function-auto-ready:v0.2.1 \ No newline at end of file diff --git a/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml b/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml new file mode 100644 index 0000000..2eb9729 --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml @@ -0,0 +1,47 @@ +apiVersion: apiextensions.crossplane.io/v1 +kind: Composition +metadata: + name: xcompdiffresources.compdiff.example.org +spec: + compositeTypeRef: + apiVersion: compdiff.example.org/v1alpha1 + kind: XCompDiffResource + mode: Pipeline + pipeline: + - step: generate-resources + functionRef: + name: function-patch-and-transform + input: + apiVersion: pt.fn.crossplane.io/v1beta1 + kind: Resources + resources: + - name: nop-resource + base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + spec: + forProvider: + conditionAfter: + - conditionType: Ready + conditionStatus: "True" + time: 0s + patches: + - type: FromCompositeFieldPath + fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[config-data] + transforms: + - type: string + string: + type: Format + fmt: "updated-%s" + - type: FromCompositeFieldPath + fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[resource-tier] + transforms: + - type: string + string: + type: Format + fmt: "premium" + - step: detect-ready-resources + functionRef: + name: function-auto-ready \ No newline at end of file From 04dddefdf8fcbd6ec0805dd28ef16c6f1f15d22c Mon Sep 17 00:00:00 2001 From: Jonathan Ogilvie Date: Mon, 22 Sep 2025 22:13:09 -0400 Subject: [PATCH 8/8] Lint Signed-off-by: Jonathan Ogilvie --- test/e2e/diff_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/diff_test.go b/test/e2e/diff_test.go index c0c4003..3294666 100644 --- a/test/e2e/diff_test.go +++ b/test/e2e/diff_test.go @@ -86,12 +86,14 @@ func runCrossplaneDiff(t *testing.T, c *envconf.Config, binPath, subcommand stri // RunXRDiff runs the crossplane xr diff command on the provided resources. // It returns the output and any error encountered. func RunXRDiff(t *testing.T, c *envconf.Config, binPath string, resourcePaths ...string) (string, string, error) { + t.Helper() return runCrossplaneDiff(t, c, binPath, "xr", resourcePaths...) } // RunCompDiff runs the crossplane comp diff command on the provided compositions. // It returns the output and any error encountered. func RunCompDiff(t *testing.T, c *envconf.Config, binPath string, compositionPaths ...string) (string, string, error) { + t.Helper() return runCrossplaneDiff(t, c, binPath, "comp", compositionPaths...) }