minor updates

Signed-off-by: Shubham Pampattiwar <[email protected]>

use VolumeFilterData struct in GetMatchAction func

Signed-off-by: Shubham Pampattiwar <[email protected]>

update parsePVC func and add more ut

Signed-off-by: Shubham Pampattiwar <[email protected]>

lint fix

Signed-off-by: Shubham Pampattiwar <[email protected]>

internal/resourcepolicies/resource_policies.go

@@ -135,27 +135,28 @@ func (p *Policies) match(res *structuredVolume) *Action {
 	return nil
 }
 
-func (p *Policies) GetMatchAction(volumeRes any, pvcRes any) (*Action, error) {
-	volume := &structuredVolume{}
-	switch obj := volumeRes.(type) {
-	case *v1.PersistentVolume:
-		volume.parsePV(obj)
-	case *v1.Volume:
-		volume.parsePodVolume(obj)
-	default:
-		return nil, errors.New("failed to convert object")
+func (p *Policies) GetMatchAction(res any) (*Action, error) {
+	data, ok := res.(VolumeFilterData)
+	if !ok {
+		return nil, errors.New("failed to convert input to VolumeFilterData")
 	}
 
-	// If a PVC is provided, set the pvcLabels on structured volume
-	if pvcRes != nil {
-		pvc, ok := pvcRes.(*v1.PersistentVolumeClaim)
-		if !ok {
-			return nil, errors.New("failed to convert object")
+	volume := &structuredVolume{}
+	switch {
+	case data.PersistentVolume != nil:
+		volume.parsePV(data.PersistentVolume)
+		if data.PVC != nil {
+			volume.parsePVC(data.PVC)
 		}
-		if pvc != nil && len(pvc.GetLabels()) > 0 {
-			volume.pvcLabels = pvc.Labels
+	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
 }
 

internal/resourcepolicies/resource_policies_test.go

@@ -419,6 +419,7 @@ func TestGetMatchAction(t *testing.T) {
 		name     string
 		yamlData string
 		vol      *v1.PersistentVolume
+		podVol   *v1.Volume
 		pvc      *v1.PersistentVolumeClaim
 		skip     bool
 	}{
@@ -875,6 +876,87 @@ volumePolicies:
 			},
 			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) {
@@ -886,8 +968,20 @@ volumePolicies:
 			policies := &Policies{}
 			err = policies.BuildPolicy(resPolicies)
 			assert.NoError(t, err)
+			vfd := VolumeFilterData{}
+			if tc.pvc != nil {
+				vfd.PVC = tc.pvc
+			}
+
+			if tc.vol != nil {
+				vfd.PersistentVolume = tc.vol
+			}
 
-			action, err := policies.GetMatchAction(tc.vol, tc.pvc)
+			if tc.podVol != nil {
+				vfd.PodVolume = tc.podVol
+			}
+
+			action, err := policies.GetMatchAction(vfd)
 			assert.NoError(t, err)
 
 			if tc.skip {
@@ -904,19 +998,78 @@ volumePolicies:
 func TestGetMatchAction_Errors(t *testing.T) {
 	p := &Policies{}
 
-	action, err := p.GetMatchAction("invalid volume", nil)
-	assert.Error(t, err, "Expected error when volumeRes is not a valid volume type")
-	assert.Contains(t, err.Error(), "failed to convert object")
-	assert.Nil(t, action)
+	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)
+		})
+	}
+}
 
-	vol := &v1.PersistentVolume{
-		Spec: v1.PersistentVolumeSpec{
-			Capacity: v1.ResourceList{v1.ResourceStorage: resource.MustParse("1Gi")},
+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,
 		},
 	}
 
-	action, err = p.GetMatchAction(vol, 123)
-	assert.Error(t, err, "Expected error when pvcRes is not a valid PVC type")
-	assert.Contains(t, err.Error(), "failed to convert object")
-	assert.Nil(t, action)
+	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)
+		})
+	}
 }

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,
+	}
+}

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)
+			}
+		})
+	}
+}

internal/resourcepolicies/volume_resources.go

@@ -69,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 {