diff --git a/cmd/diff/diffprocessor/diff_processor.go b/cmd/diff/diffprocessor/diff_processor.go index f5d5cda..a7a32d0 100644 --- a/cmd/diff/diffprocessor/diff_processor.go +++ b/cmd/diff/diffprocessor/diff_processor.go @@ -160,6 +160,12 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer 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)) + + // Write error message to stdout so user can see it + errMsg := fmt.Sprintf("ERROR: Failed to process %s: %v\n\n", resourceID, err) + if _, writeErr := fmt.Fprint(stdout, errMsg); writeErr != nil { + p.config.Logger.Debug("Failed to write error message", "error", writeErr) + } } else { // Merge the diffs into our combined map for k, v := range diffs { diff --git a/cmd/diff/diffprocessor/diff_processor_test.go b/cmd/diff/diffprocessor/diff_processor_test.go index de7160f..f070cf9 100644 --- a/cmd/diff/diffprocessor/diff_processor_test.go +++ b/cmd/diff/diffprocessor/diff_processor_test.go @@ -149,7 +149,18 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { }, resources: []*un.Unstructured{resource1}, processorOpts: testProcessorOptions(t), - want: errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"), + verifyOutput: func(t *testing.T, output string) { + t.Helper() + // Verify that the error message was written to stdout + if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-1") { + t.Errorf("Expected stdout to contain error message, got: %s", output) + } + // Also verify it contains the composition not found error + if !strings.Contains(output, "composition not found") { + t.Errorf("Expected stdout to contain 'composition not found' error detail, got: %s", output) + } + }, + want: errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"), }, "MultipleResourceErrors": { setupMocks: func() (k8.Clients, xp.Clients) { @@ -178,6 +189,22 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { }, resources: []*un.Unstructured{resource1, resource2}, processorOpts: testProcessorOptions(t), + verifyOutput: func(t *testing.T, output string) { + t.Helper() + // Verify that error messages for both resources were written to stdout + if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-1") { + t.Errorf("Expected stdout to contain error message for my-xr-1, got: %s", output) + } + + if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-2") { + t.Errorf("Expected stdout to contain error message for my-xr-2, got: %s", output) + } + // Both should contain the composition not found error + expectedCount := strings.Count(output, "composition not found") + if expectedCount < 2 { + t.Errorf("Expected stdout to contain 'composition not found' at least twice, found %d times in: %s", expectedCount, output) + } + }, 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]"), }, @@ -565,6 +592,11 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { } err := processor.PerformDiff(ctx, &stdout, tt.resources, compositionProvider) + // Check output if verification function is provided (do this first, before error checks) + if tt.verifyOutput != nil { + tt.verifyOutput(t, stdout.String()) + } + if tt.want != nil { if err == nil { t.Errorf("PerformDiff(...): expected error but got none") @@ -581,11 +613,6 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) { if err != nil { t.Errorf("PerformDiff(...): unexpected error: %v", err) } - - // Check output if verification function is provided - if tt.verifyOutput != nil { - tt.verifyOutput(t, stdout.String()) - } }) } } diff --git a/test/e2e/diff_test.go b/test/e2e/diff_test.go index c3187c7..39dd8d4 100644 --- a/test/e2e/diff_test.go +++ b/test/e2e/diff_test.go @@ -531,6 +531,7 @@ var clusterNopList = composed.NewList(composed.FromReferenceToList(corev1.Object // Regular expressions to match the dynamic parts. var ( resourceNameRegex = regexp.MustCompile(`(existing-resource)-[a-z0-9]{5,}(?:-nop-resource)?`) + compResourceNameRegex = regexp.MustCompile(`(test-comp-resource)-[a-z0-9]{5,}`) claimNameRegex = regexp.MustCompile(`(test-claim)-[a-z0-9]{5,}(?:-[a-z0-9]{5,})?`) claimCompositionRevisionRegex = regexp.MustCompile(`(xnopclaims\.claim\.diff\.example\.org)-[a-z0-9]{7,}`) compositionRevisionRegex = regexp.MustCompile(`(xnopresources\.(cluster\.|legacy\.)?diff\.example\.org)-[a-z0-9]{7,}`) @@ -545,6 +546,7 @@ func normalizeLine(line string) string { // Replace resource names with random suffixes line = resourceNameRegex.ReplaceAllString(line, "${1}-XXXXX") + line = compResourceNameRegex.ReplaceAllString(line, "${1}-XXXXX") line = claimNameRegex.ReplaceAllString(line, "${1}-XXXXX") // Replace composition revision refs with random hash @@ -689,6 +691,7 @@ 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") + expectPath := filepath.Join(manifests, "expect") environment.Test(t, features.New("DiffExistingComposition"). @@ -717,23 +720,7 @@ func TestDiffExistingComposition(t *testing.T) { 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) + assertDiffMatchesFile(t, output, filepath.Join(expectPath, "existing-xr.ansi"), log) return ctx }). diff --git a/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi b/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi new file mode 100644 index 0000000..dae5a9c --- /dev/null +++ b/test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi @@ -0,0 +1,94 @@ +=== Composition Changes === + +~~~ Composition/xcompdiffresources.compdiff.example.org + 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: + - functionRef: + name: function-patch-and-transform + input: + apiVersion: pt.fn.crossplane.io/v1beta1 + kind: Resources + resources: + - base: + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + spec: + forProvider: + conditionAfter: + - conditionStatus: "True" + conditionType: Ready + time: 0s + name: nop-resource + patches: + - fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[config-data] ++ transforms: ++ - string: ++ fmt: updated-%s ++ type: Format ++ type: string + type: FromCompositeFieldPath + - fromFieldPath: spec.coolField + toFieldPath: metadata.annotations[resource-tier] + transforms: + - string: +- fmt: basic-%s ++ fmt: premium-%s + type: Format + type: string + type: FromCompositeFieldPath + step: generate-resources + - functionRef: + name: function-auto-ready + step: detect-ready-resources + +--- + +Summary: 1 modified + +=== Affected Composite Resources === + +- XCompDiffResource/test-comp-resource (namespace: ) + +=== Impact Analysis === + +~~~ ClusterNopResource/test-comp-resource-XXXXX + apiVersion: nop.crossplane.io/v1alpha1 + kind: ClusterNopResource + metadata: + annotations: +- config-data: existing-value ++ config-data: updated-existing-value + crossplane.io/composition-resource-name: nop-resource + crossplane.io/external-name: test-comp-resource-XXXXX +- resource-tier: basic-existing-value ++ resource-tier: premium-existing-value + finalizers: + - finalizer.managedresource.crossplane.io + generateName: test-comp-resource- + labels: + crossplane.io/composite: test-comp-resource + name: test-comp-resource-XXXXX + spec: + deletionPolicy: Delete + forProvider: + conditionAfter: + - conditionStatus: "True" + conditionType: Ready + time: 0s + managementPolicies: + - '*' + providerConfigRef: + name: default + +--- + +Summary: 1 modified diff --git a/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml b/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml index 0037305..62f5f08 100644 --- a/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml +++ b/test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml @@ -36,7 +36,7 @@ spec: - type: string string: type: Format - fmt: "basic" + fmt: "basic-%s" - 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/updated-composition.yaml b/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml index 2eb9729..8aaa90b 100644 --- a/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml +++ b/test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml @@ -41,7 +41,7 @@ spec: - type: string string: type: Format - fmt: "premium" + fmt: "premium-%s" - step: detect-ready-resources functionRef: name: function-auto-ready \ No newline at end of file