diff --git a/README.md b/README.md index 79cb39fb..d24ee312 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,7 @@ Flags: --post-renderer string the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path --reset-values reset the values to the ones built into the chart and merge in any new values --reuse-values reuse the last release's values and merge in any new values + --strip-trailing-cr strip trailing carriage return on input --set stringArray set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2) --suppress stringArray allows suppression of the values listed in the diff output -q, --suppress-secrets suppress secrets in the output @@ -135,6 +136,7 @@ Flags: -h, --help help for release --home string location of your Helm config. Overrides $HELM_HOME (default "/home/aananth/.helm") --include-tests enable the diffing of the helm test hooks + --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the values listed in the diff output -q, --suppress-secrets suppress secrets in the output --tls enable TLS for request @@ -172,6 +174,7 @@ Usage: Flags: -h, --help help for revision + --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the values listed in the diff output -q, --suppress-secrets suppress secrets in the output @@ -197,6 +200,7 @@ Examples: Flags: -h, --help help for rollback + --strip-trailing-cr strip trailing carriage return on input --suppress stringArray allows suppression of the values listed in the diff output -q, --suppress-secrets suppress secrets in the output diff --git a/cmd/release.go b/cmd/release.go index feb1ffaa..0706d0ac 100644 --- a/cmd/release.go +++ b/cmd/release.go @@ -21,6 +21,7 @@ type release struct { includeTests bool showSecrets bool output string + stripTrailingCR bool } const releaseCmdLongUsage = ` @@ -79,6 +80,7 @@ func releaseCmd() *cobra.Command { releaseCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes") releaseCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks") releaseCmd.Flags().StringVar(&diff.output, "output", "diff", "Possible values: diff, simple, template. When set to \"template\", use the env var HELM_DIFF_TPL to specify the template.") + releaseCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") releaseCmd.SuggestionsMinimumDistance = 1 @@ -121,6 +123,7 @@ func (d *release) differentiateHelm3() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { @@ -155,6 +158,7 @@ func (d *release) differentiate() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { diff --git a/cmd/revision.go b/cmd/revision.go index c021354a..c187a311 100644 --- a/cmd/revision.go +++ b/cmd/revision.go @@ -23,6 +23,7 @@ type revision struct { includeTests bool showSecrets bool output string + stripTrailingCR bool } const revisionCmdLongUsage = ` @@ -89,6 +90,7 @@ func revisionCmd() *cobra.Command { revisionCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes") revisionCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks") revisionCmd.Flags().StringVar(&diff.output, "output", "diff", "Possible values: diff, simple, template. When set to \"template\", use the env var HELM_DIFF_TPL to specify the template.") + revisionCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") revisionCmd.SuggestionsMinimumDistance = 1 @@ -126,6 +128,7 @@ func (d *revision) differentiateHelm3() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) case 2: @@ -152,6 +155,7 @@ func (d *revision) differentiateHelm3() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { @@ -191,6 +195,7 @@ func (d *revision) differentiate() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) case 2: @@ -217,6 +222,7 @@ func (d *revision) differentiate() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { diff --git a/cmd/rollback.go b/cmd/rollback.go index cc1cd0ab..3cec5f67 100644 --- a/cmd/rollback.go +++ b/cmd/rollback.go @@ -23,6 +23,7 @@ type rollback struct { includeTests bool showSecrets bool output string + stripTrailingCR bool } const rollbackCmdLongUsage = ` @@ -81,6 +82,7 @@ func rollbackCmd() *cobra.Command { rollbackCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes") rollbackCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks") rollbackCmd.Flags().StringVar(&diff.output, "output", "diff", "Possible values: diff, simple, template. When set to \"template\", use the env var HELM_DIFF_TPL to specify the template.") + rollbackCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") rollbackCmd.SuggestionsMinimumDistance = 1 @@ -119,6 +121,7 @@ func (d *rollback) backcastHelm3() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { @@ -155,6 +158,7 @@ func (d *rollback) backcast() error { d.showSecrets, d.outputContext, d.output, + d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { diff --git a/cmd/upgrade.go b/cmd/upgrade.go index 033c70a9..1cea6598 100644 --- a/cmd/upgrade.go +++ b/cmd/upgrade.go @@ -39,6 +39,7 @@ type diffCmd struct { postRenderer string output string install bool + stripTrailingCR bool } func (d *diffCmd) isAllowUnreleased() bool { @@ -117,6 +118,7 @@ func newChartCommand() *cobra.Command { f.BoolVar(&diff.dryRun, "dry-run", false, "disables cluster access and show diff as if it was install. Implies --install, --reset-values, and --disable-validation") f.StringVar(&diff.postRenderer, "post-renderer", "", "the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path") f.StringVar(&diff.output, "output", "diff", "Possible values: diff, simple, json, template. When set to \"template\", use the env var HELM_DIFF_TPL to specify the template.") + f.BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") if !isHelm3() { f.StringVar(&diff.namespace, "namespace", "default", "namespace to assume the release to be installed into") } @@ -184,7 +186,7 @@ func (d *diffCmd) runHelm3() error { } else { newSpecs = manifest.Parse(string(installManifest), d.namespace, helm3TestHook, helm2TestSuccessHook) } - seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, os.Stdout) + seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { return Error{ @@ -270,7 +272,7 @@ func (d *diffCmd) run() error { } } - seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, os.Stdout) + seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout) if d.detailedExitCode && seenAnyChanges { return Error{ diff --git a/diff/diff.go b/diff/diff.go index fe7c9b19..bce80e86 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -19,7 +19,7 @@ import ( ) // Manifests diff on manifests -func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, to io.Writer) bool { +func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool { report.setupReportFormat(output) seenAnyChanges := false emptyMapping := &manifest.MappingResult{} @@ -33,7 +33,7 @@ func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressed redactSecrets(oldContent, newContent) } - diffs := diffMappingResults(oldContent, newContent) + diffs := diffMappingResults(oldContent, newContent, stripTrailingCR) if len(diffs) > 0 { seenAnyChanges = true } @@ -45,7 +45,7 @@ func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressed redactSecrets(oldContent, nil) } - diffs := diffMappingResults(oldContent, emptyMapping) + diffs := diffMappingResults(oldContent, emptyMapping, stripTrailingCR) if len(diffs) > 0 { seenAnyChanges = true } @@ -61,7 +61,7 @@ func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressed if !showSecrets { redactSecrets(nil, newContent) } - diffs := diffMappingResults(emptyMapping, newContent) + diffs := diffMappingResults(emptyMapping, newContent, stripTrailingCR) if len(diffs) > 0 { seenAnyChanges = true } @@ -142,19 +142,31 @@ func getComment(s string) string { } // Releases reindex the content based on the template names and pass it to Manifests -func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, to io.Writer) bool { +func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool { oldIndex = reIndexForRelease(oldIndex) newIndex = reIndexForRelease(newIndex) - return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, output, to) + return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, output, stripTrailingCR, to) } -func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult) []difflib.DiffRecord { - return diffStrings(oldContent.Content, newContent.Content) +func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult, stripTrailingCR bool ) []difflib.DiffRecord { + return diffStrings(oldContent.Content, newContent.Content, stripTrailingCR) } -func diffStrings(before, after string) []difflib.DiffRecord { +func diffStrings(before, after string, stripTrailingCR bool) []difflib.DiffRecord { + return difflib.Diff(split(before, stripTrailingCR), split(after, stripTrailingCR)) +} + +func split(value string, stripTrailingCR bool) []string { const sep = "\n" - return difflib.Diff(strings.Split(before, sep), strings.Split(after, sep)) + split := strings.Split(value, sep) + if !stripTrailingCR { + return split + } + var stripped []string + for _, s := range split { + stripped = append(stripped, strings.TrimSuffix(s, "\r")) + } + return stripped } func printDiffRecords(suppressedKinds []string, kind string, context int, diffs []difflib.DiffRecord, to io.Writer) { diff --git a/diff/diff_test.go b/diff/diff_test.go index d90d84c9..0cab7cc8 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -35,10 +35,22 @@ var text2 = "" + "line9\n" + "line10" +var text3 = "" + + "line1\r\n" + + "line2\r\n" + + "line3\r\n" + + "line4\r\n" + + "line5\r\n" + + "line6\r\n" + + "line7\r\n" + + "line8\r\n" + + "line9\r\n" + + "line10" + func TestPrintDiffWithContext(t *testing.T) { t.Run("context-disabled", func(t *testing.T) { - assertDiff(t, text1, text2, -1, ""+ + assertDiff(t, text1, text2, -1, false, ""+ "- line1\n"+ "- line2\n"+ "+ line1 - different!\n"+ @@ -55,7 +67,7 @@ func TestPrintDiffWithContext(t *testing.T) { }) t.Run("context-0", func(t *testing.T) { - assertDiff(t, text1, text2, 0, ""+ + assertDiff(t, text1, text2, 0, false, ""+ "- line1\n"+ "- line2\n"+ "+ line1 - different!\n"+ @@ -66,8 +78,36 @@ func TestPrintDiffWithContext(t *testing.T) { "...\n") }) + t.Run("context-0-no-strip-cr", func(t *testing.T) { + assertDiff(t, text1, text3, 0, false, ""+ + "- line1\n"+ + "- line2\n"+ + "- line3\n"+ + "- line4\n"+ + "- line5\n"+ + "- line6\n"+ + "- line7\n"+ + "- line8\n"+ + "- line9\n"+ + "+ line1\r\n"+ + "+ line2\r\n"+ + "+ line3\r\n"+ + "+ line4\r\n"+ + "+ line5\r\n"+ + "+ line6\r\n"+ + "+ line7\r\n"+ + "+ line8\r\n"+ + "+ line9\r\n"+ + "...\n") + }) + + t.Run("context-0-strip-cr", func(t *testing.T) { + assertDiff(t, text1, text3, 0, true, ""+ + "...\n") + }) + t.Run("context-1", func(t *testing.T) { - assertDiff(t, text1, text2, 1, ""+ + assertDiff(t, text1, text2, 1, false, ""+ "- line1\n"+ "- line2\n"+ "+ line1 - different!\n"+ @@ -82,7 +122,7 @@ func TestPrintDiffWithContext(t *testing.T) { }) t.Run("context-2", func(t *testing.T) { - assertDiff(t, text1, text2, 2, ""+ + assertDiff(t, text1, text2, 2, false, ""+ "- line1\n"+ "- line2\n"+ "+ line1 - different!\n"+ @@ -99,7 +139,7 @@ func TestPrintDiffWithContext(t *testing.T) { }) t.Run("context-3", func(t *testing.T) { - assertDiff(t, text1, text2, 3, ""+ + assertDiff(t, text1, text2, 3, false, ""+ "- line1\n"+ "- line2\n"+ "+ line1 - different!\n"+ @@ -117,10 +157,10 @@ func TestPrintDiffWithContext(t *testing.T) { } -func assertDiff(t *testing.T, before, after string, context int, expected string) { +func assertDiff(t *testing.T, before, after string, context int, stripTrailingCR bool, expected string) { ansi.DisableColors(true) var output bytes.Buffer - diffs := diffStrings(before, after) + diffs := diffStrings(before, after, stripTrailingCR) printDiffRecords([]string{}, "some-resource", context, diffs, &output) actual := output.String() if actual != expected { @@ -159,7 +199,7 @@ metadata: var buf1 bytes.Buffer - if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "diff", &buf1); !changesSeen { + if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "diff", false, &buf1); !changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") } @@ -177,7 +217,7 @@ metadata: t.Run("OnNoChange", func(t *testing.T) { var buf2 bytes.Buffer - if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "diff", &buf2); changesSeen { + if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "diff", false, &buf2); changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`") } @@ -188,7 +228,7 @@ metadata: var buf1 bytes.Buffer - if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "simple", &buf1); !changesSeen { + if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "simple", false, &buf1); !changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") } @@ -200,7 +240,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnNoChangeSimple", func(t *testing.T) { var buf2 bytes.Buffer - if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "simple", &buf2); changesSeen { + if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "simple", false, &buf2); changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`") } @@ -211,7 +251,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. var buf1 bytes.Buffer - if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "template", &buf1); !changesSeen { + if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "template", false, &buf1); !changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") } @@ -229,7 +269,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. var buf1 bytes.Buffer - if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "json", &buf1); !changesSeen { + if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "json", false, &buf1); !changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`") } @@ -246,7 +286,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnNoChangeTemplate", func(t *testing.T) { var buf2 bytes.Buffer - if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "template", &buf2); changesSeen { + if changesSeen := Manifests(specRelease, specRelease, []string{}, true, 10, "template", false, &buf2); changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`") } @@ -256,7 +296,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnChangeCustomTemplate", func(t *testing.T) { var buf1 bytes.Buffer os.Setenv("HELM_DIFF_TPL", "testdata/customTemplate.tpl") - if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "template", &buf1); !changesSeen { + if changesSeen := Manifests(specBeta, specRelease, []string{}, true, 10, "template", false, &buf1); !changesSeen { t.Error("Unexpected return value from Manifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`") }