From 58e7d16fb75b23b26380bda56efddb0c5be1c36d Mon Sep 17 00:00:00 2001 From: Kharun Samuel Date: Wed, 14 Aug 2019 18:12:10 +0530 Subject: [PATCH 1/2] FlattenListVisitor now continues traversal on errors and returns an aggregate error --- .../cli-runtime/pkg/resource/visitor.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go index 99a2f1f7537..9bd842ceca6 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go @@ -372,10 +372,10 @@ func (v ContinueOnErrorVisitor) Visit(fn VisitorFunc) error { // FlattenListVisitor flattens any objects that runtime.ExtractList recognizes as a list // - has an "Items" public field that is a slice of runtime.Objects or objects satisfying -// that interface - into multiple Infos. An error on any sub item (for instance, if a List -// contains an object that does not have a registered client or resource) will terminate -// the visit. -// TODO: allow errors to be aggregated? +// that interface - into multiple Infos. Returns nil in the case of no errors. +// When an error is hit on sub items (for instance, if a List contains an object that does +// not have a registered client or resource), returns an aggregate error. + type FlattenListVisitor struct { visitor Visitor typer runtime.ObjectTyper @@ -425,20 +425,23 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error { if info.Mapping != nil && !info.Mapping.GroupVersionKind.Empty() { preferredGVKs = append(preferredGVKs, info.Mapping.GroupVersionKind) } - + errs := []error{} for i := range items { item, err := v.mapper.infoForObject(items[i], v.typer, preferredGVKs) + if err != nil { - return err + errs = append(errs, err) + continue } if len(info.ResourceVersion) != 0 { item.ResourceVersion = info.ResourceVersion } if err := fn(item, nil); err != nil { - return err + errs = append(errs, err) } } - return nil + return utilerrors.NewAggregate(errs) + }) } From 05c45b5cfc878bdf05deb7dd2d3f9fd618025d47 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Thu, 29 Aug 2019 12:21:01 +0200 Subject: [PATCH 2/2] Adds visitor test for traversal errors --- .../k8s.io/cli-runtime/pkg/resource/visitor.go | 2 -- .../cli-runtime/pkg/resource/visitor_test.go | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go index 9bd842ceca6..946016c1349 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor.go @@ -375,7 +375,6 @@ func (v ContinueOnErrorVisitor) Visit(fn VisitorFunc) error { // that interface - into multiple Infos. Returns nil in the case of no errors. // When an error is hit on sub items (for instance, if a List contains an object that does // not have a registered client or resource), returns an aggregate error. - type FlattenListVisitor struct { visitor Visitor typer runtime.ObjectTyper @@ -428,7 +427,6 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error { errs := []error{} for i := range items { item, err := v.mapper.infoForObject(items[i], v.typer, preferredGVKs) - if err != nil { errs = append(errs, err) continue diff --git a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go index 9868ad9d6b4..414d460f9b4 100644 --- a/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go +++ b/staging/src/k8s.io/cli-runtime/pkg/resource/visitor_test.go @@ -18,9 +18,11 @@ package resource import ( "bytes" + "errors" "fmt" "io" "io/ioutil" + "strings" "testing" "time" @@ -180,3 +182,18 @@ func TestFlattenListVisitor(t *testing.T) { t.Fatal(spew.Sdump(test.Infos)) } } + +func TestFlattenListVisitorWithVisitorError(t *testing.T) { + b := newDefaultBuilder(). + FilenameParam(false, &FilenameOptions{Recursive: false, Filenames: []string{"../../artifacts/deeply-nested.yaml"}}). + Flatten() + + test := &testVisitor{InjectErr: errors.New("visitor error")} + err := b.Do().Visit(test.Handle) + if err == nil || !strings.Contains(err.Error(), "visitor error") { + t.Fatal(err) + } + if len(test.Infos) != 6 { + t.Fatal(spew.Sdump(test.Infos)) + } +}