Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow suppress diff line output by regex #475

Merged
merged 8 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
})
}
}