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>

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)
 			}
 

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")
+}

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)
 			}
 

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")
+}

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)
 			}
 

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")
+}

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))

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"))
+}