Introduce ResultV2 for update results

darkowlzz · darkowlzz · commit 1c4db83b0849 · 2024-03-15T21:05:50.000+05:30
ResultV2 update result contains Result (original), which provides
information in terms of files, objects and images, and also includes
file changes which provides information in terms of files, objects and
changes. The changes contain the old value, new value and the setter
that was involved in the update.
ResultV2 can be used to obtain detailed information about updates in
terms of the previous value and the new value, which could be beneficial
in commit messages to summarize the granular changes.

Signed-off-by: Sunny &lt;github@darkowlzz.space&gt;
diff --git a/pkg/update/result.go b/pkg/update/result.go
@@ -96,3 +96,66 @@ func (r Result) Objects() map[ObjectIdentifier][]ImageRef {
 	}
 	return result
 }
+
+// ResultV2 contains Result of update and also the file changes made during the
+// update. This extends the Result to include details about the exact changes
+// made to the files and the objects in them. It has a nested structure
+// file->objects->changes.
+type ResultV2 struct {
+	ImageResult Result
+	FileChanges map[string]ObjectChanges
+}
+
+// ObjectChanges contains all the changes made to objects.
+type ObjectChanges map[ObjectIdentifier][]Change
+
+// Change contains the setter that resulted in a Change, the old and the new
+// value after the Change.
+type Change struct {
+	OldValue string
+	NewValue string
+	Setter   string
+}
+
+// AddChange adds changes to Resultv2 for a given file, object and changes
+// associated with it.
+func (r *ResultV2) AddChange(file string, objectID ObjectIdentifier, changes ...Change) {
+	if r.FileChanges == nil {
+		r.FileChanges = map[string]ObjectChanges{}
+	}
+	// Create an entry for the file if not present.
+	_, ok := r.FileChanges[file]
+	if !ok {
+		r.FileChanges[file] = ObjectChanges{}
+	}
+	// Append to the changes for the object.
+	r.FileChanges[file][objectID] = append(r.FileChanges[file][objectID], changes...)
+}
+
+// Changes returns all the changes that were made in at least one update.
+func (r ResultV2) Changes() []Change {
+	seen := make(map[Change]struct{})
+	var result []Change
+	for _, objChanges := range r.FileChanges {
+		for _, changes := range objChanges {
+			for _, change := range changes {
+				if _, ok := seen[change]; !ok {
+					seen[change] = struct{}{}
+					result = append(result, change)
+				}
+			}
+		}
+	}
+	return result
+}
+
+// Objects returns ObjectChanges, regardless of which file they appear in.
+func (r ResultV2) Objects() ObjectChanges {
+	result := make(ObjectChanges)
+	for _, objChanges := range r.FileChanges {
+		for obj, change := range objChanges {
+			result[obj] = change
+		}
+	}
+	return result
+}
diff --git a/pkg/update/result_test.go b/pkg/update/result_test.go
@@ -92,3 +92,80 @@ func TestUpdateResults(t *testing.T) {
 		},
 	}))
 }
+
+func TestResultV2(t *testing.T) {
+	g := NewWithT(t)
+
+	var result ResultV2
+	objectNames := []ObjectIdentifier{
+		{yaml.ResourceIdentifier{
+			NameMeta: yaml.NameMeta{Namespace: "ns", Name: "foo"},
+		}},
+		{yaml.ResourceIdentifier{
+			NameMeta: yaml.NameMeta{Namespace: "ns", Name: "bar"},
+		}},
+	}
+
+	result.AddChange("foo.yaml", objectNames[0], Change{
+		OldValue: "aaa",
+		NewValue: "bbb",
+		Setter:   "foo-ns:policy:name",
+	})
+	result.AddChange("bar.yaml", objectNames[1], Change{
+		OldValue: "cccc:v1.0",
+		NewValue: "cccc:v1.2",
+		Setter:   "foo-ns:policy",
+	})
+
+	result = ResultV2{
+		FileChanges: map[string]ObjectChanges{
+			"foo.yaml": {
+				objectNames[0]: []Change{
+					{
+						OldValue: "aaa",
+						NewValue: "bbb",
+						Setter:   "foo-ns:policy:name",
+					},
+				},
+			},
+			"bar.yaml": {
+				objectNames[1]: []Change{
+					{
+						OldValue: "cccc:v1.0",
+						NewValue: "cccc:v1.2",
+						Setter:   "foo-ns:policy",
+					},
+				},
+			},
+		},
+	}
+
+	g.Expect(result.Changes()).To(ContainElements([]Change{
+		{
+			OldValue: "aaa",
+			NewValue: "bbb",
+			Setter:   "foo-ns:policy:name",
+		},
+		{
+			OldValue: "cccc:v1.0",
+			NewValue: "cccc:v1.2",
+			Setter:   "foo-ns:policy",
+		},
+	}))
+	g.Expect(result.Objects()).To(Equal(ObjectChanges{
+		objectNames[0]: []Change{
+			{
+				OldValue: "aaa",
+				NewValue: "bbb",
+				Setter:   "foo-ns:policy:name",
+			},
+		},
+		objectNames[1]: []Change{
+			{
+				OldValue: "cccc:v1.0",
+				NewValue: "cccc:v1.2",
+				Setter:   "foo-ns:policy",
+			},
+		},
+	}))
+}
diff --git a/pkg/update/setters.go b/pkg/update/setters.go
@@ -56,6 +56,15 @@ func init() {
 // that contain an "in scope" image policy marker, and writes files it
 // updated (and only those files) back to `outpath`.
 func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []imagev1_reflect.ImagePolicy) (Result, error) {
+	result, err := UpdateV2WithSetters(tracelog, inpath, outpath, policies)
+	return result.ImageResult, err
+}
+
+// UpdateV2WithSetters takes all YAML files from `inpath`, updates any
+// that contain an "in scope" image policy marker, and writes files it
+// updated (and only those files) back to `outpath`. It also returns the result
+// of the changes it made as ResultV2.
+func UpdateV2WithSetters(tracelog logr.Logger, inpath, outpath string, policies []imagev1_reflect.ImagePolicy) (ResultV2, error) {
 	// the OpenAPI schema is a package variable in kyaml/openapi. In
 	// lieu of being able to isolate invocations (per
 	// https://github.com/kubernetes-sigs/kustomize/issues/3058), I
@@ -99,13 +108,15 @@ func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []
 		Files: make(map[string]FileResult),
 	}
 
+	var resultV2 ResultV2
+
 	// Compilng the result needs the file, the image ref used, and the
 	// object. Each setter will supply its own name to its callback,
 	// which can be used to look up the image ref; the file and object
 	// we will get from `setAll` which keeps track of those as it
 	// iterates.
 	imageRefs := make(map[string]imageRef)
-	setAllCallback := func(file, setterName string, node *yaml.RNode) {
+	setAllCallback := func(file, setterName string, node *yaml.RNode, old, new string) {
 		ref, ok := imageRefs[setterName]
 		if !ok {
 			return
@@ -117,6 +128,15 @@ func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []
 		}
 		oid := ObjectIdentifier{meta.GetIdentifier()}
 
+		// Record the change.
+		ch := Change{
+			OldValue: old,
+			NewValue: new,
+			Setter:   setterName,
+		}
+		// Append the change for the file and identifier.
+		resultV2.AddChange(file, oid, ch)
+
 		fileres, ok := result.Files[file]
 		if !ok {
 			fileres = FileResult{
@@ -148,7 +168,7 @@ func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []
 		image := policy.Status.LatestImage
 		r, err := name.ParseReference(image, name.WeakValidation)
 		if err != nil {
-			return Result{}, fmt.Errorf("encountered invalid image ref %q: %w", policy.Status.LatestImage, err)
+			return ResultV2{}, fmt.Errorf("encountered invalid image ref %q: %w", policy.Status.LatestImage, err)
 		}
 		ref := imageRef{
 			Reference: r,
@@ -204,9 +224,12 @@ func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []
 	// go!
 	err := pipeline.Execute()
 	if err != nil {
-		return Result{}, err
+		return ResultV2{}, err
 	}
-	return result, nil
+
+	// Combine the results.
+	resultV2.ImageResult = result
+	return resultV2, nil
 }
 
 // setAll returns a kio.Filter using the supplied SetAllCallback
@@ -215,7 +238,7 @@ func UpdateWithSetters(tracelog logr.Logger, inpath, outpath string, policies []
 // files with changed nodes. This is based on
 // [`SetAll`](https://github.com/kubernetes-sigs/kustomize/blob/kyaml/v0.10.16/kyaml/setters2/set.go#L503
 // from kyaml/kio.
-func setAll(schema *spec.Schema, tracelog logr.Logger, callback func(file, setterName string, node *yaml.RNode)) kio.Filter {
+func setAll(schema *spec.Schema, tracelog logr.Logger, callback func(file, setterName string, node *yaml.RNode, old, new string)) kio.Filter {
 	filter := &SetAllCallback{
 		SettersSchema: schema,
 		Trace:         tracelog,
@@ -231,7 +254,7 @@ func setAll(schema *spec.Schema, tracelog logr.Logger, callback func(file, sette
 
 				filter.Callback = func(setter, oldValue, newValue string) {
 					if newValue != oldValue {
-						callback(path, setter, nodes[i])
+						callback(path, setter, nodes[i], oldValue, newValue)
 						filesToUpdate.Insert(path)
 					}
 				}
diff --git a/pkg/update/update_test.go b/pkg/update/update_test.go
@@ -17,8 +17,6 @@ limitations under the License.
 package update
 
 import (
-	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/go-logr/logr"
@@ -56,10 +54,7 @@ func TestUpdateWithSetters(t *testing.T) {
 		},
 	}
 
-	tmp, err := ioutil.TempDir("", "gotest")
-	g.Expect(err).ToNot(HaveOccurred())
-	defer os.RemoveAll(tmp)
-
+	tmp := t.TempDir()
 	result, err := UpdateWithSetters(logr.Discard(), "testdata/setters/original", tmp, policies)
 	g.Expect(err).ToNot(HaveOccurred())
 	test.ExpectMatchingDirectories(g, tmp, "testdata/setters/expected")
@@ -106,4 +101,41 @@ func TestUpdateWithSetters(t *testing.T) {
 	}
 
 	g.Expect(result).To(Equal(expectedResult))
+
+	// Test ResultV2.
+	tmp2 := t.TempDir()
+	resultV2, err := UpdateV2WithSetters(logr.Discard(), "testdata/setters/original", tmp2, policies)
+	g.Expect(err).ToNot(HaveOccurred())
+	test.ExpectMatchingDirectories(g, tmp2, "testdata/setters/expected")
+
+	expectedResultV2 := ResultV2{
+		ImageResult: expectedResult,
+		FileChanges: map[string]ObjectChanges{
+			"kustomization.yaml": {
+				kustomizeResourceID: []Change{
+					{
+						OldValue: "replaced",
+						NewValue: "index.repo.fake/updated",
+						Setter:   "automation-ns:policy:name",
+					},
+					{
+						OldValue: "v1",
+						NewValue: "v1.0.1",
+						Setter:   "automation-ns:policy:tag",
+					},
+				},
+			},
+			"marked.yaml": {
+				markedResourceID: []Change{
+					{
+						OldValue: "image:v1.0.0",
+						NewValue: "index.repo.fake/updated:v1.0.1",
+						Setter:   "automation-ns:policy",
+					},
+				},
+			},
+		},
+	}
+
+	g.Expect(resultV2).To(Equal(expectedResultV2))
 }
