Skip to content

Commit 31c22b2

Browse files
ernestas2kernestas2k
committed
Add gce disk labels support via create volume parameters
Add labels validation to match gce requirements add e2e test case for labels
1 parent a259069 commit 31c22b2

File tree

11 files changed

+338
-3
lines changed

11 files changed

+338
-3
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ See Github [Issues](https://github.com/kubernetes-sigs/gcp-compute-persistent-di
5757
| type | `pd-ssd` OR `pd-standard` | `pd-standard` | Type allows you to choose between standard Persistent Disks or Solid State Drive Persistent Disks |
5858
| replication-type | `none` OR `regional-pd` | `none` | Replication type allows you to choose between Zonal Persistent Disks or Regional Persistent Disks |
5959
| 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. |
60+
| labels | `key1=value1,key2=value2` | | Labels allow you to assign custom GCE Disk labels |
6061

6162
### Topology
6263

‎examples/kubernetes/demo-labeled-sc.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
apiVersion: storage.k8s.io/v1
2+
kind: StorageClass
3+
metadata:
4+
name: csi-gce-pd
5+
provisioner: pd.csi.storage.gke.io
6+
parameters:
7+
labels: key1=value1,key2=value2
8+
volumeBindingMode: WaitForFirstConsumer

‎pkg/common/parameters.go

+12
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const (
2525
ParameterKeyType = "type"
2626
ParameterKeyReplicationType = "replication-type"
2727
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
28+
ParameterKeyLabels = "labels"
2829

2930
replicationTypeNone = "none"
3031

@@ -54,6 +55,9 @@ type DiskParameters struct {
5455
// Values: {map[string]string}
5556
// Default: ""
5657
Tags map[string]string
58+
// Values: map with arbitrary keys and values
59+
// Default: empty map
60+
Labels map[string]string
5761
}
5862

5963
// ExtractAndDefaultParameters will take the relevant parameters from a map and
@@ -64,6 +68,7 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
6468
ReplicationType: replicationTypeNone, // Default
6569
DiskEncryptionKMSKey: "", // Default
6670
Tags: make(map[string]string), // Default
71+
Labels: map[string]string{}, // Default
6772
}
6873

6974
for k, v := range parameters {
@@ -89,6 +94,13 @@ func ExtractAndDefaultParameters(parameters map[string]string, driverName string
8994
p.Tags[tagKeyCreatedForClaimNamespace] = v
9095
case ParameterKeyPVName:
9196
p.Tags[tagKeyCreatedForVolumeName] = v
97+
case ParameterKeyLabels:
98+
labels, err := ConvertLabelsStringToMap(v)
99+
if err != nil {
100+
return p, fmt.Errorf("parameters contain invalid labels parameter: %w", err)
101+
}
102+
103+
p.Labels = labels
92104
default:
93105
return p, fmt.Errorf("parameters contains invalid option %q", k)
94106
}

‎pkg/common/parameters_test.go

+12-3
Original file line numberDiff line numberDiff line change
@@ -36,31 +36,37 @@ func TestExtractAndDefaultParameters(t *testing.T) {
3636
ReplicationType: "none",
3737
DiskEncryptionKMSKey: "",
3838
Tags: make(map[string]string),
39+
Labels: map[string]string{},
3940
},
4041
},
4142
{
4243
name: "specified empties",
43-
parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: ""},
44+
parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
4445
expectParams: DiskParameters{
4546
DiskType: "pd-standard",
4647
ReplicationType: "none",
4748
DiskEncryptionKMSKey: "",
4849
Tags: make(map[string]string),
50+
Labels: map[string]string{},
4951
},
5052
},
5153
{
5254
name: "random keys",
53-
parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: ""},
55+
parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
5456
expectErr: true,
5557
},
5658
{
5759
name: "real values",
58-
parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"},
60+
parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2"},
5961
expectParams: DiskParameters{
6062
DiskType: "pd-ssd",
6163
ReplicationType: "regional-pd",
6264
DiskEncryptionKMSKey: "foo/key",
6365
Tags: make(map[string]string),
66+
Labels: map[string]string{
67+
"key1": "value1",
68+
"key2": "value2",
69+
},
6470
},
6571
},
6672
{
@@ -71,6 +77,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
7177
ReplicationType: "regional-pd",
7278
DiskEncryptionKMSKey: "foo/key",
7379
Tags: make(map[string]string),
80+
Labels: map[string]string{},
7481
},
7582
},
7683
{
@@ -81,6 +88,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
8188
ReplicationType: "none",
8289
DiskEncryptionKMSKey: "foo/key",
8390
Tags: make(map[string]string),
91+
Labels: map[string]string{},
8492
},
8593
},
8694
{
@@ -91,6 +99,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
9199
ReplicationType: "none",
92100
DiskEncryptionKMSKey: "",
93101
Tags: map[string]string{tagKeyCreatedForClaimName: "testPVCName", tagKeyCreatedForClaimNamespace: "testPVCNamespace", tagKeyCreatedForVolumeName: "testPVName", tagKeyCreatedBy: "testDriver"},
102+
Labels: map[string]string{},
94103
},
95104
},
96105
}

‎pkg/common/utils.go

+58
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package common
1818

1919
import (
2020
"fmt"
21+
"regexp"
2122
"strings"
2223

2324
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -148,3 +149,60 @@ func CreateNodeID(project, zone, name string) string {
148149
func CreateZonalVolumeID(project, zone, name string) string {
149150
return fmt.Sprintf(volIDZonalFmt, project, zone, name)
150151
}
152+
153+
// ConvertLabelsStringToMap converts the labels from string to map
154+
// example: "key1=value1,key2=value2" gets converted into {"key1": "value1", "key2": "value2"}
155+
func ConvertLabelsStringToMap(labels string) (map[string]string, error) {
156+
const labelsDelimiter = ","
157+
const labelsKeyValueDelimiter = "="
158+
159+
labelsMap := make(map[string]string)
160+
if labels == "" {
161+
return labelsMap, nil
162+
}
163+
164+
regexKey, _ := regexp.Compile(`^\p{Ll}[\p{Ll}0-9_-]{0,62}$`)
165+
checkLabelKeyFn := func(key string) error {
166+
if !regexKey.MatchString(key) {
167+
return fmt.Errorf("label value %q is invalid (should start with lowercase letter / lowercase letter, digit, _ and - chars are allowed / 1-63 characters", key)
168+
}
169+
return nil
170+
}
171+
172+
regexValue, _ := regexp.Compile(`^[\p{Ll}0-9_-]{0,63}$`)
173+
checkLabelValueFn := func(value string) error {
174+
if !regexValue.MatchString(value) {
175+
return fmt.Errorf("label value %q is invalid (lowercase letter, digit, _ and - chars are allowed / 0-63 characters", value)
176+
}
177+
178+
return nil
179+
}
180+
181+
keyValueStrings := strings.Split(labels, labelsDelimiter)
182+
for _, keyValue := range keyValueStrings {
183+
keyValue := strings.Split(keyValue, labelsKeyValueDelimiter)
184+
185+
if len(keyValue) != 2 {
186+
return nil, fmt.Errorf("labels %q are invalid, correct format: 'key1=value1,key2=value2'", labels)
187+
}
188+
189+
key := strings.TrimSpace(keyValue[0])
190+
if err := checkLabelKeyFn(key); err != nil {
191+
return nil, err
192+
}
193+
194+
value := strings.TrimSpace(keyValue[1])
195+
if err := checkLabelValueFn(value); err != nil {
196+
return nil, err
197+
}
198+
199+
labelsMap[key] = value
200+
}
201+
202+
const maxNumberOfLabels = 64
203+
if len(labelsMap) > maxNumberOfLabels {
204+
return nil, fmt.Errorf("more than %d labels is not allowed, given: %d", maxNumberOfLabels, len(labelsMap))
205+
}
206+
207+
return labelsMap, nil
208+
}

‎pkg/common/utils_test.go

+182
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,185 @@ func TestKeyToVolumeID(t *testing.T) {
295295
}
296296

297297
}
298+
299+
func TestConvertLabelsStringToMap(t *testing.T) {
300+
t.Run("parsing labels string into map", func(t *testing.T) {
301+
testCases := []struct {
302+
name string
303+
labels string
304+
expectedOutput map[string]string
305+
expectedError bool
306+
}{
307+
{
308+
name: "should return empty map when labels string is empty",
309+
labels: "",
310+
expectedOutput: map[string]string{},
311+
expectedError: false,
312+
},
313+
{
314+
name: "single label string",
315+
labels: "key=value",
316+
expectedOutput: map[string]string{
317+
"key": "value",
318+
},
319+
expectedError: false,
320+
},
321+
{
322+
name: "multiple label string",
323+
labels: "key1=value1,key2=value2",
324+
expectedOutput: map[string]string{
325+
"key1": "value1",
326+
"key2": "value2",
327+
},
328+
expectedError: false,
329+
},
330+
{
331+
name: "multiple labels string with whitespaces gets trimmed",
332+
labels: "key1=value1, key2=value2",
333+
expectedOutput: map[string]string{
334+
"key1": "value1",
335+
"key2": "value2",
336+
},
337+
expectedError: false,
338+
},
339+
{
340+
name: "malformed labels string (no keys and values)",
341+
labels: ",,",
342+
expectedOutput: nil,
343+
expectedError: true,
344+
},
345+
{
346+
name: "malformed labels string (incorrect format)",
347+
labels: "foo,bar",
348+
expectedOutput: nil,
349+
expectedError: true,
350+
},
351+
{
352+
name: "malformed labels string (missing key)",
353+
labels: "key1=value1,=bar",
354+
expectedOutput: nil,
355+
expectedError: true,
356+
},
357+
{
358+
name: "malformed labels string (missing key and value)",
359+
labels: "key1=value1,=bar,=",
360+
expectedOutput: nil,
361+
expectedError: true,
362+
},
363+
}
364+
365+
for _, tc := range testCases {
366+
t.Logf("test case: %s", tc.name)
367+
output, err := ConvertLabelsStringToMap(tc.labels)
368+
if tc.expectedError && err == nil {
369+
t.Errorf("Expected error but got none")
370+
}
371+
if err != nil {
372+
if !tc.expectedError {
373+
t.Errorf("Did not expect error but got: %v", err)
374+
}
375+
continue
376+
}
377+
378+
if !reflect.DeepEqual(output, tc.expectedOutput) {
379+
t.Errorf("Got labels %v, but expected %v", output, tc.expectedOutput)
380+
}
381+
}
382+
})
383+
384+
t.Run("checking google requirements", func(t *testing.T) {
385+
testCases := []struct {
386+
name string
387+
labels string
388+
expectedError bool
389+
}{
390+
{
391+
name: "64 labels at most",
392+
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,
393+
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,
394+
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,
395+
k61=v,k62=v,k63=v,k64=v,k65=v`,
396+
expectedError: true,
397+
},
398+
{
399+
name: "label key must start with lowercase char (# case)",
400+
labels: "#k=v",
401+
expectedError: true,
402+
},
403+
{
404+
name: "label key must start with lowercase char (_ case)",
405+
labels: "_k=v",
406+
expectedError: true,
407+
},
408+
{
409+
name: "label key must start with lowercase char (- case)",
410+
labels: "-k=v",
411+
expectedError: true,
412+
},
413+
{
414+
name: "label key can only contain lowercase chars, digits, _ and -)",
415+
labels: "k*=v",
416+
expectedError: true,
417+
},
418+
{
419+
name: "label key may not have over 63 characters",
420+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
421+
expectedError: true,
422+
},
423+
{
424+
name: "label key cannot contain . and /",
425+
labels: "kubernetes.io/created-for/pvc/namespace=v",
426+
expectedError: true,
427+
},
428+
{
429+
name: "label value can only contain lowercase chars, digits, _ and -)",
430+
labels: "k1=###",
431+
expectedError: true,
432+
},
433+
{
434+
name: "label value may not have over 63 characters",
435+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
436+
expectedError: true,
437+
},
438+
{
439+
name: "label value cannot contain . and /",
440+
labels: "kubernetes_io_created-for_pvc_namespace=v./",
441+
expectedError: true,
442+
},
443+
{
444+
name: "label key can have up to 63 characters",
445+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
446+
expectedError: false,
447+
},
448+
{
449+
name: "label value can have up to 63 characters",
450+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
451+
expectedError: false,
452+
},
453+
{
454+
name: "label key can contain _ and -",
455+
labels: "kubernetes_io_created-for_pvc_namespace=v",
456+
expectedError: false,
457+
},
458+
{
459+
name: "label value can contain _ and -",
460+
labels: "k=my_value-2",
461+
expectedError: false,
462+
},
463+
}
464+
465+
for _, tc := range testCases {
466+
t.Logf("test case: %s", tc.name)
467+
_, err := ConvertLabelsStringToMap(tc.labels)
468+
469+
if tc.expectedError && err == nil {
470+
t.Errorf("Expected error but got none")
471+
}
472+
473+
if !tc.expectedError && err != nil {
474+
t.Errorf("Did not expect error but got: %v", err)
475+
}
476+
}
477+
})
478+
479+
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
271271
SelfLink: fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
272272
SourceSnapshotId: snapshotID,
273273
Status: cloud.mockDiskStatus,
274+
Labels: params.Labels,
274275
}
275276
if params.DiskEncryptionKMSKey != "" {
276277
diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
@@ -287,6 +288,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
287288
SelfLink: fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
288289
SourceSnapshotId: snapshotID,
289290
Status: cloud.mockDiskStatus,
291+
Labels: params.Labels,
290292
}
291293
if params.DiskEncryptionKMSKey != "" {
292294
diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{

0 commit comments

Comments
 (0)