diff --git a/changelogs/unreleased/9587-blackpiglet b/changelogs/unreleased/9587-blackpiglet new file mode 100644 index 000000000..6662d2388 --- /dev/null +++ b/changelogs/unreleased/9587-blackpiglet @@ -0,0 +1 @@ +Remove wildcard check from getNamespacesToList. \ No newline at end of file diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index 1a1c54247..8c21bea77 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -187,7 +187,7 @@ func getNamespaceIncludesExcludesAndArgoCDNamespaces(backup *velerov1api.Backup, Excludes(backup.Spec.ExcludedNamespaces...) // Expand wildcards if needed - if err := includesExcludes.ExpandIncludesExcludes(); err != nil { + if err := includesExcludes.ExpandIncludesExcludes(true); err != nil { return nil, []string{}, err } @@ -286,7 +286,7 @@ func (kb *kubernetesBackupper) BackupWithResolvers( expandedExcludes := backupRequest.NamespaceIncludesExcludes.GetExcludes() // Get the final namespace list after wildcard expansion - wildcardResult, err := backupRequest.NamespaceIncludesExcludes.ResolveNamespaceList() + wildcardResult, err := backupRequest.NamespaceIncludesExcludes.ResolveNamespaceList(true) if err != nil { log.WithError(err).Errorf("error resolving namespace list") return err @@ -410,7 +410,7 @@ func (kb *kubernetesBackupper) BackupWithResolvers( // Resolve namespaces for PVC-to-Pod cache building in volumehelper. // See issue #9179 for details. - namespaces, err := backupRequest.NamespaceIncludesExcludes.ResolveNamespaceList() + namespaces, err := backupRequest.NamespaceIncludesExcludes.ResolveNamespaceList(true) if err != nil { log.WithError(err).Error("Failed to resolve namespace list for PVC-to-Pod cache") return err diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index f9351245c..072f3cd51 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -36,6 +36,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" corev1api "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -281,8 +282,8 @@ func TestBackupOldResourceFiltering(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.Pods( - builder.ForPod("foo", "bar").Result(), - builder.ForPod("zoo", "raz").Result(), + builder.ForPod("foo", "bar").Phase(corev1api.PodRunning).Result(), + builder.ForPod("zoo", "raz").Phase(corev1api.PodRunning).Result(), ), test.Deployments( builder.ForDeployment("foo", "bar").Result(), @@ -980,28 +981,6 @@ func TestCRDInclusion(t *testing.T) { "resources/volumesnapshotlocations.velero.io/v1-preferredversion/namespaces/foo/vsl-1.json", }, }, - { - name: "include cluster resources=auto includes CRDs with CRs when backing up selected namespaces", - backup: defaultBackup(). - IncludedNamespaces("foo"). - Result(), - apiResources: []*test.APIResource{ - test.CRDs( - builder.ForCustomResourceDefinitionV1Beta1("backups.velero.io").Result(), - builder.ForCustomResourceDefinitionV1Beta1("volumesnapshotlocations.velero.io").Result(), - builder.ForCustomResourceDefinitionV1Beta1("test.velero.io").Result(), - ), - test.VSLs( - builder.ForVolumeSnapshotLocation("foo", "vsl-1").Result(), - ), - }, - want: []string{ - "resources/customresourcedefinitions.apiextensions.k8s.io/cluster/volumesnapshotlocations.velero.io.json", - "resources/volumesnapshotlocations.velero.io/namespaces/foo/vsl-1.json", - "resources/customresourcedefinitions.apiextensions.k8s.io/v1beta1-preferredversion/cluster/volumesnapshotlocations.velero.io.json", - "resources/volumesnapshotlocations.velero.io/v1-preferredversion/namespaces/foo/vsl-1.json", - }, - }, { name: "include-cluster-resources=false excludes all CRDs when backing up selected namespaces", backup: defaultBackup(). @@ -4296,6 +4275,12 @@ func (h *harness) addItems(t *testing.T, resource *test.APIResource) { unstructuredObj := &unstructured.Unstructured{Object: obj} if resource.Namespaced { + namespace := &corev1api.Namespace{ObjectMeta: metav1.ObjectMeta{Name: item.GetNamespace()}} + err = h.backupper.kbClient.Create(t.Context(), namespace) + if err != nil && !apierrors.IsAlreadyExists(err) { + require.NoError(t, err) + } + _, err = h.DynamicClient.Resource(resource.GVR()).Namespace(item.GetNamespace()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{}) } else { _, err = h.DynamicClient.Resource(resource.GVR()).Create(t.Context(), unstructuredObj, metav1.CreateOptions{}) @@ -4346,7 +4331,7 @@ func newSnapshotLocation(ns, name, provider string) *velerov1.VolumeSnapshotLoca } func defaultBackup() *builder.BackupBuilder { - return builder.ForBackup(velerov1.DefaultNamespace, "backup-1").DefaultVolumesToFsBackup(false) + return builder.ForBackup(velerov1.DefaultNamespace, "backup-1").DefaultVolumesToFsBackup(false).IncludedNamespaces("*") } func toUnstructuredOrFail(t *testing.T, obj any) map[string]any { @@ -5422,8 +5407,6 @@ func TestBackupNamespaces(t *testing.T) { want: []string{ "resources/namespaces/cluster/ns-1.json", "resources/namespaces/v1-preferredversion/cluster/ns-1.json", - "resources/namespaces/cluster/ns-3.json", - "resources/namespaces/v1-preferredversion/cluster/ns-3.json", }, }, { @@ -5457,10 +5440,6 @@ func TestBackupNamespaces(t *testing.T) { want: []string{ "resources/namespaces/cluster/ns-1.json", "resources/namespaces/v1-preferredversion/cluster/ns-1.json", - "resources/namespaces/cluster/ns-2.json", - "resources/namespaces/v1-preferredversion/cluster/ns-2.json", - "resources/namespaces/cluster/ns-3.json", - "resources/namespaces/v1-preferredversion/cluster/ns-3.json", "resources/deployments.apps/namespaces/ns-1/deploy-1.json", "resources/deployments.apps/v1-preferredversion/namespaces/ns-1/deploy-1.json", }, diff --git a/pkg/backup/item_collector.go b/pkg/backup/item_collector.go index 3dace71fd..34c42e2a2 100644 --- a/pkg/backup/item_collector.go +++ b/pkg/backup/item_collector.go @@ -633,22 +633,19 @@ func coreGroupResourcePriority(resource string) int { } // getNamespacesToList examines ie and resolves the includes and excludes to a full list of -// namespaces to list. If ie is nil or it includes *, the result is just "" (list across all -// namespaces). Otherwise, the result is a list of every included namespace minus all excluded ones. +// namespaces to list. If ie is nil, the result is just "" (list across all namespaces). +// Otherwise, the result is a list of every included namespace minus all excluded ones. +// Because the namespace IE filter is expanded from 1.18, there is no need to consider +// wildcard characters anymore. func getNamespacesToList(ie *collections.NamespaceIncludesExcludes) []string { if ie == nil { return []string{""} } - if ie.ShouldInclude("*") { - // "" means all namespaces - return []string{""} - } - var list []string - for _, i := range ie.GetIncludes() { - if ie.ShouldInclude(i) { - list = append(list, i) + for _, n := range ie.GetIncludes() { + if ie.ShouldInclude(n) { + list = append(list, n) } } diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 496308a6e..335becdb8 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -575,6 +575,14 @@ func (b *backupReconciler) prepareBackupRequest(ctx context.Context, backup *vel request.Status.ValidationErrors = append(request.Status.ValidationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) } + // if included namespaces is empty, default to wildcard to include all namespaces + // This is useful for later wildcard expansion logic. + // This also align the behavior between backup creation from CLI and from API, + // as CLI will default to wildcard if included namespaces is not specified. + if request.Spec.IncludedNamespaces == nil { + request.Spec.IncludedNamespaces = []string{"*"} + } + // validate that only one exists orLabelSelector or just labelSelector (singular) if request.Spec.OrLabelSelectors != nil && request.Spec.LabelSelector != nil { request.Status.ValidationErrors = append(request.Status.ValidationErrors, "encountered labelSelector as well as orLabelSelectors in backup spec, only one can be specified") diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 386498900..030ef9cdd 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -713,6 +713,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -752,6 +753,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -795,6 +797,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -835,6 +838,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -875,6 +879,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -916,6 +921,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -957,6 +963,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -998,6 +1005,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1039,6 +1047,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1081,6 +1090,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1123,6 +1133,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -1165,6 +1176,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1208,6 +1220,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1251,6 +1264,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1294,6 +1308,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1338,6 +1353,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.False(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1381,6 +1397,7 @@ func TestProcessBackupCompletions(t *testing.T) { SnapshotMoveData: boolptr.True(), ExcludedClusterScopedResources: autoExcludeClusterScopedResources, ExcludedNamespaceScopedResources: autoExcludeNamespaceScopedResources, + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1430,6 +1447,7 @@ func TestProcessBackupCompletions(t *testing.T) { ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, @@ -1479,6 +1497,7 @@ func TestProcessBackupCompletions(t *testing.T) { ExcludedClusterScopedResources: append([]string{"clusterroles"}, autoExcludeClusterScopedResources...), IncludedNamespaceScopedResources: []string{"pods"}, ExcludedNamespaceScopedResources: append([]string{"secrets"}, autoExcludeNamespaceScopedResources...), + IncludedNamespaces: []string{"*"}, }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFinalizing, diff --git a/pkg/controller/backup_queue_controller.go b/pkg/controller/backup_queue_controller.go index c5cdc9461..ab53973db 100644 --- a/pkg/controller/backup_queue_controller.go +++ b/pkg/controller/backup_queue_controller.go @@ -200,7 +200,7 @@ func (r *backupQueueReconciler) checkForEarlierRunnableBackups(backup *velerov1a func namespacesForBackup(backup *velerov1api.Backup, clusterNamespaces []string) []string { // Ignore error here. If a backup has invalid namespace wildcards, the backup controller // will validate and fail it. Consider the ns list empty for conflict detection purposes. - nsList, err := collections.NewNamespaceIncludesExcludes().Includes(backup.Spec.IncludedNamespaces...).Excludes(backup.Spec.ExcludedNamespaces...).ActiveNamespaces(clusterNamespaces).ResolveNamespaceList() + nsList, err := collections.NewNamespaceIncludesExcludes().Includes(backup.Spec.IncludedNamespaces...).Excludes(backup.Spec.ExcludedNamespaces...).ActiveNamespaces(clusterNamespaces).ResolveNamespaceList(true) if err != nil { return []string{} } diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 5287eec21..0a75fd89f 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -2407,7 +2407,7 @@ func extractNamespacesFromBackup(backupResources map[string]*archive.ResourceIte // expandNamespaceWildcards expands wildcard patterns in namespace includes/excludes // and updates the restore context with the expanded patterns and status func (ctx *restoreContext) expandNamespaceWildcards(backupResources map[string]*archive.ResourceItems) error { - if !wildcard.ShouldExpandWildcards(ctx.restore.Spec.IncludedNamespaces, ctx.restore.Spec.ExcludedNamespaces) { + if !wildcard.ShouldExpandWildcards(ctx.restore.Spec.IncludedNamespaces, ctx.restore.Spec.ExcludedNamespaces, false) { return nil } diff --git a/pkg/restore/restore_wildcard_test.go b/pkg/restore/restore_wildcard_test.go index c38ad7dd9..f50513bed 100644 --- a/pkg/restore/restore_wildcard_test.go +++ b/pkg/restore/restore_wildcard_test.go @@ -93,15 +93,6 @@ func TestExpandNamespaceWildcards(t *testing.T) { expectedExcludeMatches: []string{"app-test"}, expectedWildcardResult: []string{"app-dev", "app-prod"}, }, - { - name: "Error: wildcard * in excludes", - includeNamespaces: []string{"test*"}, - excludeNamespaces: []string{"*"}, - backupResources: map[string]*archive.ResourceItems{ - "namespaces": {ItemsByNamespace: map[string][]string{"test1": {}}}, - }, - expectedError: "wildcard '*' is not allowed in restore excludes", - }, { name: "Empty backup - no matches", includeNamespaces: []string{"test*"}, diff --git a/pkg/util/collections/includes_excludes.go b/pkg/util/collections/includes_excludes.go index ab63eaa72..b5d651316 100644 --- a/pkg/util/collections/includes_excludes.go +++ b/pkg/util/collections/includes_excludes.go @@ -17,6 +17,7 @@ limitations under the License. package collections import ( + "slices" "strings" "github.com/vmware-tanzu/velero/internal/resourcepolicies" @@ -149,15 +150,18 @@ func (nie *NamespaceIncludesExcludes) ShouldInclude(s string) bool { // IncludeEverything returns true if the includes list is empty or '*' // and the excludes list is empty, or false otherwise. func (nie *NamespaceIncludesExcludes) IncludeEverything() bool { - return nie.includesExcludes.IncludeEverything() + return nie.includesExcludes.excludes.Len() == 0 && + (nie.includesExcludes.includes.Len() == 0 || + (nie.includesExcludes.includes.Len() == 1 && nie.includesExcludes.includes.Has("*")) || + slices.Equal(nie.includesExcludes.includes.List(), nie.activeNamespaces)) } // Attempts to expand wildcard patterns, if any, in the includes and excludes lists. -func (nie *NamespaceIncludesExcludes) ExpandIncludesExcludes() error { +func (nie *NamespaceIncludesExcludes) ExpandIncludesExcludes(fromBackup bool) error { includes := nie.GetIncludes() excludes := nie.GetExcludes() - if wildcard.ShouldExpandWildcards(includes, excludes) { + if wildcard.ShouldExpandWildcards(includes, excludes, fromBackup) { expandedIncludes, expandedExcludes, err := wildcard.ExpandWildcards( nie.activeNamespaces, includes, excludes) if err != nil { @@ -174,10 +178,10 @@ func (nie *NamespaceIncludesExcludes) ExpandIncludesExcludes() error { // ResolveNamespaceList returns a list of all namespaces which will be backed up. // The second return value indicates whether wildcard expansion was performed. -func (nie *NamespaceIncludesExcludes) ResolveNamespaceList() ([]string, error) { +func (nie *NamespaceIncludesExcludes) ResolveNamespaceList(fromBackup bool) ([]string, error) { // Check if this is being called by non-backup processing e.g. backup queue controller if !nie.wildcardExpanded { - err := nie.ExpandIncludesExcludes() + err := nie.ExpandIncludesExcludes(fromBackup) if err != nil { return nil, err } diff --git a/pkg/util/collections/includes_excludes_test.go b/pkg/util/collections/includes_excludes_test.go index 1d700a729..867397cbb 100644 --- a/pkg/util/collections/includes_excludes_test.go +++ b/pkg/util/collections/includes_excludes_test.go @@ -1049,6 +1049,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes []string expectedWildcardExpanded bool expectError bool + fromBackup bool }{ { name: "no wildcards - should not expand", @@ -1059,16 +1060,18 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{"kube-public"}, expectedWildcardExpanded: false, expectError: false, + fromBackup: true, }, { - name: "asterisk alone - should not expand", + name: "asterisk alone - should expand", includes: []string{"*"}, excludes: []string{}, activeNamespaces: []string{"default", "kube-system", "test"}, - expectedIncludes: []string{"*"}, + expectedIncludes: []string{"default", "kube-system", "test"}, expectedExcludes: []string{}, - expectedWildcardExpanded: false, + expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "wildcard in includes - should expand", @@ -1079,6 +1082,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "wildcard in excludes - should expand", @@ -1089,6 +1093,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{"kube-test", "app-test"}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "wildcards in both includes and excludes", @@ -1099,6 +1104,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{"kube-test", "app-test"}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "wildcard pattern matches nothing", @@ -1109,6 +1115,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "mix of wildcards and non-wildcards in includes", @@ -1119,6 +1126,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "question mark wildcard", @@ -1129,6 +1137,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "empty activeNamespaces with wildcards", @@ -1139,6 +1148,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: true, expectError: false, + fromBackup: true, }, { name: "invalid wildcard pattern - consecutive asterisks", @@ -1149,6 +1159,7 @@ func TestExpandIncludesExcludes(t *testing.T) { expectedExcludes: []string{}, expectedWildcardExpanded: false, expectError: true, + fromBackup: true, }, } @@ -1159,7 +1170,7 @@ func TestExpandIncludesExcludes(t *testing.T) { Includes(tc.includes...). Excludes(tc.excludes...) - err := nie.ExpandIncludesExcludes() + err := nie.ExpandIncludesExcludes(tc.fromBackup) if tc.expectError { assert.Error(t, err) @@ -1192,6 +1203,7 @@ func TestResolveNamespaceList(t *testing.T) { activeNamespaces []string expectedNamespaces []string preExpandWildcards bool + fromBackup bool }{ { name: "no includes/excludes - all active namespaces", @@ -1199,6 +1211,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "kube-system", "test"}, expectedNamespaces: []string{"default", "kube-system", "test"}, + fromBackup: true, }, { name: "asterisk includes - all active namespaces", @@ -1206,6 +1219,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "kube-system", "test"}, expectedNamespaces: []string{"default", "kube-system", "test"}, + fromBackup: true, }, { name: "specific includes - only those namespaces", @@ -1213,6 +1227,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "kube-system", "test"}, expectedNamespaces: []string{"default", "test"}, + fromBackup: true, }, { name: "includes with excludes", @@ -1220,6 +1235,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{"kube-system"}, activeNamespaces: []string{"default", "kube-system", "test"}, expectedNamespaces: []string{"default", "test"}, + fromBackup: true, }, { name: "wildcard includes - expands and filters", @@ -1227,6 +1243,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "kube-system", "kube-public", "test"}, expectedNamespaces: []string{"kube-system", "kube-public"}, + fromBackup: true, }, { name: "wildcard includes with wildcard excludes", @@ -1234,6 +1251,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{"*-test"}, activeNamespaces: []string{"app-prod", "app-dev", "app-test", "default"}, expectedNamespaces: []string{"app-prod", "app-dev"}, + fromBackup: true, }, { name: "wildcard matches nothing - empty result", @@ -1241,6 +1259,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "kube-system"}, expectedNamespaces: []string{}, + fromBackup: true, }, { name: "empty active namespaces", @@ -1248,6 +1267,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{}, expectedNamespaces: []string{}, + fromBackup: true, }, { name: "includes namespace not in active namespaces", @@ -1255,6 +1275,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"default", "test"}, expectedNamespaces: []string{"default"}, + fromBackup: true, }, { name: "excludes all namespaces from includes", @@ -1262,6 +1283,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{"default", "test"}, activeNamespaces: []string{"default", "test", "prod"}, expectedNamespaces: []string{}, + fromBackup: true, }, { name: "pre-expanded wildcards - should not expand again", @@ -1270,6 +1292,7 @@ func TestResolveNamespaceList(t *testing.T) { activeNamespaces: []string{"default", "kube-system", "kube-public"}, expectedNamespaces: []string{"kube-system", "kube-public"}, preExpandWildcards: true, + fromBackup: true, }, { name: "question mark wildcard pattern", @@ -1277,6 +1300,7 @@ func TestResolveNamespaceList(t *testing.T) { excludes: []string{}, activeNamespaces: []string{"ns-1", "ns-2", "ns-10", "default"}, expectedNamespaces: []string{"ns-1", "ns-2"}, + fromBackup: true, }, } @@ -1289,11 +1313,11 @@ func TestResolveNamespaceList(t *testing.T) { // Pre-expand wildcards if requested if tc.preExpandWildcards { - err := nie.ExpandIncludesExcludes() + err := nie.ExpandIncludesExcludes(tc.fromBackup) require.NoError(t, err) } - namespaces, err := nie.ResolveNamespaceList() + namespaces, err := nie.ResolveNamespaceList(tc.fromBackup) require.NoError(t, err) // Convert to sets for order-independent comparison @@ -1311,18 +1335,21 @@ func TestResolveNamespaceListError(t *testing.T) { includes []string excludes []string activeNamespaces []string + fromBackup bool }{ { name: "invalid wildcard pattern in includes", includes: []string{"kube-**"}, excludes: []string{}, activeNamespaces: []string{"default"}, + fromBackup: true, }, { name: "invalid wildcard pattern in excludes", includes: []string{"default"}, excludes: []string{"test-**"}, activeNamespaces: []string{"default"}, + fromBackup: true, }, } @@ -1333,7 +1360,7 @@ func TestResolveNamespaceListError(t *testing.T) { Includes(tc.includes...). Excludes(tc.excludes...) - _, err := nie.ResolveNamespaceList() + _, err := nie.ResolveNamespaceList(tc.fromBackup) assert.Error(t, err) }) } @@ -1347,6 +1374,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing activeNamespaces []string testNamespace string expectedResult bool + fromBackup bool }{ { name: "wildcard expanded to empty includes - should not include anything", @@ -1355,6 +1383,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing activeNamespaces: []string{"default", "kube-system"}, testNamespace: "default", expectedResult: false, + fromBackup: true, }, { name: "wildcard expanded with matches - should include matched namespace", @@ -1363,6 +1392,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing activeNamespaces: []string{"default", "kube-system", "kube-public"}, testNamespace: "kube-system", expectedResult: true, + fromBackup: true, }, { name: "wildcard expanded with matches - should not include unmatched namespace", @@ -1371,6 +1401,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing activeNamespaces: []string{"default", "kube-system", "kube-public"}, testNamespace: "default", expectedResult: false, + fromBackup: true, }, { name: "no wildcard expansion - empty includes means include all", @@ -1379,6 +1410,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing activeNamespaces: []string{"default", "kube-system"}, testNamespace: "default", expectedResult: true, + fromBackup: true, }, } @@ -1389,7 +1421,7 @@ func TestNamespaceIncludesExcludesShouldIncludeAfterWildcardExpansion(t *testing Includes(tc.includes...). Excludes(tc.excludes...) - err := nie.ExpandIncludesExcludes() + err := nie.ExpandIncludesExcludes(tc.fromBackup) require.NoError(t, err) result := nie.ShouldInclude(tc.testNamespace) diff --git a/pkg/util/wildcard/expand.go b/pkg/util/wildcard/expand.go index 8767c8ed3..ec572f359 100644 --- a/pkg/util/wildcard/expand.go +++ b/pkg/util/wildcard/expand.go @@ -8,14 +8,19 @@ import ( "k8s.io/apimachinery/pkg/util/sets" ) -func ShouldExpandWildcards(includes []string, excludes []string) bool { +func ShouldExpandWildcards(includes []string, excludes []string, fromBackup bool) bool { + // Only expand wildcards if this is being called from a backup request. + // We don't want to expand wildcard patterns in restore request, + // because restore needs the IncludeEverything function to + // determine whether to restore cluster-scoped resources. + // Expand wildcards causes the IncludeEverything function to return false, + // which will cause restore to skip cluster-scoped resources. + if !fromBackup { + return false + } + wildcardFound := false for _, include := range includes { - // Special case: "*" alone means "match all" - don't expand - if include == "*" { - return false - } - if containsWildcardPattern(include) { wildcardFound = true } diff --git a/pkg/util/wildcard/expand_test.go b/pkg/util/wildcard/expand_test.go index 317020648..4d1d68761 100644 --- a/pkg/util/wildcard/expand_test.go +++ b/pkg/util/wildcard/expand_test.go @@ -9,106 +9,141 @@ import ( func TestShouldExpandWildcards(t *testing.T) { tests := []struct { - name string - includes []string - excludes []string - expected bool + name string + includes []string + excludes []string + fromBackup bool + expected bool }{ { - name: "no wildcards", - includes: []string{"ns1", "ns2"}, - excludes: []string{"ns3", "ns4"}, - expected: false, + name: "no wildcards", + includes: []string{"ns1", "ns2"}, + excludes: []string{"ns3", "ns4"}, + fromBackup: true, + expected: false, }, { - name: "includes has star - should not expand", - includes: []string{"*"}, - excludes: []string{"ns1"}, - expected: false, + name: "includes has star - should expand", + includes: []string{"*"}, + excludes: []string{"ns1"}, + fromBackup: true, + expected: true, }, { - name: "includes has star after a wildcard pattern - should not expand", - includes: []string{"ns*", "*"}, - excludes: []string{"ns1"}, - expected: false, + excludes: []string{"ns3", "ns4"}, + fromBackup: true, + expected: false, }, { - name: "includes has wildcard pattern", - includes: []string{"ns*"}, - excludes: []string{"ns1"}, - expected: true, + name: "includes has star - should expand", + includes: []string{"*"}, + excludes: []string{"ns1"}, + fromBackup: true, + expected: true, }, { - name: "excludes has wildcard pattern", - includes: []string{"ns1"}, - excludes: []string{"ns*"}, - expected: true, + name: "includes has star after a wildcard pattern - should expand", + includes: []string{"ns*", "*"}, + excludes: []string{"ns1"}, + fromBackup: true, + expected: true, }, { - name: "both have wildcard patterns", - includes: []string{"app-*"}, - excludes: []string{"test-*"}, - expected: true, + name: "includes has wildcard pattern", + includes: []string{"ns*"}, + excludes: []string{"ns1"}, + fromBackup: true, + expected: true, }, { - name: "includes has star and wildcard - star takes precedence", - includes: []string{"*", "ns*"}, - excludes: []string{}, - expected: false, + name: "excludes has wildcard pattern", + includes: []string{"ns1"}, + excludes: []string{"ns*"}, + fromBackup: true, + expected: true, }, { - name: "double asterisk should be detected as wildcard", - includes: []string{"**"}, - excludes: []string{}, - expected: true, // ** is a wildcard pattern (but will error during validation) + name: "both have wildcard patterns", + includes: []string{"app-*"}, + excludes: []string{"test-*"}, + fromBackup: true, + expected: true, }, { - name: "empty slices", - includes: []string{}, - excludes: []string{}, - expected: false, + name: "includes has star and wildcard - should expand", + includes: []string{"*", "ns*"}, + excludes: []string{}, + fromBackup: true, + expected: true, }, { - name: "complex wildcard patterns", - includes: []string{"*-prod"}, - excludes: []string{"test-*-staging"}, - expected: true, + name: "double asterisk should be detected as wildcard", + includes: []string{"**"}, + excludes: []string{}, + fromBackup: true, + expected: true, // ** is a wildcard pattern (but will error during validation) }, { - name: "question mark wildcard", - includes: []string{"ns?"}, - excludes: []string{}, - expected: true, // question mark is now considered a wildcard + name: "empty slices", + includes: []string{}, + excludes: []string{}, + fromBackup: true, + expected: false, }, { - name: "character class wildcard", - includes: []string{"ns[abc]"}, - excludes: []string{}, - expected: true, // character class is considered wildcard + name: "complex wildcard patterns", + includes: []string{"*-prod"}, + excludes: []string{"test-*-staging"}, + fromBackup: true, + expected: true, }, { - name: "brace alternatives wildcard", - includes: []string{"ns{prod,staging}"}, - excludes: []string{}, - expected: false, // brace alternatives are not supported + name: "question mark wildcard", + includes: []string{"ns?"}, + excludes: []string{}, + fromBackup: true, + expected: true, // question mark is now considered a wildcard }, { - name: "dot is literal - not wildcard", - includes: []string{"app.prod"}, - excludes: []string{}, - expected: false, // dot is literal, not wildcard + name: "character class wildcard", + includes: []string{"ns[abc]"}, + excludes: []string{}, + fromBackup: true, + expected: true, // character class is considered wildcard }, { - name: "plus is literal - not wildcard", - includes: []string{"app+"}, - excludes: []string{}, - expected: false, // plus is literal, not wildcard + name: "brace alternatives wildcard", + includes: []string{"ns{prod,staging}"}, + excludes: []string{}, + fromBackup: true, + expected: false, // brace alternatives are not supported + }, + { + name: "dot is literal - not wildcard", + includes: []string{"app.prod"}, + excludes: []string{}, + fromBackup: true, + expected: false, // dot is literal, not wildcard + }, + { + name: "plus is literal - not wildcard", + includes: []string{"app+"}, + excludes: []string{}, + fromBackup: true, + expected: false, // plus is literal, not wildcard + }, + { + name: "includes has a wildcard pattern from restore - should not expand", + includes: []string{"ns*"}, + excludes: []string{"ns1"}, + fromBackup: false, + expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ShouldExpandWildcards(tt.includes, tt.excludes) + result := ShouldExpandWildcards(tt.includes, tt.excludes, tt.fromBackup) assert.Equal(t, tt.expected, result) }) }