From 94d90a553d753628a20635e7996b617179730ca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Mon, 21 Aug 2023 02:01:34 +0200 Subject: [PATCH] feat: Allow suppress diff line output by regex (#475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Suppress diff output by regex Signed-off-by: Jan-Otto Kröpke * Added unit tests Signed-off-by: Jan-Otto Kröpke * Keep changed filename in output Signed-off-by: Jan-Otto Kröpke * Update diff/diff.go Co-authored-by: Yusuke Kuoka * extract method Signed-off-by: Jan-Otto Kröpke * hide filteredReport Signed-off-by: Jan-Otto Kröpke * skip doSuppress, if report is empty Signed-off-by: Jan-Otto Kröpke * Add unit tests for DoSuppress --------- Signed-off-by: Jan-Otto Kröpke Co-authored-by: Yusuke Kuoka --- cmd/options.go | 1 + diff/diff.go | 74 ++++++++++++++++++-- diff/diff_test.go | 169 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 224 insertions(+), 20 deletions(-) diff --git a/cmd/options.go b/cmd/options.go index 50846887..3ff87dba 100644 --- a/cmd/options.go +++ b/cmd/options.go @@ -15,6 +15,7 @@ func AddDiffOptions(f *pflag.FlagSet, o *diff.Options) { f.StringVar(&o.OutputFormat, "output", "diff", "Possible values: diff, simple, template, dyff. When set to \"template\", use the env var HELM_DIFF_TPL to specify the template.") f.BoolVar(&o.StripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input") f.Float32VarP(&o.FindRenames, "find-renames", "D", 0, "Enable rename detection if set to any value greater than 0. If specified, the value denotes the maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a rename. Only objects of the same Kind are attempted to be matched") + f.StringArrayVar(&o.SuppressedOutputLineRegex, "suppress-output-line-regex", []string{}, "a regex to suppress diff output lines that match") } // ProcessDiffOptions processes the set flags and handles possible interactions between them diff --git a/diff/diff.go b/diff/diff.go index 28d861dd..b8ebea28 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "math" + "regexp" "sort" "strings" @@ -20,12 +21,13 @@ import ( // Options are all the options to be passed to generate a diff type Options struct { - OutputFormat string - OutputContext int - StripTrailingCR bool - ShowSecrets bool - SuppressedKinds []string - FindRenames float32 + OutputFormat string + OutputContext int + StripTrailingCR bool + ShowSecrets bool + SuppressedKinds []string + FindRenames float32 + SuppressedOutputLineRegex []string } // Manifests diff on manifests @@ -65,11 +67,71 @@ func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, options *O } seenAnyChanges := len(report.entries) > 0 + + report, err := doSuppress(report, options.SuppressedOutputLineRegex) + if err != nil { + panic(err) + } + report.print(to) report.clean() return seenAnyChanges } +func doSuppress(report Report, suppressedOutputLineRegex []string) (Report, error) { + if len(report.entries) == 0 || len(suppressedOutputLineRegex) == 0 { + return report, nil + } + + filteredReport := Report{} + filteredReport.format = report.format + filteredReport.entries = []ReportEntry{} + + var suppressOutputRegexes []*regexp.Regexp + + for _, suppressOutputRegex := range suppressedOutputLineRegex { + regex, err := regexp.Compile(suppressOutputRegex) + if err != nil { + return Report{}, err + } + + suppressOutputRegexes = append(suppressOutputRegexes, regex) + } + + for _, entry := range report.entries { + var diffs []difflib.DiffRecord + + DIFFS: + for _, diff := range entry.diffs { + for _, suppressOutputRegex := range suppressOutputRegexes { + if suppressOutputRegex.MatchString(diff.Payload) { + continue DIFFS + } + } + + diffs = append(diffs, diff) + } + + containsDiff := false + + // Add entry to the report, if diffs are present. + for _, diff := range diffs { + if diff.Delta.String() != " " { + containsDiff = true + break + } + } + + if containsDiff { + filteredReport.addEntry(entry.key, entry.suppressedKinds, entry.kind, entry.context, diffs, entry.changeType) + } else { + filteredReport.addEntry(entry.key, entry.suppressedKinds, entry.kind, entry.context, []difflib.DiffRecord{}, entry.changeType) + } + } + + return filteredReport, nil +} + func actualChanges(diff []difflib.DiffRecord) int { changes := 0 for _, record := range diff { diff --git a/diff/diff_test.go b/diff/diff_test.go index 0474e9aa..28711b74 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/aryann/difflib" "github.com/mgutz/ansi" "github.com/stretchr/testify/require" @@ -260,7 +261,7 @@ spec: t.Run("OnChange", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.0} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") @@ -277,9 +278,21 @@ spec: `, buf1.String()) }) + t.Run("OnChangeWithSuppress", func(t *testing.T) { + var buf1 bytes.Buffer + diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{"apiVersion"}} + + if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") + } + + require.Equal(t, `default, nginx, Deployment (apps) has changed: +`, buf1.String()) + }) + t.Run("OnChangeRename", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.5} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} if changesSeen := Manifests(specReleaseSpec, specReleaseRenamed, &diffOptions, &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`") @@ -300,7 +313,7 @@ spec: t.Run("OnChangeRenameAndUpdate", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.5} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndUpdated, &diffOptions, &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`") @@ -322,7 +335,7 @@ spec: t.Run("OnChangeRenameAndAdded", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.5} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndAdded, &diffOptions, &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`") @@ -341,12 +354,35 @@ spec: + matchLabels: + app: nginx-renamed +`, buf1.String()) + }) + + t.Run("OnChangeRenameAndAddedWithPartialSuppress", func(t *testing.T) { + var buf1 bytes.Buffer + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{"app: "}} + + if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndAdded, &diffOptions, &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`") + } + + require.Equal(t, `default, nginx, Deployment (apps) has changed: + + apiVersion: apps/v1 + kind: Deployment + metadata: +- name: nginx ++ name: nginx-renamed + spec: + replicas: 3 ++ selector: ++ matchLabels: + `, buf1.String()) }) t.Run("OnChangeRenameAndRemoved", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.5} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}} if changesSeen := Manifests(specReleaseRenamedAndAdded, specReleaseSpec, &diffOptions, &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`") @@ -365,12 +401,35 @@ spec: - matchLabels: - app: nginx-renamed +`, buf1.String()) + }) + + t.Run("OnChangeRenameAndRemovedWithPartialSuppress", func(t *testing.T) { + var buf1 bytes.Buffer + diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{"app: "}} + + if changesSeen := Manifests(specReleaseRenamedAndAdded, specReleaseSpec, &diffOptions, &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`") + } + + require.Equal(t, `default, nginx-renamed, Deployment (apps) has changed: + + apiVersion: apps/v1 + kind: Deployment + metadata: +- name: nginx-renamed ++ name: nginx + spec: + replicas: 3 +- selector: +- matchLabels: + `, buf1.String()) }) t.Run("OnNoChange", func(t *testing.T) { var buf2 bytes.Buffer - diffOptions := Options{"diff", 10, false, true, []string{}, 0.0} + diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &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`") @@ -381,7 +440,7 @@ spec: t.Run("OnChangeSimple", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"simple", 10, false, true, []string{}, 0.0} + diffOptions := Options{"simple", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") @@ -394,7 +453,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnNoChangeSimple", func(t *testing.T) { var buf2 bytes.Buffer - diffOptions := Options{"simple", 10, false, true, []string{}, 0.0} + diffOptions := Options{"simple", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &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`") } @@ -404,7 +463,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnChangeTemplate", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"template", 10, false, true, []string{}, 0.0} + diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") @@ -422,7 +481,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnChangeJSON", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"json", 10, false, true, []string{}, 0.0} + diffOptions := Options{"json", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") @@ -440,7 +499,7 @@ Plan: 0 to add, 1 to change, 0 to destroy. t.Run("OnNoChangeTemplate", func(t *testing.T) { var buf2 bytes.Buffer - diffOptions := Options{"template", 10, false, true, []string{}, 0.0} + diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &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`") @@ -452,7 +511,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") - diffOptions := Options{"template", 10, false, true, []string{}, 0.0} + diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}} if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &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`") @@ -535,7 +594,7 @@ stringData: t.Run("OnChangeSecretWithByteData", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false + diffOptions := Options{"diff", 10, false, false, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false if changesSeen := Manifests(specSecretWithByteData, specSecretWithByteDataChanged, &diffOptions, &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`") @@ -560,7 +619,7 @@ stringData: t.Run("OnChangeSecretWithStringData", func(t *testing.T) { var buf1 bytes.Buffer - diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false + diffOptions := Options{"diff", 10, false, false, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false if changesSeen := Manifests(specSecretWithStringData, specSecretWithStringDataChanged, &diffOptions, &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`") @@ -582,3 +641,85 @@ stringData: `, buf1.String()) }) } + +func TestDoSuppress(t *testing.T) { + for _, tt := range []struct { + name string + input Report + supressRegex []string + expected Report + }{ + { + name: "noop", + input: Report{}, + supressRegex: []string{}, + expected: Report{}, + }, + { + name: "simple", + input: Report{ + entries: []ReportEntry{ + { + diffs: diffStrings("hello: world", "hello: world2", false), + }, + }, + }, + supressRegex: []string{}, + expected: Report{ + entries: []ReportEntry{ + { + diffs: diffStrings("hello: world", "hello: world2", false), + }, + }, + }, + }, + { + name: "ignore all", + input: Report{ + entries: []ReportEntry{ + { + diffs: diffStrings("hello: world", "hello: world2", false), + }, + }, + }, + supressRegex: []string{".*world2?"}, + expected: Report{ + entries: []ReportEntry{ + { + diffs: []difflib.DiffRecord{}, + }, + }, + }, + }, + { + name: "ignore partial", + input: Report{ + entries: []ReportEntry{ + { + diffs: diffStrings("hello: world", "hello: world2", false), + }, + }, + }, + supressRegex: []string{".*world2"}, + expected: Report{ + entries: []ReportEntry{ + { + diffs: []difflib.DiffRecord{ + { + Payload: "hello: world", + Delta: difflib.LeftOnly, + }, + }, + }, + }, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + report, err := doSuppress(tt.input, tt.supressRegex) + require.NoError(t, err) + + require.Equal(t, tt.expected, report) + }) + } +}