Adding Support For VolumeAttributes in Resource Policy (#8383)

* Adding VolumeAttributes validations in resource policy Signed-off-by: mayaggar <[email protected]> * adding tests Signed-off-by: mayaggar <[email protected]> * adding tests Signed-off-by: mayaggar <[email protected]> * adding tests Signed-off-by: mayaggar <[email protected]> * added changelog Signed-off-by: mayaggar <[email protected]> * changelog Signed-off-by: mayaggar <[email protected]> * design spec Signed-off-by: mayaggar <[email protected]> * lint fixes Signed-off-by: mayaggar <[email protected]> * doc update Signed-off-by: mayaggar <[email protected]> * doc update Signed-off-by: mayaggar <[email protected]> * Update internal/resourcepolicies/volume_resources_validator.go Co-authored-by: Tiger Kaovilai <[email protected]> Signed-off-by: Mayank Aggarwal <[email protected]> * doc name update Signed-off-by: mayaggar <[email protected]> --------- Signed-off-by: mayaggar <[email protected]> Signed-off-by: Mayank Aggarwal <[email protected]> Co-authored-by: Tiger Kaovilai <[email protected]>
vmware-tanzu · Nov 28, 2024 · 074f265 · 074f265
1 parent 3c06fc8
commit 074f265
Show file tree

Hide file tree

Showing 7 changed files with 502 additions and 29 deletions.
diff --git a/changelogs/unreleased/8383-mayankagg9722 b/changelogs/unreleased/8383-mayankagg9722
@@ -0,0 +1 @@
+Adding support in velero Resource Policies for filtering PVs based on additional VolumeAttributes properties under CSI PVs
diff --git a/design/Implemented/supporting-volumeattributes-resource-policy.md b/design/Implemented/supporting-volumeattributes-resource-policy.md
@@ -0,0 +1,84 @@
+# Adding Support For VolumeAttributes in Resource Policy
+
+## Abstract
+Currently [Velero Resource policies](https://velero.io/docs/main/resource-filtering/#creating-resource-policies) are only supporting "Driver" to be filtered for [CSI volume conditions](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources_validator.go#L28)
+
+If user want to skip certain CSI volumes based on other volume attributes like protocol or SKU, etc, they can't do it with the current Velero resource policies. It would be convenient if Velero resource policies could be extended to filter on volume attributes along with existing driver filter in the resource policies `conditions` to handle the backup of volumes just by `some specific volumes attributes conditions`.
+
+## Background
+As of Today, Velero resource policy already provides us the way to filter volumes based on the `driver` name. But it's not enough to handle the volumes based on other volume attributes like protocol, SKU, etc.
+
+## Example:
+  - Provision Azure NFS: Define the Storage class with `protocol: nfs` under storage class parameters to provision [CSI NFS Azure File Shares](https://learn.microsoft.com/en-us/azure/aks/azure-files-csi#nfs-file-shares).
+  - User wants to back up AFS (Azure file shares) but only want to backup `SMB` type of file share volumes and not `NFS` file share volumes.
+
+## Goals
+- We are only bringing additional support in the resource policy to only handle volumes during backup.
+- Introducing support for `VolumeAttributes` filter along with `driver` filter in CSI volume conditions to handle volumes.
+
+## Non-Goals
+- Currently, only handles volumes, and does not support other resources.
+
+## Use-cases/Scenarios
+### Skip backup volumes by some volume attributes:
+Users want to skip PV with the requirements:
+- option to skip specified PV on volume attributes type (like Protocol as NFS, SMB, etc)
+
+### Sample Storage Class Used to create such Volumes
+```
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: azurefile-csi-nfs
+provisioner: file.csi.azure.com
+allowVolumeExpansion: true
+parameters:
+  protocol: nfs 
+```
+
+## High-Level Design
+Modifying the existing Resource Policies code for [csiVolumeSource](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources_validator.go#L28C6-L28C22) to add the new `VolumeAttributes` filter for CSI volumes and adding validations in existing [csiCondition](https://github.com/vmware-tanzu/velero/blob/8e23752a6ea83f101bd94a69dcf17f519a805388/internal/resourcepolicies/volume_resources.go#L150) to match with volume attributes in the conditions from Resource Policy config map and original persistent volume.
+
+## Detailed Design
+The volume resources policies should contain a list of policies which is the combination of conditions and related `action`, when target volumes meet the conditions, the related `action` will take effection.
+
+Below is the API Design for the user configuration:
+
+### API Design
+```go
+type csiVolumeSource struct {
+	Driver string `yaml:"driver,omitempty"`
+	// [NEW] CSI volume attributes
+	VolumeAttributes map[string]string `yaml:"volumeAttributes,omitempty"`
+}
+```
+
+The policies YAML config file would look like this:
+```yaml
+version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: disk.csi.azure.com
+    action:
+      type: skip
+  - conditions:
+      csi:
+        driver: file.csi.azure.com
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip`
+```
+ 
+### New Supported Conditions
+#### VolumeAttributes
+Existing CSI Volume Condition can now add `volumeAttributes` which will be key and value pairs.  
+
+ Specify details for the related volume source (currently only csi driver is supported filter)
+     ```yaml
+     csi: // match volume using `file.csi.azure.com` and with volumeAttributes protocol as nfs
+       driver: file.csi.azure.com 
+       volumeAttributes:
+          protocol: nfs
+     ```
diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go
@@ -93,20 +93,32 @@ func TestLoadResourcePolicies(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name: "supported formart volume policies",
+			name: "supported format volume policies",
 			yamlData: `version: v1
-	volumePolicies:
-	- conditions:
-		capacity: "0,100Gi"
-		csi:
-		  driver: aws.efs.csi.driver
-		nfs: {}
-		storageClass:
-		- gp2
-		- ebs-sc
-	  action:
-		type: skip`,
-			wantErr: true,
+volumePolicies:
+  - conditions:
+      capacity: '0,100Gi'
+      csi:
+        driver: aws.efs.csi.driver
+    action:
+      type: skip
+`,
+			wantErr: false,
+		},
+		{
+			name: "supported format csi driver with volumeAttributes for volume policies",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      capacity: '0,100Gi'
+      csi:
+        driver: aws.efs.csi.driver
+        volumeAttributes:
+          key1: value1
+    action:
+      type: skip
+`,
+			wantErr: false,
 		},
 	}
 	for _, tc := range testCases {
@@ -135,6 +147,16 @@ func TestGetResourceMatchedAction(t *testing.T) {
 						}),
 				},
 			},
+			{
+				Action: Action{Type: "skip"},
+				Conditions: map[string]interface{}{
+					"csi": interface{}(
+						map[string]interface{}{
+							"driver":           "files.csi.driver",
+							"volumeAttributes": map[string]string{"protocol": "nfs"},
+						}),
+				},
+			},
 			{
 				Action: Action{Type: "snapshot"},
 				Conditions: map[string]interface{}{
@@ -172,6 +194,24 @@ func TestGetResourceMatchedAction(t *testing.T) {
 			},
 			expectedAction: &Action{Type: "skip"},
 		},
+		{
+			name: "match policy AFS NFS",
+			volume: &structuredVolume{
+				capacity:     *resource.NewQuantity(5<<30, resource.BinarySI),
+				storageClass: "afs-nfs",
+				csi:          &csiVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"protocol": "nfs"}},
+			},
+			expectedAction: &Action{Type: "skip"},
+		},
+		{
+			name: "match policy AFS SMB",
+			volume: &structuredVolume{
+				capacity:     *resource.NewQuantity(5<<30, resource.BinarySI),
+				storageClass: "afs-smb",
+				csi:          &csiVolumeSource{Driver: "files.csi.driver"},
+			},
+			expectedAction: nil,
+		},
 		{
 			name: "both matches return the first policy",
 			volume: &structuredVolume{
@@ -226,7 +266,7 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {
 			Namespace: "test-namespace",
 		},
 		Data: map[string]string{
-			"test-data": "version: v1\nvolumePolicies:\n- conditions:\n    capacity: '0,10Gi'\n  action:\n    type: skip",
+			"test-data": "version: v1\nvolumePolicies:\n  - conditions:\n      capacity: '0,10Gi'\n      csi:\n        driver: disks.csi.driver\n    action:\n      type: skip\n  - conditions:\n      csi:\n        driver: files.csi.driver\n        volumeAttributes:\n          protocol: nfs\n    action:\n      type: skip",
 		},
 	}
 
@@ -236,13 +276,27 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {
 
 	// Check that the returned resourcePolicies object contains the expected data
 	assert.Equal(t, "v1", resPolicies.version)
-	assert.Len(t, resPolicies.volumePolicies, 1)
+	assert.Len(t, resPolicies.volumePolicies, 2)
 	policies := ResourcePolicies{
 		Version: "v1",
 		VolumePolicies: []VolumePolicy{
 			{
 				Conditions: map[string]interface{}{
 					"capacity": "0,10Gi",
+					"csi": map[string]interface{}{
+						"driver": "disks.csi.driver",
+					},
+				},
+				Action: Action{
+					Type: Skip,
+				},
+			},
+			{
+				Conditions: map[string]interface{}{
+					"csi": map[string]interface{}{
+						"driver":           "files.csi.driver",
+						"volumeAttributes": map[string]string{"protocol": "nfs"},
+					},
 				},
 				Action: Action{
 					Type: Skip,
@@ -298,7 +352,173 @@ volumePolicies:
 			skip: false,
 		},
 		{
-			name: "csi not configured",
+			name: "Skip AFS CSI condition with Disk volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: files.csi.driver
+    action:
+      type: skip`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver"},
+					}},
+			},
+			skip: false,
+		},
+		{
+			name: "Skip AFS CSI condition with AFS volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: files.csi.driver
+    action:
+      type: skip`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver"},
+					}},
+			},
+			skip: true,
+		},
+		{
+			name: "Skip AFS NFS CSI condition with Disk volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip
+`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver"},
+					}},
+			},
+			skip: false,
+		},
+		{
+			name: "Skip AFS NFS CSI condition with AFS SMB volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip
+`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
+					}},
+			},
+			skip: false,
+		},
+		{
+			name: "Skip AFS NFS CSI condition with AFS NFS volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip
+`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"protocol": "nfs"}},
+					}},
+			},
+			skip: true,
+		},
+		{
+			name: "Skip Disk and AFS NFS CSI condition with Disk volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: disks.csi.driver
+    action:
+      type: skip
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "disks.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
+					}},
+			},
+			skip: true,
+		},
+		{
+			name: "Skip Disk and AFS NFS CSI condition with AFS SMB volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: disks.csi.driver
+    action:
+      type: skip
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1"}},
+					}},
+			},
+			skip: false,
+		},
+		{
+			name: "Skip Disk and AFS NFS CSI condition with AFS NFS volumes",
+			yamlData: `version: v1
+volumePolicies:
+  - conditions:
+      csi:
+        driver: disks.csi.driver
+    action:
+      type: skip
+  - conditions:
+      csi:
+        driver: files.csi.driver
+        volumeAttributes:
+          protocol: nfs
+    action:
+      type: skip`,
+			vol: &v1.PersistentVolume{
+				Spec: v1.PersistentVolumeSpec{
+					PersistentVolumeSource: v1.PersistentVolumeSource{
+						CSI: &v1.CSIPersistentVolumeSource{Driver: "files.csi.driver", VolumeAttributes: map[string]string{"key1": "val1", "protocol": "nfs"}},
+					}},
+			},
+			skip: true,
+		},
+		{
+			name: "csi not configured and testing capacity condition",
 			yamlData: `version: v1
 volumePolicies:
 - conditions:
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Adding support in velero Resource Policies for filtering PVs based on additional VolumeAttributes properties under CSI PVs