Skip to content

Commit fcf340c

Browse files
fwieselfwiesel
committed
Discover renames based on content
If we rename objects, they will appear as add and remove actions, which makes it difficult to assess the semantic delta, which may only be a simple renaming. The current implementation creates potentially a diff between every add/remove item, which means O(n^2) runtime. So it should only be enabled if such changes are rare
1 parent 97f7749 commit fcf340c

File tree

6 files changed

+177
-49
lines changed

6 files changed

+177
-49
lines changed

‎cmd/release.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ type release struct {
1818
suppressedKinds []string
1919
releases []string
2020
outputContext int
21+
maxContentDelta int
2122
includeTests bool
2223
showSecrets bool
2324
output string
@@ -79,6 +80,7 @@ func releaseCmd() *cobra.Command {
7980
releaseCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
8081
releaseCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
8182
releaseCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
83+
releaseCmd.Flags().IntVarP(&diff.maxContentDelta, "max-content-delta", "D", 0, "maximum delta between two diffs")
8284
releaseCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
8385
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.")
8486
releaseCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -124,6 +126,7 @@ func (d *release) differentiateHelm3() error {
124126
d.suppressedKinds,
125127
d.showSecrets,
126128
d.outputContext,
129+
d.maxContentDelta,
127130
d.output,
128131
d.stripTrailingCR,
129132
os.Stdout)
@@ -159,6 +162,7 @@ func (d *release) differentiate() error {
159162
d.suppressedKinds,
160163
d.showSecrets,
161164
d.outputContext,
165+
d.maxContentDelta,
162166
d.output,
163167
d.stripTrailingCR,
164168
os.Stdout)

‎cmd/revision.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type revision struct {
2020
suppressedKinds []string
2121
revisions []string
2222
outputContext int
23+
maxContentDelta int
2324
includeTests bool
2425
showSecrets bool
2526
output string
@@ -89,6 +90,7 @@ func revisionCmd() *cobra.Command {
8990
revisionCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
9091
revisionCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
9192
revisionCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
93+
revisionCmd.Flags().IntVarP(&diff.maxContentDelta, "max-content-delta", "D", 0, "maximum delta between two diffs")
9294
revisionCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
9395
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.")
9496
revisionCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -129,6 +131,7 @@ func (d *revision) differentiateHelm3() error {
129131
d.suppressedKinds,
130132
d.showSecrets,
131133
d.outputContext,
134+
d.maxContentDelta,
132135
d.output,
133136
d.stripTrailingCR,
134137
os.Stdout)
@@ -156,6 +159,7 @@ func (d *revision) differentiateHelm3() error {
156159
d.suppressedKinds,
157160
d.showSecrets,
158161
d.outputContext,
162+
d.maxContentDelta,
159163
d.output,
160164
d.stripTrailingCR,
161165
os.Stdout)
@@ -196,6 +200,7 @@ func (d *revision) differentiate() error {
196200
d.suppressedKinds,
197201
d.showSecrets,
198202
d.outputContext,
203+
d.maxContentDelta,
199204
d.output,
200205
d.stripTrailingCR,
201206
os.Stdout)
@@ -223,6 +228,7 @@ func (d *revision) differentiate() error {
223228
d.suppressedKinds,
224229
d.showSecrets,
225230
d.outputContext,
231+
d.maxContentDelta,
226232
d.output,
227233
d.stripTrailingCR,
228234
os.Stdout)

‎cmd/rollback.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ type rollback struct {
2020
suppressedKinds []string
2121
revisions []string
2222
outputContext int
23+
maxContentDelta int
2324
includeTests bool
2425
showSecrets bool
2526
output string
@@ -81,6 +82,7 @@ func rollbackCmd() *cobra.Command {
8182
rollbackCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
8283
rollbackCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
8384
rollbackCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
85+
rollbackCmd.Flags().IntVarP(&diff.maxContentDelta, "max-content-delta", "D", 0, "maximum delta between two diffs")
8486
rollbackCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
8587
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.")
8688
rollbackCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -122,6 +124,7 @@ func (d *rollback) backcastHelm3() error {
122124
d.suppressedKinds,
123125
d.showSecrets,
124126
d.outputContext,
127+
d.maxContentDelta,
125128
d.output,
126129
d.stripTrailingCR,
127130
os.Stdout)
@@ -159,6 +162,7 @@ func (d *rollback) backcast() error {
159162
d.suppressedKinds,
160163
d.showSecrets,
161164
d.outputContext,
165+
d.maxContentDelta,
162166
d.output,
163167
d.stripTrailingCR,
164168
os.Stdout)

‎cmd/upgrade.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ type diffCmd struct {
3636
includeTests bool
3737
suppressedKinds []string
3838
outputContext int
39+
maxContentDelta int
3940
showSecrets bool
4041
postRenderer string
4142
output string
@@ -116,6 +117,7 @@ func newChartCommand() *cobra.Command {
116117
f.BoolVar(&diff.devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.")
117118
f.StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
118119
f.IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
120+
f.IntVarP(&diff.maxContentDelta, "max-content-delta", "D", 0, "maximum delta between two diffs")
119121
f.BoolVar(&diff.disableValidation, "disable-validation", false, "disables rendered templates validation against the Kubernetes cluster you are currently pointing to. This is the same validation performed on an install")
120122
f.BoolVar(&diff.disableOpenAPIValidation, "disable-openapi-validation", false, "disables rendered templates validation against the Kubernetes OpenAPI Schema")
121123
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")
@@ -190,7 +192,7 @@ func (d *diffCmd) runHelm3() error {
190192
} else {
191193
newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests, helm3TestHook, helm2TestSuccessHook)
192194
}
193-
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout)
195+
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.maxContentDelta, d.output, d.stripTrailingCR, os.Stdout)
194196

195197
if d.detailedExitCode && seenAnyChanges {
196198
return Error{
@@ -276,7 +278,7 @@ func (d *diffCmd) run() error {
276278
}
277279
}
278280

279-
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout)
281+
seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.maxContentDelta, d.output, d.stripTrailingCR, os.Stdout)
280282

281283
if d.detailedExitCode && seenAnyChanges {
282284
return Error{

‎diff/diff.go‎

Lines changed: 104 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,58 +19,121 @@ import (
1919
)
2020

2121
// Manifests diff on manifests
22-
func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool {
22+
func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, maxDelta int, output string, stripTrailingCR bool, to io.Writer) bool {
2323
report.setupReportFormat(output)
2424
seenAnyChanges := false
2525
emptyMapping := &manifest.MappingResult{}
26+
var possiblyRemoved []string
27+
2628
for _, key := range sortedKeys(oldIndex) {
2729
oldContent := oldIndex[key]
2830

2931
if newContent, ok := newIndex[key]; ok {
30-
if oldContent.Content != newContent.Content {
31-
// modified
32-
if !showSecrets {
33-
redactSecrets(oldContent, newContent)
34-
}
35-
36-
diffs := diffMappingResults(oldContent, newContent, stripTrailingCR)
37-
if len(diffs) > 0 {
38-
seenAnyChanges = true
39-
}
40-
report.addEntry(key, suppressedKinds, oldContent.Kind, context, diffs, "MODIFY")
41-
}
42-
} else {
43-
// removed
44-
if !showSecrets {
45-
redactSecrets(oldContent, nil)
46-
47-
}
48-
diffs := diffMappingResults(oldContent, emptyMapping, stripTrailingCR)
49-
if len(diffs) > 0 {
32+
// modified
33+
if doDiff(key, oldContent, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
5034
seenAnyChanges = true
5135
}
52-
report.addEntry(key, suppressedKinds, oldContent.Kind, context, diffs, "REMOVE")
36+
} else {
37+
possiblyRemoved = append(possiblyRemoved, key)
5338
}
5439
}
5540

41+
var possiblyAdded []string
5642
for _, key := range sortedKeys(newIndex) {
43+
if _, ok := oldIndex[key]; !ok {
44+
possiblyAdded = append(possiblyAdded, key)
45+
}
46+
}
47+
48+
removed, added, anyChanges := contentSearch(possiblyRemoved, oldIndex, possiblyAdded, newIndex, showSecrets, stripTrailingCR, suppressedKinds, context, maxDelta)
49+
if anyChanges {
50+
seenAnyChanges = true
51+
}
52+
53+
for _, key := range removed {
54+
oldContent := oldIndex[key]
55+
56+
if doDiff(key, oldContent, emptyMapping, suppressedKinds, showSecrets, stripTrailingCR, context) {
57+
seenAnyChanges = true
58+
}
59+
}
60+
61+
for _, key := range added {
5762
newContent := newIndex[key]
5863

59-
if _, ok := oldIndex[key]; !ok {
60-
// added
64+
if doDiff(key, emptyMapping, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
65+
seenAnyChanges = true
66+
}
67+
}
68+
69+
report.print(to)
70+
report.clean()
71+
return seenAnyChanges
72+
}
73+
74+
func contentSearch(possiblyRemoved []string, oldIndex map[string]*manifest.MappingResult, possiblyAdded []string, newIndex map[string]*manifest.MappingResult, showSecrets bool, stripTrailingCR bool, suppressedKinds []string, context int, maxDelta int) ([]string, []string, bool) {
75+
if maxDelta < 1 {
76+
return possiblyRemoved, possiblyAdded, false
77+
}
78+
79+
var removed []string
80+
seenAnyChanges := false
81+
82+
for _, removedKey := range possiblyRemoved {
83+
oldContent := oldIndex[removedKey]
84+
var smallestKey string
85+
var smallestDiff []difflib.DiffRecord
86+
for _, addedKey := range possiblyAdded {
87+
newContent := newIndex[addedKey]
6188
if !showSecrets {
62-
redactSecrets(nil, newContent)
89+
redactSecrets(oldContent, newContent)
90+
}
91+
92+
diffs := diffMappingResults(oldContent, newContent, stripTrailingCR)
93+
if delta := len(diffs); delta > 0 && (len(smallestDiff) == 0 || delta < len(smallestDiff)) {
94+
smallestKey = addedKey
95+
smallestDiff = diffs
6396
}
64-
diffs := diffMappingResults(emptyMapping, newContent, stripTrailingCR)
65-
if len(diffs) > 0 {
97+
}
98+
99+
if 0 < len(smallestDiff) && len(smallestDiff) < maxDelta {
100+
index := sort.SearchStrings(possiblyAdded, smallestKey)
101+
possiblyAdded = append(possiblyAdded[:index], possiblyAdded[index+1:]...)
102+
newContent := newIndex[smallestKey]
103+
if doDiff(removedKey, oldContent, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
66104
seenAnyChanges = true
67105
}
68-
report.addEntry(key, suppressedKinds, newContent.Kind, context, diffs, "ADD")
106+
107+
} else {
108+
removed = append(removed, removedKey)
69109
}
70110
}
71-
report.print(to)
72-
report.clean()
73-
return seenAnyChanges
111+
112+
return removed, possiblyAdded, seenAnyChanges
113+
}
114+
115+
func doDiff(key string, oldContent *manifest.MappingResult, newContent *manifest.MappingResult, suppressedKinds []string, showSecrets bool, stripTrailingCR bool, context int) bool {
116+
if !showSecrets {
117+
redactSecrets(oldContent, newContent)
118+
}
119+
120+
diffs := diffMappingResults(oldContent, newContent, stripTrailingCR)
121+
var kind string
122+
var changeType string
123+
if oldContent == nil {
124+
kind = newContent.Kind
125+
changeType = "ADD"
126+
} else if newContent == nil {
127+
kind = oldContent.Kind
128+
changeType = "REMOVE"
129+
} else {
130+
if oldContent.Content == newContent.Content {
131+
return false
132+
}
133+
changeType = "MODIFY"
134+
}
135+
report.addEntry(key, suppressedKinds, kind, context, diffs, changeType)
136+
return len(diffs) > 0
74137
}
75138

76139
func redactSecrets(old, new *manifest.MappingResult) {
@@ -116,15 +179,15 @@ func redactSecrets(old, new *manifest.MappingResult) {
116179
if old != nil {
117180
oldSecret.Data = nil
118181
if err := serializer.Encode(&oldSecret, &buf); err != nil {
119-
182+
new.Content = fmt.Sprintf("Error encoding new secret: %s", err)
120183
}
121184
old.Content = getComment(old.Content) + strings.Replace(strings.Replace(buf.String(), "stringData", "data", 1), " creationTimestamp: null\n", "", 1)
122185
buf.Reset() //reuse buffer for new secret
123186
}
124187
if new != nil {
125188
newSecret.Data = nil
126189
if err := serializer.Encode(&newSecret, &buf); err != nil {
127-
190+
new.Content = fmt.Sprintf("Error encoding new secret: %s", err)
128191
}
129192
new.Content = getComment(new.Content) + strings.Replace(strings.Replace(buf.String(), "stringData", "data", 1), " creationTimestamp: null\n", "", 1)
130193
}
@@ -142,13 +205,13 @@ func getComment(s string) string {
142205
}
143206

144207
// Releases reindex the content based on the template names and pass it to Manifests
145-
func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool {
208+
func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, maxContentDelta int, output string, stripTrailingCR bool, to io.Writer) bool {
146209
oldIndex = reIndexForRelease(oldIndex)
147210
newIndex = reIndexForRelease(newIndex)
148-
return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, output, stripTrailingCR, to)
211+
return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, maxContentDelta, output, stripTrailingCR, to)
149212
}
150213

151-
func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult, stripTrailingCR bool ) []difflib.DiffRecord {
214+
func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult, stripTrailingCR bool) []difflib.DiffRecord {
152215
return diffStrings(oldContent.Content, newContent.Content, stripTrailingCR)
153216
}
154217

@@ -209,7 +272,11 @@ func printDiffRecord(diff difflib.DiffRecord, to io.Writer) {
209272
case difflib.LeftOnly:
210273
fmt.Fprintf(to, "%s\n", ansi.Color("- "+text, "red"))
211274
case difflib.Common:
212-
fmt.Fprintf(to, "%s\n", " "+text)
275+
if text == "\n" || text == "" {
276+
fmt.Fprintln(to)
277+
} else {
278+
fmt.Fprintf(to, "%s\n", " "+text)
279+
}
213280
}
214281
}
215282

0 commit comments

Comments
 (0)