From ecab5836801a1cf8aea92b85c74fa381c17f3948 Mon Sep 17 00:00:00 2001 From: Ashish Amarnath Date: Wed, 11 Nov 2020 06:50:57 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Do=20not=20run=20ItemAction=20pl?= =?UTF-8?q?ugins=20for=20unresolvable=20types=20for=20all=20types=20(#3059?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Ashish Amarnath --- changelogs/unreleased/3059-ashish-amarnath | 1 + pkg/backup/backup.go | 30 +++------------------- pkg/util/collections/includes_excludes.go | 5 +++- 3 files changed, 8 insertions(+), 28 deletions(-) create mode 100644 changelogs/unreleased/3059-ashish-amarnath diff --git a/changelogs/unreleased/3059-ashish-amarnath b/changelogs/unreleased/3059-ashish-amarnath new file mode 100644 index 000000000..f365a8380 --- /dev/null +++ b/changelogs/unreleased/3059-ashish-amarnath @@ -0,0 +1 @@ +🐛 ItemAction plugins for unresolvable types should not be run for all types diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index c75e6fc42..e5a6b1438 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -127,7 +127,7 @@ func resolveActions(actions []velero.BackupItemAction, helper discovery.Helper) return nil, err } - resources := getResourceIncludesExcludes(helper, resourceSelector.IncludedResources, resourceSelector.ExcludedResources) + resources := collections.GetResourceIncludesExcludes(helper, resourceSelector.IncludedResources, resourceSelector.ExcludedResources) namespaces := collections.NewIncludesExcludes().Includes(resourceSelector.IncludedNamespaces...).Excludes(resourceSelector.ExcludedNamespaces...) selector := labels.Everything() @@ -150,30 +150,6 @@ func resolveActions(actions []velero.BackupItemAction, helper discovery.Helper) return resolved, nil } -// getResourceIncludesExcludes takes the lists of resources to include and exclude, uses the -// discovery helper to resolve them to fully-qualified group-resource names, and returns an -// IncludesExcludes list. -func getResourceIncludesExcludes(helper discovery.Helper, includes, excludes []string) *collections.IncludesExcludes { - resources := collections.GenerateIncludesExcludes( - includes, - excludes, - func(item string) string { - gvr, _, err := helper.ResourceFor(schema.ParseGroupResource(item).WithVersion("")) - if err != nil { - // If we can't resolve it, return it as-is. This prevents the generated - // includes-excludes list from including *everything*, if none of the includes - // can be resolved. ref. https://github.com/vmware-tanzu/velero/issues/2461 - return item - } - - gr := gvr.GroupResource() - return gr.String() - }, - ) - - return resources -} - // getNamespaceIncludesExcludes returns an IncludesExcludes list containing which namespaces to // include and exclude from the backup. func getNamespaceIncludesExcludes(backup *velerov1api.Backup) *collections.IncludesExcludes { @@ -200,7 +176,7 @@ func getResourceHook(hookSpec velerov1api.BackupResourceHookSpec, discoveryHelpe Name: hookSpec.Name, Selector: hook.ResourceHookSelector{ Namespaces: collections.NewIncludesExcludes().Includes(hookSpec.IncludedNamespaces...).Excludes(hookSpec.ExcludedNamespaces...), - Resources: getResourceIncludesExcludes(discoveryHelper, hookSpec.IncludedResources, hookSpec.ExcludedResources), + Resources: collections.GetResourceIncludesExcludes(discoveryHelper, hookSpec.IncludedResources, hookSpec.ExcludedResources), }, Pre: hookSpec.PreHooks, Post: hookSpec.PostHooks, @@ -242,7 +218,7 @@ func (kb *kubernetesBackupper) Backup(log logrus.FieldLogger, backupRequest *Req log.Infof("Including namespaces: %s", backupRequest.NamespaceIncludesExcludes.IncludesString()) log.Infof("Excluding namespaces: %s", backupRequest.NamespaceIncludesExcludes.ExcludesString()) - backupRequest.ResourceIncludesExcludes = getResourceIncludesExcludes(kb.discoveryHelper, backupRequest.Spec.IncludedResources, backupRequest.Spec.ExcludedResources) + backupRequest.ResourceIncludesExcludes = collections.GetResourceIncludesExcludes(kb.discoveryHelper, backupRequest.Spec.IncludedResources, backupRequest.Spec.ExcludedResources) log.Infof("Including resources: %s", backupRequest.ResourceIncludesExcludes.IncludesString()) log.Infof("Excluding resources: %s", backupRequest.ResourceIncludesExcludes.ExcludesString()) log.Infof("Backing up all pod volumes using restic: %t", *backupRequest.Backup.Spec.DefaultVolumesToRestic) diff --git a/pkg/util/collections/includes_excludes.go b/pkg/util/collections/includes_excludes.go index 02f84d21f..c113f0d95 100644 --- a/pkg/util/collections/includes_excludes.go +++ b/pkg/util/collections/includes_excludes.go @@ -201,7 +201,10 @@ func GetResourceIncludesExcludes(helper discovery.Helper, includes, excludes []s func(item string) string { gvr, _, err := helper.ResourceFor(schema.ParseGroupResource(item).WithVersion("")) if err != nil { - return "" + // If we can't resolve it, return it as-is. This prevents the generated + // includes-excludes list from including *everything*, if none of the includes + // can be resolved. ref. https://github.com/vmware-tanzu/velero/issues/2461 + return item } gr := gvr.GroupResource()