From 5fa1cc3bf5d12c6634bc66d347112c67f98b77e4 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Thu, 23 Jul 2026 12:08:39 -0700 Subject: [PATCH] Add SnapshotClassParameter constant and GetSnapshotClass getter Add a new snapshotClass action parameter to volume policies, allowing users to specify which VolumeSnapshotClass to use for CSI snapshots. This follows the existing dataMover parameter pattern with a typed constant and getter method on the Action struct. Ref: #8807 Signed-off-by: Shubham Pampattiwar --- .../resourcepolicies/resource_policies.go | 29 +++++++++- .../resource_policies_test.go | 57 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 39504d6ff..22830356a 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -54,6 +54,10 @@ const ( // DataMoverParameter is the key of the action parameter that selects the data // mover to be used for the matched volumes when the action type is snapshot. DataMoverParameter = "dataMover" + + // SnapshotClassParameter is the key of the action parameter that selects the + // VolumeSnapshotClass to use for CSI snapshots when the action type is snapshot. + SnapshotClassParameter = "snapshotClass" ) // validDataMovers is the set of data mover values accepted in the snapshot @@ -101,6 +105,30 @@ func (a *Action) GetDataMover() (string, error) { return dataMover, nil } +// GetSnapshotClass returns the VolumeSnapshotClass name configured in the +// snapshot action's snapshotClass parameter. The snapshotClass parameter is +// only meaningful for the snapshot action, so it returns an error when the +// action is nil or its type is not snapshot. When the parameter is absent, +// it returns an empty string, meaning the caller should fall back to the +// existing VolumeSnapshotClass selection logic. +func (a *Action) GetSnapshotClass() (string, error) { + if a == nil || a.Type != Snapshot { + return "", fmt.Errorf("the %q parameter is only supported for the %q action", SnapshotClassParameter, Snapshot) + } + if len(a.Parameters) == 0 { + return "", nil + } + raw, ok := a.Parameters[SnapshotClassParameter] + if !ok { + return "", nil + } + snapshotClass, ok := raw.(string) + if !ok { + return "", fmt.Errorf("parameter %q must be a string, got %T", SnapshotClassParameter, raw) + } + return snapshotClass, nil +} + // PolicyLabelSelector mirrors metav1.LabelSelector with yaml tags for ConfigMap decode. // metav1.LabelSelector only has json tags, which do not populate under go.yaml.in/yaml/v3. type PolicyLabelSelector struct { @@ -153,7 +181,6 @@ func validatePolicyLabelSelector(s *PolicyLabelSelector) error { _, err := SelectorFromPolicyLabelSelector(s) return err } - // ResourceFilter defines a filter for specific resource kinds. type ResourceFilter struct { Kinds []string `yaml:"kinds"` diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 7a7da6d3d..1c9e4635f 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -3063,3 +3063,60 @@ func TestActionGetDataMover(t *testing.T) { }) } } + +func TestActionGetSnapshotClass(t *testing.T) { + testCases := []struct { + name string + action *Action + expectedClass string + expectErr bool + }{ + { + name: "nil action", + action: nil, + expectErr: true, + }, + { + name: "snapshot action without parameters", + action: &Action{Type: Snapshot}, + expectedClass: "", + }, + { + name: "snapshot action without snapshotClass parameter", + action: &Action{Type: Snapshot, Parameters: map[string]any{"other": "value"}}, + expectedClass: "", + }, + { + name: "snapshot action with snapshotClass", + action: &Action{Type: Snapshot, Parameters: map[string]any{"snapshotClass": "my-vsc"}}, + expectedClass: "my-vsc", + }, + { + name: "non-snapshot action returns error", + action: &Action{Type: FSBackup, Parameters: map[string]any{"snapshotClass": "my-vsc"}}, + expectErr: true, + }, + { + name: "snapshot action with non-string snapshotClass returns error", + action: &Action{Type: Snapshot, Parameters: map[string]any{"snapshotClass": 123}}, + expectErr: true, + }, + { + name: "snapshot action with both snapshotClass and dataMover", + action: &Action{Type: Snapshot, Parameters: map[string]any{"snapshotClass": "my-vsc", "dataMover": "velero-fs"}}, + expectedClass: "my-vsc", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + snapshotClass, err := tc.action.GetSnapshotClass() + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tc.expectedClass, snapshotClass) + }) + } +}