From 12c1ef4ff2781482e996d88521090db3e00c5af7 Mon Sep 17 00:00:00 2001 From: samay43 Date: Wed, 5 Aug 2026 14:03:42 +0530 Subject: [PATCH 01/14] validate backup name format before contacting the API server Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 5e18f468f..f42a4dc70 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -26,6 +26,7 @@ import ( "github.com/spf13/pflag" kubeerrs "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/cache" + "k8s.io/apimachinery/pkg/util/validation" kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -186,10 +187,12 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - // Ensure that unless FromSchedule is set, args contains a backup name - if o.FromSchedule == "" && len(args) != 1 { - return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") - } + // Ensure the backup name is a valid Kubernetes resource name + if o.FromSchedule == "" { + if errs := validation.IsDNS1123Subdomain(o.Name); len(errs) > 0 { + return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) + } +} errs := collections.ValidateNamespaceIncludesExcludes(o.IncludeNamespaces, o.ExcludeNamespaces) if len(errs) > 0 { From be6b4d38f2e828045801096ad74224744cb26e52 Mon Sep 17 00:00:00 2001 From: samay43 Date: Wed, 5 Aug 2026 20:24:35 +0530 Subject: [PATCH 02/14] gofmt pkg/cmd/cli/backup/create.go Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index f42a4dc70..fd46c50fa 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -25,8 +25,8 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" kubeerrs "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/client-go/tools/cache" "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/client-go/tools/cache" kbclient "sigs.k8s.io/controller-runtime/pkg/client" velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" @@ -189,10 +189,10 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto // Ensure the backup name is a valid Kubernetes resource name if o.FromSchedule == "" { - if errs := validation.IsDNS1123Subdomain(o.Name); len(errs) > 0 { - return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) - } -} + if errs := validation.IsDNS1123Subdomain(o.Name); len(errs) > 0 { + return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) + } + } errs := collections.ValidateNamespaceIncludesExcludes(o.IncludeNamespaces, o.ExcludeNamespaces) if len(errs) > 0 { From e97d01405b49c936971bbdc764c0798a1cac1bdc Mon Sep 17 00:00:00 2001 From: samay43 Date: Thu, 6 Aug 2026 10:52:03 +0530 Subject: [PATCH 03/14] add changelog entry Signed-off-by: samay43 --- changelogs/unreleased/10167-samay43 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/10167-samay43 diff --git a/changelogs/unreleased/10167-samay43 b/changelogs/unreleased/10167-samay43 new file mode 100644 index 000000000..0955fcc99 --- /dev/null +++ b/changelogs/unreleased/10167-samay43 @@ -0,0 +1 @@ +Validate backup name format before contacting the API server From 164d9343aad08b9a4091ab042487af2d608c1d50 Mon Sep 17 00:00:00 2001 From: samay43 Date: Wed, 12 Aug 2026 09:51:10 +0530 Subject: [PATCH 04/14] move backup name validation to Args to run before cluster contact, and validate regardless of --from-schedule Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 12 +++++++++++- pkg/cmd/cli/backup/create_test.go | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 7046f83e2..74cfe861d 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -46,7 +46,17 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: use + " NAME", Short: "Create a backup", - Args: cobra.MaximumNArgs(1), + Args: func(c *cobra.Command, args []string) error { + if err := cobra.MaximumNArgs(1)(c, args); err != nil { + return err + } + if len(args) == 1 { + if errs := validation.IsDNS1123Subdomain(args[0]); len(errs) > 0 { + return fmt.Errorf("invalid backup name %q: %s", args[0], strings.Join(errs, "; ")) + } + } + return nil + }, Run: func(c *cobra.Command, args []string) { cmd.CheckError(o.Complete(args, f)) cmd.CheckError(o.Validate(c, args, f)) diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 718ab0e96..4512ccd6c 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -233,7 +233,7 @@ func TestCreateOptions_OrderedResources(t *testing.T) { } func TestCreateCommand(t *testing.T) { - name := "nameToBeCreated" + name := "name-to-be-created" args := []string{name} t.Run("create a backup create command with full options except fromSchedule and wait, then run by create option", func(t *testing.T) { From 52f7c24084fef0062c10f76a251826cfdc0d7167 Mon Sep 17 00:00:00 2001 From: samay43 Date: Wed, 12 Aug 2026 16:39:47 +0530 Subject: [PATCH 05/14] address review feedback: validate backup name and require name unless from-schedule Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 74cfe861d..b5821a9b1 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -46,10 +46,14 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: use + " NAME", Short: "Create a backup", - Args: func(c *cobra.Command, args []string) error { + Args: func(c *cobra.Command, args []string) error { if err := cobra.MaximumNArgs(1)(c, args); err != nil { return err } + fromSchedule, _ := c.Flags().GetString("from-schedule") + if fromSchedule == "" && len(args) == 0 { + return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") + } if len(args) == 1 { if errs := validation.IsDNS1123Subdomain(args[0]); len(errs) > 0 { return fmt.Errorf("invalid backup name %q: %s", args[0], strings.Join(errs, "; ")) @@ -202,13 +206,16 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - // Ensure the backup name is a valid Kubernetes resource name - if o.FromSchedule == "" { + // Ensure that unless FromSchedule is set, a backup name is required + if o.FromSchedule == "" && o.Name == "" { + return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") + } + // Validate the backup name format whenever a name is provided + if o.Name != "" { if errs := validation.IsDNS1123Subdomain(o.Name); len(errs) > 0 { return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) } } - errs := collections.ValidateNamespaceIncludesExcludes(o.IncludeNamespaces, o.ExcludeNamespaces) if len(errs) > 0 { return kubeerrs.NewAggregate(errs) From c27a661d1d269eaea2f90251c22aba10fff8b959 Mon Sep 17 00:00:00 2001 From: samay43 Date: Thu, 13 Aug 2026 09:44:47 +0530 Subject: [PATCH 06/14] gofmt: fix indentation in create.go Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index b5821a9b1..3ef773b47 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -46,7 +46,7 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { c := &cobra.Command{ Use: use + " NAME", Short: "Create a backup", - Args: func(c *cobra.Command, args []string) error { + Args: func(c *cobra.Command, args []string) error { if err := cobra.MaximumNArgs(1)(c, args); err != nil { return err } From fa3f5737c896c2836d7b922bca744309eed7e25e Mon Sep 17 00:00:00 2001 From: samay43 Date: Thu, 13 Aug 2026 10:57:54 +0530 Subject: [PATCH 07/14] add test coverage for Args validation and Validate method Signed-off-by: samay43 --- pkg/cmd/cli/backup/create_test.go | 110 ++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index fe321f9c6..e955719eb 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -449,3 +449,113 @@ func TestCreateCommand(t *testing.T) { assert.NoError(t, e) }) } +func TestCreateCommand_Args(t *testing.T) { + testCases := []struct { + name string + args []string + fromSchedule string + expectError bool + }{ + { + name: "should error when no name and no from-schedule", + args: []string{}, + expectError: true, + }, + { + name: "should pass when a valid name is provided", + args: []string{"my-backup"}, + expectError: false, + }, + { + name: "should error when the name is not a valid DNS1123 subdomain", + args: []string{"Invalid_Name!"}, + expectError: true, + }, + { + name: "should pass with no name when from-schedule is set", + args: []string{}, + fromSchedule: "daily-backup", + expectError: false, + }, + { + name: "should error when more than one arg is given", + args: []string{"name1", "name2"}, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := &factorymocks.Factory{} + cmd := NewCreateCommand(f, "") + if tc.fromSchedule != "" { + err := cmd.Flags().Set("from-schedule", tc.fromSchedule) + assert.NoError(t, err) + } + + err := cmd.Args(cmd, tc.args) + + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestCreateOptions_Validate(t *testing.T) { + testCases := []struct { + name string + optName string + fromSchedule string + args []string + expectError bool + }{ + { + name: "should error when no name and no from-schedule", + optName: "", + args: []string{}, + expectError: true, + }, + { + name: "should pass with a valid name and no from-schedule", + optName: "my-backup", + args: []string{"my-backup"}, + expectError: false, + }, + { + name: "should error when name is invalid, regardless of from-schedule", + optName: "Invalid_Name!", + fromSchedule: "daily-backup", + args: []string{"Invalid_Name!"}, + expectError: true, + }, + { + name: "should pass when from-schedule is set and no name given", + optName: "", + fromSchedule: "daily-backup", + args: []string{}, + expectError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + f := &factorymocks.Factory{} + cmd := NewCreateCommand(f, "") + + o := NewCreateOptions() + o.Name = tc.optName + o.FromSchedule = tc.fromSchedule + + err := o.Validate(cmd, tc.args, f) + + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} From e2c3b4b2699836b7df83787ba2293c2053729eb2 Mon Sep 17 00:00:00 2001 From: samay43 Date: Thu, 13 Aug 2026 11:08:21 +0530 Subject: [PATCH 08/14] fix lint: use require.NoError for error assertion Signed-off-by: samay43 --- pkg/cmd/cli/backup/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index e955719eb..5f9b9aac0 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -490,7 +490,7 @@ func TestCreateCommand_Args(t *testing.T) { cmd := NewCreateCommand(f, "") if tc.fromSchedule != "" { err := cmd.Flags().Set("from-schedule", tc.fromSchedule) - assert.NoError(t, err) + require.NoError(t, err) } err := cmd.Args(cmd, tc.args) From efe1279d49b945b437f1ed598a4eaef798c8816d Mon Sep 17 00:00:00 2001 From: samay43 Date: Fri, 14 Aug 2026 10:37:57 +0530 Subject: [PATCH 09/14] validate schedule-derived backup names don't exceed length limit Signed-off-by: samay43 --- pkg/cmd/cli/backup/create.go | 11 +++++++++++ pkg/cmd/cli/backup/create_test.go | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 7fe6b0e64..a8d988d72 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -216,6 +216,17 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return fmt.Errorf("invalid backup name %q: %s", o.Name, strings.Join(errs, "; ")) } } + // When a backup name will be generated from the schedule (i.e. FromSchedule + // is set and no explicit name was given), ensure the schedule name leaves + // enough room for the generated timestamp suffix ("-" + 14-digit timestamp, + // 15 characters total) within the DNS1123 subdomain length limit. + if o.FromSchedule != "" && o.Name == "" { + const timestampSuffixLen = 15 // "-" + "20060102150405" + maxScheduleNameLen := validation.DNS1123SubdomainMaxLength - timestampSuffixLen + if len(o.FromSchedule) > maxScheduleNameLen { + return fmt.Errorf("schedule name %q is too long: must be %d characters or fewer to leave room for the generated timestamp suffix", o.FromSchedule, maxScheduleNameLen) + } + } errs := collections.ValidateNamespaceIncludesExcludes(o.IncludeNamespaces, o.ExcludeNamespaces) if len(errs) > 0 { return kubeerrs.NewAggregate(errs) diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 5f9b9aac0..528e76943 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -538,6 +538,20 @@ func TestCreateOptions_Validate(t *testing.T) { args: []string{}, expectError: false, }, + { + name: "should pass when schedule name leaves room for timestamp suffix", + optName: "", + fromSchedule: strings.Repeat("a", 238), // exactly at the 238-char limit + args: []string{}, + expectError: false, + }, + { + name: "should error when schedule name is too long to leave room for timestamp suffix", + optName: "", + fromSchedule: strings.Repeat("a", 239), // one over the 238-char limit + args: []string{}, + expectError: true, + }, } for _, tc := range testCases { From 1d9d45ee917b3a6ec5905a2aa5f4f26f1fbcf60f Mon Sep 17 00:00:00 2001 From: opbot_xd Date: Mon, 24 Aug 2026 14:04:26 +0530 Subject: [PATCH 10/14] 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 11/14] 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 cf2ad280d7970247c04113b2e4c96f60abf4e51a Mon Sep 17 00:00:00 2001 From: Adeet Phanse Date: Fri, 28 Aug 2026 16:02:01 -0400 Subject: [PATCH 12/14] chore(deps): bump klauspost/compress to v1.19.2 Signed-off-by: Adeet Phanse --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c38ab050a..e47ac9190 100644 --- a/go.mod +++ b/go.mod @@ -148,7 +148,7 @@ require ( github.com/kcp-dev/kcp/cli v0.27.1 // indirect github.com/kcp-dev/kcp/sdk v0.27.1 // indirect github.com/kcp-dev/logicalcluster/v3 v3.0.5 // indirect - github.com/klauspost/compress v1.18.6 // indirect + github.com/klauspost/compress v1.19.2 // indirect github.com/klauspost/cpuid/v2 v2.3.0 // indirect github.com/klauspost/crc32 v1.3.0 // indirect github.com/klauspost/pgzip v1.2.6 // indirect diff --git a/go.sum b/go.sum index f744bb2e4..0d195bfba 100644 --- a/go.sum +++ b/go.sum @@ -297,8 +297,8 @@ github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRt github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= -github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/klauspost/compress v1.19.2 h1:hMRETovs/pu/dVWN7zIT1PGG8t509MwT6bO7XSi26R8= +github.com/klauspost/compress v1.19.2/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/klauspost/cpuid/v2 v2.0.1/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/klauspost/cpuid/v2 v2.3.0 h1:S4CRMLnYUhGeDFDqkGriYKdfoFlDnMtqTiI/sFzhA9Y= github.com/klauspost/cpuid/v2 v2.3.0/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= 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 13/14] 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 14/14] 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) + } + }) + } +}