From d429d38ea1b626b50c26c3c0463bf5bbe63aa1e0 Mon Sep 17 00:00:00 2001 From: Ming Date: Thu, 30 Mar 2023 11:17:52 +0000 Subject: [PATCH] Fix csi contidion policy match Signed-off-by: Ming --- .../resourcepolicies/resource_policies.go | 17 ++- .../resource_policies_test.go | 137 ++++++++++++++++++ internal/resourcepolicies/volume_resources.go | 8 +- 3 files changed, 152 insertions(+), 10 deletions(-) diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index c50133806..35e8b9f3e 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -97,15 +97,16 @@ func (p *Policies) match(res *structuredVolume) *Action { } func (p *Policies) GetMatchAction(res interface{}) (*Action, error) { - structuredVolume := new(structuredVolume) - if pv, ok := res.(*v1.PersistentVolume); ok { - structuredVolume.parsePV(pv) - } else if volume, ok := res.(*v1.Volume); ok { - structuredVolume.parsePodVolume(volume) - } else { - return nil, errors.Errorf("failed to convert object") + volume := &structuredVolume{} + switch obj := res.(type) { + case *v1.PersistentVolume: + volume.parsePV(obj) + case *v1.Volume: + volume.parsePodVolume(obj) + default: + return nil, errors.New("failed to convert object") } - return p.match(structuredVolume), nil + return p.match(volume), nil } func (p *Policies) Validate() error { diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 846adb2b5..51b2a4f75 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -242,3 +242,140 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) { } assert.Equal(t, p, resPolicies) } + +func TestGetMatchAction(t *testing.T) { + testCases := []struct { + name string + yamlData string + vol *v1.PersistentVolume + skip bool + }{ + { + name: "empty csi", + yamlData: `version: v1 +volumePolicies: +- conditions: + csi: {} + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + CSI: &v1.CSIPersistentVolumeSource{Driver: "aws.ebs.csi.driver"}, + }}, + }, + skip: true, + }, + { + name: "empty csi with pv no csi driver", + yamlData: `version: v1 +volumePolicies: +- conditions: + csi: {} + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }}, + }, + skip: false, + }, + { + name: "csi not configured", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{ + CSI: &v1.CSIPersistentVolumeSource{Driver: "aws.ebs.csi.driver"}, + }}, + }, + skip: true, + }, + { + name: "empty nfs", + yamlData: `version: v1 +volumePolicies: +- conditions: + nfs: {} + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + PersistentVolumeSource: v1.PersistentVolumeSource{ + NFS: &v1.NFSVolumeSource{Server: "192.168.1.20"}, + }}, + }, + skip: true, + }, + { + name: "nfs not configured", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{ + NFS: &v1.NFSVolumeSource{Server: "192.168.1.20"}, + }, + }, + }, + skip: true, + }, + { + name: "empty nfs with pv no nfs volume source", + yamlData: `version: v1 +volumePolicies: +- conditions: + capacity: "0,100Gi" + nfs: {} + action: + type: skip`, + vol: &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }, + skip: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resPolicies, err := unmarshalResourcePolicies(&tc.yamlData) + if err != nil { + t.Fatalf("got error when get match action %v", err) + } + assert.Nil(t, err) + policies := &Policies{} + err = policies.buildPolicy(resPolicies) + assert.Nil(t, err) + action, err := policies.GetMatchAction(tc.vol) + assert.Nil(t, err) + + if tc.skip { + if action.Type != Skip { + t.Fatalf("Expected action skip but is %v", action.Type) + } + } else if action != nil && action.Type == Skip { + t.Fatalf("Expected action not skip but is %v", action.Type) + } + }) + } +} diff --git a/internal/resourcepolicies/volume_resources.go b/internal/resourcepolicies/volume_resources.go index fcf8c0def..5c60d88e7 100644 --- a/internal/resourcepolicies/volume_resources.go +++ b/internal/resourcepolicies/volume_resources.go @@ -103,8 +103,8 @@ func (c *nfsCondition) match(v *structuredVolume) bool { } if c.nfs.Path == "" { - if c.nfs.Server == "" { - return true + if c.nfs.Server == "" { // match nfs: {} + return v.nfs != nil } if c.nfs.Server != v.nfs.Server { return false @@ -133,6 +133,10 @@ func (c *csiCondition) match(v *structuredVolume) bool { return true } + if c.csi.Driver == "" { // match csi: {} + return v.csi != nil + } + if v.csi == nil { return false }