From a3f75422f159f90ddc5f495357c09081317bafd5 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Wed, 12 Apr 2023 23:05:09 +0000 Subject: [PATCH 1/5] verifying PV and PVC names against label restrictions --- pkg/common/parameters.go | 37 +++++++++++++++++++++++++++++++++++ pkg/common/parameters_test.go | 26 +++++++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index e7f0104dc..1b0befbfe 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -19,6 +19,8 @@ package common import ( "fmt" "strings" + + "k8s.io/klog/v2" ) const ( @@ -58,6 +60,11 @@ const ( tagKeyCreatedForSnapshotName = "kubernetes.io/created-for/volumesnapshot/name" tagKeyCreatedForSnapshotNamespace = "kubernetes.io/created-for/volumesnapshot/namespace" tagKeyCreatedForSnapshotContentName = "kubernetes.io/created-for/volumesnapshotcontent/name" + + // Keys for labels to tag to PV + labelKeyCreatedForClaimNamespace = "kubernetes_io_created_for_pvc_namespace" + labelKeyCreatedForVolumeName = "kubernetes_io_created_for_pv_name" + labelKeyCreatedForClaimName = "kubernetes_io_created_for_pvc_name" ) // DiskParameters contains normalized and defaulted disk parameters @@ -130,10 +137,40 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.DiskEncryptionKMSKey = v case ParameterKeyPVCName: p.Tags[tagKeyCreatedForClaimName] = v + // Verify that the parameters satisfy label restrictions before adding them as label + var pvc_name = strings.ToLower(v) + if len(pvc_name) > 63 { + pvc_name = pvc_name[:63] + } + _, err := ConvertLabelsStringToMap(labelKeyCreatedForClaimNamespace + "=" + pvc_name) + if err != nil { + klog.V(4).Info("Unable to add PVC name as a label", err) + } else { + p.Labels[labelKeyCreatedForClaimName] = pvc_name + } case ParameterKeyPVCNamespace: p.Tags[tagKeyCreatedForClaimNamespace] = v + // Verify that the parameters satisfy label restrictions before adding them as label + var namespace_name = strings.ToLower(v) + _, err := ConvertLabelsStringToMap(labelKeyCreatedForClaimNamespace + "=" + namespace_name) + if err != nil { + klog.V(4).Info("Unable to add Namespace name as a label", err) + } else { + p.Labels[labelKeyCreatedForClaimNamespace] = namespace_name + } case ParameterKeyPVName: p.Tags[tagKeyCreatedForVolumeName] = v + // Verify that the parameters satisfy label restrictions before adding them as label + var pv_name = strings.ToLower(v) + if len(pv_name) > 63 { + pv_name = pv_name[:63] + } + _, err := ConvertLabelsStringToMap(labelKeyCreatedForVolumeName + "=" + pv_name) + if err != nil { + klog.V(4).Info("Unable to add PV name as a label", err) + } else { + p.Labels[labelKeyCreatedForVolumeName] = pv_name + } case ParameterKeyLabels: paramLabels, err := ConvertLabelsStringToMap(v) if err != nil { diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go index 2d1a4b6b0..31735d585 100644 --- a/pkg/common/parameters_test.go +++ b/pkg/common/parameters_test.go @@ -139,7 +139,7 @@ func TestExtractAndDefaultParameters(t *testing.T) { ReplicationType: "none", DiskEncryptionKMSKey: "", Tags: map[string]string{tagKeyCreatedForClaimName: "testPVCName", tagKeyCreatedForClaimNamespace: "testPVCNamespace", tagKeyCreatedForVolumeName: "testPVName", tagKeyCreatedBy: "testDriver"}, - Labels: map[string]string{}, + Labels: map[string]string{labelKeyCreatedForClaimNamespace: "testpvcnamespace", labelKeyCreatedForVolumeName: "testpvname", labelKeyCreatedForClaimName: "testpvcname"}, }, }, { @@ -178,6 +178,30 @@ func TestExtractAndDefaultParameters(t *testing.T) { Labels: map[string]string{"key1": "value1", "label-1": "value-a", "label-2": "label-value-2"}, }, }, + { + name: "pv labels invalid", + parameters: map[string]string{ParameterKeyPVCNamespace: "test-namespace-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", ParameterKeyPVName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63-these-are-additional_with*+", ParameterKeyPVCName: "test-pvc-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char"}, + labels: map[string]string{}, + expectParams: DiskParameters{ + DiskType: "pd-standard", + ReplicationType: "none", + DiskEncryptionKMSKey: "", + Tags: map[string]string{tagKeyCreatedForClaimNamespace: "test-namespace-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63-these-are-additional_with*+", tagKeyCreatedForClaimName: "test-pvc-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedBy: "testDriver"}, + Labels: map[string]string{labelKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63"}, + }, + }, + { + name: "pv labels", + parameters: map[string]string{ParameterKeyPVCNamespace: "test-namespace-name-with-label-restrictions-allow_-and-63char", ParameterKeyPVName: "test-pv-name-with-label-restrictions-allow_-and-63char", ParameterKeyPVCName: "test-pvc-name-as-label-with-label-restriction-allow_-and-63char"}, + labels: map[string]string{}, + expectParams: DiskParameters{ + DiskType: "pd-standard", + ReplicationType: "none", + DiskEncryptionKMSKey: "", + Tags: map[string]string{tagKeyCreatedForClaimNamespace: "test-namespace-name-with-label-restrictions-allow_-and-63char", tagKeyCreatedForVolumeName: "test-pv-name-with-label-restrictions-allow_-and-63char", tagKeyCreatedForClaimName: "test-pvc-name-as-label-with-label-restriction-allow_-and-63char", tagKeyCreatedBy: "testDriver"}, + Labels: map[string]string{labelKeyCreatedForClaimNamespace: "test-namespace-name-with-label-restrictions-allow_-and-63char", labelKeyCreatedForVolumeName: "test-pv-name-with-label-restrictions-allow_-and-63char", labelKeyCreatedForClaimName: "test-pvc-name-as-label-with-label-restriction-allow_-and-63char"}, + }, + }, } for _, tc := range tests { From ab6217d3948655bfb72ab03c598eab5d37e950f3 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Wed, 12 Apr 2023 23:10:16 +0000 Subject: [PATCH 2/5] typo fix --- pkg/common/parameters_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go index 31735d585..a6a3cd61b 100644 --- a/pkg/common/parameters_test.go +++ b/pkg/common/parameters_test.go @@ -191,7 +191,7 @@ func TestExtractAndDefaultParameters(t *testing.T) { }, }, { - name: "pv labels", + name: "pv labels valid", parameters: map[string]string{ParameterKeyPVCNamespace: "test-namespace-name-with-label-restrictions-allow_-and-63char", ParameterKeyPVName: "test-pv-name-with-label-restrictions-allow_-and-63char", ParameterKeyPVCName: "test-pvc-name-as-label-with-label-restriction-allow_-and-63char"}, labels: map[string]string{}, expectParams: DiskParameters{ From 3ff902d981c4da206ad19ebdc285db49e09ebb4e Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 13 Apr 2023 05:48:51 +0000 Subject: [PATCH 3/5] Updating strategy to drop invalid labels instead of truncating longer labels --- pkg/common/parameters.go | 6 ------ pkg/common/parameters_test.go | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index 1b0befbfe..23683a288 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -139,9 +139,6 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.Tags[tagKeyCreatedForClaimName] = v // Verify that the parameters satisfy label restrictions before adding them as label var pvc_name = strings.ToLower(v) - if len(pvc_name) > 63 { - pvc_name = pvc_name[:63] - } _, err := ConvertLabelsStringToMap(labelKeyCreatedForClaimNamespace + "=" + pvc_name) if err != nil { klog.V(4).Info("Unable to add PVC name as a label", err) @@ -162,9 +159,6 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.Tags[tagKeyCreatedForVolumeName] = v // Verify that the parameters satisfy label restrictions before adding them as label var pv_name = strings.ToLower(v) - if len(pv_name) > 63 { - pv_name = pv_name[:63] - } _, err := ConvertLabelsStringToMap(labelKeyCreatedForVolumeName + "=" + pv_name) if err != nil { klog.V(4).Info("Unable to add PV name as a label", err) diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go index a6a3cd61b..fa538c6d6 100644 --- a/pkg/common/parameters_test.go +++ b/pkg/common/parameters_test.go @@ -187,7 +187,7 @@ func TestExtractAndDefaultParameters(t *testing.T) { ReplicationType: "none", DiskEncryptionKMSKey: "", Tags: map[string]string{tagKeyCreatedForClaimNamespace: "test-namespace-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63-these-are-additional_with*+", tagKeyCreatedForClaimName: "test-pvc-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedBy: "testDriver"}, - Labels: map[string]string{labelKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63"}, + Labels: map[string]string{}, }, }, { From e7c3360895975430682a1fb65b7b4b8876e2cb45 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Thu, 13 Apr 2023 05:54:47 +0000 Subject: [PATCH 4/5] Revert "Updating strategy to drop invalid labels instead of truncating longer labels" This reverts commit 3ff902d981c4da206ad19ebdc285db49e09ebb4e. --- pkg/common/parameters.go | 6 ++++++ pkg/common/parameters_test.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index 23683a288..1b0befbfe 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -139,6 +139,9 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.Tags[tagKeyCreatedForClaimName] = v // Verify that the parameters satisfy label restrictions before adding them as label var pvc_name = strings.ToLower(v) + if len(pvc_name) > 63 { + pvc_name = pvc_name[:63] + } _, err := ConvertLabelsStringToMap(labelKeyCreatedForClaimNamespace + "=" + pvc_name) if err != nil { klog.V(4).Info("Unable to add PVC name as a label", err) @@ -159,6 +162,9 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string p.Tags[tagKeyCreatedForVolumeName] = v // Verify that the parameters satisfy label restrictions before adding them as label var pv_name = strings.ToLower(v) + if len(pv_name) > 63 { + pv_name = pv_name[:63] + } _, err := ConvertLabelsStringToMap(labelKeyCreatedForVolumeName + "=" + pv_name) if err != nil { klog.V(4).Info("Unable to add PV name as a label", err) diff --git a/pkg/common/parameters_test.go b/pkg/common/parameters_test.go index fa538c6d6..a6a3cd61b 100644 --- a/pkg/common/parameters_test.go +++ b/pkg/common/parameters_test.go @@ -187,7 +187,7 @@ func TestExtractAndDefaultParameters(t *testing.T) { ReplicationType: "none", DiskEncryptionKMSKey: "", Tags: map[string]string{tagKeyCreatedForClaimNamespace: "test-namespace-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63-these-are-additional_with*+", tagKeyCreatedForClaimName: "test-pvc-name-as-labels-with-label-restrictions-does-not-allow*_--and-63+char", tagKeyCreatedBy: "testDriver"}, - Labels: map[string]string{}, + Labels: map[string]string{labelKeyCreatedForVolumeName: "test-pv-name-as-label-with-label-restriction-truncate-char-at63"}, }, }, { From 112c869accb1925ece18581daaf013ea846f54f1 Mon Sep 17 00:00:00 2001 From: Sneha Aradhey Date: Fri, 28 Apr 2023 17:39:18 +0000 Subject: [PATCH 5/5] Adding warning messages --- pkg/common/parameters.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/common/parameters.go b/pkg/common/parameters.go index 1b0befbfe..fc112305c 100644 --- a/pkg/common/parameters.go +++ b/pkg/common/parameters.go @@ -141,6 +141,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string var pvc_name = strings.ToLower(v) if len(pvc_name) > 63 { pvc_name = pvc_name[:63] + klog.V(4).Info("The disk label for PVC-name is truncated to match label restriction (<64 characters allowed)") } _, err := ConvertLabelsStringToMap(labelKeyCreatedForClaimNamespace + "=" + pvc_name) if err != nil { @@ -164,6 +165,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string var pv_name = strings.ToLower(v) if len(pv_name) > 63 { pv_name = pv_name[:63] + klog.V(4).Info("The disk label for PV-name is truncated to match label restriction (<64 characters allowed)") } _, err := ConvertLabelsStringToMap(labelKeyCreatedForVolumeName + "=" + pv_name) if err != nil {