Implement namespace selection by label in resource policy

Add includedNamespacesByLabel, excludedNamespacesByLabel, and labelSelectorLogic to IncludeExcludePolicy in the ResourcePolicy ConfigMap (realizes design in velero-io/velero#9772), letting a backup select or exclude namespaces by label instead of (or in addition to) name/wildcard. The backup controller resolves label selectors against the live namespace list once per backup, merges the results into spec.includedNamespaces/excludedNamespaces, then proceeds through the existing name-based filtering unchanged. A defaulted "*" include list is replaced by the resolved set; an explicitly-configured include list (including an explicit "*") is unioned with it instead, and stays canonical rather than widening. Namespaces matching an exclude selector are always subtracted from the merged includes, regardless of how the includes were populated. Because Velero's namespace-includes/excludes model requires at least one name (an empty list means "match everything"), a selector that resolves to zero namespaces is represented with a sentinel glob pattern ("[-]*") guaranteed to match no real namespace, rather than an empty list that would silently fall back to including/excluding everything. labelSelectorLogic ("AND"/"OR", case-insensitive) controls whether multiple included/excluded label selectors are combined by intersection or union; it is validated up front, including inside ResolveNamespacesByLabel itself, so an invalid value fails fast instead of silently falling through to OR semantics. Namespace-selection-by-label and resource-selection-by-label act as independent axes and do not affect each other, matching the design discussion in #9772. Known limitations: - Selectors are evaluated once per backup against the namespace list at that point in time; namespaces created or relabeled mid-backup are not picked up. - Backup-only for now; restore-side namespace mapping is unaffected. Testing: - Unit coverage in internal/resourcepolicies for validation, selector resolution (including AND/OR logic, case-insensitivity, and malformed-selector/invalid-logic error paths), and the no-match sentinel. - Unit coverage in pkg/controller for the merge logic between resolved label selections and explicit/defaulted includes and excludes. - End-to-end coverage in pkg/backup exercising the full backup pipeline with label-selected namespaces, including the velero.io/exclude-from-backup hard-exclusion interaction and the zero-match/fully-excluded sentinel path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
2026-09-29 03:05:54 +00:00 · 2026-09-18 23:04:55 -04:00
parent d074047b06
commit 40af5efdd0
7 changed files with 1319 additions and 2 deletions
@@ -5543,6 +5543,32 @@ func TestBackupNamespaces(t *testing.T) {
 				"resources/deployments.apps/v1-preferredversion/namespaces/ns-1/deploy-1.json",
 			},
 		},
+		{
+			// Regression guard for the design/namespace-label-selector-in-resource-policy_design.md
+			// Precedence and Interaction trade-off: a namespace admitted into
+			// BackupSpec.IncludedNamespaces by name (which is exactly what
+			// resourcepolicies.ResolveNamespacesByLabel + prepareBackupRequest produce for
+			// includedNamespacesByLabel) still gets its own Namespace object backed up even
+			// when nothing in it matches a separately configured LabelSelector -
+			// namespace-selection and resource-selection-by-label are independent axes.
+			name: "namespace explicitly included is backed up even when nothing inside matches LabelSelector",
+			backup: defaultBackup().IncludedNamespaces("ns-1").
+				LabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"team": "platform"}}).
+				Result(),
+			apiResources: []*test.APIResource{
+				test.Namespaces(
+					builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(),
+					builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(),
+				),
+				test.Deployments(
+					builder.ForDeployment("ns-1", "deploy-1").Result(),
+				),
+			},
+			want: []string{
+				"resources/namespaces/cluster/ns-1.json",
+				"resources/namespaces/v1-preferredversion/cluster/ns-1.json",
+			},
+		},
 	}

 	itemBlockPool := StartItemBlockWorkerPool(t.Context(), 1, logrus.StandardLogger())
@@ -5571,6 +5597,212 @@ func TestBackupNamespaces(t *testing.T) {
 	}
 }

+// TestBackupWithResourcePolicyNamespaceLabelSelector is an integration-style test spanning
+// resourcepolicies.ResolveNamespacesByLabel and the real backup item-collection path: it
+// resolves includedNamespacesByLabel against live namespaces the way prepareBackupRequest
+// does, combines the result with an explicit BackupSpec.IncludedNamespaces entry the way
+// mergeNamespacesByLabel's union branch does, and verifies the resulting backup contains both
+// namespaces' resources - simulating a Schedule configured with both a ResourcePolicy label
+// selector and an explicit include.
+func TestBackupWithResourcePolicyNamespaceLabelSelector(t *testing.T) {
+	nsPlatform := builder.ForNamespace("platform-ns").ObjectMeta(builder.WithLabels("team", "platform")).Result()
+	nsOps := builder.ForNamespace("ops-ns").Result()
+	nsOther := builder.ForNamespace("other-ns").ObjectMeta(builder.WithLabels("team", "infra")).Result()
+
+	fakeClient := test.NewFakeControllerRuntimeClient(t, nsPlatform, nsOps, nsOther)
+
+	resolvedIncluded, _, err := resourcepolicies.ResolveNamespacesByLabel(
+		t.Context(), fakeClient, []string{"team=platform"}, nil, "")
+	require.NoError(t, err)
+	require.Equal(t, []string{"platform-ns"}, resolvedIncluded)
+
+	// "ops-ns" stands in for BackupSpec.IncludedNamespaces already having an explicit entry;
+	// mergeNamespacesByLabel would union resolvedIncluded into it additively (see
+	// TestMergeNamespacesByLabel in pkg/controller for that merge decision in isolation).
+	effectiveIncludes := append([]string{"ops-ns"}, resolvedIncluded...)
+
+	backup := defaultBackup().IncludedNamespaces(effectiveIncludes...).Result()
+
+	itemBlockPool := StartItemBlockWorkerPool(t.Context(), 1, logrus.StandardLogger())
+	defer itemBlockPool.Stop()
+
+	h := newHarness(t, itemBlockPool)
+	req := &Request{
+		Backup:           backup,
+		SkippedPVTracker: NewSkipPVTracker(),
+		BackedUpItems:    NewBackedUpItemsMap(),
+		WorkerPool:       itemBlockPool,
+	}
+	backupFile := bytes.NewBuffer([]byte{})
+
+	h.addItems(t, test.Namespaces(
+		builder.ForNamespace("platform-ns").Phase(corev1api.NamespaceActive).ObjectMeta(builder.WithLabels("team", "platform")).Result(),
+		builder.ForNamespace("ops-ns").Phase(corev1api.NamespaceActive).Result(),
+		builder.ForNamespace("other-ns").Phase(corev1api.NamespaceActive).ObjectMeta(builder.WithLabels("team", "infra")).Result(),
+	))
+	h.addItems(t, test.Deployments(
+		builder.ForDeployment("platform-ns", "app-1").Result(),
+		builder.ForDeployment("ops-ns", "app-2").Result(),
+		builder.ForDeployment("other-ns", "app-3").Result(),
+	))
+
+	h.backupper.Backup(h.log, req, backupFile, nil, nil, nil)
+
+	assertTarballContents(t, backupFile,
+		"metadata/version",
+		"resources/namespaces/cluster/platform-ns.json",
+		"resources/namespaces/v1-preferredversion/cluster/platform-ns.json",
+		"resources/namespaces/cluster/ops-ns.json",
+		"resources/namespaces/v1-preferredversion/cluster/ops-ns.json",
+		"resources/deployments.apps/namespaces/platform-ns/app-1.json",
+		"resources/deployments.apps/v1-preferredversion/namespaces/platform-ns/app-1.json",
+		"resources/deployments.apps/namespaces/ops-ns/app-2.json",
+		"resources/deployments.apps/v1-preferredversion/namespaces/ops-ns/app-2.json",
+	)
+}
+
+// TestBackupResourcePolicyNamespaceLabelSelectorEdgeCases runs several scenarios through the
+// real backup item-collection path (not just prepareBackupRequest's intermediate spec value),
+// each constructing the same effective IncludedNamespaces/ExcludedNamespaces that
+// mergeNamespacesByLabel (pkg/controller) produces for the given resource-policy configuration.
+// Guards against an empty include-selector result silently expanding to "back up everything"
+// downstream, and covers the explicit-wildcard and exclude-precedence handling.
+func TestBackupResourcePolicyNamespaceLabelSelectorEdgeCases(t *testing.T) {
+	runBackup := func(t *testing.T, backup *velerov1.Backup, namespaces *test.APIResource, resources ...*test.APIResource) *bytes.Buffer {
+		t.Helper()
+
+		itemBlockPool := StartItemBlockWorkerPool(t.Context(), 1, logrus.StandardLogger())
+		defer itemBlockPool.Stop()
+
+		h := newHarness(t, itemBlockPool)
+		req := &Request{
+			Backup:           backup,
+			SkippedPVTracker: NewSkipPVTracker(),
+			BackedUpItems:    NewBackedUpItemsMap(),
+			WorkerPool:       itemBlockPool,
+		}
+		backupFile := bytes.NewBuffer([]byte{})
+
+		h.addItems(t, namespaces)
+		for _, r := range resources {
+			h.addItems(t, r)
+		}
+
+		require.NoError(t, h.backupper.Backup(h.log, req, backupFile, nil, nil, nil))
+		return backupFile
+	}
+
+	t.Run("include selector matching zero namespaces backs up nothing, not everything", func(t *testing.T) {
+		// mergeNamespacesByLabel's fix: an include selector matching zero namespaces must
+		// resolve to resourcepolicies.NoNamespaceMatchesPattern, not a bare empty slice
+		// (which wildcard.ShouldExpandWildcards would otherwise treat as "match everything").
+		backup := defaultBackup().IncludedNamespaces(resourcepolicies.NoNamespaceMatchesPattern).Result()
+
+		backupFile := runBackup(t, backup,
+			test.Namespaces(
+				builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(),
+				builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(),
+			),
+			test.Deployments(builder.ForDeployment("ns-1", "app-1").Result()),
+		)
+
+		assertTarballContents(t, backupFile, "metadata/version")
+	})
+
+	t.Run("explicit wildcard plus include selector keeps everything, not narrowed", func(t *testing.T) {
+		// mergeNamespacesByLabel's other fix: an explicitly-configured ["*"] keeps everything
+		// included regardless of includedNamespacesByLabel, rather than being narrowed down to
+		// just the label matches. The merge canonicalizes this case back down to ["*"] (see
+		// TestMergeNamespacesByLabel), which is what's fed in here.
+		backup := defaultBackup().IncludedNamespaces("*").Result()
+
+		backupFile := runBackup(t, backup,
+			test.Namespaces(
+				builder.ForNamespace("platform-ns").Phase(corev1api.NamespaceActive).ObjectMeta(builder.WithLabels("team", "platform")).Result(),
+				builder.ForNamespace("other-ns").Phase(corev1api.NamespaceActive).Result(),
+			),
+			test.Deployments(
+				builder.ForDeployment("platform-ns", "app-1").Result(),
+				builder.ForDeployment("other-ns", "app-2").Result(),
+			),
+		)
+
+		assertTarballContents(t, backupFile,
+			"metadata/version",
+			"resources/namespaces/cluster/platform-ns.json",
+			"resources/namespaces/v1-preferredversion/cluster/platform-ns.json",
+			"resources/namespaces/cluster/other-ns.json",
+			"resources/namespaces/v1-preferredversion/cluster/other-ns.json",
+			"resources/deployments.apps/namespaces/platform-ns/app-1.json",
+			"resources/deployments.apps/v1-preferredversion/namespaces/platform-ns/app-1.json",
+			"resources/deployments.apps/namespaces/other-ns/app-2.json",
+			"resources/deployments.apps/v1-preferredversion/namespaces/other-ns/app-2.json",
+		)
+	})
+
+	t.Run("namespace matching both included and excluded label selectors is excluded", func(t *testing.T) {
+		// A namespace resolved into both resolvedIncluded and resolvedExcluded - exclusion
+		// wins, same as BackupSpec.ExcludedNamespaces vs IncludedNamespaces always has.
+		backup := defaultBackup().
+			IncludedNamespaces("both-ns", "include-only-ns").
+			ExcludedNamespaces("both-ns").
+			Result()
+
+		backupFile := runBackup(t, backup, test.Namespaces(
+			builder.ForNamespace("both-ns").Phase(corev1api.NamespaceActive).Result(),
+			builder.ForNamespace("include-only-ns").Phase(corev1api.NamespaceActive).Result(),
+		))
+
+		assertTarballContents(t, backupFile,
+			"metadata/version",
+			"resources/namespaces/cluster/include-only-ns.json",
+			"resources/namespaces/v1-preferredversion/cluster/include-only-ns.json",
+		)
+	})
+
+	t.Run("velero.io/exclude-from-backup hard exclusion wins over an include-label match", func(t *testing.T) {
+		// prepareBackupRequest's ordering guarantee: hard-excluded namespaces are already in
+		// ExcludedNamespaces by the time includedNamespacesByLabel resolution runs, so a
+		// namespace that also matches an include selector must still end up excluded.
+		backup := defaultBackup().
+			IncludedNamespaces("hard-excluded-ns", "platform-ns").
+			ExcludedNamespaces("hard-excluded-ns").
+			Result()
+
+		backupFile := runBackup(t, backup, test.Namespaces(
+			builder.ForNamespace("hard-excluded-ns").Phase(corev1api.NamespaceActive).
+				ObjectMeta(builder.WithLabels("velero.io/exclude-from-backup", "true")).Result(),
+			builder.ForNamespace("platform-ns").Phase(corev1api.NamespaceActive).Result(),
+		))
+
+		assertTarballContents(t, backupFile,
+			"metadata/version",
+			"resources/namespaces/cluster/platform-ns.json",
+			"resources/namespaces/v1-preferredversion/cluster/platform-ns.json",
+		)
+	})
+
+	t.Run("every included namespace also excluded backs up nothing, not everything", func(t *testing.T) {
+		// mergeNamespacesByLabel's exclude-subtraction step (added to satisfy
+		// collections.ValidateIncludesExcludes' invariants) can itself empty out the
+		// included set when every included name is also excluded. When that happens, the
+		// merge must emit resourcepolicies.NoNamespaceMatchesPattern rather than a bare
+		// empty include list, which wildcard.ShouldExpandWildcards would otherwise treat as
+		// "match everything" - the same hazard the zero-match sentinel exists for, reached
+		// through a different path.
+		backup := defaultBackup().
+			IncludedNamespaces(resourcepolicies.NoNamespaceMatchesPattern).
+			ExcludedNamespaces("only-ns").
+			Result()
+
+		backupFile := runBackup(t, backup, test.Namespaces(
+			builder.ForNamespace("only-ns").Phase(corev1api.NamespaceActive).Result(),
+		))
+
+		assertTarballContents(t, backupFile, "metadata/version")
+	})
+}
+
 func TestUpdateVolumeInfos(t *testing.T) {
 	timeExample := time.Date(2014, 6, 5, 11, 56, 45, 0, time.Local)
 	now := metav1.NewTime(timeExample)