From 40b8be288b7caf5d8f7e36feb55eb6823ab62fdf Mon Sep 17 00:00:00 2001 From: Moritz Rieger Date: Thu, 11 Aug 2022 13:40:23 +0200 Subject: [PATCH] support storageclass from annotations (#149) providing the storage-class via annotations is deprecated but still respected when provided CLOSES #150 --- checks/doks/dobs_pod_owner.go | 17 +++- checks/doks/dobs_pod_owner_test.go | 137 ++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 4 deletions(-) diff --git a/checks/doks/dobs_pod_owner.go b/checks/doks/dobs_pod_owner.go index e58f917a..77221aa9 100644 --- a/checks/doks/dobs_pod_owner.go +++ b/checks/doks/dobs_pod_owner.go @@ -90,11 +90,12 @@ func isDOBSVolume(volume corev1.Volume, namespace string, objects *kube.Objects) if claim == nil { return false } - if claim.Spec.StorageClassName == nil && isDOCSI(objects.DefaultStorageClass.Provisioner) { + + scn := getStorageClassName(claim) + if scn == nil && isDOCSI(objects.DefaultStorageClass.Provisioner) { return true } - - sc := getStorageClass(objects.StorageClasses, claim.Spec.StorageClassName) + sc := getStorageClass(objects.StorageClasses, scn) if sc != nil && isDOCSI(sc.Provisioner) { return true } @@ -108,6 +109,16 @@ func isDOBSVolume(volume corev1.Volume, namespace string, objects *kube.Objects) return false } +func getStorageClassName(claim *corev1.PersistentVolumeClaim) *string{ + if claim.Spec.StorageClassName != nil { + return claim.Spec.StorageClassName + } + if scn, found := claim.Annotations["volume.beta.kubernetes.io/storage-class"]; found { + return &scn + } + return nil +} + func isDOCSI(referrer string) bool { return referrer == DOCSIDriver || referrer == LegacyCSIDriver } diff --git a/checks/doks/dobs_pod_owner_test.go b/checks/doks/dobs_pod_owner_test.go index a0a67e78..444d6010 100644 --- a/checks/doks/dobs_pod_owner_test.go +++ b/checks/doks/dobs_pod_owner_test.go @@ -17,9 +17,10 @@ limitations under the License. package doks import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/digitalocean/clusterlint/checks" "github.com/digitalocean/clusterlint/kube" "github.com/stretchr/testify/assert" @@ -72,6 +73,19 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "bare dobs pod referenced by deprecated pvc annotation", + objs: provideStorageClassInAnnotation(pvcDobs(DOBlockStorageName, DOCSIDriver)), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{Name: "foo", Namespace: metav1.NamespaceDefault}, + Owners: nil, + }, + }, + }, { name: "bare dobs pod referenced by pvc -- with legacy driver", objs: pvcDobs(DOBlockStorageName, LegacyCSIDriver), @@ -85,6 +99,19 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "bare dobs pod referenced by deprecated pvc annotation -- with legacy driver", + objs: provideStorageClassInAnnotation(pvcDobs(DOBlockStorageName, LegacyCSIDriver)), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{Name: "foo", Namespace: metav1.NamespaceDefault}, + Owners: nil, + }, + }, + }, { name: "bare dobs pod referenced by pvc with default storage class", objs: pvcDobs("", DOCSIDriver), @@ -98,6 +125,19 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "bare dobs pod referenced by deprecated pvc with default storage class", + objs: provideStorageClassInAnnotation(pvcDobs("", DOCSIDriver)), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{Name: "foo", Namespace: metav1.NamespaceDefault}, + Owners: nil, + }, + }, + }, { name: "bare dobs pod referenced by pvc with default storage class -- with legacy driver", objs: pvcDobs("", LegacyCSIDriver), @@ -111,6 +151,19 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "bare dobs pod referenced by deprecated pvc annotation with default storage class -- with legacy driver", + objs: provideStorageClassInAnnotation(pvcDobs("", LegacyCSIDriver)), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{Name: "foo", Namespace: metav1.NamespaceDefault}, + Owners: nil, + }, + }, + }, { name: "bare dobs pod referenced by csi", objs: csiDobs(DOCSIDriver), @@ -166,6 +219,35 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "dobs pod owned by deployment, with deprecated pvc annotation", + objs: deployment(provideStorageClassInAnnotation(pvcDobs("", DOCSIDriver))), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{ + Name: "foo", + Namespace: metav1.NamespaceDefault, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "web-app", + }, + }, + }, + Owners: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "web-app", + }, + }, + }, + }, + }, { name: "dobs pods owned by multiple deployments", objs: multiDeployment(), @@ -247,16 +329,55 @@ func TestDobsPodOwnerWarning(t *testing.T) { }, }, }, + { + name: "dobs pod owned by deployment, with deprecated pvc annotation -- with legacy driver", + objs: deployment(provideStorageClassInAnnotation(pvcDobs("", LegacyCSIDriver))), + expected: []checks.Diagnostic{ + { + Severity: checks.Warning, + Message: "Pod referencing DOBS volumes must be owned by StatefulSet", + Kind: checks.Pod, + Object: &metav1.ObjectMeta{ + Name: "foo", + Namespace: metav1.NamespaceDefault, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "web-app", + }, + }, + }, + Owners: []metav1.OwnerReference{ + { + APIVersion: "apps/v1", + Kind: "Deployment", + Name: "web-app", + }, + }, + }, + }, + }, { name: "dobs pod owned by statefulset", objs: statefulSet(pvcDobs("", DOCSIDriver)), expected: nil, }, + { + name: "dobs pod owned by statefulset, with deprecated pvc annotation", + objs: statefulSet(provideStorageClassInAnnotation(pvcDobs("", DOCSIDriver))), + expected: nil, + }, { name: "dobs pod owned by statefulset -- with legacy driver", objs: statefulSet(pvcDobs("", LegacyCSIDriver)), expected: nil, }, + { + name: "dobs pod owned by statefulset, with deprecated pvc annotation -- with legacy driver", + objs: statefulSet(provideStorageClassInAnnotation(pvcDobs("", LegacyCSIDriver))), + expected: nil, + }, } for _, test := range tests { @@ -316,6 +437,20 @@ func deployment(objs *kube.Objects) *kube.Objects { return objs } +// Users request dynamically provisioned storage by including a storage class in their PersistentVolumeClaim. +// Before Kubernetes v1.6, this was done via the volume.beta.kubernetes.io/storage-class annotation. However, this annotation is deprecated since v1.9. +// https://kubernetes.io/docs/concepts/storage/dynamic-provisioning/#:~:text=Users%20request%20dynamically,the%20PersistentVolumeClaim%20object. +func provideStorageClassInAnnotation(objs *kube.Objects) *kube.Objects { + if objs.PersistentVolumeClaims.Items[0].Spec.StorageClassName != nil { + if objs.PersistentVolumeClaims.Items[0].Annotations == nil { + objs.PersistentVolumeClaims.Items[0].Annotations = make(map[string]string) + } + objs.PersistentVolumeClaims.Items[0].Annotations["volume.beta.kubernetes.io/storage-class"] = *objs.PersistentVolumeClaims.Items[0].Spec.StorageClassName + objs.PersistentVolumeClaims.Items[0].Spec.StorageClassName = nil + } + return objs +} + func multiDeployment() *kube.Objects { objs := &kube.Objects{ Pods: &corev1.PodList{