From 4c007c0af49d6d7336eae7a74aa53aa8232ba939 Mon Sep 17 00:00:00 2001 From: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:52:11 -0400 Subject: [PATCH] Scope schedule and repo CLI list calls to the Velero namespace (#10482) * Scope schedule and repo CLI list calls to the Velero namespace velero schedule get, velero schedule describe, velero schedule pause/unpause and velero repo get built a ctrlclient.ListOptions with a LabelSelector but no Namespace, so the list ran across every namespace in the cluster. Their single-name paths in the same functions already scope to f.Namespace(), and every sibling command (backup get, restore get, backup describe, restore describe, snapshot-location get, schedule delete) passes Namespace too, so the omission was an oversight rather than intent. The read commands print another installation's Schedules and BackupRepositories. runPause is worse: velero schedule pause --all and velero schedule unpause --all fetch Schedules from every namespace and then write Spec.Paused on each, so pausing one installation's schedules pauses every other installation's schedules as well. Add Namespace: f.Namespace() to the four List calls: pkg/cmd/cli/schedule/get.go:61 pkg/cmd/cli/schedule/describe.go:59 pkg/cmd/cli/schedule/pause.go:114 pkg/cmd/cli/repo/get.go:61 Neither pkg/cmd/cli/schedule nor pkg/cmd/cli/repo had any tests, so the regression tests are new files. Each seeds a fake client with one object in the Velero namespace and one in another-velero, and asserts the second is neither listed, described, nor paused. Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> * Add changelog for PR 10482 Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> --------- Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com> --- changelogs/unreleased/10482-MaxFreedomPollard | 1 + pkg/cmd/cli/repo/get.go | 2 +- pkg/cmd/cli/repo/get_test.go | 94 +++++++++++++++ pkg/cmd/cli/schedule/describe.go | 2 +- pkg/cmd/cli/schedule/describe_test.go | 50 ++++++++ pkg/cmd/cli/schedule/get.go | 2 +- pkg/cmd/cli/schedule/get_test.go | 77 ++++++++++++ pkg/cmd/cli/schedule/pause.go | 1 + pkg/cmd/cli/schedule/pause_test.go | 112 ++++++++++++++++++ 9 files changed, 338 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/10482-MaxFreedomPollard create mode 100644 pkg/cmd/cli/repo/get_test.go create mode 100644 pkg/cmd/cli/schedule/describe_test.go create mode 100644 pkg/cmd/cli/schedule/get_test.go create mode 100644 pkg/cmd/cli/schedule/pause_test.go diff --git a/changelogs/unreleased/10482-MaxFreedomPollard b/changelogs/unreleased/10482-MaxFreedomPollard new file mode 100644 index 000000000..59e7bfcfb --- /dev/null +++ b/changelogs/unreleased/10482-MaxFreedomPollard @@ -0,0 +1 @@ +Scope velero schedule get/describe/pause/unpause and velero repo get to the Velero namespace instead of listing across all namespaces diff --git a/pkg/cmd/cli/repo/get.go b/pkg/cmd/cli/repo/get.go index b3b914ae3..a0eac788c 100644 --- a/pkg/cmd/cli/repo/get.go +++ b/pkg/cmd/cli/repo/get.go @@ -58,7 +58,7 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command { selector, err = labels.Parse(listOptions.LabelSelector) cmd.CheckError(err) } - err = crClient.List(context.TODO(), repos, &ctrlclient.ListOptions{LabelSelector: selector}) + err = crClient.List(context.TODO(), repos, &ctrlclient.ListOptions{LabelSelector: selector, Namespace: f.Namespace()}) cmd.CheckError(err) } diff --git a/pkg/cmd/cli/repo/get_test.go b/pkg/cmd/cli/repo/get_test.go new file mode 100644 index 000000000..23474cbc7 --- /dev/null +++ b/pkg/cmd/cli/repo/get_test.go @@ -0,0 +1,94 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package repo + +import ( + "bytes" + "io" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func backupRepository(namespace, name string) *velerov1api.BackupRepository { + return &velerov1api.BackupRepository{ + TypeMeta: metav1.TypeMeta{ + APIVersion: velerov1api.SchemeGroupVersion.String(), + Kind: "BackupRepository", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Spec: velerov1api.BackupRepositorySpec{ + RepositoryType: velerov1api.BackupRepositoryTypeKopia, + }, + } +} + +// captureStdout runs execute and returns everything it wrote to os.Stdout. The +// output helpers write to os.Stdout directly rather than to the command's +// output writer, so the file has to be swapped to read them back. +func captureStdout(t *testing.T, execute func()) string { + t.Helper() + + r, w, err := os.Pipe() + require.NoError(t, err) + + original := os.Stdout + os.Stdout = w + defer func() { os.Stdout = original }() + + execute() + + require.NoError(t, w.Close()) + + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + require.NoError(t, err) + + return buf.String() +} + +func TestNewGetCommandListsOnlyTheVeleroNamespace(t *testing.T) { + ours := backupRepository(cmdtest.VeleroNameSpace, "ours") + theirs := backupRepository("another-velero", "theirs") + + crClient := velerotest.NewFakeControllerRuntimeClient(t, ours, theirs) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(crClient, nil) + + c := NewGetCommand(f, "get") + c.SetArgs([]string{}) + + out := captureStdout(t, func() { + require.NoError(t, c.Execute()) + }) + + assert.Contains(t, out, "ours") + assert.NotContains(t, out, "theirs") +} diff --git a/pkg/cmd/cli/schedule/describe.go b/pkg/cmd/cli/schedule/describe.go index b657245e9..6b6a2b950 100644 --- a/pkg/cmd/cli/schedule/describe.go +++ b/pkg/cmd/cli/schedule/describe.go @@ -56,7 +56,7 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { selector, err = labels.Parse(listOptions.LabelSelector) cmd.CheckError(err) } - err = crClient.List(context.TODO(), schedules, &ctrlclient.ListOptions{LabelSelector: selector}) + err = crClient.List(context.TODO(), schedules, &ctrlclient.ListOptions{LabelSelector: selector, Namespace: f.Namespace()}) cmd.CheckError(err) } diff --git a/pkg/cmd/cli/schedule/describe_test.go b/pkg/cmd/cli/schedule/describe_test.go new file mode 100644 index 000000000..6b3695213 --- /dev/null +++ b/pkg/cmd/cli/schedule/describe_test.go @@ -0,0 +1,50 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schedule + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func TestNewDescribeCommandDescribesOnlyTheVeleroNamespace(t *testing.T) { + ours := builder.ForSchedule(cmdtest.VeleroNameSpace, "ours").CronSchedule("@daily").Result() + theirs := builder.ForSchedule("another-velero", "theirs").CronSchedule("@daily").Result() + + crClient := velerotest.NewFakeControllerRuntimeClient(t, ours, theirs) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(crClient, nil) + + c := NewDescribeCommand(f, "describe") + c.SetArgs([]string{}) + + out := captureStdout(t, func() { + require.NoError(t, c.Execute()) + }) + + assert.Contains(t, out, "ours") + assert.NotContains(t, out, "theirs") +} diff --git a/pkg/cmd/cli/schedule/get.go b/pkg/cmd/cli/schedule/get.go index ba8ddb122..cb280e120 100644 --- a/pkg/cmd/cli/schedule/get.go +++ b/pkg/cmd/cli/schedule/get.go @@ -58,7 +58,7 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command { selector, err = labels.Parse(listOptions.LabelSelector) cmd.CheckError(err) } - err := crClient.List(context.TODO(), schedules, &ctrlclient.ListOptions{LabelSelector: selector}) + err := crClient.List(context.TODO(), schedules, &ctrlclient.ListOptions{LabelSelector: selector, Namespace: f.Namespace()}) cmd.CheckError(err) } diff --git a/pkg/cmd/cli/schedule/get_test.go b/pkg/cmd/cli/schedule/get_test.go new file mode 100644 index 000000000..7aa71844e --- /dev/null +++ b/pkg/cmd/cli/schedule/get_test.go @@ -0,0 +1,77 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schedule + +import ( + "bytes" + "io" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +// captureStdout runs execute and returns everything it wrote to os.Stdout. The +// describe and output helpers write to os.Stdout directly rather than to the +// command's output writer, so the file has to be swapped to read them back. +func captureStdout(t *testing.T, execute func()) string { + t.Helper() + + r, w, err := os.Pipe() + require.NoError(t, err) + + original := os.Stdout + os.Stdout = w + defer func() { os.Stdout = original }() + + execute() + + require.NoError(t, w.Close()) + + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + require.NoError(t, err) + + return buf.String() +} + +func TestNewGetCommandListsOnlyTheVeleroNamespace(t *testing.T) { + ours := builder.ForSchedule(cmdtest.VeleroNameSpace, "ours").CronSchedule("@daily").Result() + theirs := builder.ForSchedule("another-velero", "theirs").CronSchedule("@daily").Result() + + crClient := velerotest.NewFakeControllerRuntimeClient(t, ours, theirs) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(crClient, nil) + + c := NewGetCommand(f, "get") + c.SetArgs([]string{}) + + out := captureStdout(t, func() { + require.NoError(t, c.Execute()) + }) + + assert.Contains(t, out, "ours") + assert.NotContains(t, out, "theirs") +} diff --git a/pkg/cmd/cli/schedule/pause.go b/pkg/cmd/cli/schedule/pause.go index 06fc43f5c..d8ce6b00e 100644 --- a/pkg/cmd/cli/schedule/pause.go +++ b/pkg/cmd/cli/schedule/pause.go @@ -114,6 +114,7 @@ func runPause(f client.Factory, o *cli.SelectOptions, paused bool, skipImmediate res := new(velerov1api.ScheduleList) err := crClient.List(context.TODO(), res, &ctrlclient.ListOptions{ LabelSelector: selector, + Namespace: f.Namespace(), }) if err != nil { errs = append(errs, errors.WithStack(err)) diff --git a/pkg/cmd/cli/schedule/pause_test.go b/pkg/cmd/cli/schedule/pause_test.go new file mode 100644 index 000000000..85ae1797d --- /dev/null +++ b/pkg/cmd/cli/schedule/pause_test.go @@ -0,0 +1,112 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package schedule + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + "github.com/vmware-tanzu/velero/pkg/builder" + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + "github.com/vmware-tanzu/velero/pkg/cmd/cli" + cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test" + "github.com/vmware-tanzu/velero/pkg/cmd/util/flag" + velerotest "github.com/vmware-tanzu/velero/pkg/test" +) + +func scheduleIsPaused(t *testing.T, crClient ctrlclient.Client, namespace, name string) bool { + t.Helper() + + schedule := new(velerov1api.Schedule) + require.NoError(t, crClient.Get(t.Context(), ctrlclient.ObjectKey{Namespace: namespace, Name: name}, schedule)) + return schedule.Spec.Paused +} + +func TestRunPauseOnlyTouchesTheVeleroNamespace(t *testing.T) { + labeled := func(s *velerov1api.Schedule) *velerov1api.Schedule { + s.Labels = map[string]string{"foo": "bar"} + return s + } + + tests := []struct { + name string + options func(o *cli.SelectOptions) + }{ + { + name: "--all", + options: func(o *cli.SelectOptions) { o.All = true }, + }, + { + name: "--selector", + options: func(o *cli.SelectOptions) { + o.Selector = flag.LabelSelector{LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}} + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ours := labeled(builder.ForSchedule(cmdtest.VeleroNameSpace, "ours").CronSchedule("@daily").Result()) + theirs := labeled(builder.ForSchedule("another-velero", "theirs").CronSchedule("@daily").Result()) + + crClient := velerotest.NewFakeControllerRuntimeClient(t, ours, theirs) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(crClient, nil) + + o := cli.NewSelectOptions("pause", "schedule") + tc.options(o) + require.NoError(t, o.Validate()) + + require.NoError(t, runPause(f, o, true, nil)) + + assert.True(t, scheduleIsPaused(t, crClient, cmdtest.VeleroNameSpace, "ours")) + assert.False(t, scheduleIsPaused(t, crClient, "another-velero", "theirs")) + }) + } +} + +func TestRunUnpauseOnlyTouchesTheVeleroNamespace(t *testing.T) { + paused := func(ns, name string) *velerov1api.Schedule { + s := builder.ForSchedule(ns, name).CronSchedule("@daily").Result() + s.Spec.Paused = true + return s + } + + ours := paused(cmdtest.VeleroNameSpace, "ours") + theirs := paused("another-velero", "theirs") + + crClient := velerotest.NewFakeControllerRuntimeClient(t, ours, theirs) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(cmdtest.VeleroNameSpace) + f.On("KubebuilderClient").Return(crClient, nil) + + o := cli.NewSelectOptions("pause", "schedule") + o.All = true + + require.NoError(t, runPause(f, o, false, nil)) + + assert.False(t, scheduleIsPaused(t, crClient, cmdtest.VeleroNameSpace, "ours")) + assert.True(t, scheduleIsPaused(t, crClient, "another-velero", "theirs")) +}