Merge pull request #10275 from kaovilai/namespace-selection-by-label

Implement namespace selection by label in resource policy

pkg/controller/backup_controller.go

@@ -603,7 +603,12 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel
 	// Empty IncludedNamespaces means "include all namespaces". Normalize
 	// to ["*"] so that downstream wildcard expansion does not collapse
 	// an empty-includes + wildcard-excludes combination into "back up nothing".
-	if len(request.Spec.IncludedNamespaces) == 0 {
+	// Recorded separately from the normalized value below: once normalized, an
+	// originally-empty list and an explicitly-configured ["*"] are indistinguishable,
+	// but mergeNamespacesByLabel's replace-vs-union decision needs to tell them apart
+	// (see its doc comment).
+	includedNamespacesWereDefaulted := len(request.Spec.IncludedNamespaces) == 0
+	if includedNamespacesWereDefaulted {
 		request.Spec.IncludedNamespaces = []string{"*"}
 	}
 
@@ -636,10 +641,130 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel
 		request.Status.ValidationErrors = append(request.Status.ValidationErrors, "include-resources, exclude-resources and include-cluster-resources are old filter parameters.\n"+
 			"They cannot be used with namespace-scoped or fine-grained global filter policies.")
 	}
+
+	// Resolve includedNamespacesByLabel/excludedNamespacesByLabel from the resource policy
+	// (if configured) against the live namespace list, and merge into the effective
+	// namespace filter. Must run after the velero.io/exclude-from-backup hard-exclusion
+	// above, so that the "- BackupSpec.ExcludedNamespaces" term below already carries
+	// hard-excluded namespaces.
+	if resourcePolicies != nil && resourcePolicies.GetIncludeExcludePolicy() != nil {
+		iep := resourcePolicies.GetIncludeExcludePolicy()
+		if len(iep.IncludedNamespacesByLabel) > 0 || len(iep.ExcludedNamespacesByLabel) > 0 {
+			resolvedIncluded, resolvedExcluded, err := resourcepolicies.ResolveNamespacesByLabel(
+				ctx, b.kbClient,
+				iep.IncludedNamespacesByLabel, iep.ExcludedNamespacesByLabel, iep.LabelSelectorLogic)
+			if err != nil {
+				request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("error resolving namespace label selectors: %v", err))
+			} else {
+				logger.WithFields(logrus.Fields{
+					"includedNamespacesByLabelCount": len(resolvedIncluded),
+					"excludedNamespacesByLabelCount": len(resolvedExcluded),
+				}).Info("resolved namespaces by label selector")
+				logger.WithFields(logrus.Fields{
+					"includedNamespacesByLabel": resolvedIncluded,
+					"excludedNamespacesByLabel": resolvedExcluded,
+				}).Debug("resolved namespaces by label selector detail")
+
+				request.Spec.IncludedNamespaces, request.Spec.ExcludedNamespaces = mergeNamespacesByLabel(
+					request.Spec.IncludedNamespaces,
+					request.Spec.ExcludedNamespaces,
+					len(iep.IncludedNamespacesByLabel) > 0,
+					includedNamespacesWereDefaulted,
+					resolvedIncluded,
+					resolvedExcluded,
+				)
+			}
+		}
+	}
+
 	request.ResPolicies = resourcePolicies
 	return request
 }
 
+// mergeNamespacesByLabel merges a resource policy's resolved includedNamespacesByLabel/
+// excludedNamespacesByLabel name sets into the backup's effective IncludedNamespaces/
+// ExcludedNamespaces, per the design's Precedence and Interaction rules:
+//
+//   - includedNamespaces is assumed already normalized so that an originally-empty
+//     BackupSpec.IncludedNamespaces reads as the ["*"] wildcard (prepareBackupRequest does
+//     this normalization earlier, before resource-policy processing runs).
+//   - includedNamespacesWereDefaulted reports whether that normalization actually fired -
+//     i.e. whether BackupSpec.IncludedNamespaces was originally empty, as opposed to the user
+//     having explicitly written ["*"] themselves. The two are indistinguishable by the time
+//     includedNamespaces reaches this function (both read as ["*"]), so the caller must track
+//     and pass this separately; inspecting includedNamespaces alone would wrongly narrow an
+//     explicit ["*"] down to only the label matches instead of leaving it as "everything".
+//   - labelIncludeActive reports whether includedNamespacesByLabel was *configured* at all
+//     (not whether it matched anything - a configured selector matching zero namespaces must
+//     still produce an empty-selection baseline, not fall through to "all namespaces").
+//   - When labelIncludeActive and includedNamespacesWereDefaulted, resolvedIncluded REPLACES
+//     the wildcard baseline (unioning into "all" would still be "all", defeating the feature's
+//     primary use case of a schedule with no explicit includes) - represented via
+//     resourcepolicies.RepresentNamespaceSelection so a zero-match result is expressed as a
+//     wildcard pattern guaranteed to match nothing, not a plain empty slice (which
+//     wildcard.ShouldExpandWildcards treats as "match everything" - see that function's doc
+//     comment for why a bare empty list cannot be reused to mean the opposite here). When
+//     explicit concrete names were already present, resolvedIncluded is unioned in additively
+//     instead. When the user explicitly wrote ["*"] themselves (includedNamespacesWereDefaulted
+//     is false but includedNamespaces is already ["*"]), unioning concrete names into it is a
+//     no-op at match time (IncludesExcludes.ShouldInclude treats a "*" entry as match-everything
+//     regardless of what else is in the list) but would also violate
+//     collections.ValidateIncludesExcludes' "'*' must be alone in includes" invariant if the
+//     merged result were ever re-validated - so this case canonicalizes back down to ["*"]
+//     instead of widening it.
+//   - resolvedExcluded is unioned into excludedNamespaces whenever non-empty, regardless of
+//     labelIncludeActive - excludedNamespacesByLabel is purely subtractive, same role as
+//     BackupSpec.ExcludedNamespaces today. An empty resolvedExcluded needs no such translation:
+//     "exclude nothing" is unambiguous as a plain empty list, unlike "include nothing".
+//   - Finally, unless mergedIncluded is the wildcard, any name present in both merged lists is
+//     dropped from mergedIncluded (not mergedExcluded) so the two stay mutually exclusive.
+//     Exclusion already wins over inclusion at match time regardless (same ShouldInclude
+//     precedence as above), so this changes only the returned representation, not resolved
+//     backup behavior - it keeps the merged lists satisfying
+//     collections.ValidateIncludesExcludes' "excludes list cannot contain an item in the
+//     includes list" invariant too, for the same reason the "*" case above is canonicalized
+//     rather than left as an invariant-violating pair.
+func mergeNamespacesByLabel(
+	includedNamespaces []string,
+	excludedNamespaces []string,
+	labelIncludeActive bool,
+	includedNamespacesWereDefaulted bool,
+	resolvedIncluded []string,
+	resolvedExcluded []string,
+) (mergedIncluded []string, mergedExcluded []string) {
+	mergedIncluded = includedNamespaces
+	if labelIncludeActive {
+		switch {
+		case includedNamespacesWereDefaulted:
+			mergedIncluded = resourcepolicies.RepresentNamespaceSelection(resolvedIncluded)
+		case sets.NewString(includedNamespaces...).Has("*"):
+			mergedIncluded = []string{"*"}
+		default:
+			mergedIncluded = sets.NewString(includedNamespaces...).Insert(resolvedIncluded...).List()
+		}
+	}
+
+	mergedExcluded = excludedNamespaces
+	if len(resolvedExcluded) > 0 {
+		mergedExcluded = sets.NewString(excludedNamespaces...).Insert(resolvedExcluded...).List()
+	}
+
+	if !sets.NewString(mergedIncluded...).Has("*") {
+		// Difference can legitimately empty this out entirely (every resolved or explicit
+		// include also landed in mergedExcluded) - route back through
+		// RepresentNamespaceSelection so that comes back as the no-match sentinel, not a bare
+		// empty slice. The same "empty means include everything" hazard that motivated the
+		// zero-match sentinel above applies here too: an empty result at this point means
+		// "everything that was included is now excluded", i.e. include nothing, and a plain
+		// empty []string would be silently reinterpreted downstream as the opposite.
+		mergedIncluded = resourcepolicies.RepresentNamespaceSelection(
+			sets.NewString(mergedIncluded...).Difference(sets.NewString(mergedExcluded...)).List(),
+		)
+	}
+
+	return mergedIncluded, mergedExcluded
+}
+
 // validateAndGetSnapshotLocations gets a collection of VolumeSnapshotLocation objects that
 // this backup will use (returned as a map of provider name -> VSL), and ensures:
 //   - each location name in .spec.volumeSnapshotLocations exists as a location