Skip to content

Commit 94d90a5

Browse files
jkroepkemumoshu
andauthored
feat: Allow suppress diff line output by regex (databus23#475)
* 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]>
1 parent fdfb30b commit 94d90a5

File tree

3 files changed

+224
-20
lines changed

3 files changed

+224
-20
lines changed

cmd/options.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ func AddDiffOptions(f *pflag.FlagSet, o *diff.Options) {
1515
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.")
1616
f.BoolVar(&o.StripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
1717
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")
18+
f.StringArrayVar(&o.SuppressedOutputLineRegex, "suppress-output-line-regex", []string{}, "a regex to suppress diff output lines that match")
1819
}
1920

2021
// ProcessDiffOptions processes the set flags and handles possible interactions between them

diff/diff.go

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"io"
77
"math"
8+
"regexp"
89
"sort"
910
"strings"
1011

@@ -20,12 +21,13 @@ import (
2021

2122
// Options are all the options to be passed to generate a diff
2223
type Options struct {
23-
OutputFormat string
24-
OutputContext int
25-
StripTrailingCR bool
26-
ShowSecrets bool
27-
SuppressedKinds []string
28-
FindRenames float32
24+
OutputFormat string
25+
OutputContext int
26+
StripTrailingCR bool
27+
ShowSecrets bool
28+
SuppressedKinds []string
29+
FindRenames float32
30+
SuppressedOutputLineRegex []string
2931
}
3032

3133
// Manifests diff on manifests
@@ -65,11 +67,71 @@ func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, options *O
6567
}
6668

6769
seenAnyChanges := len(report.entries) > 0
70+
71+
report, err := doSuppress(report, options.SuppressedOutputLineRegex)
72+
if err != nil {
73+
panic(err)
74+
}
75+
6876
report.print(to)
6977
report.clean()
7078
return seenAnyChanges
7179
}
7280

81+
func doSuppress(report Report, suppressedOutputLineRegex []string) (Report, error) {
82+
if len(report.entries) == 0 || len(suppressedOutputLineRegex) == 0 {
83+
return report, nil
84+
}
85+
86+
filteredReport := Report{}
87+
filteredReport.format = report.format
88+
filteredReport.entries = []ReportEntry{}
89+
90+
var suppressOutputRegexes []*regexp.Regexp
91+
92+
for _, suppressOutputRegex := range suppressedOutputLineRegex {
93+
regex, err := regexp.Compile(suppressOutputRegex)
94+
if err != nil {
95+
return Report{}, err
96+
}
97+
98+
suppressOutputRegexes = append(suppressOutputRegexes, regex)
99+
}
100+
101+
for _, entry := range report.entries {
102+
var diffs []difflib.DiffRecord
103+
104+
DIFFS:
105+
for _, diff := range entry.diffs {
106+
for _, suppressOutputRegex := range suppressOutputRegexes {
107+
if suppressOutputRegex.MatchString(diff.Payload) {
108+
continue DIFFS
109+
}
110+
}
111+
112+
diffs = append(diffs, diff)
113+
}
114+
115+
containsDiff := false
116+
117+
// Add entry to the report, if diffs are present.
118+
for _, diff := range diffs {
119+
if diff.Delta.String() != " " {
120+
containsDiff = true
121+
break
122+
}
123+
}
124+
125+
if containsDiff {
126+
filteredReport.addEntry(entry.key, entry.suppressedKinds, entry.kind, entry.context, diffs, entry.changeType)
127+
} else {
128+
filteredReport.addEntry(entry.key, entry.suppressedKinds, entry.kind, entry.context, []difflib.DiffRecord{}, entry.changeType)
129+
}
130+
}
131+
132+
return filteredReport, nil
133+
}
134+
73135
func actualChanges(diff []difflib.DiffRecord) int {
74136
changes := 0
75137
for _, record := range diff {

diff/diff_test.go

Lines changed: 155 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"testing"
77

8+
"github.com/aryann/difflib"
89
"github.com/mgutz/ansi"
910
"github.com/stretchr/testify/require"
1011

@@ -260,7 +261,7 @@ spec:
260261

261262
t.Run("OnChange", func(t *testing.T) {
262263
var buf1 bytes.Buffer
263-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.0}
264+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{}}
264265

265266
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
266267
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:
277278
`, buf1.String())
278279
})
279280

281+
t.Run("OnChangeWithSuppress", func(t *testing.T) {
282+
var buf1 bytes.Buffer
283+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{"apiVersion"}}
284+
285+
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
286+
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`")
287+
}
288+
289+
require.Equal(t, `default, nginx, Deployment (apps) has changed:
290+
`, buf1.String())
291+
})
292+
280293
t.Run("OnChangeRename", func(t *testing.T) {
281294
var buf1 bytes.Buffer
282-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5}
295+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}}
283296

284297
if changesSeen := Manifests(specReleaseSpec, specReleaseRenamed, &diffOptions, &buf1); !changesSeen {
285298
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:
300313

301314
t.Run("OnChangeRenameAndUpdate", func(t *testing.T) {
302315
var buf1 bytes.Buffer
303-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5}
316+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}}
304317

305318
if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndUpdated, &diffOptions, &buf1); !changesSeen {
306319
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:
322335

323336
t.Run("OnChangeRenameAndAdded", func(t *testing.T) {
324337
var buf1 bytes.Buffer
325-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5}
338+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}}
326339

327340
if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndAdded, &diffOptions, &buf1); !changesSeen {
328341
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:
341354
+ matchLabels:
342355
+ app: nginx-renamed
343356
357+
`, buf1.String())
358+
})
359+
360+
t.Run("OnChangeRenameAndAddedWithPartialSuppress", func(t *testing.T) {
361+
var buf1 bytes.Buffer
362+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{"app: "}}
363+
364+
if changesSeen := Manifests(specReleaseSpec, specReleaseRenamedAndAdded, &diffOptions, &buf1); !changesSeen {
365+
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`")
366+
}
367+
368+
require.Equal(t, `default, nginx, Deployment (apps) has changed:
369+
370+
apiVersion: apps/v1
371+
kind: Deployment
372+
metadata:
373+
- name: nginx
374+
+ name: nginx-renamed
375+
spec:
376+
replicas: 3
377+
+ selector:
378+
+ matchLabels:
379+
344380
`, buf1.String())
345381
})
346382

347383
t.Run("OnChangeRenameAndRemoved", func(t *testing.T) {
348384
var buf1 bytes.Buffer
349-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5}
385+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{}}
350386

351387
if changesSeen := Manifests(specReleaseRenamedAndAdded, specReleaseSpec, &diffOptions, &buf1); !changesSeen {
352388
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:
365401
- matchLabels:
366402
- app: nginx-renamed
367403
404+
`, buf1.String())
405+
})
406+
407+
t.Run("OnChangeRenameAndRemovedWithPartialSuppress", func(t *testing.T) {
408+
var buf1 bytes.Buffer
409+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.5, []string{"app: "}}
410+
411+
if changesSeen := Manifests(specReleaseRenamedAndAdded, specReleaseSpec, &diffOptions, &buf1); !changesSeen {
412+
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`")
413+
}
414+
415+
require.Equal(t, `default, nginx-renamed, Deployment (apps) has changed:
416+
417+
apiVersion: apps/v1
418+
kind: Deployment
419+
metadata:
420+
- name: nginx-renamed
421+
+ name: nginx
422+
spec:
423+
replicas: 3
424+
- selector:
425+
- matchLabels:
426+
368427
`, buf1.String())
369428
})
370429

371430
t.Run("OnNoChange", func(t *testing.T) {
372431
var buf2 bytes.Buffer
373-
diffOptions := Options{"diff", 10, false, true, []string{}, 0.0}
432+
diffOptions := Options{"diff", 10, false, true, []string{}, 0.0, []string{}}
374433

375434
if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &buf2); changesSeen {
376435
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:
381440

382441
t.Run("OnChangeSimple", func(t *testing.T) {
383442
var buf1 bytes.Buffer
384-
diffOptions := Options{"simple", 10, false, true, []string{}, 0.0}
443+
diffOptions := Options{"simple", 10, false, true, []string{}, 0.0, []string{}}
385444

386445
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
387446
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.
394453

395454
t.Run("OnNoChangeSimple", func(t *testing.T) {
396455
var buf2 bytes.Buffer
397-
diffOptions := Options{"simple", 10, false, true, []string{}, 0.0}
456+
diffOptions := Options{"simple", 10, false, true, []string{}, 0.0, []string{}}
398457
if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &buf2); changesSeen {
399458
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`")
400459
}
@@ -404,7 +463,7 @@ Plan: 0 to add, 1 to change, 0 to destroy.
404463

405464
t.Run("OnChangeTemplate", func(t *testing.T) {
406465
var buf1 bytes.Buffer
407-
diffOptions := Options{"template", 10, false, true, []string{}, 0.0}
466+
diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}}
408467

409468
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
410469
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.
422481

423482
t.Run("OnChangeJSON", func(t *testing.T) {
424483
var buf1 bytes.Buffer
425-
diffOptions := Options{"json", 10, false, true, []string{}, 0.0}
484+
diffOptions := Options{"json", 10, false, true, []string{}, 0.0, []string{}}
426485

427486
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
428487
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.
440499

441500
t.Run("OnNoChangeTemplate", func(t *testing.T) {
442501
var buf2 bytes.Buffer
443-
diffOptions := Options{"template", 10, false, true, []string{}, 0.0}
502+
diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}}
444503

445504
if changesSeen := Manifests(specRelease, specRelease, &diffOptions, &buf2); changesSeen {
446505
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.
452511
t.Run("OnChangeCustomTemplate", func(t *testing.T) {
453512
var buf1 bytes.Buffer
454513
os.Setenv("HELM_DIFF_TPL", "testdata/customTemplate.tpl")
455-
diffOptions := Options{"template", 10, false, true, []string{}, 0.0}
514+
diffOptions := Options{"template", 10, false, true, []string{}, 0.0, []string{}}
456515

457516
if changesSeen := Manifests(specBeta, specRelease, &diffOptions, &buf1); !changesSeen {
458517
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:
535594

536595
t.Run("OnChangeSecretWithByteData", func(t *testing.T) {
537596
var buf1 bytes.Buffer
538-
diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false
597+
diffOptions := Options{"diff", 10, false, false, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false
539598

540599
if changesSeen := Manifests(specSecretWithByteData, specSecretWithByteDataChanged, &diffOptions, &buf1); !changesSeen {
541600
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:
560619

561620
t.Run("OnChangeSecretWithStringData", func(t *testing.T) {
562621
var buf1 bytes.Buffer
563-
diffOptions := Options{"diff", 10, false, false, []string{}, 0.5} //NOTE: ShowSecrets = false
622+
diffOptions := Options{"diff", 10, false, false, []string{}, 0.5, []string{}} //NOTE: ShowSecrets = false
564623

565624
if changesSeen := Manifests(specSecretWithStringData, specSecretWithStringDataChanged, &diffOptions, &buf1); !changesSeen {
566625
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:
582641
`, buf1.String())
583642
})
584643
}
644+
645+
func TestDoSuppress(t *testing.T) {
646+
for _, tt := range []struct {
647+
name string
648+
input Report
649+
supressRegex []string
650+
expected Report
651+
}{
652+
{
653+
name: "noop",
654+
input: Report{},
655+
supressRegex: []string{},
656+
expected: Report{},
657+
},
658+
{
659+
name: "simple",
660+
input: Report{
661+
entries: []ReportEntry{
662+
{
663+
diffs: diffStrings("hello: world", "hello: world2", false),
664+
},
665+
},
666+
},
667+
supressRegex: []string{},
668+
expected: Report{
669+
entries: []ReportEntry{
670+
{
671+
diffs: diffStrings("hello: world", "hello: world2", false),
672+
},
673+
},
674+
},
675+
},
676+
{
677+
name: "ignore all",
678+
input: Report{
679+
entries: []ReportEntry{
680+
{
681+
diffs: diffStrings("hello: world", "hello: world2", false),
682+
},
683+
},
684+
},
685+
supressRegex: []string{".*world2?"},
686+
expected: Report{
687+
entries: []ReportEntry{
688+
{
689+
diffs: []difflib.DiffRecord{},
690+
},
691+
},
692+
},
693+
},
694+
{
695+
name: "ignore partial",
696+
input: Report{
697+
entries: []ReportEntry{
698+
{
699+
diffs: diffStrings("hello: world", "hello: world2", false),
700+
},
701+
},
702+
},
703+
supressRegex: []string{".*world2"},
704+
expected: Report{
705+
entries: []ReportEntry{
706+
{
707+
diffs: []difflib.DiffRecord{
708+
{
709+
Payload: "hello: world",
710+
Delta: difflib.LeftOnly,
711+
},
712+
},
713+
},
714+
},
715+
},
716+
},
717+
} {
718+
t.Run(tt.name, func(t *testing.T) {
719+
report, err := doSuppress(tt.input, tt.supressRegex)
720+
require.NoError(t, err)
721+
722+
require.Equal(t, tt.expected, report)
723+
})
724+
}
725+
}

0 commit comments

Comments
 (0)