From da916846b1e86701a1f44d36d736b1bc74eafd6a Mon Sep 17 00:00:00 2001 From: Scott Seago Date: Thu, 5 May 2022 08:26:50 -0400 Subject: [PATCH] continue rather than return for non-matching restore action label When iterating over applicable restore actions, if a non-matching label selector is found, velero should continue to the next action rather than returning from the restoreItem func, which ends up preventing the item's restore entirely. Signed-off-by: Scott Seago --- changelogs/unreleased/4890-sseago | 1 + pkg/restore/restore.go | 2 +- pkg/restore/restore_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/4890-sseago diff --git a/changelogs/unreleased/4890-sseago b/changelogs/unreleased/4890-sseago new file mode 100644 index 000000000..7245436dc --- /dev/null +++ b/changelogs/unreleased/4890-sseago @@ -0,0 +1 @@ +continue rather than return for non-matching restore action label diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 67681eea5..496b6cc57 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -1106,7 +1106,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso for _, action := range ctx.getApplicableActions(groupResource, namespace) { if !action.Selector.Matches(labels.Set(obj.GetLabels())) { - return warnings, errs + continue } ctx.log.Infof("Executing item action for %v", &groupResource) diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index b31c87f7c..dadf6bea5 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -1270,6 +1270,11 @@ func (a *pluggableAction) AppliesTo() (velero.ResourceSelector, error) { return a.selector, nil } +func (a *pluggableAction) addSelector(selector velero.ResourceSelector) *pluggableAction { + a.selector = selector + return a +} + // TestRestoreActionModifications runs restores with restore item actions that modify resources, and // verifies that that the modified item is correctly created in the API. Verification is done by looking // at the full object in the API. @@ -1335,6 +1340,26 @@ func TestRestoreActionModifications(t *testing.T) { test.Pods(builder.ForPod("ns-1", "pod-1").Result()), }, }, + { + name: "action with non-matching label selector doesn't prevent restore", + restore: defaultRestore().Result(), + backup: defaultBackup().Result(), + tarball: test.NewTarWriter(t).AddItems("pods", builder.ForPod("ns-1", "pod-1").Result()).Done(), + apiResources: []*test.APIResource{test.Pods()}, + actions: []velero.RestoreItemAction{ + modifyingActionGetter(func(item *unstructured.Unstructured) { + item.SetLabels(map[string]string{"updated": "true"}) + }).addSelector(velero.ResourceSelector{ + IncludedResources: []string{ + "Pod", + }, + LabelSelector: "nonmatching=label", + }), + }, + want: []*test.APIResource{ + test.Pods(builder.ForPod("ns-1", "pod-1").Result()), + }, + }, // TODO action that modifies namespace/name - what's the expected behavior? }