Skip to content

Commit

Permalink
feat: Allow suppress diff line output by regex (#475)
Browse files Browse the repository at this point in the history
* Suppress diff output by regex

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Added unit tests

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Keep changed filename in output

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Update diff/diff.go

Co-authored-by: Yusuke Kuoka <[email protected]>

* extract method

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* hide filteredReport

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* skip doSuppress, if report is empty

Signed-off-by: Jan-Otto Kröpke <[email protected]>

* Add unit tests for DoSuppress

---------

Signed-off-by: Jan-Otto Kröpke <[email protected]>
Co-authored-by: Yusuke Kuoka <[email protected]>
  • Loading branch information
jkroepke and mumoshu authored Aug 21, 2023
1 parent fdfb30b commit 94d90a5
Show file tree
Hide file tree
Showing 3 changed files with 224 additions and 20 deletions.
1 change: 1 addition & 0 deletions cmd/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 68 additions & 6 deletions diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"
"math"
"regexp"
"sort"
"strings"

Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
169 changes: 155 additions & 14 deletions diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"testing"

"github.com/aryann/difflib"
"github.com/mgutz/ansi"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
}
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand All @@ -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`")
Expand Down Expand Up @@ -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`")
Expand All @@ -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`")
Expand All @@ -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)
})
}
}

0 comments on commit 94d90a5

Please sign in to comment.