Skip to content

Commit

Permalink
Add labels as a criteria for volume policy
Browse files Browse the repository at this point in the history 
Signed-off-by: Shubham Pampattiwar <[email protected]>

add changelog file

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

handle err

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

use labels selector.matches

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

make update

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

remove fetching pvc from volume policy filtering

Signed-off-by: Shubham Pampattiwar <[email protected]>
  • Loading branch information
@shubham-pampattiwar
shubham-pampattiwar committed Feb 21, 2025
1 parent 9235fe1 commit 487f9fc
Show file tree
Hide file tree
Showing 10 changed files with 501 additions and 34 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/8713-shubham-pampattiwar
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add labels as a criteria for volume policy
28 changes: 26 additions & 2 deletions internal/resourcepolicies/resource_policies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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)
}

Expand All @@ -122,16 +135,27 @@ func (p *Policies) match(res *structuredVolume) *Action {
return nil
}

func (p *Policies) GetMatchAction(res any) (*Action, error) {
func (p *Policies) GetMatchAction(volumeRes any, pvcRes any) (*Action, error) {
volume := &structuredVolume{}
switch obj := res.(type) {
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")
}

// 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")
}

Check warning on line 154 in internal/resourcepolicies/resource_policies.go

View check run for this annotation

Codecov / codecov/patch

internal/resourcepolicies/resource_policies.go#L153-L154 

Added lines #L153 - L154 were not covered by tests
if pvc != nil && len(pvc.GetLabels()) > 0 {
volume.pvcLabels = pvc.Labels
}
}
return p.match(volume), nil
}

Expand Down
249 changes: 245 additions & 4 deletions internal/resourcepolicies/resource_policies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
`,
},
}

Expand All @@ -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{
Expand All @@ -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)
}

Expand All @@ -317,6 +419,7 @@ func TestGetMatchAction(t *testing.T) {
name string
yamlData string
vol *v1.PersistentVolume
pvc *v1.PersistentVolumeClaim
skip bool
}{
{
Expand Down Expand Up @@ -635,6 +738,143 @@ 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,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -646,7 +886,8 @@ volumePolicies:
policies := &Policies{}
err = policies.BuildPolicy(resPolicies)
assert.NoError(t, err)
action, err := policies.GetMatchAction(tc.vol)

action, err := policies.GetMatchAction(tc.vol, tc.pvc)
assert.NoError(t, err)

if tc.skip {
Expand Down
Loading

0 comments on commit 487f9fc

Please sign in to comment.