From a12b373e4cd379d2ab619c0cfbbd988f1f2684f1 Mon Sep 17 00:00:00 2001 From: Adam Zhang Date: Sat, 25 Jul 2026 04:03:51 +0800 Subject: [PATCH 1/3] Support set-based filter label selectors (#10064) * Support set-based filter label selectors Use matchLabels/matchExpressions in fine-grained filters. Signed-off-by: Adam Zhang * omit the details of resource policy for cli The reason to not resolve and display CLI is because it may go out of sync, we want to avoid display it to mislead users. We may consider to cpature those information and display it in later release. Signed-off-by: Adam Zhang --------- Signed-off-by: Adam Zhang Co-authored-by: Scott Seago --- changelogs/unreleased/10064-adam-jian-zhang | 1 + .../fine-grained-backup-filters-design.md | 137 ++++++------ .../fine-grained-restore-filters-design.md | 64 ++++-- .../resourcepolicies/resource_policies.go | 88 +++++++- .../resource_policies_test.go | 197 ++++++++++++++++-- pkg/backup/backup.go | 16 +- pkg/backup/backup_test.go | 92 ++++++-- pkg/cmd/util/output/backup_describer.go | 118 ----------- pkg/cmd/util/output/backup_describer_test.go | 85 -------- .../output/backup_structured_describer.go | 85 -------- .../backup_structured_describer_test.go | 96 --------- pkg/restore/restore.go | 16 +- pkg/restore/restore_policies_test.go | 3 +- .../docs/main/fine-grained-backup-filters.md | 127 ++++++++--- 14 files changed, 575 insertions(+), 550 deletions(-) create mode 100644 changelogs/unreleased/10064-adam-jian-zhang diff --git a/changelogs/unreleased/10064-adam-jian-zhang b/changelogs/unreleased/10064-adam-jian-zhang new file mode 100644 index 000000000..9d45481db --- /dev/null +++ b/changelogs/unreleased/10064-adam-jian-zhang @@ -0,0 +1 @@ +Add set based label selectors for fine-grained filters diff --git a/design/backup-filter-enhancement/fine-grained-backup-filters-design.md b/design/backup-filter-enhancement/fine-grained-backup-filters-design.md index 0bb52ffd5..3fef82974 100644 --- a/design/backup-filter-enhancement/fine-grained-backup-filters-design.md +++ b/design/backup-filter-enhancement/fine-grained-backup-filters-design.md @@ -41,7 +41,7 @@ This creates three critical gaps for common backup scenarios: - Maintain full backward compatibility — existing backups with no `namespacedFilterPolicies` behave exactly as they do today - Define clear precedence rules for how per-namespace filters interact with global filters - Add corresponding validation within the Resource Policies validation pipeline using existing Velero wildcard validation functions -- Update `velero backup describe` output to display per-namespace filter information when present +- Update `velero backup describe` output to display the referenced ResourcePolicy ConfigMap name when configured - Ensure the restore process works correctly with backups produced by namespace-scoped filters, without requiring restore-side code changes in the initial phase ## Non-Goals @@ -77,7 +77,8 @@ clusterScopedFilterPolicy: names: ["my-app-*"] - kinds: [CustomResourceDefinition] labelSelector: - app: my-app + matchLabels: + app: my-app namespacedFilterPolicies: # NEW: per-namespace filter overrides - namespaces: @@ -85,7 +86,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret, Deployment] labelSelector: - app: my-app + matchLabels: + app: my-app - namespaces: - ns-b resourceFilters: @@ -93,7 +95,8 @@ namespacedFilterPolicies: names: [app-1, app-2] - kinds: [ConfigMap] labelSelector: - app: my-service + matchLabels: + app: my-service ``` All four sections coexist in the same ConfigMap. They are independent — `volumePolicies` handles volume backup strategy, `includeExcludePolicy` handles global resource type filtering, `clusterScopedFilterPolicy` handles cluster-scoped resource filtering by kind/name/label, and `namespacedFilterPolicies` handles per-namespace, per-kind overrides. @@ -107,7 +110,9 @@ namespacedFilterPolicies: - namespaces: [ns-a] resourceFilters: - kinds: [ConfigMap, Secret] # these kinds share a selector - labelSelector: {app: my-app} + labelSelector: + matchLabels: + app: my-app names: ["app-*"] - kinds: [Deployment] # this kind has its own selector names: [workload-1, workload-2] @@ -116,6 +121,24 @@ namespacedFilterPolicies: This model has one way to express filters — there is no ambiguity about how to structure the configuration. Only resource kinds listed in `resourceFilters` entries are included in the backup for the matched namespaces; unlisted kinds are implicitly excluded. +#### Label selectors (`matchLabels` / `matchExpressions`) + +`labelSelector` and each entry of `orLabelSelectors` use the standard Kubernetes selector shape (same as `BackupSpec.labelSelector`): + +```yaml +labelSelector: + matchLabels: + app: my-app + matchExpressions: + - key: environment + operator: In + values: [prod, staging] + - key: do-not-backup + operator: DoesNotExist +``` + +Supported `matchExpressions` operators: `In`, `NotIn`, `Exists`, `DoesNotExist`. Prefer `In` for value-OR on one key; use `orLabelSelectors` for OR across independent multi-key groups. `labelSelector` and `orLabelSelectors` cannot co-exist in the same `resourceFilters` entry. + #### Catch-All Resource Filter (Empty `kinds` or `["*"]`) A `ResourceFilter` entry with an empty (or omitted) `kinds` field, or a field explicitly set to `["*"]`, acts as a **catch-all**. Its `labelSelector` or `orLabelSelectors` (if provided) is applied to **all resource types in the namespace that are not already matched by a kind-specific filter entry**. If no selectors are provided, all unlisted resources are included. Using `["*"]` is highly recommended as it makes the catch-all intention explicit and self-documenting. @@ -319,9 +342,10 @@ resourceFilters: resourceFilters: - kinds: ["Pod"] labelSelector: - "invalid label key!": "value" # invalid key syntax + matchLabels: + "invalid label key!": "value" # invalid key syntax ``` -**Behavior:** Validation error during backup creation when `labels.SelectorFromSet()` fails: +**Behavior:** Validation error during backup creation when `metav1.LabelSelectorAsSelector()` fails: ``` namespacedFilterPolicies[0].resourceFilters[0]: invalid label selector: "invalid label key!" is not a valid label key ``` @@ -340,7 +364,33 @@ This is consistent with how other discovery-dependent features handle this error ## ResourceFilter Field Notes -**`labelSelector`** supports equality-based selectors only (`key=value`). Set-based requirements (e.g., `environment in (prod, staging)`) are not supported. To match resources with any of several label combinations, use `orLabelSelectors` with multiple maps — each map is AND-evaluated internally, and the maps are OR-evaluated across the list. `labelSelector` and `orLabelSelectors` cannot co-exist in the same entry. +**`labelSelector`** uses the standard Kubernetes shape: `matchLabels` (equality) and `matchExpressions` (set-based: `In`, `NotIn`, `Exists`, `DoesNotExist`). All requirements within one selector are AND-ed. Example: + +```yaml +labelSelector: + matchLabels: + app: my-app + matchExpressions: + - key: environment + operator: In + values: [prod, staging] + - key: do-not-backup + operator: DoesNotExist +``` + +**`orLabelSelectors`** is a list of the same selector shape. Match if **any** entry matches (AND within each entry, OR across the list). Prefer `In` for value-OR on one key; use `orLabelSelectors` for OR of independent multi-key groups. `labelSelector` and `orLabelSelectors` cannot co-exist in the same entry. + +```yaml +orLabelSelectors: + - matchLabels: + tier: frontend + matchExpressions: + - key: track + operator: In + values: [canary] + - matchLabels: + tier: backend +``` **`names` / `excludedNames`** accept exact resource names or glob patterns. If `names` is empty, all resource names are included (subject to label filters). `excludedNames` takes precedence over `names` when a name matches both. @@ -420,7 +470,8 @@ data: resourceFilters: - kinds: [ConfigMap, Secret, Deployment] labelSelector: - app: my-app + matchLabels: + app: my-app # ns-b has no filter policy entry, so global filters apply (include everything) ``` @@ -462,10 +513,12 @@ data: resourceFilters: - kinds: [Deployment] labelSelector: - app: production-workload-1 + matchLabels: + app: production-workload-1 - kinds: [StatefulSet] labelSelector: - app: production-workload-2 + matchLabels: + app: production-workload-2 ``` ### Per-Kind Exact Names @@ -561,7 +614,8 @@ data: resourceFilters: - kinds: ["*"] # catch-all: applies to every kind not listed below labelSelector: - backup: "true" # back up any resource carrying this label + matchLabels: + backup: "true" # back up any resource carrying this label ``` **Result:** Every resource type in `production` that has the label `backup=true` is backed up. Resources without that label are excluded. No kind enumeration is required. @@ -589,7 +643,8 @@ data: names: [db-credentials, tls-cert] # these exact Secrets by name - kinds: ["*"] # catch-all for all other kinds labelSelector: - backup: "true" # back up by label + matchLabels: + backup: "true" # back up by label ``` **Result:** @@ -666,7 +721,8 @@ data: names: [workload-1, workload-2] - kinds: [StatefulSet] labelSelector: - app: my-app + matchLabels: + app: my-app - kinds: [ConfigMap, Secret] names: ["app-*"] excludedNames: ["*-tmp", "*-debug"] @@ -697,7 +753,7 @@ spec: ### `velero backup describe` -The output is extended to display namespace-scoped filter policies when present in the ResourcePolicy ConfigMap: +The output displays the referenced ResourcePolicy ConfigMap name when configured on the backup. It intentionally avoids resolving and displaying the live ConfigMap contents, because the ConfigMap content in the cluster may be modified or deleted after the backup execution, which could lead to displaying out-of-sync or inaccurate information: ``` Name: selective-backup @@ -721,46 +777,9 @@ Resources: Label selector: -Resource Policy: backup-filter-policy - -Namespace-Scoped Filter Policies: - ns-a: - Resource Filters: - ConfigMap, Secret, Deployment: - Label selector: app=my-app - Included names: - Excluded names: - target-namespace: - Resource Filters: - Deployment: - Label selector: app=production-workload-1 - Included names: - Excluded names: - StatefulSet: - Label selector: app=production-workload-2 - Included names: - Excluded names: - production: - Resource Filters: - Deployment: - Label selector: - Included names: [api-server, worker] - Excluded names: - (all other kinds): - Label selector: backup=true - Included names: - Excluded names: - -Fine-Grained Global Filter Policy: - Resource Filters: - ClusterRole, ClusterRoleBinding: - Label selector: - Included names: [my-app-*] - Excluded names: - CustomResourceDefinition: - Label selector: app=my-app - Included names: - Excluded names: +Resource policies: + Type: configmap + Name: backup-filter-policy Storage Location: default @@ -795,7 +814,7 @@ Notes: - Global filters (--include-resources, --selector, etc.) apply to all included namespaces - Namespace-scoped filters defined in --resource-policies-configmap override global filters for matching namespaces - Fine-grained global filter policies defined in --resource-policies-configmap override global filters for cluster-scoped resources -- Use 'velero backup describe' to view resolved filter policies after backup creation +- Use 'velero backup describe' to view the referenced ResourcePolicy ConfigMap name after backup creation ``` ### CLI Integration Points @@ -808,12 +827,12 @@ Notes: **Help and Discovery:** - `velero backup create --help` includes updated filtering documentation -- `velero backup describe` shows resolved filter policies for troubleshooting +- `velero backup describe` shows the referenced ResourcePolicy ConfigMap name - Validation errors include ConfigMap field references for easy debugging **Configuration Discovery:** - `velero backup create --help` includes namespace-scoped filtering documentation -- `velero backup describe` shows resolved filter policies for verification +- `velero backup describe` shows the referenced ResourcePolicy ConfigMap name for verification ## User Perspective @@ -823,7 +842,7 @@ This design provides fine-grained, per-namespace, per-kind control over backup f - **For users adopting namespace-scoped filter policies**: Create a ConfigMap with the `namespacedFilterPolicies` section and reference it via `BackupSpec.ResourcePolicy` (or the existing `--resource-policies-configmap` flag). The backup will selectively include/exclude resources per namespace based on the filter rules. - **For users already using ResourcePolicy for volume policies**: Add the `namespacedFilterPolicies` section to the same ConfigMap. Both volume policies and namespace-scoped filters coexist. - **For restore from a namespace-filtered backup**: No changes to restore workflow. Restore processes whatever is in the archive. Users can use existing `RestoreSpec.IncludedNamespaces` for additional filtering at restore time. -- **`velero backup describe` output**: Extended to show per-namespace, per-kind filter details when the ResourcePolicy ConfigMap contains `namespacedFilterPolicies`. +- **`velero backup describe` output**: Displays the referenced ResourcePolicy ConfigMap name when configured on the backup. - **Validation errors**: Reported at backup start when the ResourcePolicy ConfigMap contains invalid `namespacedFilterPolicies` configurations. Consistent with how volume policy validation errors are reported today. ## Alternatives Considered diff --git a/design/restore-filter-enhancement/fine-grained-restore-filters-design.md b/design/restore-filter-enhancement/fine-grained-restore-filters-design.md index 913c056c0..fd3069b68 100644 --- a/design/restore-filter-enhancement/fine-grained-restore-filters-design.md +++ b/design/restore-filter-enhancement/fine-grained-restore-filters-design.md @@ -108,14 +108,16 @@ clusterScopedFilterPolicy: names: ["my-app-*"] - kinds: [CustomResourceDefinition] labelSelector: - app: my-app + matchLabels: + app: my-app namespacedFilterPolicies: - namespaces: - ns-a resourceFilters: - kinds: [ConfigMap, Secret, Deployment] labelSelector: - app: my-app + matchLabels: + app: my-app - namespaces: - ns-b resourceFilters: @@ -123,7 +125,8 @@ namespacedFilterPolicies: names: [app-1, app-2] - kinds: [ConfigMap] labelSelector: - app: my-service + matchLabels: + app: my-service ``` The restore-side ConfigMap does **not** require `volumePolicies` or `includeExcludePolicy` sections. Those are backup-specific. The YAML parser will ignore unknown fields gracefully, so a user can technically point to the same ConfigMap used for backup — the restore pipeline will only read `namespacedFilterPolicies` and `clusterScopedFilterPolicy`. @@ -137,7 +140,9 @@ namespacedFilterPolicies: - namespaces: [ns-a] resourceFilters: - kinds: [ConfigMap, Secret] # these kinds share a selector - labelSelector: {app: my-app} + labelSelector: + matchLabels: + app: my-app names: ["app-*"] - kinds: [Deployment] # this kind has its own selector names: [workload-1, workload-2] @@ -146,6 +151,36 @@ namespacedFilterPolicies: Only resource kinds listed in `resourceFilters` entries are restored for the matched namespaces; unlisted kinds are implicitly excluded (globally excluded kinds cannot be re-included — see precedence model). +#### Label selectors (`matchLabels` / `matchExpressions`) + +`labelSelector` and each entry of `orLabelSelectors` use the standard Kubernetes selector shape (same as `RestoreSpec.labelSelector`): + +```yaml +labelSelector: + matchLabels: + app: my-app + matchExpressions: + - key: environment + operator: In + values: [prod, staging] + - key: do-not-restore + operator: DoesNotExist +``` + +Supported `matchExpressions` operators: `In`, `NotIn`, `Exists`, `DoesNotExist`. Prefer `In` for value-OR on one key; use `orLabelSelectors` for OR across independent multi-key groups. `labelSelector` and `orLabelSelectors` cannot co-exist in the same `resourceFilters` entry. + +```yaml +orLabelSelectors: + - matchLabels: + tier: frontend + matchExpressions: + - key: track + operator: In + values: [canary] + - matchLabels: + tier: backend +``` + #### Peek-and-Map Fallback for Unresolved Kinds The `kinds` field accepts both plural resource names (e.g., `configmaps`, `mycustomkinds.mygroup.io`) and singular `Kind` names (e.g., `ConfigMap`, `MyCustomKind`). @@ -382,9 +417,10 @@ resourceFilters: resourceFilters: - kinds: ["Deployment"] labelSelector: - "invalid label key!": "value" # invalid key syntax + matchLabels: + "invalid label key!": "value" # invalid key syntax ``` -**Behavior:** Validation error during restore creation when `labels.ValidatedSelectorFromSet()` fails: +**Behavior:** Validation error during restore creation when `metav1.LabelSelectorAsSelector()` fails: ``` namespacedFilterPolicies[0].resourceFilters[0]: invalid label selector: "invalid label key!" is not a valid label key ``` @@ -420,7 +456,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret] # Secret listed here is ineffective — globally excluded labelSelector: - app: my-app + matchLabels: + app: my-app - kinds: [Deployment] ``` @@ -461,8 +498,8 @@ After existing filter setup, the filter policies are resolved into the runtime m The `resolveRestoreNamespacedFilterPolicies` function: - For each `NamespacedFilterPolicy`, iterates its `ResourceFilters` entries - Resolves kind names to fully-qualified group-resource strings using the discovery helper -- Converts `labelSelector` maps into `labels.Selector` objects using `labels.ValidatedSelectorFromSet()` -- Converts `orLabelSelectors` maps into `[]labels.Selector` +- Converts `labelSelector` into a `labels.Selector` via `ToMetaV1LabelSelector` + `metav1.LabelSelectorAsSelector()` +- Converts `orLabelSelectors` into `[]labels.Selector` the same way - Creates `IncludesExcludes` instances for `names`/`excludedNames` patterns - Identifies catch-all entries (empty or `["*"]` kinds) and stores them in `catchAllFilter` - Builds a `resourceFilterMap` keyed by the resolved group-resource string @@ -537,7 +574,8 @@ data: resourceFilters: - kinds: [Deployment, ConfigMap] labelSelector: - app: my-app + matchLabels: + app: my-app # ns-b has no filter policy entry, so global filters apply (restore everything) ``` @@ -631,7 +669,8 @@ data: names: [db-credentials, tls-cert] # these exact Secrets by name - kinds: ["*"] # catch-all for all other kinds labelSelector: - backup: "true" # restore by label + matchLabels: + backup: "true" # restore by label ``` **Result:** @@ -658,7 +697,8 @@ data: names: ["my-app-*"] - kinds: [CustomResourceDefinition] labelSelector: - app: my-app + matchLabels: + app: my-app namespacedFilterPolicies: - namespaces: - production diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 235f48ed5..39504d6ff 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -21,12 +21,13 @@ import ( "fmt" "strings" - "k8s.io/apimachinery/pkg/util/sets" - "github.com/cockroachdb/errors" "github.com/gobwas/glob" "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" crclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -100,13 +101,66 @@ func (a *Action) GetDataMover() (string, error) { return dataMover, nil } +// PolicyLabelSelector mirrors metav1.LabelSelector with yaml tags for ConfigMap decode. +// metav1.LabelSelector only has json tags, which do not populate under go.yaml.in/yaml/v3. +type PolicyLabelSelector struct { + MatchLabels map[string]string `yaml:"matchLabels,omitempty"` + MatchExpressions []PolicyLabelSelectorRequirement `yaml:"matchExpressions,omitempty"` +} + +// PolicyLabelSelectorRequirement mirrors metav1.LabelSelectorRequirement with yaml tags. +type PolicyLabelSelectorRequirement struct { + Key string `yaml:"key"` + Operator string `yaml:"operator"` + Values []string `yaml:"values,omitempty"` +} + +// IsPresentLabelSelector reports whether s defines any label constraints. +// Empty {} (nil MatchLabels and empty MatchExpressions) is treated as absent. +func IsPresentLabelSelector(s *PolicyLabelSelector) bool { + return s != nil && (len(s.MatchLabels) > 0 || len(s.MatchExpressions) > 0) +} + +// ToMetaV1LabelSelector converts the YAML mirror type to metav1.LabelSelector. +// Conversion itself is infallible; call LabelSelectorAsSelector (or +// SelectorFromPolicyLabelSelector) to validate operators and values. +func ToMetaV1LabelSelector(s *PolicyLabelSelector) *metav1.LabelSelector { + if s == nil { + return nil + } + ls := &metav1.LabelSelector{MatchLabels: s.MatchLabels} + for _, expr := range s.MatchExpressions { + ls.MatchExpressions = append(ls.MatchExpressions, metav1.LabelSelectorRequirement{ + Key: expr.Key, + Operator: metav1.LabelSelectorOperator(expr.Operator), + Values: expr.Values, + }) + } + return ls +} + +// SelectorFromPolicyLabelSelector converts a present policy label selector to a +// runtime labels.Selector. Returns (nil, nil) when s defines no constraints. +func SelectorFromPolicyLabelSelector(s *PolicyLabelSelector) (labels.Selector, error) { + if !IsPresentLabelSelector(s) { + return nil, nil + } + return metav1.LabelSelectorAsSelector(ToMetaV1LabelSelector(s)) +} + +// validatePolicyLabelSelector converts and validates a policy label selector. +func validatePolicyLabelSelector(s *PolicyLabelSelector) error { + _, err := SelectorFromPolicyLabelSelector(s) + return err +} + // ResourceFilter defines a filter for specific resource kinds. type ResourceFilter struct { - Kinds []string `yaml:"kinds"` - LabelSelector map[string]string `yaml:"labelSelector,omitempty"` - OrLabelSelectors []map[string]string `yaml:"orLabelSelectors,omitempty"` - Names []string `yaml:"names,omitempty"` - ExcludedNames []string `yaml:"excludedNames,omitempty"` + Kinds []string `yaml:"kinds"` + LabelSelector *PolicyLabelSelector `yaml:"labelSelector,omitempty"` + OrLabelSelectors []*PolicyLabelSelector `yaml:"orLabelSelectors,omitempty"` + Names []string `yaml:"names,omitempty"` + ExcludedNames []string `yaml:"excludedNames,omitempty"` } // IsCatchAll returns true if the filter is a catch-all entry (empty kinds or ["*"]) @@ -605,9 +659,17 @@ func (p *Policies) validateNamespacedFilterPolicies() error { seenKinds[kind] = j } - if len(rf.LabelSelector) > 0 && len(rf.OrLabelSelectors) > 0 { + if IsPresentLabelSelector(rf.LabelSelector) && len(rf.OrLabelSelectors) > 0 { return fmt.Errorf("namespacedFilterPolicies[%d].resourceFilters[%d]: labelSelector and orLabelSelectors cannot co-exist", i, j) } + if err := validatePolicyLabelSelector(rf.LabelSelector); err != nil { + return fmt.Errorf("namespacedFilterPolicies[%d].resourceFilters[%d]: invalid label selector: %w", i, j, err) + } + for k, ols := range rf.OrLabelSelectors { + if err := validatePolicyLabelSelector(ols); err != nil { + return fmt.Errorf("namespacedFilterPolicies[%d].resourceFilters[%d].orLabelSelectors[%d]: invalid label selector: %w", i, j, k, err) + } + } // Validate glob patterns for names and excludedNames using gobwas/glob for k, pattern := range rf.Names { @@ -657,9 +719,17 @@ func (p *Policies) validateClusterScopedFilterPolicy() error { seenKinds[kind] = j } - if len(rf.LabelSelector) > 0 && len(rf.OrLabelSelectors) > 0 { + if IsPresentLabelSelector(rf.LabelSelector) && len(rf.OrLabelSelectors) > 0 { return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d]: labelSelector and orLabelSelectors cannot co-exist", j) } + if err := validatePolicyLabelSelector(rf.LabelSelector); err != nil { + return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d]: invalid label selector: %w", j, err) + } + for k, ols := range rf.OrLabelSelectors { + if err := validatePolicyLabelSelector(ols); err != nil { + return fmt.Errorf("clusterScopedFilterPolicy.resourceFilters[%d].orLabelSelectors[%d]: invalid label selector: %w", j, k, err) + } + } for k, pattern := range rf.Names { if _, err := glob.Compile(pattern); err != nil { diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 445b479f0..7a7da6d3d 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -25,6 +25,7 @@ import ( corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -2027,7 +2028,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["Pod", "ConfigMap"] labelSelector: - app: web + matchLabels: + app: web names: ["app-*"] - kinds: ["Secret"] excludedNames: ["temp-*"]`, @@ -2041,8 +2043,10 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["Pod"] orLabelSelectors: - - env: prod - - env: staging`, + - matchLabels: + env: prod + - matchLabels: + env: staging`, wantErr: false, }, { @@ -2084,7 +2088,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["*"] labelSelector: - app: web`, + matchLabels: + app: web`, wantErr: false, }, { @@ -2095,10 +2100,12 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["*"] labelSelector: - app: web + matchLabels: + app: web - kinds: ["*"] labelSelector: - app: db`, + matchLabels: + app: db`, wantErr: true, errMsg: "only one catch-all resource filter is allowed", }, @@ -2110,10 +2117,12 @@ namespacedFilterPolicies: resourceFilters: - kinds: [] labelSelector: - app: web + matchLabels: + app: web - kinds: ["*"] labelSelector: - app: db`, + matchLabels: + app: db`, wantErr: true, errMsg: "only one catch-all resource filter is allowed", }, @@ -2125,10 +2134,12 @@ namespacedFilterPolicies: resourceFilters: - kinds: [] labelSelector: - app: web + matchLabels: + app: web - kinds: [] labelSelector: - app: db`, + matchLabels: + app: db`, wantErr: true, errMsg: "only one catch-all resource filter is allowed", }, @@ -2141,7 +2152,8 @@ namespacedFilterPolicies: - kinds: [] names: ["app-*"] labelSelector: - app: web`, + matchLabels: + app: web`, wantErr: true, errMsg: "names or excludedNames cannot be specified for catch-all filters", }, @@ -2154,7 +2166,8 @@ namespacedFilterPolicies: - kinds: [] excludedNames: ["app-*"] labelSelector: - app: web`, + matchLabels: + app: web`, wantErr: true, errMsg: "names or excludedNames cannot be specified for catch-all filters", }, @@ -2186,9 +2199,11 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["Pod"] labelSelector: - app: web + matchLabels: + app: web orLabelSelectors: - - env: prod`, + - matchLabels: + env: prod`, wantErr: true, errMsg: "labelSelector and orLabelSelectors cannot co-exist", }, @@ -2272,7 +2287,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["Pod"] labelSelector: - app: web` + matchLabels: + app: web` resPolicies, err := unmarshalResourcePolicies(&yamlData) require.NoError(t, err) @@ -2290,7 +2306,135 @@ namespacedFilterPolicies: rf := policy.ResourceFilters[0] assert.Equal(t, []string{"Pod"}, rf.Kinds) - assert.Equal(t, map[string]string{"app": "web"}, rf.LabelSelector) + assert.Equal(t, &PolicyLabelSelector{MatchLabels: map[string]string{"app": "web"}}, rf.LabelSelector) +} + +func TestPolicyLabelSelectorSetBased(t *testing.T) { + t.Run("yaml decode matchLabels and matchExpressions", func(t *testing.T) { + yamlData := `version: v1 +namespacedFilterPolicies: +- namespaces: ["ns1"] + resourceFilters: + - kinds: ["Pod"] + labelSelector: + matchLabels: + app: web + matchExpressions: + - key: environment + operator: In + values: [prod, staging] + - key: do-not-backup + operator: DoesNotExist` + + resPolicies, err := unmarshalResourcePolicies(&yamlData) + require.NoError(t, err) + + policies := &Policies{} + require.NoError(t, policies.BuildPolicy(resPolicies)) + require.NoError(t, policies.Validate()) + + rf := policies.GetNamespacedFilterPolicies()[0].ResourceFilters[0] + require.NotNil(t, rf.LabelSelector) + assert.Equal(t, map[string]string{"app": "web"}, rf.LabelSelector.MatchLabels) + require.Len(t, rf.LabelSelector.MatchExpressions, 2) + assert.Equal(t, "environment", rf.LabelSelector.MatchExpressions[0].Key) + assert.Equal(t, "In", rf.LabelSelector.MatchExpressions[0].Operator) + assert.Equal(t, []string{"prod", "staging"}, rf.LabelSelector.MatchExpressions[0].Values) + assert.Equal(t, "do-not-backup", rf.LabelSelector.MatchExpressions[1].Key) + assert.Equal(t, "DoesNotExist", rf.LabelSelector.MatchExpressions[1].Operator) + }) + + t.Run("empty labelSelector is no filter", func(t *testing.T) { + yamlData := `version: v1 +namespacedFilterPolicies: +- namespaces: ["ns1"] + resourceFilters: + - kinds: ["Pod"] + labelSelector: {}` + + resPolicies, err := unmarshalResourcePolicies(&yamlData) + require.NoError(t, err) + + policies := &Policies{} + require.NoError(t, policies.BuildPolicy(resPolicies)) + require.NoError(t, policies.Validate()) + + rf := policies.GetNamespacedFilterPolicies()[0].ResourceFilters[0] + assert.False(t, IsPresentLabelSelector(rf.LabelSelector)) + }) + + t.Run("invalid operator rejected", func(t *testing.T) { + yamlData := `version: v1 +namespacedFilterPolicies: +- namespaces: ["ns1"] + resourceFilters: + - kinds: ["Pod"] + labelSelector: + matchExpressions: + - key: environment + operator: Equals + values: [prod]` + + resPolicies, err := unmarshalResourcePolicies(&yamlData) + require.NoError(t, err) + + policies := &Policies{} + require.NoError(t, policies.BuildPolicy(resPolicies)) + err = policies.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid label selector") + }) + + t.Run("NotIn Exists operators validate", func(t *testing.T) { + yamlData := `version: v1 +clusterScopedFilterPolicy: + resourceFilters: + - kinds: ["ClusterRole"] + labelSelector: + matchExpressions: + - key: tier + operator: NotIn + values: [debug] + - key: managed-by + operator: Exists` + + resPolicies, err := unmarshalResourcePolicies(&yamlData) + require.NoError(t, err) + + policies := &Policies{} + require.NoError(t, policies.BuildPolicy(resPolicies)) + require.NoError(t, policies.Validate()) + }) + + t.Run("ToMetaV1LabelSelector and IsPresentLabelSelector", func(t *testing.T) { + assert.False(t, IsPresentLabelSelector(nil)) + assert.False(t, IsPresentLabelSelector(&PolicyLabelSelector{})) + assert.True(t, IsPresentLabelSelector(&PolicyLabelSelector{MatchLabels: map[string]string{"a": "b"}})) + + ls := ToMetaV1LabelSelector(&PolicyLabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + MatchExpressions: []PolicyLabelSelectorRequirement{ + {Key: "env", Operator: "In", Values: []string{"prod"}}, + }, + }) + require.NotNil(t, ls) + assert.Equal(t, map[string]string{"app": "web"}, ls.MatchLabels) + require.Len(t, ls.MatchExpressions, 1) + assert.Equal(t, metav1.LabelSelectorOpIn, ls.MatchExpressions[0].Operator) + + assert.Nil(t, ToMetaV1LabelSelector(nil)) + + sel, err := SelectorFromPolicyLabelSelector(&PolicyLabelSelector{ + MatchLabels: map[string]string{"app": "web"}, + }) + require.NoError(t, err) + require.NotNil(t, sel) + assert.True(t, sel.Matches(labels.Set{"app": "web"})) + + emptySel, err := SelectorFromPolicyLabelSelector(&PolicyLabelSelector{}) + require.NoError(t, err) + assert.Nil(t, emptySel) + }) } func TestClusterScopedFilterPoliciesAccessor(t *testing.T) { @@ -2394,7 +2538,8 @@ clusterScopedFilterPolicy: resourceFilters: - kinds: ["ClusterRole", "ClusterRoleBinding"] labelSelector: - app: my-app`, + matchLabels: + app: my-app`, wantErr: false, }, { @@ -2404,8 +2549,10 @@ clusterScopedFilterPolicy: resourceFilters: - kinds: ["CustomResourceDefinition"] orLabelSelectors: - - app: my-app - - app: other-app`, + - matchLabels: + app: my-app + - matchLabels: + app: other-app`, wantErr: false, }, { @@ -2443,7 +2590,8 @@ clusterScopedFilterPolicy: resourceFilters: - kinds: ["*"] labelSelector: - app: my-app`, + matchLabels: + app: my-app`, wantErr: true, errMsg: "kinds must be specified", }, @@ -2456,7 +2604,8 @@ clusterScopedFilterPolicy: names: ["my-app-*"] - kinds: ["ClusterRole"] labelSelector: - app: other`, + matchLabels: + app: other`, wantErr: true, errMsg: `kind "ClusterRole" appears in both`, }, @@ -2467,9 +2616,11 @@ clusterScopedFilterPolicy: resourceFilters: - kinds: ["ClusterRole"] labelSelector: - app: my-app + matchLabels: + app: my-app orLabelSelectors: - - app: other`, + - matchLabels: + app: other`, wantErr: true, errMsg: "labelSelector and orLabelSelectors cannot co-exist", }, diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index dc60bba8c..30eb26a36 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -1428,22 +1428,20 @@ func resolveClusterScopedFilterPolicy( } func resolveResourceFilter(rf resourcepolicies.ResourceFilter) (*ResolvedResourceFilter, error) { - var selector labels.Selector - if len(rf.LabelSelector) > 0 { - var err error - selector, err = labels.ValidatedSelectorFromSet(labels.Set(rf.LabelSelector)) - if err != nil { - return nil, fmt.Errorf("invalid label selector in resource filter: %w", err) - } + selector, err := resourcepolicies.SelectorFromPolicyLabelSelector(rf.LabelSelector) + if err != nil { + return nil, fmt.Errorf("invalid label selector in resource filter: %w", err) } var orSelectors []labels.Selector for _, ols := range rf.OrLabelSelectors { - s, err := labels.ValidatedSelectorFromSet(labels.Set(ols)) + s, err := resourcepolicies.SelectorFromPolicyLabelSelector(ols) if err != nil { return nil, fmt.Errorf("invalid OR label selector in resource filter: %w", err) } - orSelectors = append(orSelectors, s) + if s != nil { + orSelectors = append(orSelectors, s) + } } var nameIE *collections.IncludesExcludes diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 56f4aaf33..3baae0131 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -5741,7 +5741,7 @@ func TestResolveResourceFilter(t *testing.T) { { name: "valid label selector", rf: resourcepolicies.ResourceFilter{ - LabelSelector: map[string]string{"app": "foo"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"app": "foo"}}, }, expectErr: false, checkResult: func(t *testing.T, r *ResolvedResourceFilter) { @@ -5754,16 +5754,16 @@ func TestResolveResourceFilter(t *testing.T) { { name: "invalid label selector", rf: resourcepolicies.ResourceFilter{ - LabelSelector: map[string]string{"invalid/label/key": "value"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, expectErr: true, }, { name: "valid or label selectors", rf: resourcepolicies.ResourceFilter{ - OrLabelSelectors: []map[string]string{ - {"app": "foo"}, - {"app": "bar"}, + OrLabelSelectors: []*resourcepolicies.PolicyLabelSelector{ + {MatchLabels: map[string]string{"app": "foo"}}, + {MatchLabels: map[string]string{"app": "bar"}}, }, }, expectErr: false, @@ -5776,8 +5776,8 @@ func TestResolveResourceFilter(t *testing.T) { { name: "invalid or label selectors", rf: resourcepolicies.ResourceFilter{ - OrLabelSelectors: []map[string]string{ - {"invalid/label/key": "value"}, + OrLabelSelectors: []*resourcepolicies.PolicyLabelSelector{ + {MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, }, expectErr: true, @@ -5797,6 +5797,68 @@ func TestResolveResourceFilter(t *testing.T) { assert.False(t, r.NameIE.ShouldInclude("exc1")) }, }, + { + name: "empty labelSelector is no filter", + rf: resourcepolicies.ResourceFilter{ + LabelSelector: &resourcepolicies.PolicyLabelSelector{}, + }, + expectErr: false, + checkResult: func(t *testing.T, r *ResolvedResourceFilter) { + t.Helper() + require.NotNil(t, r) + assert.Nil(t, r.LabelSelector) + }, + }, + { + name: "set-based In and DoesNotExist", + rf: resourcepolicies.ResourceFilter{ + LabelSelector: &resourcepolicies.PolicyLabelSelector{ + MatchExpressions: []resourcepolicies.PolicyLabelSelectorRequirement{ + {Key: "environment", Operator: "In", Values: []string{"prod", "staging"}}, + {Key: "do-not-backup", Operator: "DoesNotExist"}, + }, + }, + }, + expectErr: false, + checkResult: func(t *testing.T, r *ResolvedResourceFilter) { + t.Helper() + require.NotNil(t, r.LabelSelector) + assert.True(t, r.LabelSelector.Matches(labels.Set{"environment": "prod"})) + assert.True(t, r.LabelSelector.Matches(labels.Set{"environment": "staging"})) + assert.False(t, r.LabelSelector.Matches(labels.Set{"environment": "dev"})) + assert.False(t, r.LabelSelector.Matches(labels.Set{"environment": "prod", "do-not-backup": "true"})) + }, + }, + { + name: "set-based NotIn and Exists", + rf: resourcepolicies.ResourceFilter{ + LabelSelector: &resourcepolicies.PolicyLabelSelector{ + MatchExpressions: []resourcepolicies.PolicyLabelSelectorRequirement{ + {Key: "tier", Operator: "NotIn", Values: []string{"debug"}}, + {Key: "app", Operator: "Exists"}, + }, + }, + }, + expectErr: false, + checkResult: func(t *testing.T, r *ResolvedResourceFilter) { + t.Helper() + require.NotNil(t, r.LabelSelector) + assert.True(t, r.LabelSelector.Matches(labels.Set{"app": "web", "tier": "frontend"})) + assert.False(t, r.LabelSelector.Matches(labels.Set{"app": "web", "tier": "debug"})) + assert.False(t, r.LabelSelector.Matches(labels.Set{"tier": "frontend"})) + }, + }, + { + name: "invalid operator", + rf: resourcepolicies.ResourceFilter{ + LabelSelector: &resourcepolicies.PolicyLabelSelector{ + MatchExpressions: []resourcepolicies.PolicyLabelSelectorRequirement{ + {Key: "env", Operator: "Equals", Values: []string{"prod"}}, + }, + }, + }, + expectErr: true, + }, } for _, tc := range tests { @@ -5834,11 +5896,11 @@ func TestResolveClusterScopedFilterPolicy(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods", "secrets"}, - LabelSelector: map[string]string{"app": "foo"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"app": "foo"}}, }, { Kinds: []string{"invalid-kind"}, - LabelSelector: map[string]string{"invalid/label/key": "value"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, }, } @@ -5852,7 +5914,7 @@ func TestResolveClusterScopedFilterPolicy(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods", "secrets"}, - LabelSelector: map[string]string{"app": "foo"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"app": "foo"}}, }, }, } @@ -5900,11 +5962,11 @@ func TestResolveNamespacedFilterPolicies(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods"}, - LabelSelector: map[string]string{"app": "foo"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"app": "foo"}}, }, { Kinds: []string{"*"}, - LabelSelector: map[string]string{"catch": "all"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"catch": "all"}}, }, }, }, @@ -5932,7 +5994,7 @@ func TestResolveNamespacedFilterPolicies(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods"}, - LabelSelector: map[string]string{"invalid/label/key": "value"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, }, }, @@ -6016,7 +6078,7 @@ func TestBackupWithResPoliciesLogs(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods"}, - LabelSelector: map[string]string{"invalid/label/key": "value"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, }, } @@ -6035,7 +6097,7 @@ func TestBackupWithResPoliciesLogs(t *testing.T) { ResourceFilters: []resourcepolicies.ResourceFilter{ { Kinds: []string{"pods"}, - LabelSelector: map[string]string{"invalid/label/key": "value"}, + LabelSelector: &resourcepolicies.PolicyLabelSelector{MatchLabels: map[string]string{"invalid/label/key": "value"}}, }, }, }, diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 4c8222f81..445ce3df5 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -21,7 +21,6 @@ import ( "context" "encoding/json" "fmt" - "io" "sort" "strconv" "strings" @@ -31,7 +30,6 @@ import ( "github.com/cockroachdb/errors" snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1" - "github.com/sirupsen/logrus" "github.com/fatih/color" kbclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -94,9 +92,6 @@ func DescribeBackup( if backup.Spec.ResourcePolicy != nil { d.Println() DescribeResourcePolicies(d, backup.Spec.ResourcePolicy) - - // Display fine-grained filter policies if they exist - DescribeFineGrainedFilterPolicies(ctx, kbClient, d, backup) } DescribeGlobalVolumePolicy(d, backup) @@ -151,119 +146,6 @@ func DescribeGlobalVolumePolicy(d *Describer, backup *velerov1api.Backup) { d.Printf("\tName:\t%s\n", name) } -// DescribeFineGrainedFilterPolicies describes cluster-scoped and namespace-scoped filter policies if present -func DescribeFineGrainedFilterPolicies(ctx context.Context, kbClient kbclient.Client, d *Describer, backup *velerov1api.Backup) { - if backup.Spec.ResourcePolicy == nil { - return - } - - // Create a discard logger for the resource policies function since this is CLI output context - discardLogger := logrus.New() - discardLogger.Out = io.Discard - - resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*backup, kbClient, discardLogger) - if err != nil { - // Don't fail the describe if we can't read policies, just skip - return - } - - if resourcePolicies == nil { - return - } - - clusterScopedFilterPolicy := resourcePolicies.GetClusterScopedFilterPolicy() - if clusterScopedFilterPolicy != nil { - d.Printf("\nCluster Scoped Filter Policy:\n") - d.Printf(" Resource Filters:\n") - for _, rf := range clusterScopedFilterPolicy.ResourceFilters { - kindsStr := strings.Join(rf.Kinds, ", ") - d.Printf(" %s:\n", kindsStr) - - // Label selector - if len(rf.LabelSelector) > 0 { - selectorStr := formatLabelMap(rf.LabelSelector) - d.Printf(" Label selector: %s\n", selectorStr) - } else if len(rf.OrLabelSelectors) > 0 { - var orStrs []string - for _, ols := range rf.OrLabelSelectors { - orStrs = append(orStrs, formatLabelMap(ols)) - } - d.Printf(" OR label selectors: [%s]\n", strings.Join(orStrs, ", ")) - } else { - d.Printf(" Label selector: \n") - } - - // Name patterns - if len(rf.Names) > 0 { - d.Printf(" Included names: [%s]\n", strings.Join(rf.Names, ", ")) - } else { - d.Printf(" Included names: \n") - } - - if len(rf.ExcludedNames) > 0 { - d.Printf(" Excluded names: [%s]\n", strings.Join(rf.ExcludedNames, ", ")) - } else { - d.Printf(" Excluded names: \n") - } - } - } - - nfPolicies := resourcePolicies.GetNamespacedFilterPolicies() - if len(nfPolicies) > 0 { - d.Printf("\nNamespace-Scoped Filter Policies:\n") - for _, policy := range nfPolicies { - for _, ns := range policy.Namespaces { - d.Printf(" %s:\n", ns) - d.Printf(" Resource Filters:\n") - for _, rf := range policy.ResourceFilters { - var kindsStr string - if rf.IsCatchAll() { - kindsStr = " (all other kinds)" - } else { - kindsStr = strings.Join(rf.Kinds, ", ") - } - d.Printf(" %s:\n", kindsStr) - - // Label selector - if len(rf.LabelSelector) > 0 { - selectorStr := formatLabelMap(rf.LabelSelector) - d.Printf(" Label selector: %s\n", selectorStr) - } else if len(rf.OrLabelSelectors) > 0 { - var orStrs []string - for _, ols := range rf.OrLabelSelectors { - orStrs = append(orStrs, formatLabelMap(ols)) - } - d.Printf(" OR label selectors: [%s]\n", strings.Join(orStrs, ", ")) - } else { - d.Printf(" Label selector: \n") - } - - // Name patterns - if len(rf.Names) > 0 { - d.Printf(" Included names: [%s]\n", strings.Join(rf.Names, ", ")) - } else { - d.Printf(" Included names: \n") - } - - if len(rf.ExcludedNames) > 0 { - d.Printf(" Excluded names: [%s]\n", strings.Join(rf.ExcludedNames, ", ")) - } else { - d.Printf(" Excluded names: \n") - } - } - } - } - } -} - -func formatLabelMap(labelMap map[string]string) string { - var pairs []string - for k, v := range labelMap { - pairs = append(pairs, fmt.Sprintf("%s=%s", k, v)) - } - return strings.Join(pairs, ",") -} - // DescribeUploaderConfigForBackup describes uploader config in human-readable format func DescribeUploaderConfigForBackup(d *Describer, spec velerov1api.BackupSpec) { d.Printf("Uploader config:\n") diff --git a/pkg/cmd/util/output/backup_describer_test.go b/pkg/cmd/util/output/backup_describer_test.go index 248b0a45b..da28f6c87 100644 --- a/pkg/cmd/util/output/backup_describer_test.go +++ b/pkg/cmd/util/output/backup_describer_test.go @@ -18,7 +18,6 @@ package output import ( "bytes" - "context" "testing" "text/tabwriter" "time" @@ -26,8 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/velero/internal/volume" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -897,85 +894,3 @@ func TestDescribeBackupItemOperation(t *testing.T) { d.out.Flush() assert.Equal(t, expected, d.buf.String()) } - -func TestDescribeFineGrainedFilterPolicies(t *testing.T) { - yamlData := ` -version: v1 -clusterScopedFilterPolicy: - resourceFilters: - - kinds: ["StorageClass"] - labelSelector: {"app": "velero"} - - kinds: ["ClusterRole"] - orLabelSelectors: - - {"app": "velero"} - - {"app": "test"} - names: ["role1"] - excludedNames: ["role2"] -namespacedFilterPolicies: -- namespaces: ["ns1", "ns2"] - resourceFilters: - - kinds: ["Pod", "ConfigMap"] - labelSelector: {"app": "velero"} - - kinds: ["*"] -` - cm := &corev1api.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "velero", - }, - Data: map[string]string{ - "policy.yaml": yamlData, - }, - } - - client := fake.NewClientBuilder().WithRuntimeObjects(cm).Build() - - backup := builder.ForBackup("velero", "test-backup"). - ResourcePolicies("test-policy").Result() - - d := &Describer{ - Prefix: "", - out: &tabwriter.Writer{}, - buf: &bytes.Buffer{}, - } - d.out.Init(d.buf, 0, 8, 2, ' ', 0) - - DescribeFineGrainedFilterPolicies(context.Background(), client, d, backup) - d.out.Flush() - - expected := ` -Cluster Scoped Filter Policy: - Resource Filters: - StorageClass: - Label selector: app=velero - Included names: - Excluded names: - ClusterRole: - OR label selectors: [app=velero, app=test] - Included names: [role1] - Excluded names: [role2] - -Namespace-Scoped Filter Policies: - ns1: - Resource Filters: - Pod, ConfigMap: - Label selector: app=velero - Included names: - Excluded names: - (all other kinds): - Label selector: - Included names: - Excluded names: - ns2: - Resource Filters: - Pod, ConfigMap: - Label selector: app=velero - Included names: - Excluded names: - (all other kinds): - Label selector: - Included names: - Excluded names: -` - assert.Equal(t, expected, d.buf.String()) -} diff --git a/pkg/cmd/util/output/backup_structured_describer.go b/pkg/cmd/util/output/backup_structured_describer.go index dfffcda06..b2541df4b 100644 --- a/pkg/cmd/util/output/backup_structured_describer.go +++ b/pkg/cmd/util/output/backup_structured_describer.go @@ -21,10 +21,8 @@ import ( "context" "encoding/json" "fmt" - "io" "strings" - "github.com/sirupsen/logrus" corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -57,7 +55,6 @@ func DescribeBackupInSF( if backup.Spec.ResourcePolicy != nil { DescribeResourcePoliciesInSF(d, backup.Spec.ResourcePolicy) - DescribeFineGrainedFilterPoliciesInSF(ctx, kbClient, d, backup) } DescribeGlobalVolumePolicyInSF(d, backup) @@ -228,88 +225,6 @@ func DescribeBackupSpecInSF(d *StructuredDescriber, spec velerov1api.BackupSpec) d.Describe("spec", backupSpecInfo) } -// DescribeFineGrainedFilterPoliciesInSF adds the clusterScopedFilterPolicy -// and namespacedFilterPolicies sections to the structured describer output when present -// in the ResourcePolicy ConfigMap referenced by the backup. -func DescribeFineGrainedFilterPoliciesInSF(ctx context.Context, kbClient kbclient.Client, d *StructuredDescriber, backup *velerov1api.Backup) { - if backup.Spec.ResourcePolicy == nil { - return - } - - discardLogger := logrus.New() - discardLogger.Out = io.Discard - - resPolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(*backup, kbClient, discardLogger) - if err != nil || resPolicies == nil { - return - } - - clusterScopedFilterPolicy := resPolicies.GetClusterScopedFilterPolicy() - if clusterScopedFilterPolicy != nil { - var clusterScopedFilters []map[string]any - for _, rf := range clusterScopedFilterPolicy.ResourceFilters { - entry := map[string]any{ - "kinds": rf.Kinds, - } - if len(rf.LabelSelector) > 0 { - entry["labelSelector"] = rf.LabelSelector - } - if len(rf.OrLabelSelectors) > 0 { - entry["orLabelSelectors"] = rf.OrLabelSelectors - } - if len(rf.Names) > 0 { - entry["names"] = rf.Names - } - if len(rf.ExcludedNames) > 0 { - entry["excludedNames"] = rf.ExcludedNames - } - clusterScopedFilters = append(clusterScopedFilters, entry) - } - d.Describe("clusterScopedFilterPolicy", map[string]any{ - "resourceFilters": clusterScopedFilters, - }) - } - - nfPolicies := resPolicies.GetNamespacedFilterPolicies() - if len(nfPolicies) == 0 { - return - } - - var structuredPolicies []map[string]any - for _, policy := range nfPolicies { - for _, ns := range policy.Namespaces { - var rfEntries []map[string]any - for _, rf := range policy.ResourceFilters { - entry := map[string]any{} - if rf.IsCatchAll() { - entry["kinds"] = []string{} - entry["isCatchAll"] = true - } else { - entry["kinds"] = rf.Kinds - } - if len(rf.LabelSelector) > 0 { - entry["labelSelector"] = rf.LabelSelector - } - if len(rf.OrLabelSelectors) > 0 { - entry["orLabelSelectors"] = rf.OrLabelSelectors - } - if len(rf.Names) > 0 { - entry["names"] = rf.Names - } - if len(rf.ExcludedNames) > 0 { - entry["excludedNames"] = rf.ExcludedNames - } - rfEntries = append(rfEntries, entry) - } - structuredPolicies = append(structuredPolicies, map[string]any{ - "namespace": ns, - "resourceFilters": rfEntries, - }) - } - } - d.Describe("namespacedFilterPolicies", structuredPolicies) -} - // DescribeBackupStatusInSF describes a backup status in structured format. func DescribeBackupStatusInSF(ctx context.Context, kbClient kbclient.Client, d *StructuredDescriber, backup *velerov1api.Backup, details bool, insecureSkipTLSVerify bool, caCertPath string, podVolumeBackups []velerov1api.PodVolumeBackup) { diff --git a/pkg/cmd/util/output/backup_structured_describer_test.go b/pkg/cmd/util/output/backup_structured_describer_test.go index cb46a4676..88af0f95f 100644 --- a/pkg/cmd/util/output/backup_structured_describer_test.go +++ b/pkg/cmd/util/output/backup_structured_describer_test.go @@ -17,7 +17,6 @@ limitations under the License. package output import ( - "context" "reflect" "testing" "time" @@ -25,8 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/velero/internal/volume" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -731,96 +728,3 @@ func TestDescribeDeleteBackupRequestsInSF(t *testing.T) { }) } } - -func TestDescribeFineGrainedFilterPoliciesInSF(t *testing.T) { - yamlData := ` -version: v1 -clusterScopedFilterPolicy: - resourceFilters: - - kinds: ["StorageClass"] - labelSelector: {"app": "velero"} - - kinds: ["ClusterRole"] - orLabelSelectors: - - {"app": "velero"} - - {"app": "test"} - names: ["role1"] - excludedNames: ["role2"] -namespacedFilterPolicies: -- namespaces: ["ns1", "ns2"] - resourceFilters: - - kinds: ["Pod", "ConfigMap"] - labelSelector: {"app": "velero"} - - kinds: ["*"] -` - cm := &corev1api.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "velero", - }, - Data: map[string]string{ - "policy.yaml": yamlData, - }, - } - - client := fake.NewClientBuilder().WithRuntimeObjects(cm).Build() - - backup := builder.ForBackup("velero", "test-backup"). - ResourcePolicies("test-policy").Result() - - sd := &StructuredDescriber{ - output: make(map[string]any), - format: "", - } - - DescribeFineGrainedFilterPoliciesInSF(context.Background(), client, sd, backup) - - expect := map[string]any{ - "clusterScopedFilterPolicy": map[string]any{ - "resourceFilters": []map[string]any{ - { - "kinds": []string{"StorageClass"}, - "labelSelector": map[string]string{"app": "velero"}, - }, - { - "kinds": []string{"ClusterRole"}, - "orLabelSelectors": []map[string]string{ - {"app": "velero"}, - {"app": "test"}, - }, - "names": []string{"role1"}, - "excludedNames": []string{"role2"}, - }, - }, - }, - "namespacedFilterPolicies": []map[string]any{ - { - "namespace": "ns1", - "resourceFilters": []map[string]any{ - { - "kinds": []string{"Pod", "ConfigMap"}, - "labelSelector": map[string]string{"app": "velero"}, - }, - { - "kinds": []string{}, - "isCatchAll": true, - }, - }, - }, - { - "namespace": "ns2", - "resourceFilters": []map[string]any{ - { - "kinds": []string{"Pod", "ConfigMap"}, - "labelSelector": map[string]string{"app": "velero"}, - }, - { - "kinds": []string{}, - "isCatchAll": true, - }, - }, - }, - }, - } - - assert.True(t, reflect.DeepEqual(sd.output, expect)) -} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 8205accb9..bc452b49c 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -638,21 +638,19 @@ func resolveRestoreNamespacedFilterPolicies( func resolveResourceFilter( rf resourcepolicies.ResourceFilter, ) (*resolvedResourceFilter, error) { - var selector labels.Selector - if len(rf.LabelSelector) > 0 { - var err error - selector, err = labels.ValidatedSelectorFromSet(labels.Set(rf.LabelSelector)) - if err != nil { - return nil, fmt.Errorf("invalid label selector in resource filter: %w", err) - } + selector, err := resourcepolicies.SelectorFromPolicyLabelSelector(rf.LabelSelector) + if err != nil { + return nil, fmt.Errorf("invalid label selector in resource filter: %w", err) } var orSelectors []labels.Selector for _, ols := range rf.OrLabelSelectors { - s, err := labels.ValidatedSelectorFromSet(labels.Set(ols)) + s, err := resourcepolicies.SelectorFromPolicyLabelSelector(ols) if err != nil { return nil, fmt.Errorf("invalid OR label selector in resource filter: %w", err) } - orSelectors = append(orSelectors, s) + if s != nil { + orSelectors = append(orSelectors, s) + } } var nameIE *collections.IncludesExcludes if len(rf.Names) > 0 || len(rf.ExcludedNames) > 0 { diff --git a/pkg/restore/restore_policies_test.go b/pkg/restore/restore_policies_test.go index a027f66aa..569d8923d 100644 --- a/pkg/restore/restore_policies_test.go +++ b/pkg/restore/restore_policies_test.go @@ -170,7 +170,8 @@ namespacedFilterPolicies: - kinds: - '*' labelSelector: - app: test + matchLabels: + app: test `, tarball: test.NewTarWriter(t). AddItems("pods", diff --git a/site/content/docs/main/fine-grained-backup-filters.md b/site/content/docs/main/fine-grained-backup-filters.md index d9f90debd..d49cf6c93 100644 --- a/site/content/docs/main/fine-grained-backup-filters.md +++ b/site/content/docs/main/fine-grained-backup-filters.md @@ -67,7 +67,8 @@ data: resourceFilters: - kinds: [ConfigMap] labelSelector: - app: my-app + matchLabels: + app: my-app ``` **Backup:** @@ -158,7 +159,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret, Deployment, Pod] labelSelector: - app: my-app + matchLabels: + app: my-app ``` **Backup:** @@ -197,7 +199,8 @@ namespacedFilterPolicies: - kinds: [ConfigMap] names: [vm-1, vm-2] labelSelector: - resource-type: VirtualMachine + matchLabels: + resource-type: VirtualMachine ``` **Backup:** `includedNamespaces: [target-namespace]` plus `resourcePolicy` reference. @@ -250,15 +253,62 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap] orLabelSelectors: - - app: production-workload-1 - component: vm-group - - app: production-workload-2 - component: vm-service + - matchLabels: + app: production-workload-1 + component: vm-group + - matchLabels: + app: production-workload-2 + component: vm-service ``` **Expected outcome:** ConfigMaps matching either label combination are backed up; other ConfigMaps in the namespace are not (for this kind). -**Note:** Use `orLabelSelectors` when you need OR across label sets. `labelSelector` and `orLabelSelectors` cannot appear in the same `resourceFilters` entry. +**Note:** Prefer `matchExpressions` with `In` for value-OR on a single key (see next example). Use `orLabelSelectors` when you need OR across **independent multi-key groups**. `labelSelector` and `orLabelSelectors` cannot appear in the same `resourceFilters` entry. + +--- + +### Example 4b — Set-based label selectors (`matchExpressions`) + +**Goal:** Back up Deployments and Pods that are in `prod` or `staging`, belong to `app=my-app`, and do **not** carry a skip label. + +**Policy:** + +```yaml +version: v1 +namespacedFilterPolicies: + - namespaces: + - production + resourceFilters: + - kinds: [Deployment, Pod] + labelSelector: + matchLabels: + app: my-app + matchExpressions: + - key: environment + operator: In + values: [prod, staging] + - key: do-not-backup + operator: DoesNotExist +``` + +**Supported operators:** `In`, `NotIn`, `Exists`, `DoesNotExist` (same as Kubernetes / Velero global `--selector`). + +**Other useful patterns:** + +```yaml +# Exclude environments +matchExpressions: + - key: environment + operator: NotIn + values: [dev, test] + +# Require a label key to be present (any value) +matchExpressions: + - key: tier + operator: Exists +``` + +**Expected outcome:** Only Deployments/Pods with `app=my-app`, `environment` in `{prod, staging}`, and without `do-not-backup` are backed up. --- @@ -276,16 +326,21 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret] orLabelSelectors: - - app: my-app - - app: monitoring + - matchLabels: + app: my-app + - matchLabels: + app: monitoring - kinds: [Deployment] orLabelSelectors: - - app: my-app - - app: monitoring - - component: backend + - matchLabels: + app: my-app + - matchLabels: + app: monitoring + - matchLabels: + component: backend ``` -**Expected outcome:** Resources included if they match **any** map in `orLabelSelectors` for their kind (AND within each map, OR across maps). +**Expected outcome:** Resources included if they match **any** selector in `orLabelSelectors` for their kind (AND within each selector, OR across the list). --- @@ -304,9 +359,12 @@ namespacedFilterPolicies: - kinds: [ConfigMap] names: [vm-1, vm-2] orLabelSelectors: - - resource-type: VirtualMachine - - component: vm-group - - component: vm-service + - matchLabels: + resource-type: VirtualMachine + - matchLabels: + component: vm-group + - matchLabels: + component: vm-service ``` **Expected outcome:** Only `vm-1` and `vm-2` that also satisfy one of the label OR branches. @@ -330,7 +388,8 @@ namespacedFilterPolicies: - kinds: [ConfigMap] - kinds: [Deployment] labelSelector: - tier: web + matchLabels: + tier: web ``` **Expected outcome:** @@ -393,10 +452,12 @@ namespacedFilterPolicies: resourceFilters: - kinds: ["*"] # catch-all labelSelector: - app: common-app + matchLabels: + app: common-app - kinds: [ConfigMap, Secret] # override for these kinds labelSelector: - app: specialized-app + matchLabels: + app: specialized-app ``` **Equivalent:** `kinds: []` (empty) also denotes a catch-all; `kinds: ["*"]` is preferred for readability. @@ -429,7 +490,8 @@ namespacedFilterPolicies: names: [db-credentials, tls-cert] - kinds: ["*"] labelSelector: - backup: "true" + matchLabels: + backup: "true" ``` **Expected outcome:** @@ -482,7 +544,8 @@ clusterScopedFilterPolicy: names: ["my-app-*"] - kinds: [ClusterRole, ClusterRoleBinding] labelSelector: - app: my-app + matchLabels: + app: my-app ``` **Backup (required):** You must still include cluster-scoped kinds on the Backup: @@ -535,7 +598,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret] labelSelector: - app: my-app + matchLabels: + app: my-app - namespaces: - production resourceFilters: @@ -561,7 +625,8 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret, Deployment] labelSelector: - app: my-app + matchLabels: + app: my-app ``` **Result:** No Secrets in the backup — the namespace policy cannot re-include a globally excluded kind. Velero logs a warning at backup start if you list an excluded kind in `namespacedFilterPolicies`. @@ -598,7 +663,8 @@ namespacedFilterPolicies: excludedNames: ["*-tmp-*", "*-debug-*", "*-tmp", "*-debug"] - kinds: [Secret] labelSelector: - workload: application + matchLabels: + workload: application ``` **Expected outcome:** Volume actions apply to PVCs per `volumePolicies`; resource inclusion follows `namespacedFilterPolicies`. The sections are independent. @@ -619,10 +685,12 @@ namespacedFilterPolicies: resourceFilters: - kinds: [ConfigMap, Secret] labelSelector: - app: my-app + matchLabels: + app: my-app - kinds: ["*"] labelSelector: - app: my-app + matchLabels: + app: my-app ``` **On resources to exclude**, set: @@ -644,8 +712,8 @@ metadata: | Field | Description | |-------|-------------| | `kinds` | Resource type names (e.g. `ConfigMap`, `deployments`). Empty or `["*"]` = catch-all (namespace policies only). | -| `labelSelector` | Equality labels (`key: value`), AND across keys. No `in`, `exists`, etc. — use `orLabelSelectors` for OR. | -| `orLabelSelectors` | List of label maps; match if **any** map matches (AND within each map). Mutually exclusive with `labelSelector`. | +| `labelSelector` | Kubernetes-style selector with `matchLabels` and/or `matchExpressions` (`In`, `NotIn`, `Exists`, `DoesNotExist`). All requirements are AND-ed. | +| `orLabelSelectors` | List of selectors; match if **any** entry matches (AND within each, OR across the list). Use for OR of multi-key groups; prefer `In` for value-OR on one key. Mutually exclusive with `labelSelector`. | | `names` | Exact names or glob patterns to include. | | `excludedNames` | Patterns to exclude; wins over `names` when both match. | @@ -761,6 +829,7 @@ Velero validates the ResourcePolicy when a backup starts. Common errors: | `only one catch-all resource filter is allowed` | Multiple catch-alls in one policy entry | | `kind "X" appears in both resourceFilters[...]` | Same kind in two entries | | `labelSelector and orLabelSelectors cannot co-exist` | Both set in one entry | +| `invalid label selector` | Bad operator, values, or label key/value syntax | | `duplicate namespace pattern` | Same namespace string in two policy entries | | `invalid glob pattern` | Bad characters in namespace or name pattern | | `clusterScopedFilterPolicy... kinds must be specified (catch-all is not supported)` | Empty or `["*"]` kinds in cluster policy | From ff7273548b5d2d1283120db5c7d0b1bc9de5fe9f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:12:25 +0000 Subject: [PATCH 2/3] Bump codecov/codecov-action from 6 to 7 Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 6 to 7. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/v6...v7) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/pr-ci-check.yml | 2 +- .github/workflows/push.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-ci-check.yml b/.github/workflows/pr-ci-check.yml index ba55e6ab0..b189a622a 100644 --- a/.github/workflows/pr-ci-check.yml +++ b/.github/workflows/pr-ci-check.yml @@ -24,7 +24,7 @@ jobs: - name: Make ci run: make ci - name: Upload test coverage - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: token: ${{ secrets.CODECOV_TOKEN }} files: coverage.out diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index b45af38d9..528776e54 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -45,7 +45,7 @@ jobs: - name: Test run: make test - name: Upload test coverage - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: token: ${{ secrets.CODECOV_TOKEN }} files: coverage.out From 91e089d3e6d1797099bd76ebc165b82978b9aec6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 19:12:46 +0000 Subject: [PATCH 3/3] Bump actions/labeler from 5 to 7 Bumps [actions/labeler](https://github.com/actions/labeler) from 5 to 7. - [Release notes](https://github.com/actions/labeler/releases) - [Commits](https://github.com/actions/labeler/compare/v5...v7) --- updated-dependencies: - dependency-name: actions/labeler dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/auto_label_prs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/auto_label_prs.yml b/.github/workflows/auto_label_prs.yml index 21540d8cb..cc61473db 100644 --- a/.github/workflows/auto_label_prs.yml +++ b/.github/workflows/auto_label_prs.yml @@ -18,6 +18,6 @@ jobs: if: github.repository == 'velero-io/velero' runs-on: ubuntu-latest steps: - - uses: actions/labeler@v5 + - uses: actions/labeler@v7 with: configuration-path: .github/labeler.yml