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

Author: fwiesel

cmd/release.go

@@ -18,6 +18,7 @@ type release struct {
 	suppressedKinds    []string
 	releases           []string
 	outputContext      int
+	maxChangeFraction  float32
 	includeTests       bool
 	showSecrets        bool
 	output             string
@@ -79,6 +80,7 @@ func releaseCmd() *cobra.Command {
 	releaseCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
 	releaseCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
 	releaseCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
+	releaseCmd.Flags().Float32VarP(&diff.maxChangeFraction, "max-content-change-fraction", "D", 0, "maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a moved resource")
 	releaseCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
 	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.")
 	releaseCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -124,6 +126,7 @@ func (d *release) differentiateHelm3() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)
@@ -159,6 +162,7 @@ func (d *release) differentiate() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)

cmd/revision.go

@@ -20,6 +20,7 @@ type revision struct {
 	suppressedKinds    []string
 	revisions          []string
 	outputContext      int
+	maxChangeFraction  float32
 	includeTests       bool
 	showSecrets        bool
 	output             string
@@ -89,6 +90,7 @@ func revisionCmd() *cobra.Command {
 	revisionCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
 	revisionCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
 	revisionCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
+	revisionCmd.Flags().Float32VarP(&diff.maxChangeFraction, "max-content-change-fraction", "D", 0, "maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a moved resource")
 	revisionCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
 	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.")
 	revisionCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -129,6 +131,7 @@ func (d *revision) differentiateHelm3() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)
@@ -156,6 +159,7 @@ func (d *revision) differentiateHelm3() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)
@@ -196,6 +200,7 @@ func (d *revision) differentiate() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)
@@ -223,6 +228,7 @@ func (d *revision) differentiate() error {
 			d.suppressedKinds,
 			d.showSecrets,
 			d.outputContext,
+			d.maxChangeFraction,
 			d.output,
 			d.stripTrailingCR,
 			os.Stdout)

cmd/rollback.go

@@ -20,6 +20,7 @@ type rollback struct {
 	suppressedKinds    []string
 	revisions          []string
 	outputContext      int
+	maxChangeFraction  float32
 	includeTests       bool
 	showSecrets        bool
 	output             string
@@ -81,6 +82,7 @@ func rollbackCmd() *cobra.Command {
 	rollbackCmd.Flags().BoolVar(&diff.detailedExitCode, "detailed-exitcode", false, "return a non-zero exit code when there are changes")
 	rollbackCmd.Flags().StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
 	rollbackCmd.Flags().IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
+	rollbackCmd.Flags().Float32VarP(&diff.maxChangeFraction, "max-content-change-fraction", "D", 0, "maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a moved resource")
 	rollbackCmd.Flags().BoolVar(&diff.includeTests, "include-tests", false, "enable the diffing of the helm test hooks")
 	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.")
 	rollbackCmd.Flags().BoolVar(&diff.stripTrailingCR, "strip-trailing-cr", false, "strip trailing carriage return on input")
@@ -122,6 +124,7 @@ func (d *rollback) backcastHelm3() error {
 		d.suppressedKinds,
 		d.showSecrets,
 		d.outputContext,
+		d.maxChangeFraction,
 		d.output,
 		d.stripTrailingCR,
 		os.Stdout)
@@ -159,6 +162,7 @@ func (d *rollback) backcast() error {
 		d.suppressedKinds,
 		d.showSecrets,
 		d.outputContext,
+		d.maxChangeFraction,
 		d.output,
 		d.stripTrailingCR,
 		os.Stdout)

cmd/upgrade.go

@@ -36,6 +36,7 @@ type diffCmd struct {
 	includeTests             bool
 	suppressedKinds          []string
 	outputContext            int
+	maxChangeFraction        float32
 	showSecrets              bool
 	postRenderer             string
 	output                   string
@@ -116,6 +117,7 @@ func newChartCommand() *cobra.Command {
 	f.BoolVar(&diff.devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.")
 	f.StringArrayVar(&diff.suppressedKinds, "suppress", []string{}, "allows suppression of the values listed in the diff output")
 	f.IntVarP(&diff.outputContext, "context", "C", -1, "output NUM lines of context around changes")
+	f.Float32VarP(&diff.maxChangeFraction, "max-content-change-fraction", "D", 0, "maximum fraction of changed content as lines added + removed compared to total lines in a diff for considering it a moved resource")
 	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")
 	f.BoolVar(&diff.disableOpenAPIValidation, "disable-openapi-validation", false, "disables rendered templates validation against the Kubernetes OpenAPI Schema")
 	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 {
 	} else {
 		newSpecs = manifest.Parse(string(installManifest), d.namespace, d.normalizeManifests, helm3TestHook, helm2TestSuccessHook)
 	}
-	seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout)
+	seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.maxChangeFraction, d.output, d.stripTrailingCR, os.Stdout)
 
 	if d.detailedExitCode && seenAnyChanges {
 		return Error{
@@ -276,7 +278,7 @@ func (d *diffCmd) run() error {
 		}
 	}
 
-	seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.output, d.stripTrailingCR, os.Stdout)
+	seenAnyChanges := diff.Manifests(currentSpecs, newSpecs, d.suppressedKinds, d.showSecrets, d.outputContext, d.maxChangeFraction, d.output, d.stripTrailingCR, os.Stdout)
 
 	if d.detailedExitCode && seenAnyChanges {
 		return Error{

diff/diff.go

@@ -19,58 +19,135 @@ import (
 )
 
 // Manifests diff on manifests
-func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool {
+func Manifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, maxChangeFraction float32, output string, stripTrailingCR bool, to io.Writer) bool {
 	report.setupReportFormat(output)
 	seenAnyChanges := false
-	emptyMapping := &manifest.MappingResult{}
+	var possiblyRemoved []string
+
 	for _, key := range sortedKeys(oldIndex) {
 		oldContent := oldIndex[key]
 
 		if newContent, ok := newIndex[key]; ok {
-			if oldContent.Content != newContent.Content {
-				// modified
-				if !showSecrets {
-					redactSecrets(oldContent, newContent)
-				}
-
-				diffs := diffMappingResults(oldContent, newContent, stripTrailingCR)
-				if len(diffs) > 0 {
-					seenAnyChanges = true
-				}
-				report.addEntry(key, suppressedKinds, oldContent.Kind, context, diffs, "MODIFY")
-			}
-		} else {
-			// removed
-			if !showSecrets {
-				redactSecrets(oldContent, nil)
-
-			}
-			diffs := diffMappingResults(oldContent, emptyMapping, stripTrailingCR)
-			if len(diffs) > 0 {
+			// modified
+			if doDiff(key, oldContent, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
 				seenAnyChanges = true
 			}
-			report.addEntry(key, suppressedKinds, oldContent.Kind, context, diffs, "REMOVE")
+		} else {
+			possiblyRemoved = append(possiblyRemoved, key)
 		}
 	}
 
+	var possiblyAdded []string
 	for _, key := range sortedKeys(newIndex) {
+		if _, ok := oldIndex[key]; !ok {
+			possiblyAdded = append(possiblyAdded, key)
+		}
+	}
+
+	removed, added, anyChanges := contentSearch(possiblyRemoved, oldIndex, possiblyAdded, newIndex, showSecrets, stripTrailingCR, suppressedKinds, context, maxChangeFraction)
+	if anyChanges {
+		seenAnyChanges = true
+	}
+
+	for _, key := range removed {
+		oldContent := oldIndex[key]
+
+		if doDiff(key, oldContent, nil, suppressedKinds, showSecrets, stripTrailingCR, context) {
+			seenAnyChanges = true
+		}
+	}
+
+	for _, key := range added {
 		newContent := newIndex[key]
 
-		if _, ok := oldIndex[key]; !ok {
-			// added
+		if doDiff(key, nil, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
+			seenAnyChanges = true
+		}
+	}
+
+	report.print(to)
+	report.clean()
+	return seenAnyChanges
+}
+
+func actualChanges(diff []difflib.DiffRecord) int {
+	changes := 0
+	for _, record := range diff {
+		if record.Delta != difflib.Common {
+			changes++
+		}
+	}
+	return changes
+}
+
+func contentSearch(possiblyRemoved []string, oldIndex map[string]*manifest.MappingResult, possiblyAdded []string, newIndex map[string]*manifest.MappingResult, showSecrets bool, stripTrailingCR bool, suppressedKinds []string, context int, maxChangeFraction float32) ([]string, []string, bool) {
+	if maxChangeFraction <= 0 {
+		return possiblyRemoved, possiblyAdded, false
+	}
+
+	var removed []string
+	seenAnyChanges := false
+
+	for _, removedKey := range possiblyRemoved {
+		oldContent := oldIndex[removedKey]
+		var smallestKey string
+		var smallestFraction float32 = maxChangeFraction
+		for _, addedKey := range possiblyAdded {
+			newContent := newIndex[addedKey]
 			if !showSecrets {
-				redactSecrets(nil, newContent)
+				redactSecrets(oldContent, newContent)
+			}
+
+			diff := diffMappingResults(oldContent, newContent, stripTrailingCR)
+			delta := actualChanges(diff)
+			if delta == 0 || len(diff) == 0 {
+				continue // Should never happen, but better safe than sorry
+			}
+			fraction := float32(delta) / float32(len(diff))
+			if fraction > 0 && fraction < smallestFraction {
+				smallestKey = addedKey
+				smallestFraction = fraction
 			}
-			diffs := diffMappingResults(emptyMapping, newContent, stripTrailingCR)
-			if len(diffs) > 0 {
+		}
+
+		if smallestFraction < maxChangeFraction {
+			index := sort.SearchStrings(possiblyAdded, smallestKey)
+			possiblyAdded = append(possiblyAdded[:index], possiblyAdded[index+1:]...)
+			newContent := newIndex[smallestKey]
+			if doDiff(removedKey, oldContent, newContent, suppressedKinds, showSecrets, stripTrailingCR, context) {
 				seenAnyChanges = true
 			}
-			report.addEntry(key, suppressedKinds, newContent.Kind, context, diffs, "ADD")
+
+		} else {
+			removed = append(removed, removedKey)
 		}
 	}
-	report.print(to)
-	report.clean()
-	return seenAnyChanges
+
+	return removed, possiblyAdded, seenAnyChanges
+}
+
+func doDiff(key string, oldContent *manifest.MappingResult, newContent *manifest.MappingResult, suppressedKinds []string, showSecrets bool, stripTrailingCR bool, context int) bool {
+	if !showSecrets {
+		redactSecrets(oldContent, newContent)
+	}
+
+	diffs := diffMappingResults(oldContent, newContent, stripTrailingCR)
+	var kind string
+	var changeType string
+	if oldContent == nil {
+		kind = newContent.Kind
+		changeType = "ADD"
+	} else if newContent == nil {
+		kind = oldContent.Kind
+		changeType = "REMOVE"
+	} else {
+		if oldContent.Content == newContent.Content {
+			return false
+		}
+		changeType = "MODIFY"
+	}
+	report.addEntry(key, suppressedKinds, kind, context, diffs, changeType)
+	return len(diffs) > 0
 }
 
 func redactSecrets(old, new *manifest.MappingResult) {
@@ -116,15 +193,15 @@ func redactSecrets(old, new *manifest.MappingResult) {
 	if old != nil {
 		oldSecret.Data = nil
 		if err := serializer.Encode(&oldSecret, &buf); err != nil {
-
+			new.Content = fmt.Sprintf("Error encoding new secret: %s", err)
 		}
 		old.Content = getComment(old.Content) + strings.Replace(strings.Replace(buf.String(), "stringData", "data", 1), "  creationTimestamp: null\n", "", 1)
 		buf.Reset() //reuse buffer for new secret
 	}
 	if new != nil {
 		newSecret.Data = nil
 		if err := serializer.Encode(&newSecret, &buf); err != nil {
-
+			new.Content = fmt.Sprintf("Error encoding new secret: %s", err)
 		}
 		new.Content = getComment(new.Content) + strings.Replace(strings.Replace(buf.String(), "stringData", "data", 1), "  creationTimestamp: null\n", "", 1)
 	}
@@ -142,14 +219,22 @@ func getComment(s string) string {
 }
 
 // Releases reindex the content  based on the template names and pass it to Manifests
-func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, output string, stripTrailingCR bool, to io.Writer) bool {
+func Releases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, showSecrets bool, context int, maxChangeFraction float32, output string, stripTrailingCR bool, to io.Writer) bool {
 	oldIndex = reIndexForRelease(oldIndex)
 	newIndex = reIndexForRelease(newIndex)
-	return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, output, stripTrailingCR, to)
+	return Manifests(oldIndex, newIndex, suppressedKinds, showSecrets, context, maxChangeFraction, output, stripTrailingCR, to)
+}
+
+func getContentOrEmptyString(mappingResult *manifest.MappingResult) string {
+	if mappingResult == nil {
+		return ""
+	}
+
+	return mappingResult.Content
 }
 
-func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult, stripTrailingCR bool ) []difflib.DiffRecord {
-	return diffStrings(oldContent.Content, newContent.Content, stripTrailingCR)
+func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult, stripTrailingCR bool) []difflib.DiffRecord {
+	return diffStrings(getContentOrEmptyString(oldContent), getContentOrEmptyString(newContent), stripTrailingCR)
 }
 
 func diffStrings(before, after string, stripTrailingCR bool) []difflib.DiffRecord {
@@ -209,7 +294,11 @@ func printDiffRecord(diff difflib.DiffRecord, to io.Writer) {
 	case difflib.LeftOnly:
 		fmt.Fprintf(to, "%s\n", ansi.Color("- "+text, "red"))
 	case difflib.Common:
-		fmt.Fprintf(to, "%s\n", "  "+text)
+		if text == "\n" || text == "" {
+			fmt.Fprintln(to)
+		} else {
+			fmt.Fprintf(to, "%s\n", "  "+text)
+		}
 	}
 }