Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce ResultV2 for update results #642

Merged
merged 1 commit into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions pkg/update/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
77 changes: 77 additions & 0 deletions pkg/update/result_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
}))
}
35 changes: 29 additions & 6 deletions pkg/update/setters.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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)
}
}
Expand Down
44 changes: 38 additions & 6 deletions pkg/update/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package update

import (
"io/ioutil"
"os"
"testing"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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))
}
Loading