Add gce disk labels support via storageclass/CreateVolumeRequest parameters

README.md

@@ -57,6 +57,7 @@ See Github [Issues](https://github.com/kubernetes-sigs/gcp-compute-persistent-di
 | type             | `pd-ssd` OR `pd-standard` | `pd-standard` | Type allows you to choose between standard Persistent Disks  or Solid State Drive Persistent Disks |
 | replication-type | `none` OR `regional-pd`   | `none`        | Replication type allows you to choose between Zonal Persistent Disks or Regional Persistent Disks  |
 | disk-encryption-kms-key | Fully qualified resource identifier for the key to use to encrypt new disks. | Empty string. | Encrypt disk using Customer Managed Encryption Key (CMEK). See [GKE Docs](https://cloud.google.com/kubernetes-engine/docs/how-to/using-cmek#create_a_cmek_protected_attached_disk) for details. |
+| labels           | `key1=value1,key2=value2` |               | Labels allow you to assign custom GCE Disk labels                                                  |
 
 ### Topology
 

examples/kubernetes/demo-labeled-sc.yaml

@@ -0,0 +1,8 @@
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: csi-gce-pd
+provisioner: pd.csi.storage.gke.io
+parameters:
+  labels: key1=value1,key2=value2
+volumeBindingMode: WaitForFirstConsumer

pkg/common/parameters.go

@@ -25,6 +25,7 @@ const (
 	ParameterKeyType                 = "type"
 	ParameterKeyReplicationType      = "replication-type"
 	ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
+	ParameterKeyLabels               = "labels"
 
 	replicationTypeNone = "none"
 
@@ -54,6 +55,9 @@ type DiskParameters struct {
 	// Values: {map[string]string}
 	// Default: ""
 	Tags map[string]string
+	// Values: map with arbitrary keys and values
+	// Default: empty map
+	Labels map[string]string
 }
 
 // ExtractAndDefaultParameters will take the relevant parameters from a map and
@@ -64,6 +68,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 		ReplicationType:      replicationTypeNone,     // Default
 		DiskEncryptionKMSKey: "",                      // Default
 		Tags:                 make(map[string]string), // Default
+		Labels:               map[string]string{},     // Default
 	}
 
 	for k, v := range parameters {
@@ -89,6 +94,13 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
 			p.Tags[tagKeyCreatedForClaimNamespace] = v
 		case ParameterKeyPVName:
 			p.Tags[tagKeyCreatedForVolumeName] = v
+		case ParameterKeyLabels:
+			labels, err := ConvertLabelsStringToMap(v)
+			if err != nil {
+				return p, fmt.Errorf("parameters contain invalid labels parameter: %w", err)
+			}
+
+			p.Labels = labels
 		default:
 			return p, fmt.Errorf("parameters contains invalid option %q", k)
 		}

pkg/common/parameters_test.go

@@ -36,31 +36,37 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "",
 				Tags:                 make(map[string]string),
+				Labels:               map[string]string{},
 			},
 		},
 		{
 			name:       "specified empties",
-			parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: ""},
+			parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
 			expectParams: DiskParameters{
 				DiskType:             "pd-standard",
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "",
 				Tags:                 make(map[string]string),
+				Labels:               map[string]string{},
 			},
 		},
 		{
 			name:       "random keys",
-			parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: ""},
+			parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
 			expectErr:  true,
 		},
 		{
 			name:       "real values",
-			parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"},
+			parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2"},
 			expectParams: DiskParameters{
 				DiskType:             "pd-ssd",
 				ReplicationType:      "regional-pd",
 				DiskEncryptionKMSKey: "foo/key",
 				Tags:                 make(map[string]string),
+				Labels: map[string]string{
+					"key1": "value1",
+					"key2": "value2",
+				},
 			},
 		},
 		{
@@ -71,6 +77,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				ReplicationType:      "regional-pd",
 				DiskEncryptionKMSKey: "foo/key",
 				Tags:                 make(map[string]string),
+				Labels:               map[string]string{},
 			},
 		},
 		{
@@ -81,6 +88,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				ReplicationType:      "none",
 				DiskEncryptionKMSKey: "foo/key",
 				Tags:                 make(map[string]string),
+				Labels:               map[string]string{},
 			},
 		},
 		{
@@ -91,6 +99,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{},
 			},
 		},
 	}

pkg/common/utils.go

@@ -18,6 +18,7 @@ package common
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -148,3 +149,60 @@ func CreateNodeID(project, zone, name string) string {
 func CreateZonalVolumeID(project, zone, name string) string {
 	return fmt.Sprintf(volIDZonalFmt, project, zone, name)
 }
+
+// ConvertLabelsStringToMap converts the labels from string to map
+// example: "key1=value1,key2=value2" gets converted into {"key1": "value1", "key2": "value2"}
+func ConvertLabelsStringToMap(labels string) (map[string]string, error) {
+	const labelsDelimiter = ","
+	const labelsKeyValueDelimiter = "="
+
+	labelsMap := make(map[string]string)
+	if labels == "" {
+		return labelsMap, nil
+	}
+
+	regexKey, _ := regexp.Compile(`^\p{Ll}[\p{Ll}0-9_-]{0,62}$`)
+	checkLabelKeyFn := func(key string) error {
+		if !regexKey.MatchString(key) {
+			return fmt.Errorf("label value %q is invalid (should start with lowercase letter / lowercase letter, digit, _ and - chars are allowed / 1-63 characters", key)
+		}
+		return nil
+	}
+
+	regexValue, _ := regexp.Compile(`^[\p{Ll}0-9_-]{0,63}$`)
+	checkLabelValueFn := func(value string) error {
+		if !regexValue.MatchString(value) {
+			return fmt.Errorf("label value %q is invalid (lowercase letter, digit, _ and - chars are allowed / 0-63 characters", value)
+		}
+
+		return nil
+	}
+
+	keyValueStrings := strings.Split(labels, labelsDelimiter)
+	for _, keyValue := range keyValueStrings {
+		keyValue := strings.Split(keyValue, labelsKeyValueDelimiter)
+
+		if len(keyValue) != 2 {
+			return nil, fmt.Errorf("labels %q are invalid, correct format: 'key1=value1,key2=value2'", labels)
+		}
+
+		key := strings.TrimSpace(keyValue[0])
+		if err := checkLabelKeyFn(key); err != nil {
+			return nil, err
+		}
+
+		value := strings.TrimSpace(keyValue[1])
+		if err := checkLabelValueFn(value); err != nil {
+			return nil, err
+		}
+
+		labelsMap[key] = value
+	}
+
+	const maxNumberOfLabels = 64
+	if len(labelsMap) > maxNumberOfLabels {
+		return nil, fmt.Errorf("more than %d labels is not allowed, given: %d", maxNumberOfLabels, len(labelsMap))
+	}
+
+	return labelsMap, nil
+}

pkg/common/utils_test.go

@@ -295,3 +295,185 @@ func TestKeyToVolumeID(t *testing.T) {
 	}
 
 }
+
+func TestConvertLabelsStringToMap(t *testing.T) {
+	t.Run("parsing labels string into map", func(t *testing.T) {
+		testCases := []struct {
+			name           string
+			labels         string
+			expectedOutput map[string]string
+			expectedError  bool
+		}{
+			{
+				name:           "should return empty map when labels string is empty",
+				labels:         "",
+				expectedOutput: map[string]string{},
+				expectedError:  false,
+			},
+			{
+				name:   "single label string",
+				labels: "key=value",
+				expectedOutput: map[string]string{
+					"key": "value",
+				},
+				expectedError: false,
+			},
+			{
+				name:   "multiple label string",
+				labels: "key1=value1,key2=value2",
+				expectedOutput: map[string]string{
+					"key1": "value1",
+					"key2": "value2",
+				},
+				expectedError: false,
+			},
+			{
+				name:   "multiple labels string with whitespaces gets trimmed",
+				labels: "key1=value1, key2=value2",
+				expectedOutput: map[string]string{
+					"key1": "value1",
+					"key2": "value2",
+				},
+				expectedError: false,
+			},
+			{
+				name:           "malformed labels string (no keys and values)",
+				labels:         ",,",
+				expectedOutput: nil,
+				expectedError:  true,
+			},
+			{
+				name:           "malformed labels string (incorrect format)",
+				labels:         "foo,bar",
+				expectedOutput: nil,
+				expectedError:  true,
+			},
+			{
+				name:           "malformed labels string (missing key)",
+				labels:         "key1=value1,=bar",
+				expectedOutput: nil,
+				expectedError:  true,
+			},
+			{
+				name:           "malformed labels string (missing key and value)",
+				labels:         "key1=value1,=bar,=",
+				expectedOutput: nil,
+				expectedError:  true,
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Logf("test case: %s", tc.name)
+			output, err := ConvertLabelsStringToMap(tc.labels)
+			if tc.expectedError && err == nil {
+				t.Errorf("Expected error but got none")
+			}
+			if err != nil {
+				if !tc.expectedError {
+					t.Errorf("Did not expect error but got: %v", err)
+				}
+				continue
+			}
+
+			if !reflect.DeepEqual(output, tc.expectedOutput) {
+				t.Errorf("Got labels %v, but expected %v", output, tc.expectedOutput)
+			}
+		}
+	})
+
+	t.Run("checking google requirements", func(t *testing.T) {
+		testCases := []struct {
+			name          string
+			labels        string
+			expectedError bool
+		}{
+			{
+				name: "64 labels at most",
+				labels: `k1=v,k2=v,k3=v,k4=v,k5=v,k6=v,k7=v,k8=v,k9=v,k10=v,k11=v,k12=v,k13=v,k14=v,k15=v,k16=v,k17=v,k18=v,k19=v,k20=v,
+                         k21=v,k22=v,k23=v,k24=v,k25=v,k26=v,k27=v,k28=v,k29=v,k30=v,k31=v,k32=v,k33=v,k34=v,k35=v,k36=v,k37=v,k38=v,k39=v,k40=v,
+                         k41=v,k42=v,k43=v,k44=v,k45=v,k46=v,k47=v,k48=v,k49=v,k50=v,k51=v,k52=v,k53=v,k54=v,k55=v,k56=v,k57=v,k58=v,k59=v,k60=v,
+                         k61=v,k62=v,k63=v,k64=v,k65=v`,
+				expectedError: true,
+			},
+			{
+				name:          "label key must start with lowercase char (# case)",
+				labels:        "#k=v",
+				expectedError: true,
+			},
+			{
+				name:          "label key must start with lowercase char (_ case)",
+				labels:        "_k=v",
+				expectedError: true,
+			},
+			{
+				name:          "label key must start with lowercase char (- case)",
+				labels:        "-k=v",
+				expectedError: true,
+			},
+			{
+				name:          "label key can only contain lowercase chars, digits, _ and -)",
+				labels:        "k*=v",
+				expectedError: true,
+			},
+			{
+				name:          "label key may not have over 63 characters",
+				labels:        "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
+				expectedError: true,
+			},
+			{
+				name:          "label key cannot contain . and /",
+				labels:        "kubernetes.io/created-for/pvc/namespace=v",
+				expectedError: true,
+			},
+			{
+				name:          "label value can only contain lowercase chars, digits, _ and -)",
+				labels:        "k1=###",
+				expectedError: true,
+			},
+			{
+				name:          "label value may not have over 63 characters",
+				labels:        "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
+				expectedError: true,
+			},
+			{
+				name:          "label value cannot contain . and /",
+				labels:        "kubernetes_io_created-for_pvc_namespace=v./",
+				expectedError: true,
+			},
+			{
+				name:          "label key can have up to 63 characters",
+				labels:        "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
+				expectedError: false,
+			},
+			{
+				name:          "label value can have up to 63 characters",
+				labels:        "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
+				expectedError: false,
+			},
+			{
+				name:          "label key can contain _ and -",
+				labels:        "kubernetes_io_created-for_pvc_namespace=v",
+				expectedError: false,
+			},
+			{
+				name:          "label value can contain _ and -",
+				labels:        "k=my_value-2",
+				expectedError: false,
+			},
+		}
+
+		for _, tc := range testCases {
+			t.Logf("test case: %s", tc.name)
+			_, err := ConvertLabelsStringToMap(tc.labels)
+
+			if tc.expectedError && err == nil {
+				t.Errorf("Expected error but got none")
+			}
+
+			if !tc.expectedError && err != nil {
+				t.Errorf("Did not expect error but got: %v", err)
+			}
+		}
+	})
+
+}

pkg/gce-cloud-provider/compute/fake-gce.go

@@ -271,6 +271,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
 			SelfLink:         fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
 			SourceSnapshotId: snapshotID,
 			Status:           cloud.mockDiskStatus,
+			Labels:           params.Labels,
 		}
 		if params.DiskEncryptionKMSKey != "" {
 			diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
@@ -287,6 +288,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
 			SelfLink:         fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
 			SourceSnapshotId: snapshotID,
 			Status:           cloud.mockDiskStatus,
+			Labels:           params.Labels,
 		}
 		if params.DiskEncryptionKMSKey != "" {
 			diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{