From 907ae6c5b05ca50d607447a19355aa9a87f38bdd Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Fri, 1 Sep 2017 14:39:04 -0700 Subject: [PATCH] move groupresource resolution into discovery helper Signed-off-by: Steve Kriss --- pkg/backup/backup.go | 62 +++++++---------------- pkg/backup/backup_test.go | 59 +++++++++------------ pkg/discovery/helper.go | 12 +++++ pkg/util/collections/includes_excludes.go | 33 ++++++++++++ pkg/util/test/fake_discovery_helper.go | 31 ++++++++++++ 5 files changed, 118 insertions(+), 79 deletions(-) create mode 100644 pkg/util/test/fake_discovery_helper.go diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index 23b0a96fd..e1fdfcf9f 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -76,7 +76,7 @@ func NewKubernetesBackupper( dynamicFactory client.DynamicFactory, actions map[string]Action, ) (Backupper, error) { - resolvedActions, err := resolveActions(discoveryHelper.Mapper(), actions) + resolvedActions, err := resolveActions(discoveryHelper, actions) if err != nil { return nil, err } @@ -91,11 +91,11 @@ func NewKubernetesBackupper( // resolveActions resolves the string-based map of group-resources to actions and returns a map of // schema.GroupResources to actions. -func resolveActions(mapper meta.RESTMapper, actions map[string]Action) (map[schema.GroupResource]Action, error) { +func resolveActions(helper discovery.Helper, actions map[string]Action) (map[schema.GroupResource]Action, error) { ret := make(map[schema.GroupResource]Action) for resource, action := range actions { - gr, err := resolveGroupResource(mapper, resource) + gr, err := helper.ResolveGroupResource(resource) if err != nil { return nil, err } @@ -105,46 +105,22 @@ func resolveActions(mapper meta.RESTMapper, actions map[string]Action) (map[sche return ret, nil } -// resolveResources uses the RESTMapper to resolve resources to their fully-qualified group-resource -// names. fn is invoked for each resolved resource. resolveResources returns a list of any resources that failed to resolve. -func (ctx *backupContext) resolveResources(mapper meta.RESTMapper, resources []string, allowAll bool, fn func(string)) { - for _, resource := range resources { - if allowAll && resource == "*" { - fn("*") - return - } - gr, err := resolveGroupResource(mapper, resource) - if err != nil { - ctx.log("Unable to resolve resource %q: %v", resource, err) - continue - } - fn(gr.String()) - } -} +// getResourceIncludesExcludes takes the lists of resources to include and exclude from the +// backup, uses the discovery helper to resolve them to fully-qualified group-resource names, and returns +// an IncludesExcludes list. +func (ctx *backupContext) getResourceIncludesExcludes(helper discovery.Helper, includes, excludes []string) *collections.IncludesExcludes { + return collections.GenerateIncludesExcludes( + includes, + excludes, + func(item string) (string, error) { + gr, err := helper.ResolveGroupResource(item) + if err != nil { + return "", err + } -// getResourceIncludesExcludes takes the lists of resources to include and exclude, uses the -// RESTMapper to resolve them to fully-qualified group-resource names, and returns an -// IncludesExcludes list. -func (ctx *backupContext) getResourceIncludesExcludes(mapper meta.RESTMapper, includes, excludes []string) *collections.IncludesExcludes { - resources := collections.NewIncludesExcludes() - - ctx.resolveResources(mapper, includes, true, func(s string) { resources.Includes(s) }) - ctx.resolveResources(mapper, excludes, false, func(s string) { resources.Excludes(s) }) - - ctx.log("Including resources: %v", strings.Join(resources.GetIncludes(), ", ")) - ctx.log("Excluding resources: %v", strings.Join(resources.GetExcludes(), ", ")) - - return resources -} - -// resolveGroupResource uses the RESTMapper to resolve resource to a fully-qualified -// schema.GroupResource. If the RESTMapper is unable to do so, an error is returned instead. -func resolveGroupResource(mapper meta.RESTMapper, resource string) (schema.GroupResource, error) { - gvr, err := mapper.ResourceFor(schema.ParseGroupResource(resource).WithVersion("")) - if err != nil { - return schema.GroupResource{}, err - } - return gvr.GroupResource(), nil + return gr.String(), nil + }, + ) } // getNamespaceIncludesExcludes returns an IncludesExcludes list containing which namespaces to @@ -206,7 +182,7 @@ func (kb *kubernetesBackupper) Backup(backup *api.Backup, backupFile, logFile io ctx.log("Starting backup") - ctx.resourceIncludesExcludes = ctx.getResourceIncludesExcludes(kb.discoveryHelper.Mapper(), backup.Spec.IncludedResources, backup.Spec.ExcludedResources) + ctx.resourceIncludesExcludes = ctx.getResourceIncludesExcludes(kb.discoveryHelper, backup.Spec.IncludedResources, backup.Spec.ExcludedResources) for _, group := range kb.discoveryHelper.Resources() { ctx.log("Processing group %s", group.GroupVersion) diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index efe9cbd13..890563493 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -104,16 +104,18 @@ func TestResolveActions(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - mapper := &FakeMapper{ - Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ - schema.GroupVersionResource{Resource: "foo"}: schema.GroupVersionResource{Group: "somegroup", Resource: "foodies"}, - schema.GroupVersionResource{Resource: "fie"}: schema.GroupVersionResource{Group: "somegroup", Resource: "fields"}, - schema.GroupVersionResource{Resource: "bar"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "barnacles"}, - schema.GroupVersionResource{Resource: "baz"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "bazaars"}, + dh := &FakeDiscoveryHelper{ + RESTMapper: &FakeMapper{ + Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ + schema.GroupVersionResource{Resource: "foo"}: schema.GroupVersionResource{Group: "somegroup", Resource: "foodies"}, + schema.GroupVersionResource{Resource: "fie"}: schema.GroupVersionResource{Group: "somegroup", Resource: "fields"}, + schema.GroupVersionResource{Resource: "bar"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "barnacles"}, + schema.GroupVersionResource{Resource: "baz"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "bazaars"}, + }, }, } - actual, err := resolveActions(mapper, test.input) + actual, err := resolveActions(dh, test.input) gotError := err != nil if e, a := test.expectError, gotError; e != a { @@ -176,12 +178,14 @@ func TestGetResourceIncludesExcludes(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - mapper := &FakeMapper{ - Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ - schema.GroupVersionResource{Resource: "foo"}: schema.GroupVersionResource{Group: "somegroup", Resource: "foodies"}, - schema.GroupVersionResource{Resource: "fie"}: schema.GroupVersionResource{Group: "somegroup", Resource: "fields"}, - schema.GroupVersionResource{Resource: "bar"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "barnacles"}, - schema.GroupVersionResource{Resource: "baz"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "bazaars"}, + dh := &FakeDiscoveryHelper{ + RESTMapper: &FakeMapper{ + Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ + schema.GroupVersionResource{Resource: "foo"}: schema.GroupVersionResource{Group: "somegroup", Resource: "foodies"}, + schema.GroupVersionResource{Resource: "fie"}: schema.GroupVersionResource{Group: "somegroup", Resource: "fields"}, + schema.GroupVersionResource{Resource: "bar"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "barnacles"}, + schema.GroupVersionResource{Resource: "baz"}: schema.GroupVersionResource{Group: "anothergroup", Resource: "bazaars"}, + }, }, } @@ -190,7 +194,7 @@ func TestGetResourceIncludesExcludes(t *testing.T) { logger: &logger{w: b}, } - actual := ctx.getResourceIncludesExcludes(mapper, test.includes, test.excludes) + actual := ctx.getResourceIncludesExcludes(dh, test.includes, test.excludes) sort.Strings(test.expectedIncludes) actualIncludes := actual.GetIncludes() @@ -236,23 +240,6 @@ func TestGetNamespaceIncludesExcludes(t *testing.T) { } } -type fakeDiscoveryHelper struct { - resources []*metav1.APIResourceList - mapper meta.RESTMapper -} - -func (dh *fakeDiscoveryHelper) Resources() []*metav1.APIResourceList { - return dh.resources -} - -func (dh *fakeDiscoveryHelper) Mapper() meta.RESTMapper { - return dh.mapper -} - -func (dh *fakeDiscoveryHelper) Refresh() error { - return nil -} - func TestBackupMethod(t *testing.T) { // TODO ensure LabelSelector is passed through to the List() calls backup := &v1.Backup{ @@ -303,15 +290,15 @@ func TestBackupMethod(t *testing.T) { ShortNames: []string{"csr"}, } - discoveryHelper := &fakeDiscoveryHelper{ - mapper: &FakeMapper{ + discoveryHelper := &FakeDiscoveryHelper{ + RESTMapper: &FakeMapper{ Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ schema.GroupVersionResource{Resource: "cm"}: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "configmaps"}, schema.GroupVersionResource{Resource: "csr"}: schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}, schema.GroupVersionResource{Resource: "roles"}: schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1beta1", Resource: "roles"}, }, }, - resources: []*metav1.APIResourceList{ + ResourceList: []*metav1.APIResourceList{ { GroupVersion: "v1", APIResources: []metav1.APIResource{configMapsResource, podsResource}, @@ -829,8 +816,8 @@ func TestBackupResource(t *testing.T) { } } - discoveryHelper := &fakeDiscoveryHelper{ - mapper: &FakeMapper{ + discoveryHelper := &FakeDiscoveryHelper{ + RESTMapper: &FakeMapper{ Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ schema.GroupVersionResource{Resource: "certificatesigningrequests"}: schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}, schema.GroupVersionResource{Resource: "other"}: schema.GroupVersionResource{Group: "somegroup", Version: "someversion", Resource: "otherthings"}, diff --git a/pkg/discovery/helper.go b/pkg/discovery/helper.go index 2a22af179..8b7877a42 100644 --- a/pkg/discovery/helper.go +++ b/pkg/discovery/helper.go @@ -44,6 +44,10 @@ type Helper interface { // Refresh pulls an updated set of Ark-backuppable resources from the // discovery API. Refresh() error + + // ResolveGroupResource uses the RESTMapper to resolve resource to a fully-qualified + // schema.GroupResource. If the RESTMapper is unable to do so, an error is returned instead. + ResolveGroupResource(resource string) (schema.GroupResource, error) } type helper struct { @@ -139,3 +143,11 @@ func (h *helper) Resources() []*metav1.APIResourceList { defer h.lock.RUnlock() return h.resources } + +func (h *helper) ResolveGroupResource(resource string) (schema.GroupResource, error) { + gvr, err := h.mapper.ResourceFor(schema.ParseGroupResource(resource).WithVersion("")) + if err != nil { + return schema.GroupResource{}, err + } + return gvr.GroupResource(), nil +} diff --git a/pkg/util/collections/includes_excludes.go b/pkg/util/collections/includes_excludes.go index c1353e035..00cd48eb1 100644 --- a/pkg/util/collections/includes_excludes.go +++ b/pkg/util/collections/includes_excludes.go @@ -20,6 +20,8 @@ import ( "errors" "fmt" + "github.com/golang/glog" + "k8s.io/apimachinery/pkg/util/sets" ) @@ -104,3 +106,34 @@ func ValidateIncludesExcludes(includesList, excludesList []string) []error { return errs } + +func GenerateIncludesExcludes(includes []string, excludes []string, mapFunc func(string) (string, error)) *IncludesExcludes { + res := NewIncludesExcludes() + + for _, item := range includes { + if item == "*" { + res.Includes(item) + return res + } + + key, err := mapFunc(item) + if err != nil { + glog.Errorf("unable to include item %q: %v", item, err) + continue + } + + res.Includes(key) + } + + for _, item := range excludes { + key, err := mapFunc(item) + if err != nil { + glog.Errorf("unable to exclude item %q: %v", item, err) + continue + } + + res.Excludes(key) + } + + return res +} diff --git a/pkg/util/test/fake_discovery_helper.go b/pkg/util/test/fake_discovery_helper.go new file mode 100644 index 000000000..0ccdfd773 --- /dev/null +++ b/pkg/util/test/fake_discovery_helper.go @@ -0,0 +1,31 @@ +package test + +import ( + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type FakeDiscoveryHelper struct { + ResourceList []*metav1.APIResourceList + RESTMapper meta.RESTMapper +} + +func (dh *FakeDiscoveryHelper) Mapper() meta.RESTMapper { + return dh.RESTMapper +} + +func (dh *FakeDiscoveryHelper) Resources() []*metav1.APIResourceList { + return dh.ResourceList +} +func (dh *FakeDiscoveryHelper) Refresh() error { + return nil +} + +func (dh *FakeDiscoveryHelper) ResolveGroupResource(resource string) (schema.GroupResource, error) { + gvr, err := dh.RESTMapper.ResourceFor(schema.ParseGroupResource(resource).WithVersion("")) + if err != nil { + return schema.GroupResource{}, err + } + return gvr.GroupResource(), nil +}