From 1d9d45ee917b3a6ec5905a2aa5f4f26f1fbcf60f Mon Sep 17 00:00:00 2001 From: opbot_xd Date: Mon, 24 Aug 2026 14:04:26 +0530 Subject: [PATCH 1/4] Fix context propagation in GetDefaultBackupStorageLocations and add tests * Use correct context in GetDefaultBackupStorageLocations * Add TestGetDefaultBackupStorageLocations * Add TestNamespacedSecretStore * Add changelog for PR 10376 Signed-off-by: opbot_xd --- changelogs/unreleased/10376-opbot-xd | 1 + internal/credentials/secret_store_test.go | 97 +++++++++++++++++++++++ internal/storage/storagelocation.go | 2 +- internal/storage/storagelocation_test.go | 76 ++++++++++++++++++ 4 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/10376-opbot-xd create mode 100644 internal/credentials/secret_store_test.go diff --git a/changelogs/unreleased/10376-opbot-xd b/changelogs/unreleased/10376-opbot-xd new file mode 100644 index 000000000..a7f254e66 --- /dev/null +++ b/changelogs/unreleased/10376-opbot-xd @@ -0,0 +1 @@ +Fix context propagation bug in GetDefaultBackupStorageLocations and add missing test coverage for core components diff --git a/internal/credentials/secret_store_test.go b/internal/credentials/secret_store_test.go new file mode 100644 index 000000000..f9acf6c27 --- /dev/null +++ b/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)) + } + }) + } +} diff --git a/internal/storage/storagelocation.go b/internal/storage/storagelocation.go index 59afe6b79..5024553dc 100644 --- a/internal/storage/storagelocation.go +++ b/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) } diff --git a/internal/storage/storagelocation_test.go b/internal/storage/storagelocation_test.go index 44eabff48..474369487 100644 --- a/internal/storage/storagelocation_test.go +++ b/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)) + } + } + }) + } +} From f4d9bef51aded321c2113de625a6d4edc08a07be Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Fri, 28 Aug 2026 12:49:01 +0800 Subject: [PATCH 2/4] enhance the doc for backup deletion Signed-off-by: Lyndon-Li --- changelogs/unreleased/10430-Lyndon-Li | 1 + site/content/docs/main/backup-reference.md | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/10430-Lyndon-Li diff --git a/changelogs/unreleased/10430-Lyndon-Li b/changelogs/unreleased/10430-Lyndon-Li new file mode 100644 index 000000000..924b23a87 --- /dev/null +++ b/changelogs/unreleased/10430-Lyndon-Li @@ -0,0 +1 @@ +Enhance the doc for backup deletion \ No newline at end of file diff --git a/site/content/docs/main/backup-reference.md b/site/content/docs/main/backup-reference.md index f2045211f..46e0b0f5e 100644 --- a/site/content/docs/main/backup-reference.md +++ b/site/content/docs/main/backup-reference.md @@ -161,7 +161,12 @@ Pagination can be entirely disabled by setting `--client-page-size` to `0`. This ## Deleting Backups -Use the following commands to delete Velero backups and data: +Use the following commands to delete Velero backups: +`velero backup delete `: successful run of this command will: +- Immediately delete the resource backup data from the backup storage +- Immediately delete the volume snapshots associates to the backup if any (e.g., volumes are backed up with CSI snapshot backup or native snapshot method) +- Trigger the deletion of volume backup data if any data is persisted to the backup repository (e.g., volumes are backed up with CSI snapshot data movement or fs-backup method). You will not see the backup storage space is released immediately, the backup repository maintenance jobs will GC the data and finally release the storage space -* `kubectl delete backup -n ` will delete the backup custom resource only and will not delete any associated data from object/block storage -* `velero backup delete ` will delete the backup resource including all data in object/block storage +Velero backup deletion needs extra spaces in the backup storage, so make sure the backup storage is not full during the backup deletion, otherwise, the backup deletion or the following backup repository maintenance jobs may fail. + +`kubectl delete backup -n `: this command will delete the backup custom resource only and will not delete any associated data from the backup storage. So it is used for limited purposes only, e.g., all the backup data has been deleted in the backup storage, you just need to clear the orphaned CRs in the cluster. From 023d5ad471cce15a5047c5fe31a0a97cd8069c2c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:15:03 +0000 Subject: [PATCH 3/4] Bump the github-actions group with 2 updates Bumps the github-actions group with 2 updates: [helm/kind-action](https://github.com/helm/kind-action) and [github/codeql-action](https://github.com/github/codeql-action). Updates `helm/kind-action` from 7a97ed793754775518f9db3a8151ee7461dc9c31 to c72b4750145dbfb1c71734c3782a4db35a1c65c0 - [Release notes](https://github.com/helm/kind-action/releases) - [Commits](https://github.com/helm/kind-action/compare/7a97ed793754775518f9db3a8151ee7461dc9c31...c72b4750145dbfb1c71734c3782a4db35a1c65c0) Updates `github/codeql-action` from 4.37.7 to 4.37.9 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4.37.7...v4.37.9) --- updated-dependencies: - dependency-name: helm/kind-action dependency-version: c72b4750145dbfb1c71734c3782a4db35a1c65c0 dependency-type: direct:production dependency-group: github-actions - dependency-name: github/codeql-action dependency-version: 4.37.9 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/e2e-test-kind.yaml | 2 +- .github/workflows/nightly-trivy-scan.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e-test-kind.yaml b/.github/workflows/e2e-test-kind.yaml index fcf0d37c2..e3f220353 100644 --- a/.github/workflows/e2e-test-kind.yaml +++ b/.github/workflows/e2e-test-kind.yaml @@ -192,7 +192,7 @@ jobs: - name: Install MinIO run: | docker run -d --rm -p 9000:9000 -e "MINIO_ROOT_USER=minio" -e "MINIO_ROOT_PASSWORD=minio123" -e "MINIO_DEFAULT_BUCKETS=bucket,additional-bucket" bitnami/minio:local - - uses: helm/kind-action@7a97ed793754775518f9db3a8151ee7461dc9c31 # v1 + fix: add curl retry flags (https://github.com/helm/kind-action/pull/165) + - uses: helm/kind-action@c72b4750145dbfb1c71734c3782a4db35a1c65c0 # v1 + fix: add curl retry flags (https://github.com/helm/kind-action/pull/165) with: cluster_name: "kind" version: "v0.32.0" diff --git a/.github/workflows/nightly-trivy-scan.yml b/.github/workflows/nightly-trivy-scan.yml index d3f2e6062..09dcdbf4f 100644 --- a/.github/workflows/nightly-trivy-scan.yml +++ b/.github/workflows/nightly-trivy-scan.yml @@ -31,6 +31,6 @@ jobs: output: 'trivy-results.sarif' - name: Upload Trivy scan results to GitHub Security tab - uses: github/codeql-action/upload-sarif@v4.37.7 + uses: github/codeql-action/upload-sarif@v4.37.9 with: sarif_file: 'trivy-results.sarif' \ No newline at end of file From 7707681783f5812a6dd77bdbca6ae8fd78f1911b Mon Sep 17 00:00:00 2001 From: R4mbo Date: Mon, 31 Aug 2026 07:10:25 +0530 Subject: [PATCH 4/4] add test coverage for CleanupVolumeSnapshot (#10198) Signed-off-by: samay43 --- changelogs/unreleased/10198-samay43 | 1 + pkg/util/csi/volume_snapshot_test.go | 104 +++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 changelogs/unreleased/10198-samay43 diff --git a/changelogs/unreleased/10198-samay43 b/changelogs/unreleased/10198-samay43 new file mode 100644 index 000000000..ab4c0c7f9 --- /dev/null +++ b/changelogs/unreleased/10198-samay43 @@ -0,0 +1 @@ +add test coverage for CleanupVolumeSnapshot diff --git a/pkg/util/csi/volume_snapshot_test.go b/pkg/util/csi/volume_snapshot_test.go index 895935b4b..a8ab5ed4e 100644 --- a/pkg/util/csi/volume_snapshot_test.go +++ b/pkg/util/csi/volume_snapshot_test.go @@ -2179,3 +2179,107 @@ func TestGetVSCForVS(t *testing.T) { }) } } + +func TestCleanupVolumeSnapshot(t *testing.T) { + retainVSCName := "retain-vsc" + + testCases := []struct { + name string + volSnap *snapshotv1api.VolumeSnapshot + objs []runtime.Object + expectDeleted bool + // name of the VolumeSnapshotContent expected to have been patched to + // DeletionPolicy=Delete; empty when no VSC should be touched. + expectedVSC string + }{ + { + name: "should be a no-op if the VolumeSnapshot no longer exists", + volSnap: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "does-not-exist", + Namespace: "velero", + }, + }, + objs: []runtime.Object{}, + expectDeleted: false, + }, + { + name: "should delete a VolumeSnapshot with no bound VolumeSnapshotContent", + volSnap: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs-no-vsc", + Namespace: "velero", + }, + }, + objs: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs-no-vsc", + Namespace: "velero", + }, + }, + }, + expectDeleted: true, + }, + { + name: "should patch bound VSC DeletionPolicy to Delete and delete the VolumeSnapshot", + volSnap: &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs-with-vsc", + Namespace: "velero", + }, + }, + objs: []runtime.Object{ + &snapshotv1api.VolumeSnapshot{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vs-with-vsc", + Namespace: "velero", + }, + Status: &snapshotv1api.VolumeSnapshotStatus{ + BoundVolumeSnapshotContentName: &retainVSCName, + }, + }, + &snapshotv1api.VolumeSnapshotContent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "retain-vsc", + }, + Spec: snapshotv1api.VolumeSnapshotContentSpec{ + DeletionPolicy: snapshotv1api.VolumeSnapshotContentRetain, + }, + }, + }, + expectDeleted: true, + expectedVSC: "retain-vsc", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fakeClient := velerotest.NewFakeControllerRuntimeClient(t, tc.objs...) + + CleanupVolumeSnapshot(t.Context(), tc.volSnap, fakeClient, velerotest.NewLogger()) + + actual := new(snapshotv1api.VolumeSnapshot) + err := fakeClient.Get( + t.Context(), + crclient.ObjectKey{Name: tc.volSnap.Name, Namespace: tc.volSnap.Namespace}, + actual, + ) + + if tc.expectDeleted { + assert.True(t, apierrors.IsNotFound(err), "expected VolumeSnapshot to be deleted") + } + + if tc.expectedVSC != "" { + actualVSC := new(snapshotv1api.VolumeSnapshotContent) + err := fakeClient.Get( + t.Context(), + crclient.ObjectKey{Name: tc.expectedVSC}, + actualVSC, + ) + require.NoError(t, err) + assert.Equal(t, snapshotv1api.VolumeSnapshotContentDelete, actualVSC.Spec.DeletionPolicy) + } + }) + } +}