Merge pull request #9946 from adam-jian-zhang/restore_filters_via_resource_policy
Run the E2E test on kind / get-go-version (push) Failing after 1m0s
Run the E2E test on kind / build (push) Has been skipped
Run the E2E test on kind / setup-test-matrix (push) Successful in 3s
Run the E2E test on kind / run-e2e-test (push) Has been skipped
Main CI / get-go-version (push) Successful in 12s
Main CI / Build (push) Failing after 25s

restore filters via resource policy
This commit is contained in:
Adam Zhang
2026-07-11 09:03:43 +08:00
committed by GitHub
9 changed files with 909 additions and 43 deletions
@@ -0,0 +1 @@
Fix issue #9936, restore filters via resource policy implementation
@@ -146,6 +146,22 @@ 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).
#### 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`).
To ensure consistent case-insensitive behavior across all code paths, Velero normalizes all input `kinds` to lowercase *before* attempting discovery or fallback matching.
During a restore, Velero attempts to resolve `Kind` names to fully-qualified plural resource names using the cluster's discovery helper. However, for Custom Resources (CRDs), the CRD might not exist in the cluster yet when the restore begins.
To handle this, Velero implements a **peek-and-map fallback**:
1. If a normalized `Kind` cannot be resolved via the discovery helper at the start of the restore, Velero stores the normalized string as provided in the policy.
2. Later, when iterating through the backup tarball, if Velero encounters a resource type (e.g., `mycustomkinds.mygroup.io`) that doesn't match any resolved filters, it peeks at the `Kind` of the first item in the tarball for that resource type.
3. It then checks if this actual `Kind` (case-insensitively) matches any of the unresolved normalized strings in the user's policy.
4. If a match is found, the filter is applied and cached for subsequent lookups.
This ensures that users can intuitively write `kinds: [MyCustomKind]` and it will work reliably, even if the CRD hasn't been restored yet. This logic applies to both `namespacedFilterPolicies` and `clusterScopedFilterPolicy`.
#### 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.
+32 -16
View File
@@ -44,6 +44,7 @@ import (
"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/constant"
@@ -232,7 +233,7 @@ func (r *restoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
original := restore.DeepCopy()
// Validate the restore and fetch the backup
info, resourceModifiers := r.validateAndComplete(restore)
info, resourceModifiers, restoreResPolicies := r.validateAndComplete(ctx, restore)
// Register attempts after validation so we don't have to fetch the backup multiple times
backupScheduleName := restore.Spec.ScheduleName
@@ -267,7 +268,7 @@ func (r *restoreReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return ctrl.Result{}, nil
}
if err := r.runValidatedRestore(restore, info, resourceModifiers); err != nil {
if err := r.runValidatedRestore(restore, info, resourceModifiers, restoreResPolicies); err != nil {
log.WithError(err).Debug("Restore failed")
restore.Status.Phase = api.RestorePhaseFailed
restore.Status.FailureReason = err.Error()
@@ -303,7 +304,7 @@ func (r *restoreReconciler) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}
func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInfo, *resourcemodifiers.ResourceModifiers) {
func (r *restoreReconciler) validateAndComplete(ctx context.Context, restore *api.Restore) (backupInfo, *resourcemodifiers.ResourceModifiers, *resourcepolicies.Policies) {
// add non-restorable resources to restore's excluded resources
excludedResources := sets.NewString(restore.Spec.ExcludedResources...)
for _, nonrestorable := range nonRestorableResources {
@@ -338,7 +339,7 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf
// validate that exactly one of BackupName and ScheduleName have been specified
if !backupXorScheduleProvided(restore) {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "Either a backup or schedule must be specified as a source for the restore, but not both")
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
// validate Restore Init Hook's InitContainers
@@ -372,9 +373,9 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf
}))
backupList := &api.BackupList{}
if err := r.kbClient.List(context.Background(), backupList, &client.ListOptions{LabelSelector: selector}); err != nil {
if err := r.kbClient.List(ctx, backupList, &client.ListOptions{LabelSelector: selector}); err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "Unable to list backups for schedule")
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
if len(backupList.Items) == 0 {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "No backups found for schedule")
@@ -384,19 +385,19 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf
restore.Spec.BackupName = backup.Name
} else {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, "No completed backups found for schedule")
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
}
info, err := r.fetchBackupInfo(restore.Spec.BackupName)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("Error retrieving backup: %v", err))
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
if !veleroutil.BSLIsAvailable(*info.location) {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("The BSL %s is unavailable, cannot retrieve the backup", info.location.Name))
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
// reject restores from backups that are not in a usable phase
@@ -407,7 +408,7 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors,
fmt.Sprintf("backup %q is in phase %q and cannot be used as a restore source",
info.backup.Name, info.backup.Status.Phase))
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
// Fill in the ScheduleName so it's easier to consume for metrics.
@@ -415,26 +416,40 @@ func (r *restoreReconciler) validateAndComplete(restore *api.Restore) (backupInf
restore.Spec.ScheduleName = info.backup.GetLabels()[api.ScheduleNameLabel]
}
var restoreResPolicies *resourcepolicies.Policies
if restore.Spec.ResourcePolicy != nil {
var err error
restoreResPolicies, err = resourcepolicies.GetResourcePoliciesFromRestore(
ctx, restore, r.kbClient, r.logger,
)
if err != nil {
restore.Status.ValidationErrors = append(
restore.Status.ValidationErrors, err.Error(),
)
return backupInfo{}, nil, nil
}
}
var resourceModifiers *resourcemodifiers.ResourceModifiers
if restore.Spec.ResourceModifier != nil && strings.EqualFold(restore.Spec.ResourceModifier.Kind, resourcemodifiers.ConfigmapRefType) {
ResourceModifierConfigMap := &corev1api.ConfigMap{}
err := r.kbClient.Get(context.Background(), client.ObjectKey{Namespace: restore.Namespace, Name: restore.Spec.ResourceModifier.Name}, ResourceModifierConfigMap)
err := r.kbClient.Get(ctx, client.ObjectKey{Namespace: restore.Namespace, Name: restore.Spec.ResourceModifier.Name}, ResourceModifierConfigMap)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, fmt.Sprintf("failed to get resource modifiers configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name))
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
resourceModifiers, err = resourcemodifiers.GetResourceModifiersFromConfig(ResourceModifierConfigMap)
if err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrapf(err, "Error in parsing resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name).Error())
return backupInfo{}, nil
return backupInfo{}, nil, nil
} else if err = resourceModifiers.Validate(); err != nil {
restore.Status.ValidationErrors = append(restore.Status.ValidationErrors, errors.Wrapf(err, "Validation error in resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name).Error())
return backupInfo{}, nil
return backupInfo{}, nil, nil
}
r.logger.Infof("Retrieved Resource modifiers provided in configmap %s/%s", restore.Namespace, restore.Spec.ResourceModifier.Name)
}
return info, resourceModifiers
return info, resourceModifiers, restoreResPolicies
}
// backupXorScheduleProvided returns true if exactly one of BackupName and
@@ -507,7 +522,7 @@ func fetchBackupInfoInternal(kbClient client.Client, namespace, backupName strin
// The log and results files are uploaded to backup storage. Any error returned from this function
// means that the restore failed. This function updates the restore API object with warning and error
// counts, but *does not* update its phase or patch it via the API.
func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backupInfo, resourceModifiers *resourcemodifiers.ResourceModifiers) error {
func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backupInfo, resourceModifiers *resourcemodifiers.ResourceModifiers, restoreResPolicies *resourcepolicies.Policies) error {
// instantiate the per-restore logger that will output both to a temp file
// (for upload to object storage) and to stdout.
restoreLog, err := logging.NewTempFileLogger(r.restoreLogLevel, r.logFormat, nil, logrus.Fields{"restore": kubeutil.NamespaceAndName(restore)})
@@ -586,6 +601,7 @@ func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backu
VolumeSnapshots: volumeSnapshots,
BackupReader: backupFile,
ResourceModifiers: resourceModifiers,
ResPolicies: restoreResPolicies,
DisableInformerCache: r.disableInformerCache,
CSIVolumeSnapshots: csiVolumeSnapshots,
BackupVolumeInfoMap: backupVolumeInfoMap,
+136 -7
View File
@@ -785,7 +785,7 @@ func TestValidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
Phase(velerov1api.BackupPhaseCompleted).
Result()))
r.validateAndComplete(restore)
r.validateAndComplete(t.Context(), restore)
assert.Contains(t, restore.Status.ValidationErrors, "No backups found for schedule")
assert.Empty(t, restore.Spec.BackupName)
@@ -801,7 +801,7 @@ func TestValidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
Result(),
))
r.validateAndComplete(restore)
r.validateAndComplete(t.Context(), restore)
assert.Contains(t, restore.Status.ValidationErrors, "No completed backups found for schedule")
assert.Empty(t, restore.Spec.BackupName)
@@ -832,11 +832,140 @@ func TestValidateAndCompleteWhenScheduleNameSpecified(t *testing.T) {
ScheduleName: "schedule-1",
},
}
r.validateAndComplete(restore)
r.validateAndComplete(t.Context(), restore)
assert.Nil(t, restore.Status.ValidationErrors)
assert.Equal(t, "foo", restore.Spec.BackupName)
}
func TestValidateAndCompleteWithResourcePolicySpecified(t *testing.T) {
formatFlag := logging.FormatText
var (
logger = velerotest.NewLogger()
pluginManager = &pluginmocks.Manager{}
fakeClient = velerotest.NewFakeControllerRuntimeClient(t)
fakeGlobalClient = velerotest.NewFakeControllerRuntimeClient(t)
backupStore = &persistencemocks.BackupStore{}
)
r := NewRestoreReconciler(
t.Context(),
velerov1api.DefaultNamespace,
nil,
fakeClient,
logger,
logrus.DebugLevel,
func(logrus.FieldLogger) clientmgmt.Manager { return pluginManager },
NewFakeSingleObjectBackupStoreGetter(backupStore),
metrics.NewServerMetrics(),
formatFlag,
60*time.Minute,
false,
fakeGlobalClient,
10*time.Minute,
)
restore := &velerov1api.Restore{
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "restore-1",
},
Spec: velerov1api.RestoreSpec{
BackupName: "backup-1",
ResourcePolicy: &corev1api.TypedLocalObjectReference{
Kind: "configmap",
Name: "test-configmap",
},
},
}
location := builder.ForBackupStorageLocation("velero", "default").Provider("myCloud").Bucket("bucket").Phase(velerov1api.BackupStorageLocationPhaseAvailable).Result()
require.NoError(t, r.kbClient.Create(t.Context(), location))
require.NoError(t, r.kbClient.Create(
t.Context(),
defaultBackup().
ObjectMeta(
builder.WithName("backup-1"),
).StorageLocation("default").
Phase(velerov1api.BackupPhaseCompleted).
Result(),
))
r.validateAndComplete(t.Context(), restore)
assert.Contains(t, restore.Status.ValidationErrors[0], "fail to get ResourcePolicies velero/test-configmap ConfigMap")
restore1 := &velerov1api.Restore{
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "restore-1",
},
Spec: velerov1api.RestoreSpec{
BackupName: "backup-1",
ResourcePolicy: &corev1api.TypedLocalObjectReference{
Kind: "configmap",
Name: "test-configmap",
},
},
}
cm1 := &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "test-configmap",
Namespace: velerov1api.DefaultNamespace,
},
Data: map[string]string{
"policy.yaml": `version: v1
clusterScopedFilterPolicy:
resourceFilters:
- kinds:
- pods
`,
},
}
require.NoError(t, r.kbClient.Create(t.Context(), cm1))
r.validateAndComplete(t.Context(), restore1)
assert.Nil(t, restore1.Status.ValidationErrors)
restore2 := &velerov1api.Restore{
ObjectMeta: metav1.ObjectMeta{
Namespace: velerov1api.DefaultNamespace,
Name: "restore-1",
},
Spec: velerov1api.RestoreSpec{
BackupName: "backup-1",
ResourcePolicy: &corev1api.TypedLocalObjectReference{
// intentional to ensure case insensitivity works as expected
Kind: "confIGMaP",
Name: "test-configmap-invalid",
},
},
}
cm2 := &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "test-configmap-invalid",
Namespace: velerov1api.DefaultNamespace,
},
Data: map[string]string{
"policy.yaml": `version: v1
volumePolicies:
- conditions:
capacity: '0,10Gi'
csi:
driver: disks.csi.driver
action:
type: invalid_action
`,
},
}
require.NoError(t, r.kbClient.Create(t.Context(), cm2))
r.validateAndComplete(t.Context(), restore2)
assert.Contains(t, restore2.Status.ValidationErrors[0], "fail to validate ResourcePolicies in ConfigMap velero/test-configmap-invalid")
}
func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
formatFlag := logging.FormatText
@@ -892,7 +1021,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
Result(),
))
r.validateAndComplete(restore)
r.validateAndComplete(t.Context(), restore)
assert.Contains(t, restore.Status.ValidationErrors[0], "failed to get resource modifiers configmap")
restore1 := &velerov1api.Restore{
@@ -920,7 +1049,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
}
require.NoError(t, r.kbClient.Create(t.Context(), cm1))
r.validateAndComplete(restore1)
r.validateAndComplete(t.Context(), restore1)
assert.Nil(t, restore1.Status.ValidationErrors)
restore2 := &velerov1api.Restore{
@@ -949,7 +1078,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
}
require.NoError(t, r.kbClient.Create(t.Context(), invalidVersionCm))
r.validateAndComplete(restore2)
r.validateAndComplete(t.Context(), restore2)
assert.Contains(t, restore2.Status.ValidationErrors[0], "Error in parsing resource modifiers provided in configmap")
restore3 := &velerov1api.Restore{
@@ -977,7 +1106,7 @@ func TestValidateAndCompleteWithResourceModifierSpecified(t *testing.T) {
}
require.NoError(t, r.kbClient.Create(t.Context(), invalidOperatorCm))
r.validateAndComplete(restore3)
r.validateAndComplete(t.Context(), restore3)
assert.Contains(t, restore3.Status.ValidationErrors[0], "Validation error in resource modifiers provided in configmap")
}
+2
View File
@@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/itemoperation"
@@ -61,6 +62,7 @@ type Request struct {
RestoredItems map[itemKey]restoredItemStatus
itemOperationsList *[]*itemoperation.RestoreOperation
ResourceModifiers *resourcemodifiers.ResourceModifiers
ResPolicies *resourcepolicies.Policies
DisableInformerCache bool
CSIVolumeSnapshots []*snapshotv1api.VolumeSnapshot
BackupVolumeInfoMap map[string]volume.BackupVolumeInfo
+429 -20
View File
@@ -32,6 +32,7 @@ import (
"time"
"github.com/cockroachdb/errors"
"github.com/gobwas/glob"
"github.com/google/uuid"
snapshotv1api "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
"github.com/sirupsen/logrus"
@@ -55,6 +56,7 @@ import (
"github.com/vmware-tanzu/velero/internal/credentials"
"github.com/vmware-tanzu/velero/internal/hook"
"github.com/vmware-tanzu/velero/internal/resourcemodifiers"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/internal/volume"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/archive"
@@ -237,6 +239,43 @@ func (kr *kubernetesRestorer) RestoreWithResolvers(
Includes(req.Restore.Spec.IncludedNamespaces...).
Excludes(req.Restore.Spec.ExcludedNamespaces...)
var clusterScopedFilterMap map[string]*resolvedResourceFilter
var namespacedFilterMap map[string]*resolvedNamespaceFilter
var namespacedFilterPatterns []namespacedFilterPattern
if req.ResPolicies != nil {
if kr.discoveryHelper == nil {
return results.Result{}, results.Result{Velero: []string{"failed to resolve namespace filter policies: discovery client unavailable"}}
}
// Resolve clusterScopedFilterPolicy
csPolicy := req.ResPolicies.GetClusterScopedFilterPolicy()
if csPolicy != nil {
clusterScopedFilterMap, err = resolveRestoreClusterScopedFilterPolicy(
csPolicy,
kr.discoveryHelper,
req.Log,
)
if err != nil {
return results.Result{}, results.Result{Velero: []string{err.Error()}}
}
}
// Resolve namespacedFilterPolicies
nfPolicies := req.ResPolicies.GetNamespacedFilterPolicies()
if len(nfPolicies) > 0 {
namespacedFilterMap, namespacedFilterPatterns, err = resolveRestoreNamespacedFilterPolicies(
nfPolicies,
req.Restore.Spec.ExcludedResources,
kr.discoveryHelper,
req.Log,
)
if err != nil {
return results.Result{}, results.Result{Velero: []string{err.Error()}}
}
}
}
resolvedActions, err := restoreItemActionResolver.ResolveActions(kr.discoveryHelper, kr.logger)
if err != nil {
return results.Result{}, results.Result{Velero: []string{err.Error()}}
@@ -333,6 +372,10 @@ func (kr *kubernetesRestorer) RestoreWithResolvers(
restoreVolumeInfoTracker: req.RestoreVolumeInfoTracker,
hooksWaitExecutor: hooksWaitExecutor,
resourceDeletionStatusTracker: req.ResourceDeletionStatusTracker,
clusterScopedFilterMap: clusterScopedFilterMap,
namespacedFilterMap: namespacedFilterMap,
namespacedFilterPatterns: namespacedFilterPatterns,
namespaceFilterCache: make(map[string]*resolvedNamespaceFilter),
}
return restoreCtx.execute()
@@ -382,6 +425,245 @@ type restoreContext struct {
restoreVolumeInfoTracker *volume.RestoreVolumeInfoTracker
hooksWaitExecutor *hooksWaitExecutor
resourceDeletionStatusTracker kube.ResourceDeletionStatusTracker
// clusterScopedFilterMap holds resolved per-kind filters for cluster-scoped resources.
// Key is the resolved group-resource string.
clusterScopedFilterMap map[string]*resolvedResourceFilter
// namespacedFilterMap holds resolved per-namespace filters.
// Key is either an exact namespace name or a glob pattern string.
namespacedFilterMap map[string]*resolvedNamespaceFilter
// namespacedFilterPatterns preserves the order of patterns for first-match
// semantics and caches pre-compiled globs to avoid repeated compilation.
namespacedFilterPatterns []namespacedFilterPattern
// namespaceFilterCache memoizes the resolved filter for a given namespace
// to avoid re-evaluating glob patterns on every call.
namespaceFilterCache map[string]*resolvedNamespaceFilter
}
type resolvedResourceFilter struct {
labelSelector labels.Selector
orLabelSelectors []labels.Selector
nameIE *collections.IncludesExcludes
originalKinds []string
}
type resolvedNamespaceFilter struct {
// resourceFilterMap is keyed by the resolved group-resource string
resourceFilterMap map[string]*resolvedResourceFilter
// catchAllFilter holds the resolved filter for a catch-all entry (empty kinds or ["*"]).
// nil when no catch-all entry is defined.
catchAllFilter *resolvedResourceFilter
// hasUnresolvedKinds is true if any kind in the policy failed discovery.
// This is used to bypass the fast-path skip so the peek-and-map fallback can run.
hasUnresolvedKinds bool
}
// namespacedFilterPattern pairs a namespace pattern string with its pre-compiled
// glob so that getNamespaceFilter does not recompile on every call.
type namespacedFilterPattern struct {
pattern string
compiled glob.Glob // compiled once at restore start; nil for exact-match patterns
}
func (ctx *restoreContext) getNamespaceFilter(namespace string) *resolvedNamespaceFilter {
if ctx.namespacedFilterMap == nil {
return nil
}
// 1. Check the cache first
if filter, ok := ctx.namespaceFilterCache[namespace]; ok {
return filter
}
// 2. Walk patterns in definition order (first-match semantics)
// Note: namespaceFilterCache is mutated below without synchronization. This is safe
// today because resource collection runs sequentially. If the restore loop is
// parallelized in the future, these map writes will need a lock to prevent data races.
for _, p := range ctx.namespacedFilterPatterns {
if p.compiled != nil {
if p.compiled.Match(namespace) {
filter := ctx.namespacedFilterMap[p.pattern]
ctx.namespaceFilterCache[namespace] = filter
return filter
}
} else if p.pattern == namespace {
filter := ctx.namespacedFilterMap[p.pattern]
ctx.namespaceFilterCache[namespace] = filter
return filter
}
}
// 3. Cache the miss so we don't re-evaluate failed matches
ctx.namespaceFilterCache[namespace] = nil
return nil
}
// resolveRestoreClusterScopedFilterPolicy resolves the cluster-scoped filter policy
// into a map keyed by group-resource string. Note: catch-all entries (empty or ["*"] kinds)
// are NOT supported in clusterScopedFilterPolicy — validation rejects them earlier.
// Cluster-scoped filtering is a refinement overlay; unlisted kinds fall back to global
// filters via the existing pipeline, so there is no catchAllFilter field on this map.
func resolveRestoreClusterScopedFilterPolicy(
policy *resourcepolicies.ClusterScopedFilterPolicy,
helper discovery.Helper,
log logrus.FieldLogger,
) (map[string]*resolvedResourceFilter, error) {
result := make(map[string]*resolvedResourceFilter)
for _, rf := range policy.ResourceFilters {
resolved, err := resolveResourceFilter(rf)
if err != nil {
return nil, err
}
for _, kind := range resolved.originalKinds {
gr, resource, err := helper.ResourceFor(schema.ParseGroupResource(kind).WithVersion(""))
key := kind
if err != nil {
log.WithField("kind", kind).Warnf("Cannot resolve kind via discovery, using as-is")
} else {
if resource.Namespaced {
log.Warnf("kind %q in clusterScopedFilterPolicy is a namespace-scoped resource; it will never match in a cluster-scoped filter — did you mean namespacedFilterPolicies?", kind)
}
key = gr.GroupResource().String()
}
if _, exists := result[key]; exists {
return nil, fmt.Errorf("ambiguous policy: duplicate kind %q detected", key)
}
result[key] = resolved
}
}
return result, nil
}
func resolveRestoreNamespacedFilterPolicies(
policies []resourcepolicies.NamespacedFilterPolicy,
excludedResources []string,
helper discovery.Helper,
log logrus.FieldLogger,
) (map[string]*resolvedNamespaceFilter, []namespacedFilterPattern, error) {
result := make(map[string]*resolvedNamespaceFilter)
var patternOrder []namespacedFilterPattern
// Build a quick lookup map for globally excluded resources
globalExcludes := make(map[string]bool)
for _, ex := range excludedResources {
globalExcludes[ex] = true
}
for _, policy := range policies {
rfMap := make(map[string]*resolvedResourceFilter)
var catchAll *resolvedResourceFilter
hasUnresolvedKinds := false
for _, rf := range policy.ResourceFilters {
resolved, err := resolveResourceFilter(rf)
if err != nil {
return nil, nil, err
}
if rf.IsCatchAll() {
catchAll = resolved
continue
}
for _, kind := range resolved.originalKinds {
gr, resource, err := helper.ResourceFor(
schema.ParseGroupResource(kind).WithVersion(""),
)
key := kind
if err != nil {
log.WithField("kind", kind).Warnf(
"Cannot resolve kind via discovery, using as-is")
hasUnresolvedKinds = true
} else {
if !resource.Namespaced {
log.Warnf("kind %q in namespacedFilterPolicies is a cluster-scoped resource; it will never match in a namespace-scoped filter — did you mean clusterScopedFilterPolicy?", kind)
}
if globalExcludes[kind] || globalExcludes[gr.GroupResource().String()] {
log.WithFields(logrus.Fields{
"kind": kind,
"namespacePattern": strings.Join(policy.Namespaces, ","),
}).Warn("namespacedFilterPolicies entry lists a kind that is globally excluded by RestoreSpec.ExcludedResources; the per-namespace filter entry has no effect")
}
key = gr.GroupResource().String()
}
if _, exists := rfMap[key]; exists {
return nil, nil, fmt.Errorf("ambiguous policy: duplicate kind %q detected", key)
}
rfMap[key] = resolved
}
}
nsFilter := &resolvedNamespaceFilter{
resourceFilterMap: rfMap,
catchAllFilter: catchAll,
hasUnresolvedKinds: hasUnresolvedKinds,
}
for _, nsPattern := range policy.Namespaces {
result[nsPattern] = nsFilter
var compiled glob.Glob
if strings.ContainsAny(nsPattern, "*?[") {
var err error
compiled, err = glob.Compile(nsPattern)
if err != nil {
log.WithError(err).Warnf("Failed to compile namespace glob pattern %q, falling back to exact match", nsPattern)
}
}
patternOrder = append(patternOrder, namespacedFilterPattern{
pattern: nsPattern,
compiled: compiled,
})
}
}
return result, patternOrder, nil
}
// resolveResourceFilter converts a ResourceFilter's label selectors and name patterns
// into their runtime representations.
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)
}
}
var orSelectors []labels.Selector
for _, ols := range rf.OrLabelSelectors {
s, err := labels.ValidatedSelectorFromSet(labels.Set(ols))
if err != nil {
return nil, fmt.Errorf("invalid OR label selector in resource filter: %w", err)
}
orSelectors = append(orSelectors, s)
}
var nameIE *collections.IncludesExcludes
if len(rf.Names) > 0 || len(rf.ExcludedNames) > 0 {
nameIE = collections.NewIncludesExcludes().Includes(rf.Names...).Excludes(rf.ExcludedNames...)
}
normalizedKinds := make([]string, len(rf.Kinds))
for i, k := range rf.Kinds {
normalizedKinds[i] = strings.ToLower(k)
}
return &resolvedResourceFilter{
labelSelector: selector,
orLabelSelectors: orSelectors,
nameIE: nameIE,
originalKinds: normalizedKinds,
}, nil
}
type resourceClientKey struct {
@@ -1128,6 +1410,10 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
// and should be excluded. Note that we're checking the object's namespace (
// via obj.GetNamespace()) instead of the namespace parameter, because we want
// to check the *original* namespace, not the remapped one if it's been remapped.
//
// Note: Additional items intentionally bypass fine-grained resource filter policies
// (like per-namespace label/name selectors) to avoid breaking semantic dependencies,
// but they must still pass the global exclusions enforced below.
if namespace != "" {
if !ctx.namespaceIncludesExcludes.ShouldInclude(obj.GetNamespace()) && !ctx.resourceMustHave.Has(groupResource.String()) {
restoreLogger.Info("Not restoring item because namespace is excluded")
@@ -2277,6 +2563,18 @@ func (ctx *restoreContext) getOrderedResourceCollection(
continue
}
// Per-namespace resource type check from restore filter policy
if namespace != "" && !ctx.resourceMustHave.Has(groupResource.String()) {
if nsFilter := ctx.getNamespaceFilter(namespace); nsFilter != nil {
_, kindListed := nsFilter.resourceFilterMap[groupResource.String()]
if !kindListed && nsFilter.catchAllFilter == nil && !nsFilter.hasUnresolvedKinds {
ctx.log.Infof("Skipping resource %s in namespace %s: not in resourceFilters",
resource, namespace)
continue
}
}
}
res, w, e := ctx.getSelectedRestoreableItems(groupResource.String(), namespace, items)
warnings.Merge(&w)
errs.Merge(&e)
@@ -2330,6 +2628,88 @@ func (ctx *restoreContext) getSelectedRestoreableItems(resource string, original
resourceForPath = filepath.Join(resource, cgv.Dir)
}
var rf *resolvedResourceFilter
var useFilterPolicy bool
if !ctx.resourceMustHave.Has(resource) {
if originalNamespace != "" {
// Namespace-scoped path
if nsFilter := ctx.getNamespaceFilter(originalNamespace); nsFilter != nil {
// Resolve effective filter: kind-specific takes precedence over catch-all
rf = nsFilter.resourceFilterMap[resource]
// Peek-and-map logic for unresolvable kinds
if rf == nil && len(items) > 0 {
peekPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); err == nil {
actualKind := obj.GroupVersionKind().Kind
for _, filter := range nsFilter.resourceFilterMap {
for _, k := range filter.originalKinds {
if strings.EqualFold(k, actualKind) {
rf = filter
// Cache it for future lookups of this resource
// Note: resourceFilterMap is mutated in place without synchronization.
// This is safe today because resource collection runs sequentially.
// If parallelized in the future, this will need a lock to prevent data races.
nsFilter.resourceFilterMap[resource] = rf
break
}
}
if rf != nil {
break
}
}
}
}
if rf == nil {
rf = nsFilter.catchAllFilter // may be nil if no catch-all
}
useFilterPolicy = true
if rf == nil {
ctx.log.Infof("Skipping resource %s in namespace %s: not in resourceFilters", resource, originalNamespace)
return restorable, warnings, errs
}
}
} else if ctx.clusterScopedFilterMap != nil {
// Cluster-scoped path: only applies if kind is listed (refinement overlay)
if listedRF, ok := ctx.clusterScopedFilterMap[resource]; ok {
rf = listedRF
useFilterPolicy = true
} else if len(items) > 0 {
// Peek-and-map logic for unresolvable kinds.
// Note: Unlike the namespaced path, this fallback is always reachable
// because the main restore loop does not have a fast-path skip for
// unlisted cluster-scoped resources.
peekPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, items[0])
// Ignore unmarshal errors during peek; the main restore loop will catch and report them
if obj, err := archive.Unmarshal(ctx.fileSystem, peekPath); err == nil {
actualKind := obj.GroupVersionKind().Kind
for _, filter := range ctx.clusterScopedFilterMap {
for _, k := range filter.originalKinds {
if strings.EqualFold(k, actualKind) {
rf = filter
// Cache it for future lookups of this resource
// Note: clusterScopedFilterMap is mutated in place without synchronization.
// This is safe today because resource collection runs sequentially.
// If parallelized in the future, this will need a lock to prevent data races.
ctx.clusterScopedFilterMap[resource] = rf
useFilterPolicy = true
break
}
}
if rf != nil {
break
}
}
}
}
// If kind not listed, fall through to global selectors below
}
}
for _, item := range items {
itemPath := archive.GetItemFilePath(ctx.restoreDir, resourceForPath, originalNamespace, item)
@@ -2347,29 +2727,58 @@ func (ctx *restoreContext) getSelectedRestoreableItems(resource string, original
}
if !ctx.resourceMustHave.Has(resource) {
if !ctx.selector.Matches(labels.Set(obj.GetLabels())) {
continue
}
// Processing OrLabelSelectors when specified in the restore request. LabelSelectors as well as OrLabelSelectors
// cannot co-exist, only one of them can be specified
var skipItem = false
var skip = 0
ctx.log.Debugf("orSelectors specified: %s for item: %s", ctx.OrSelectors, item)
for _, s := range ctx.OrSelectors {
if !s.Matches(labels.Set(obj.GetLabels())) {
skip++
if useFilterPolicy {
if rf != nil {
// Per-kind label selector
if rf.labelSelector != nil && !rf.labelSelector.Matches(labels.Set(obj.GetLabels())) {
continue
}
// Per-kind OR label selectors
if len(rf.orLabelSelectors) > 0 {
matched := false
for _, s := range rf.orLabelSelectors {
if s.Matches(labels.Set(obj.GetLabels())) {
matched = true
break
}
}
if !matched {
ctx.log.Infof("Excluding item %s: no OR label selector matched (restore filter policy)", item)
continue
}
}
// Per-kind name filter
if rf.nameIE != nil && !rf.nameIE.ShouldInclude(obj.GetName()) {
ctx.log.Infof("Excluding item %s: name does not match restore filter policy", obj.GetName())
continue
}
}
} else {
// Existing global selector logic
if !ctx.selector.Matches(labels.Set(obj.GetLabels())) {
continue
}
if len(ctx.OrSelectors) == skip && skip > 0 {
ctx.log.Infof("setting skip flag to true for item: %s", item)
skipItem = true
}
}
// Processing OrLabelSelectors when specified in the restore request. LabelSelectors as well as OrLabelSelectors
// cannot co-exist, only one of them can be specified
var skipItem = false
var skip = 0
ctx.log.Debugf("orSelectors specified: %s for item: %s", ctx.OrSelectors, item)
for _, s := range ctx.OrSelectors {
if !s.Matches(labels.Set(obj.GetLabels())) {
skip++
}
if skipItem {
ctx.log.Infof("restore orSelector labels did not match, skipping restore of item: %s", skipItem, item)
continue
if len(ctx.OrSelectors) == skip && skip > 0 {
ctx.log.Infof("setting skip flag to true for item: %s", item)
skipItem = true
}
}
if skipItem {
ctx.log.Infof("restore orSelector labels did not match, skipping restore of item: %s", skipItem, item)
continue
}
}
}
+285
View File
@@ -0,0 +1,285 @@
package restore
import (
"io"
"testing"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
corev1api "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/builder"
"github.com/vmware-tanzu/velero/pkg/test"
)
func TestRestoreResourcePoliciesFiltering(t *testing.T) {
customKindRes := &test.APIResource{Group: "mygroup.io", Version: "v1", Name: "mycustomkinds", Kind: "MyCustomKind", Namespaced: true}
clusterCustomKindRes := &test.APIResource{Group: "mygroup.io", Version: "v1", Name: "myclustercustomkinds", Kind: "MyClusterCustomKind", Namespaced: false}
tests := []struct {
name string
restore *velerov1api.Restore
backup *velerov1api.Backup
policyYAML string
apiResources []*test.APIResource
tarball io.Reader
want map[*test.APIResource][]string
}{
{
name: "namespaced filter policy with exact namespace match",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
namespacedFilterPolicies:
- namespaces:
- ns-1
resourceFilters:
- kinds:
- pods
names:
- pod-1
`,
tarball: test.NewTarWriter(t).
AddItems("pods",
builder.ForPod("ns-1", "pod-1").Result(),
builder.ForPod("ns-1", "pod-2").Result(),
builder.ForPod("ns-2", "pod-1").Result(),
).
Done(),
apiResources: []*test.APIResource{
test.Pods(),
},
want: map[*test.APIResource][]string{
test.Pods(): {"ns-1/pod-1", "ns-2/pod-1"}, // ns-2 is not filtered, ns-1 only includes pod-1
},
},
{
name: "namespaced filter policy with glob namespace match and first-match semantics",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
namespacedFilterPolicies:
- namespaces:
- ns-*
resourceFilters:
- kinds:
- pods
names:
- pod-1
- namespaces:
- ns-1
resourceFilters:
- kinds:
- pods
names:
- pod-2
`,
tarball: test.NewTarWriter(t).
AddItems("pods",
builder.ForPod("ns-1", "pod-1").Result(),
builder.ForPod("ns-1", "pod-2").Result(),
builder.ForPod("ns-2", "pod-1").Result(),
builder.ForPod("ns-2", "pod-2").Result(),
).
Done(),
apiResources: []*test.APIResource{
test.Pods(),
},
want: map[*test.APIResource][]string{
test.Pods(): {"ns-1/pod-1", "ns-2/pod-1"},
},
},
{
name: "cluster scoped filter policy",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
clusterScopedFilterPolicy:
resourceFilters:
- kinds:
- persistentvolumes
names:
- pv-1
`,
tarball: test.NewTarWriter(t).
AddItems("persistentvolumes",
builder.ForPersistentVolume("pv-1").Result(),
builder.ForPersistentVolume("pv-2").Result(),
).
Done(),
apiResources: []*test.APIResource{
test.PVs(),
},
want: map[*test.APIResource][]string{
test.PVs(): {"/pv-1"},
},
},
{
name: "catch-all filter",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
namespacedFilterPolicies:
- namespaces:
- ns-1
resourceFilters:
- kinds:
- '*'
labelSelector:
app: test
`,
tarball: test.NewTarWriter(t).
AddItems("pods",
builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("app", "test")).Result(),
builder.ForPod("ns-1", "pod-2").Result(),
).
AddItems("deployments.apps",
builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("app", "test")).Result(),
builder.ForDeployment("ns-1", "deploy-2").Result(),
).
Done(),
apiResources: []*test.APIResource{
test.Pods(),
test.Deployments(),
},
want: map[*test.APIResource][]string{
test.Pods(): {"ns-1/pod-1"},
test.Deployments(): {"ns-1/deploy-1"},
},
},
{
name: "unresolved kind in namespaced filter policy is still restored via peek-and-map",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
namespacedFilterPolicies:
- namespaces: ["ns-1"]
resourceFilters:
- kinds: ["MyCustomKind"]
`,
tarball: test.NewTarWriter(t).AddItems("mycustomkinds.mygroup.io",
&unstructured.Unstructured{Object: map[string]any{"apiVersion": "mygroup.io/v1", "kind": "MyCustomKind", "metadata": map[string]any{"namespace": "ns-1", "name": "my-cr"}}},
).Done(),
apiResources: []*test.APIResource{
customKindRes,
},
want: map[*test.APIResource][]string{
customKindRes: {"ns-1/my-cr"},
},
},
{
name: "unresolved kind in cluster-scoped filter policy is still restored via peek-and-map",
restore: defaultRestore().Result(),
backup: defaultBackup().Result(),
policyYAML: `version: v1
clusterScopedFilterPolicy:
resourceFilters:
- kinds: ["MyClusterCustomKind"]
`,
tarball: test.NewTarWriter(t).AddItems("myclustercustomkinds.mygroup.io",
&unstructured.Unstructured{Object: map[string]any{"apiVersion": "mygroup.io/v1", "kind": "MyClusterCustomKind", "metadata": map[string]any{"name": "my-cluster-cr"}}},
).Done(),
apiResources: []*test.APIResource{
clusterCustomKindRes,
},
want: map[*test.APIResource][]string{
clusterCustomKindRes: {"/my-cluster-cr"},
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
h := newHarness(t)
for _, r := range tc.apiResources {
h.DiscoveryClient.WithAPIResource(r)
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
var resPolicies *resourcepolicies.Policies
if tc.policyYAML != "" {
cm := &corev1api.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "test-policies",
Namespace: "velero",
},
Data: map[string]string{
"policy.yaml": tc.policyYAML,
},
}
client := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithObjects(cm).Build()
restore := tc.restore.DeepCopy()
restore.Namespace = "velero"
restore.Spec.ResourcePolicy = &corev1api.TypedLocalObjectReference{
Kind: "configmap",
Name: "test-policies",
}
var err error
resPolicies, err = resourcepolicies.GetResourcePoliciesFromRestore(t.Context(), restore, client, logrus.New())
require.NoError(t, err)
}
data := &Request{
Log: h.log,
Restore: tc.restore,
Backup: tc.backup,
PodVolumeBackups: nil,
VolumeSnapshots: nil,
BackupReader: tc.tarball,
ResPolicies: resPolicies,
}
warnings, errs := h.restorer.Restore(
data,
nil, // restoreItemActions
nil, // volume snapshotter getter
)
assertEmptyResults(t, warnings, errs)
assertAPIContents(t, h, tc.want)
})
}
}
func TestResolveRestoreNamespacedFilterPolicies_Validation(t *testing.T) {
log := logrus.New()
helper := test.NewFakeDiscoveryHelper(true, nil)
policies := []resourcepolicies.NamespacedFilterPolicy{
{
Namespaces: []string{"ns-1"},
ResourceFilters: []resourcepolicies.ResourceFilter{
{
Kinds: []string{"MyKind", "mykind"},
},
},
},
}
_, _, err := resolveRestoreNamespacedFilterPolicies(policies, nil, helper, log)
require.Error(t, err)
require.Contains(t, err.Error(), "ambiguous policy: duplicate kind")
}
func TestResolveRestoreClusterScopedFilterPolicy_Validation(t *testing.T) {
log := logrus.New()
helper := test.NewFakeDiscoveryHelper(true, nil)
policy := &resourcepolicies.ClusterScopedFilterPolicy{
ResourceFilters: []resourcepolicies.ResourceFilter{
{
Kinds: []string{"MyKind", "mykind"},
},
},
}
_, err := resolveRestoreClusterScopedFilterPolicy(policy, helper, log)
require.Error(t, err)
require.Contains(t, err.Error(), "ambiguous policy: duplicate kind")
}
+6
View File
@@ -25,6 +25,7 @@ import (
"testing"
"time"
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/collections"
@@ -764,6 +765,10 @@ func TestRestoreResourceFiltering(t *testing.T) {
}
require.NoError(t, h.restorer.discoveryHelper.Refresh())
// We need to fetch the policies using the actual function
resPolicies, err := resourcepolicies.GetResourcePoliciesFromRestore(t.Context(), tc.restore, h.restorer.kbClient, h.log)
require.NoError(t, err)
data := &Request{
Log: h.log,
Restore: tc.restore,
@@ -771,6 +776,7 @@ func TestRestoreResourceFiltering(t *testing.T) {
PodVolumeBackups: nil,
VolumeSnapshots: nil,
BackupReader: tc.tarball,
ResPolicies: resPolicies,
}
warnings, errs := h.restorer.Restore(
data,
+2
View File
@@ -56,6 +56,8 @@ func NewAPIServer(t *testing.T) *APIServer {
{Group: "extensions", Version: "v1", Resource: "deployments"}: "ExtDeploymentsList",
{Group: "velero.io", Version: "v1", Resource: "deployments"}: "VeleroDeploymentsList",
{Group: "velero.io", Version: "v2alpha1", Resource: "datauploads"}: "DataUploadsList",
{Group: "mygroup.io", Version: "v1", Resource: "mycustomkinds"}: "MyCustomKindList",
{Group: "mygroup.io", Version: "v1", Resource: "myclustercustomkinds"}: "MyClusterCustomKindList",
})
discoveryClient = &DiscoveryClient{FakeDiscovery: kubeClient.Discovery().(*discoveryfake.FakeDiscovery)}
)