Merge pull request #642 from fluxcd/resultv2

Introduce ResultV2 for update results

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
+}

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",
+			},
+		},
+	}))
+}

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)
 					}
 				}

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))
 }