diff --git a/changelogs/unreleased/8713-shubham-pampattiwar b/changelogs/unreleased/8713-shubham-pampattiwar new file mode 100644 index 000000000..ebfeaef82 --- /dev/null +++ b/changelogs/unreleased/8713-shubham-pampattiwar @@ -0,0 +1 @@ +Add labels as a criteria for volume policy \ No newline at end of file diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 532c8722d..c4f9e4b8b 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -76,6 +76,16 @@ func unmarshalResourcePolicies(yamlData *string) (*ResourcePolicies, error) { if err != nil { return nil, fmt.Errorf("failed to decode yaml data into resource policies %v", err) } + + for _, vp := range resPolicies.VolumePolicies { + if raw, ok := vp.Conditions["pvcLabels"]; ok { + switch raw.(type) { + case map[string]any, map[string]string: + default: + return nil, fmt.Errorf("pvcLabels must be a map of string to string, got %T", raw) + } + } + } return resPolicies, nil } @@ -96,6 +106,9 @@ func (p *Policies) BuildPolicy(resPolicies *ResourcePolicies) error { volP.conditions = append(volP.conditions, &nfsCondition{nfs: con.NFS}) volP.conditions = append(volP.conditions, &csiCondition{csi: con.CSI}) volP.conditions = append(volP.conditions, &volumeTypeCondition{volumeTypes: con.VolumeTypes}) + if con.PVCLabels != nil && len(con.PVCLabels) > 0 { + volP.conditions = append(volP.conditions, &pvcLabelsCondition{labels: con.PVCLabels}) + } p.volumePolicies = append(p.volumePolicies, volP) } @@ -123,15 +136,27 @@ func (p *Policies) match(res *structuredVolume) *Action { } func (p *Policies) GetMatchAction(res any) (*Action, error) { + data, ok := res.(VolumeFilterData) + if !ok { + return nil, errors.New("failed to convert input to VolumeFilterData") + } + volume := &structuredVolume{} - switch obj := res.(type) { - case *v1.PersistentVolume: - volume.parsePV(obj) - case *v1.Volume: - volume.parsePodVolume(obj) + switch { + case data.PersistentVolume != nil: + volume.parsePV(data.PersistentVolume) + if data.PVC != nil { + volume.parsePVC(data.PVC) + } + case data.PodVolume != nil: + volume.parsePodVolume(data.PodVolume) + if data.PVC != nil { + volume.parsePVC(data.PVC) + } default: return nil, errors.New("failed to convert object") } + return p.match(volume), nil } diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index c9ddcd16d..d94db81e5 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -117,6 +117,43 @@ volumePolicies: key1: value1 action: type: skip +`, + wantErr: false, + }, + { + name: "supported format pvcLabels", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcLabels: + environment: production + app: database + action: + type: skip +`, + wantErr: false, + }, + { + name: "error format of pvcLabels (not a map)", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcLabels: "production" + action: + type: skip +`, + wantErr: true, + }, + { + name: "supported format pvcLabels with extra keys", + yamlData: `version: v1 +volumePolicies: + - conditions: + pvcLabels: + environment: production + region: us-west + action: + type: skip `, wantErr: false, }, @@ -178,6 +215,14 @@ func TestGetResourceMatchedAction(t *testing.T) { }), }, }, + { + Action: Action{Type: "snapshot"}, + Conditions: map[string]any{ + "pvcLabels": map[string]string{ + "environment": "production", + }, + }, + }, }, } testCases := []struct { @@ -230,6 +275,29 @@ func TestGetResourceMatchedAction(t *testing.T) { }, expectedAction: nil, }, + { + name: "match pvcLabels condition", + volume: &structuredVolume{ + capacity: *resource.NewQuantity(5<<30, resource.BinarySI), + storageClass: "some-class", + pvcLabels: map[string]string{ + "environment": "production", + "team": "backend", + }, + }, + expectedAction: &Action{Type: "snapshot"}, + }, + { + name: "mismatch pvcLabels condition", + volume: &structuredVolume{ + capacity: *resource.NewQuantity(5<<30, resource.BinarySI), + storageClass: "some-class", + pvcLabels: map[string]string{ + "environment": "staging", + }, + }, + expectedAction: nil, + }, } for _, tc := range testCases { @@ -266,7 +334,27 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) { Namespace: "test-namespace", }, Data: map[string]string{ - "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", + "test-data": `version: v1 +volumePolicies: + - conditions: + capacity: '0,10Gi' + csi: + driver: disks.csi.driver + action: + type: skip + - conditions: + csi: + driver: files.csi.driver + volumeAttributes: + protocol: nfs + action: + type: skip + - conditions: + pvcLabels: + environment: production + action: + type: skip +`, }, } @@ -276,7 +364,9 @@ 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, 2) + + assert.Len(t, resPolicies.volumePolicies, 3) + policies := ResourcePolicies{ Version: "v1", VolumePolicies: []VolumePolicy{ @@ -302,13 +392,25 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) { Type: Skip, }, }, + { + Conditions: map[string]any{ + "pvcLabels": map[string]string{ + "environment": "production", + }, + }, + Action: Action{ + Type: Skip, + }, + }, }, } + p := &Policies{} err = p.BuildPolicy(&policies) if err != nil { - t.Fatalf("failed to build policy with error %v", err) + t.Fatalf("failed to build policy: %v", err) } + assert.Equal(t, p, resPolicies) } @@ -317,6 +419,8 @@ func TestGetMatchAction(t *testing.T) { name string yamlData string vol *v1.PersistentVolume + podVol *v1.Volume + pvc *v1.PersistentVolumeClaim skip bool }{ { @@ -635,6 +739,224 @@ volumePolicies: }, skip: false, }, + { + name: "PVC labels match", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + pvcLabels: + environment: production + action: + type: skip`, + vol: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-1", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{}, + ClaimRef: &v1.ObjectReference{ + Namespace: "default", + Name: "pvc-1", + }, + }, + }, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-1", + Labels: map[string]string{"environment": "production"}, + }, + }, + skip: true, + }, + { + name: "PVC labels match, criteria label is a subset of the pvc labels", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + pvcLabels: + environment: production + action: + type: skip`, + vol: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-1", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{}, + ClaimRef: &v1.ObjectReference{ + Namespace: "default", + Name: "pvc-1", + }, + }, + }, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-1", + Labels: map[string]string{"environment": "production", "app": "backend"}, + }, + }, + skip: true, + }, + { + name: "PVC labels match don't match exactly", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + pvcLabels: + environment: production + app: frontend + action: + type: skip`, + vol: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-1", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{}, + ClaimRef: &v1.ObjectReference{ + Namespace: "default", + Name: "pvc-1", + }, + }, + }, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-1", + Labels: map[string]string{"environment": "production"}, + }, + }, + skip: false, + }, + { + name: "PVC labels mismatch", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + pvcLabels: + environment: production + action: + type: skip`, + vol: &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-2", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{}, + ClaimRef: &v1.ObjectReference{ + Namespace: "default", + Name: "pvc-2", + }, + }, + }, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-1", + Labels: map[string]string{"environment": "staging"}, + }, + }, + skip: false, + }, + { + name: "PodVolume case with PVC labels match", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcLabels: + environment: production + action: + type: skip`, + vol: nil, + podVol: &v1.Volume{Name: "pod-vol-1"}, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-1", + Labels: map[string]string{"environment": "production"}, + }, + }, + skip: true, + }, + { + name: "PodVolume case with PVC labels mismatch", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcLabels: + environment: production + action: + type: skip`, + vol: nil, + podVol: &v1.Volume{Name: "pod-vol-2"}, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-2", + Labels: map[string]string{"environment": "staging"}, + }, + }, + skip: false, + }, + { + name: "PodVolume case with PVC labels match with extra keys on PVC", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcLabels: + environment: production + action: + type: skip`, + vol: nil, + podVol: &v1.Volume{Name: "pod-vol-3"}, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-3", + Labels: map[string]string{"environment": "production", "app": "backend"}, + }, + }, + skip: true, + }, + { + name: "PodVolume case with PVC labels don't match exactly", + yamlData: `version: v1 +volumePolicies: +- conditions: + pvcLabels: + environment: production + app: frontend + action: + type: skip`, + vol: nil, + podVol: &v1.Volume{Name: "pod-vol-4"}, + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "pvc-4", + Labels: map[string]string{"environment": "production"}, + }, + }, + skip: false, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -646,7 +968,20 @@ volumePolicies: policies := &Policies{} err = policies.BuildPolicy(resPolicies) assert.NoError(t, err) - action, err := policies.GetMatchAction(tc.vol) + vfd := VolumeFilterData{} + if tc.pvc != nil { + vfd.PVC = tc.pvc + } + + if tc.vol != nil { + vfd.PersistentVolume = tc.vol + } + + if tc.podVol != nil { + vfd.PodVolume = tc.podVol + } + + action, err := policies.GetMatchAction(vfd) assert.NoError(t, err) if tc.skip { @@ -659,3 +994,82 @@ volumePolicies: }) } } + +func TestGetMatchAction_Errors(t *testing.T) { + p := &Policies{} + + testCases := []struct { + name string + input any + expectedErr string + }{ + { + name: "invalid input type", + input: "invalid input", + expectedErr: "failed to convert input to VolumeFilterData", + }, + { + name: "no volume provided", + input: VolumeFilterData{ + PersistentVolume: nil, + PodVolume: nil, + PVC: nil, + }, + expectedErr: "failed to convert object", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + action, err := p.GetMatchAction(tc.input) + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErr) + assert.Nil(t, action) + }) + } +} + +func TestParsePVC(t *testing.T) { + tests := []struct { + name string + pvc *v1.PersistentVolumeClaim + expectedLabels map[string]string + expectErr bool + }{ + { + name: "valid PVC with labels", + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"env": "prod"}, + }, + }, + expectedLabels: map[string]string{"env": "prod"}, + expectErr: false, + }, + { + name: "valid PVC with empty labels", + pvc: &v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + }, + expectedLabels: nil, + expectErr: false, + }, + { + name: "nil PVC pointer", + pvc: (*v1.PersistentVolumeClaim)(nil), + expectedLabels: nil, + expectErr: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + s := &structuredVolume{} + s.parsePVC(tc.pvc) + + assert.Equal(t, tc.expectedLabels, s.pvcLabels) + }) + } +} diff --git a/internal/resourcepolicies/volume_filter_data.go b/internal/resourcepolicies/volume_filter_data.go new file mode 100644 index 000000000..531410107 --- /dev/null +++ b/internal/resourcepolicies/volume_filter_data.go @@ -0,0 +1,21 @@ +package resourcepolicies + +import ( + corev1 "k8s.io/api/core/v1" +) + +// VolumeFilterData bundles the volume data needed for volume policy filtering +type VolumeFilterData struct { + PersistentVolume *corev1.PersistentVolume + PodVolume *corev1.Volume + PVC *corev1.PersistentVolumeClaim +} + +// NewVolumeFilterData constructs a new VolumeFilterData instance. +func NewVolumeFilterData(pv *corev1.PersistentVolume, podVol *corev1.Volume, pvc *corev1.PersistentVolumeClaim) VolumeFilterData { + return VolumeFilterData{ + PersistentVolume: pv, + PodVolume: podVol, + PVC: pvc, + } +} diff --git a/internal/resourcepolicies/volume_filter_data_test.go b/internal/resourcepolicies/volume_filter_data_test.go new file mode 100644 index 000000000..73e8929e6 --- /dev/null +++ b/internal/resourcepolicies/volume_filter_data_test.go @@ -0,0 +1,102 @@ +package resourcepolicies + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNewVolumeFilterData(t *testing.T) { + testCases := []struct { + name string + pv *corev1.PersistentVolume + podVol *corev1.Volume + pvc *corev1.PersistentVolumeClaim + expectedPVName string + expectedPodName string + expectedPVCName string + }{ + { + name: "all provided", + pv: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-test", + }, + }, + podVol: &corev1.Volume{ + Name: "pod-vol-test", + }, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-test", + }, + }, + expectedPVName: "pv-test", + expectedPodName: "pod-vol-test", + expectedPVCName: "pvc-test", + }, + { + name: "only PV provided", + pv: &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pv-only", + }, + }, + podVol: nil, + pvc: nil, + expectedPVName: "pv-only", + expectedPodName: "", + expectedPVCName: "", + }, + { + name: "only PodVolume provided", + pv: nil, + podVol: &corev1.Volume{ + Name: "pod-only", + }, + pvc: nil, + expectedPVName: "", + expectedPodName: "pod-only", + expectedPVCName: "", + }, + { + name: "only PVC provided", + pv: nil, + podVol: nil, + pvc: &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pvc-only", + }, + }, + expectedPVName: "", + expectedPodName: "", + expectedPVCName: "pvc-only", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + vfd := NewVolumeFilterData(tc.pv, tc.podVol, tc.pvc) + if tc.expectedPVName != "" { + assert.NotNil(t, vfd.PersistentVolume) + assert.Equal(t, tc.expectedPVName, vfd.PersistentVolume.Name) + } else { + assert.Nil(t, vfd.PersistentVolume) + } + if tc.expectedPodName != "" { + assert.NotNil(t, vfd.PodVolume) + assert.Equal(t, tc.expectedPodName, vfd.PodVolume.Name) + } else { + assert.Nil(t, vfd.PodVolume) + } + if tc.expectedPVCName != "" { + assert.NotNil(t, vfd.PVC) + assert.Equal(t, tc.expectedPVCName, vfd.PVC.Name) + } else { + assert.Nil(t, vfd.PVC) + } + }) + } +} diff --git a/internal/resourcepolicies/volume_resources.go b/internal/resourcepolicies/volume_resources.go index 250ebc8f2..545c1f3f6 100644 --- a/internal/resourcepolicies/volume_resources.go +++ b/internal/resourcepolicies/volume_resources.go @@ -20,6 +20,8 @@ import ( "fmt" "strings" + "k8s.io/apimachinery/pkg/labels" + "github.com/pkg/errors" "gopkg.in/yaml.v3" corev1api "k8s.io/api/core/v1" @@ -48,6 +50,7 @@ type structuredVolume struct { nfs *nFSVolumeSource csi *csiVolumeSource volumeType SupportedVolume + pvcLabels map[string]string } func (s *structuredVolume) parsePV(pv *corev1api.PersistentVolume) { @@ -66,6 +69,12 @@ func (s *structuredVolume) parsePV(pv *corev1api.PersistentVolume) { s.volumeType = getVolumeTypeFromPV(pv) } +func (s *structuredVolume) parsePVC(pvc *corev1api.PersistentVolumeClaim) { + if pvc != nil && len(pvc.GetLabels()) > 0 { + s.pvcLabels = pvc.Labels + } +} + func (s *structuredVolume) parsePodVolume(vol *corev1api.Volume) { nfs := vol.NFS if nfs != nil { @@ -80,6 +89,27 @@ func (s *structuredVolume) parsePodVolume(vol *corev1api.Volume) { s.volumeType = getVolumeTypeFromVolume(vol) } +// pvcLabelsCondition defines a condition that matches if the PVC's labels contain all the provided key/value pairs. +type pvcLabelsCondition struct { + labels map[string]string +} + +func (c *pvcLabelsCondition) match(v *structuredVolume) bool { + // No labels specified: always match. + if len(c.labels) == 0 { + return true + } + if v.pvcLabels == nil { + return false + } + selector := labels.SelectorFromSet(c.labels) + return selector.Matches(labels.Set(v.pvcLabels)) +} + +func (c *pvcLabelsCondition) validate() error { + return nil +} + type capacityCondition struct { capacity capacity } diff --git a/internal/resourcepolicies/volume_resources_test.go b/internal/resourcepolicies/volume_resources_test.go index 32dfb2f10..c93430b5b 100644 --- a/internal/resourcepolicies/volume_resources_test.go +++ b/internal/resourcepolicies/volume_resources_test.go @@ -25,12 +25,114 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ) -func setStructuredVolume(capacity resource.Quantity, sc string, nfs *nFSVolumeSource, csi *csiVolumeSource) *structuredVolume { +func setStructuredVolume(capacity resource.Quantity, sc string, nfs *nFSVolumeSource, csi *csiVolumeSource, pvcLabels map[string]string) *structuredVolume { return &structuredVolume{ capacity: capacity, storageClass: sc, nfs: nfs, csi: csi, + pvcLabels: pvcLabels, + } +} + +func TestPVCLabelsMatch(t *testing.T) { + tests := []struct { + name string + condition *pvcLabelsCondition + volume *structuredVolume + expectedMatch bool + }{ + { + name: "match exact label (single)", + condition: &pvcLabelsCondition{ + labels: map[string]string{"environment": "production"}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + map[string]string{"environment": "production", "app": "database"}, + ), + expectedMatch: true, + }, + { + name: "match exact label (multiple)", + condition: &pvcLabelsCondition{ + labels: map[string]string{"environment": "production", "app": "database"}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + map[string]string{"environment": "production", "app": "database"}, + ), + expectedMatch: true, + }, + { + name: "mismatch label value", + condition: &pvcLabelsCondition{ + labels: map[string]string{"environment": "production"}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + map[string]string{"environment": "staging", "app": "database"}, + ), + expectedMatch: false, + }, + { + name: "missing label key", + condition: &pvcLabelsCondition{ + labels: map[string]string{"environment": "production", "region": "us-west"}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + map[string]string{"environment": "production", "app": "database"}, + ), + expectedMatch: false, + }, + { + name: "empty condition always matches", + condition: &pvcLabelsCondition{ + labels: map[string]string{}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + map[string]string{"environment": "staging"}, + ), + expectedMatch: true, + }, + { + name: "nil pvcLabels fails non-empty condition", + condition: &pvcLabelsCondition{ + labels: map[string]string{"environment": "production"}, + }, + volume: setStructuredVolume( + *resource.NewQuantity(0, resource.BinarySI), + "any", + nil, + nil, + nil, + ), + expectedMatch: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + match := tt.condition.match(tt.volume) + assert.Equal(t, tt.expectedMatch, match, "expected match %v, got %v", tt.expectedMatch, match) + }) } } @@ -100,31 +202,31 @@ func TestStorageClassConditionMatch(t *testing.T) { { name: "match single storage class", condition: &storageClassCondition{[]string{"gp2"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "gp2", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "gp2", nil, nil, nil), expectedMatch: true, }, { name: "match multiple storage classes", condition: &storageClassCondition{[]string{"gp2", "ebs-sc"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "gp2", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "gp2", nil, nil, nil), expectedMatch: true, }, { name: "mismatch storage class", condition: &storageClassCondition{[]string{"gp2"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "ebs-sc", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "ebs-sc", nil, nil, nil), expectedMatch: false, }, { name: "empty storage class", condition: &storageClassCondition{[]string{}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "ebs-sc", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "ebs-sc", nil, nil, nil), expectedMatch: true, }, { name: "empty volume storage class", condition: &storageClassCondition{[]string{"gp2"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, nil, nil), expectedMatch: false, }, } @@ -149,37 +251,37 @@ func TestNFSConditionMatch(t *testing.T) { { name: "match nfs condition", condition: &nfsCondition{&nFSVolumeSource{Server: "192.168.10.20"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil, nil), expectedMatch: true, }, { name: "empty nfs condition", condition: &nfsCondition{nil}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil, nil), expectedMatch: true, }, { name: "empty nfs server and path condition", condition: &nfsCondition{&nFSVolumeSource{Server: "", Path: ""}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20"}, nil, nil), expectedMatch: true, }, { name: "server mismatch", condition: &nfsCondition{&nFSVolumeSource{Server: "192.168.10.20", Path: ""}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: ""}, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: ""}, nil, nil), expectedMatch: false, }, { name: "empty nfs server condition", condition: &nfsCondition{&nFSVolumeSource{Path: "/mnt/data"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20", Path: "/mnt/data"}, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", &nFSVolumeSource{Server: "192.168.10.20", Path: "/mnt/data"}, nil, nil), expectedMatch: true, }, { name: "empty nfs volume", condition: &nfsCondition{&nFSVolumeSource{Server: "192.168.10.20"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, nil), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, nil, nil), expectedMatch: false, }, } @@ -203,43 +305,43 @@ func TestCSIConditionMatch(t *testing.T) { { name: "match csi driver condition", condition: &csiCondition{&csiVolumeSource{Driver: "test"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}, nil), expectedMatch: true, }, { name: "empty csi driver condition", condition: &csiCondition{nil}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}, nil), expectedMatch: true, }, { name: "empty csi driver volume", condition: &csiCondition{&csiVolumeSource{Driver: "test"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{}, nil), expectedMatch: false, }, { name: "match csi volumeAttributes condition", condition: &csiCondition{&csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}, nil), expectedMatch: true, }, { name: "empty csi volumeAttributes condition", condition: &csiCondition{&csiVolumeSource{Driver: "test"}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}, nil), expectedMatch: true, }, { name: "empty csi volumeAttributes volume", condition: &csiCondition{&csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": ""}}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": ""}}, nil), expectedMatch: false, }, { name: "empty csi volumeAttributes volume", condition: &csiCondition{&csiVolumeSource{Driver: "test", VolumeAttributes: map[string]string{"protocol": "nfs"}}}, - volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}), + volume: setStructuredVolume(*resource.NewQuantity(0, resource.BinarySI), "", nil, &csiVolumeSource{Driver: "test"}, nil), expectedMatch: false, }, } @@ -301,6 +403,35 @@ func TestUnmarshalVolumeConditions(t *testing.T) { }, expectedError: "field unknown not found in type", }, + { + name: "Valid pvcLabels input as map[string]string", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcLabels": map[string]string{ + "environment": "production", + }, + }, + expectedError: "", + }, + { + name: "Valid pvcLabels input as map[string]any", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcLabels": map[string]any{ + "environment": "production", + "app": "database", + }, + }, + expectedError: "", + }, + { + name: "Invalid pvcLabels input: not a map", + input: map[string]any{ + "capacity": "1Gi,10Gi", + "pvcLabels": "production", + }, + expectedError: "!!str `production` into map[string]string", + }, } for _, tc := range testCases { diff --git a/internal/resourcepolicies/volume_resources_validator.go b/internal/resourcepolicies/volume_resources_validator.go index 93295e87d..0c719b09e 100644 --- a/internal/resourcepolicies/volume_resources_validator.go +++ b/internal/resourcepolicies/volume_resources_validator.go @@ -45,6 +45,7 @@ type volumeConditions struct { NFS *nFSVolumeSource `yaml:"nfs,omitempty"` CSI *csiVolumeSource `yaml:"csi,omitempty"` VolumeTypes []SupportedVolume `yaml:"volumeTypes,omitempty"` + PVCLabels map[string]string `yaml:"pvcLabels,omitempty"` } func (c *capacityCondition) validate() error { diff --git a/internal/resourcepolicies/volume_resources_validator_test.go b/internal/resourcepolicies/volume_resources_validator_test.go index c2acf58e8..cf795b3ae 100644 --- a/internal/resourcepolicies/volume_resources_validator_test.go +++ b/internal/resourcepolicies/volume_resources_validator_test.go @@ -420,6 +420,39 @@ func TestValidate(t *testing.T) { }, wantErr: false, }, + { + name: "supported format volume policies with pvcLabels (valid map)", + res: &ResourcePolicies{ + Version: "v1", + VolumePolicies: []VolumePolicy{ + { + Action: Action{Type: "skip"}, + Conditions: map[string]any{ + "pvcLabels": map[string]string{ + "environment": "production", + "app": "database", + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "error format volume policies with pvcLabels (not a map)", + res: &ResourcePolicies{ + Version: "v1", + VolumePolicies: []VolumePolicy{ + { + Action: Action{Type: "skip"}, + Conditions: map[string]any{ + "pvcLabels": "production", + }, + }, + }, + }, + wantErr: true, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/volumehelper/volume_policy_helper.go b/internal/volumehelper/volume_policy_helper.go index b0315d6c5..13cef2fd9 100644 --- a/internal/volumehelper/volume_policy_helper.go +++ b/internal/volumehelper/volume_policy_helper.go @@ -81,7 +81,8 @@ func (v *volumeHelperImpl) ShouldPerformSnapshot(obj runtime.Unstructured, group } if v.volumePolicy != nil { - action, err := v.volumePolicy.GetMatchAction(pv) + vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc) + action, err := v.volumePolicy.GetMatchAction(vfd) if err != nil { v.logger.WithError(err).Errorf("fail to get VolumePolicy match action for PV %s", pv.Name) return false, err @@ -144,9 +145,11 @@ func (v volumeHelperImpl) ShouldPerformFSBackup(volume corev1api.Volume, pod cor if v.volumePolicy != nil { var resource any + var err error resource = &volume + var pvc = &corev1api.PersistentVolumeClaim{} if volume.VolumeSource.PersistentVolumeClaim != nil { - pvc, err := kubeutil.GetPVCForPodVolume(&volume, &pod, v.client) + pvc, err = kubeutil.GetPVCForPodVolume(&volume, &pod, v.client) if err != nil { v.logger.WithError(err).Errorf("fail to get PVC for pod %s", pod.Namespace+"/"+pod.Name) return false, err @@ -158,7 +161,13 @@ func (v volumeHelperImpl) ShouldPerformFSBackup(volume corev1api.Volume, pod cor } } - action, err := v.volumePolicy.GetMatchAction(resource) + pv, podVolume, err := v.getVolumeFromResource(resource) + if err != nil { + return false, err + } + + vfd := resourcepolicies.NewVolumeFilterData(pv, podVolume, pvc) + action, err := v.volumePolicy.GetMatchAction(vfd) if err != nil { v.logger.WithError(err).Error("fail to get VolumePolicy match action for volume") return false, err @@ -247,3 +256,12 @@ func (v *volumeHelperImpl) shouldIncludeVolumeInBackup(vol corev1api.Volume) boo } return includeVolumeInBackup } + +func (v *volumeHelperImpl) getVolumeFromResource(resource any) (*corev1api.PersistentVolume, *corev1api.Volume, error) { + if pv, ok := resource.(*corev1api.PersistentVolume); ok { + return pv, nil, nil + } else if podVol, ok := resource.(*corev1api.Volume); ok { + return nil, podVol, nil + } + return nil, nil, fmt.Errorf("resource is not a PersistentVolume or Volume") +} diff --git a/internal/volumehelper/volume_policy_helper_test.go b/internal/volumehelper/volume_policy_helper_test.go index 4c6db87b9..9b462b39d 100644 --- a/internal/volumehelper/volume_policy_helper_test.go +++ b/internal/volumehelper/volume_policy_helper_test.go @@ -706,3 +706,37 @@ func TestVolumeHelperImpl_ShouldPerformFSBackup(t *testing.T) { }) } } + +func TestGetVolumeFromResource(t *testing.T) { + helper := &volumeHelperImpl{} + + t.Run("PersistentVolume input", func(t *testing.T) { + pv := &corev1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + } + outPV, outPod, err := helper.getVolumeFromResource(pv) + assert.NoError(t, err) + assert.NotNil(t, outPV) + assert.Nil(t, outPod) + assert.Equal(t, "test-pv", outPV.Name) + }) + + t.Run("Volume input", func(t *testing.T) { + vol := &corev1.Volume{ + Name: "test-volume", + } + outPV, outPod, err := helper.getVolumeFromResource(vol) + assert.NoError(t, err) + assert.Nil(t, outPV) + assert.NotNil(t, outPod) + assert.Equal(t, "test-volume", outPod.Name) + }) + + t.Run("Invalid input", func(t *testing.T) { + _, _, err := helper.getVolumeFromResource("invalid") + assert.Error(t, err) + assert.Contains(t, err.Error(), "resource is not a PersistentVolume or Volume") + }) +} diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index 49c39568e..b890b23c3 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -593,7 +593,18 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie } if ib.backupRequest.ResPolicies != nil { - if action, err := ib.backupRequest.ResPolicies.GetMatchAction(pv); err != nil { + pvc := new(corev1api.PersistentVolumeClaim) + if pv.Spec.ClaimRef != nil { + err = ib.kbClient.Get(context.Background(), kbClient.ObjectKey{ + Namespace: pv.Spec.ClaimRef.Namespace, + Name: pv.Spec.ClaimRef.Name}, + pvc) + if err != nil { + return err + } + } + vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc) + if action, err := ib.backupRequest.ResPolicies.GetMatchAction(vfd); err != nil { log.WithError(err).Errorf("Error getting matched resource policies for pv %s", pv.Name) return nil } else if action != nil && action.Type == resourcepolicies.Skip { @@ -696,8 +707,8 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie func (ib *itemBackupper) getMatchAction(obj runtime.Unstructured, groupResource schema.GroupResource, backupItemActionName string) (*resourcepolicies.Action, error) { if ib.backupRequest.ResPolicies != nil && groupResource == kuberesource.PersistentVolumeClaims && (backupItemActionName == csiBIAPluginName || backupItemActionName == vsphereBIAPluginName) { - pvc := corev1api.PersistentVolumeClaim{} - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pvc); err != nil { + pvc := &corev1api.PersistentVolumeClaim{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), pvc); err != nil { return nil, errors.WithStack(err) } @@ -710,7 +721,8 @@ func (ib *itemBackupper) getMatchAction(obj runtime.Unstructured, groupResource if err := ib.kbClient.Get(context.Background(), kbClient.ObjectKey{Name: pvName}, pv); err != nil { return nil, errors.WithStack(err) } - return ib.backupRequest.ResPolicies.GetMatchAction(pv) + vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc) + return ib.backupRequest.ResPolicies.GetMatchAction(vfd) } return nil, nil diff --git a/pkg/podvolume/backupper.go b/pkg/podvolume/backupper.go index 75f36c3c3..6e13dd0e6 100644 --- a/pkg/podvolume/backupper.go +++ b/pkg/podvolume/backupper.go @@ -196,11 +196,13 @@ func (b *backupper) getMatchAction(resPolicies *resourcepolicies.Policies, pvc * if err != nil { return nil, errors.Wrapf(err, "error getting pv for pvc %s", pvc.Spec.VolumeName) } - return resPolicies.GetMatchAction(pv) + vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc) + return resPolicies.GetMatchAction(vfd) } if volume != nil { - return resPolicies.GetMatchAction(volume) + vfd := resourcepolicies.NewVolumeFilterData(nil, volume, pvc) + return resPolicies.GetMatchAction(vfd) } return nil, errors.Errorf("failed to check resource policies for empty volume")