Merge pull request #10376 from opbot-xd/fix-context-propagation

Fix context propagation bug in GetDefaultBackupStorageLocations and add missing test coverage

changelogs/unreleased/10376-opbot-xd

@@ -0,0 +1 @@
+Fix context propagation bug in GetDefaultBackupStorageLocations and add missing test coverage for core components

internal/credentials/secret_store_test.go

@@ -0,0 +1,97 @@
+/*
+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 credentials
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	corev1api "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+)
+
+func TestNamespacedSecretStore(t *testing.T) {
+	scheme := runtime.NewScheme()
+	g := NewWithT(t)
+	g.Expect(corev1api.AddToScheme(scheme)).To(Succeed())
+
+	secret := &corev1api.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-secret",
+			Namespace: "velero",
+		},
+		Data: map[string][]byte{
+			"creds-key": []byte("my-super-secret-value"),
+		},
+	}
+
+	client := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(secret).Build()
+
+	store, err := NewNamespacedSecretStore(client, "velero")
+	g.Expect(err).ToNot(HaveOccurred())
+
+	tests := []struct {
+		name        string
+		selector    *corev1api.SecretKeySelector
+		expectedVal string
+		expectErr   bool
+	}{
+		{
+			name: "existing secret and key returns the correct value",
+			selector: &corev1api.SecretKeySelector{
+				LocalObjectReference: corev1api.LocalObjectReference{Name: "test-secret"},
+				Key:                  "creds-key",
+			},
+			expectedVal: "my-super-secret-value",
+			expectErr:   false,
+		},
+		{
+			name: "missing secret returns an error",
+			selector: &corev1api.SecretKeySelector{
+				LocalObjectReference: corev1api.LocalObjectReference{Name: "missing-secret"},
+				Key:                  "creds-key",
+			},
+			expectedVal: "",
+			expectErr:   true,
+		},
+		{
+			name: "missing key in existing secret returns an error",
+			selector: &corev1api.SecretKeySelector{
+				LocalObjectReference: corev1api.LocalObjectReference{Name: "test-secret"},
+				Key:                  "missing-key",
+			},
+			expectedVal: "",
+			expectErr:   true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			val, err := store.Get(tc.selector)
+			if tc.expectErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(val).To(Equal(tc.expectedVal))
+			}
+		})
+	}
+}

internal/storage/storagelocation.go

@@ -96,7 +96,7 @@ func ListBackupStorageLocations(ctx context.Context, kbClient client.Client, nam
 func GetDefaultBackupStorageLocations(ctx context.Context, kbClient client.Client, namespace string) (*velerov1api.BackupStorageLocationList, error) {
 	locations := new(velerov1api.BackupStorageLocationList)
 	defaultLocations := new(velerov1api.BackupStorageLocationList)
-	if err := kbClient.List(context.Background(), locations, &client.ListOptions{Namespace: namespace}); err != nil {
+	if err := kbClient.List(ctx, locations, &client.ListOptions{Namespace: namespace}); err != nil {
 		return defaultLocations, errors.Wrapf(err, "failed to list backup storage locations in namespace %s", namespace)
 	}
 

internal/storage/storagelocation_test.go

@@ -173,3 +173,79 @@ func TestListBackupStorageLocations(t *testing.T) {
 		})
 	}
 }
+
+func TestGetDefaultBackupStorageLocations(t *testing.T) {
+	tests := []struct {
+		name             string
+		locations        *velerov1api.BackupStorageLocationList
+		expectedDefaults []string
+		expectedErr      bool
+	}{
+		{
+			name: "no default locations",
+			locations: &velerov1api.BackupStorageLocationList{
+				Items: []velerov1api.BackupStorageLocation{
+					*builder.ForBackupStorageLocation("ns-1", "loc-1").Default(false).Result(),
+					*builder.ForBackupStorageLocation("ns-1", "loc-2").Default(false).Result(),
+				},
+			},
+			expectedDefaults: nil,
+			expectedErr:      false,
+		},
+		{
+			name: "one default location",
+			locations: &velerov1api.BackupStorageLocationList{
+				Items: []velerov1api.BackupStorageLocation{
+					*builder.ForBackupStorageLocation("ns-1", "loc-1").Default(false).Result(),
+					*builder.ForBackupStorageLocation("ns-1", "loc-2").Default(true).Result(),
+				},
+			},
+			expectedDefaults: []string{"loc-2"},
+			expectedErr:      false,
+		},
+		{
+			name: "multiple default locations",
+			locations: &velerov1api.BackupStorageLocationList{
+				Items: []velerov1api.BackupStorageLocation{
+					*builder.ForBackupStorageLocation("ns-1", "loc-1").Default(true).Result(),
+					*builder.ForBackupStorageLocation("ns-1", "loc-2").Default(true).Result(),
+					*builder.ForBackupStorageLocation("ns-1", "loc-3").Default(false).Result(),
+				},
+			},
+			expectedDefaults: []string{"loc-1", "loc-2"},
+			expectedErr:      false,
+		},
+		{
+			name:             "empty locations list",
+			locations:        &velerov1api.BackupStorageLocationList{},
+			expectedDefaults: nil,
+			expectedErr:      false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			client := fake.NewClientBuilder().WithScheme(util.VeleroScheme).WithRuntimeObjects(tt.locations).Build()
+
+			defaults, err := GetDefaultBackupStorageLocations(t.Context(), client, "ns-1")
+			if tt.expectedErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+
+				var defaultNames []string
+				for _, loc := range defaults.Items {
+					defaultNames = append(defaultNames, loc.Name)
+				}
+
+				if tt.expectedDefaults == nil {
+					g.Expect(defaultNames).To(BeEmpty())
+				} else {
+					g.Expect(defaultNames).To(ConsistOf(tt.expectedDefaults))
+				}
+			}
+		})
+	}
+}