Skip to content

Commit b11526f

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 f476c98 commit b11526f

File tree

11 files changed

+306
-3
lines changed

11 files changed

+306
-3
lines changed

‎README.md

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

5960
### Topology
6061

‎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
)
@@ -40,6 +41,9 @@ type DiskParameters struct {
4041
// Values: {string}
4142
// Default: ""
4243
DiskEncryptionKMSKey string
44+
// Values: map with arbitrary keys and values
45+
// Default: empty map
46+
Labels map[string]string
4347
}
4448

4549
// ExtractAndDefaultParameters will take the relevant parameters from a map and
@@ -49,6 +53,7 @@ func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters,
4953
DiskType: "pd-standard", // Default
5054
ReplicationType: replicationTypeNone, // Default
5155
DiskEncryptionKMSKey: "", // Default
56+
Labels: map[string]string{}, // Default
5257
}
5358
for k, v := range parameters {
5459
if k == "csiProvisionerSecretName" || k == "csiProvisionerSecretNamespace" {
@@ -67,6 +72,13 @@ func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters,
6772
case ParameterKeyDiskEncryptionKmsKey:
6873
// Resource names (e.g. "keyRings", "cryptoKeys", etc.) are case sensitive, so do not change case
6974
p.DiskEncryptionKMSKey = v
75+
case ParameterKeyLabels:
76+
labels, err := ConvertLabelsStringToMap(v)
77+
if err != nil {
78+
return p, fmt.Errorf("parameters contain invalid labels parameter: %w", err)
79+
}
80+
81+
p.Labels = labels
7082
default:
7183
return p, fmt.Errorf("parameters contains invalid option %q", k)
7284
}

‎pkg/common/parameters_test.go

+10-3
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,35 @@ func TestExtractAndDefaultParameters(t *testing.T) {
3535
DiskType: "pd-standard",
3636
ReplicationType: "none",
3737
DiskEncryptionKMSKey: "",
38+
Labels: map[string]string{},
3839
},
3940
},
4041
{
4142
name: "specified empties",
42-
parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: ""},
43+
parameters: map[string]string{ParameterKeyType: "", ParameterKeyReplicationType: "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
4344
expectParams: DiskParameters{
4445
DiskType: "pd-standard",
4546
ReplicationType: "none",
4647
DiskEncryptionKMSKey: "",
48+
Labels: map[string]string{},
4749
},
4850
},
4951
{
5052
name: "random keys",
51-
parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: ""},
53+
parameters: map[string]string{ParameterKeyType: "", "foo": "", ParameterKeyDiskEncryptionKmsKey: "", ParameterKeyLabels: ""},
5254
expectErr: true,
5355
},
5456
{
5557
name: "real values",
56-
parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key"},
58+
parameters: map[string]string{ParameterKeyType: "pd-ssd", ParameterKeyReplicationType: "regional-pd", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2"},
5759
expectParams: DiskParameters{
5860
DiskType: "pd-ssd",
5961
ReplicationType: "regional-pd",
6062
DiskEncryptionKMSKey: "foo/key",
63+
Labels: map[string]string{
64+
"key1": "value1",
65+
"key2": "value2",
66+
},
6167
},
6268
},
6369
{
@@ -67,6 +73,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
6773
DiskType: "pd-standard",
6874
ReplicationType: "none",
6975
DiskEncryptionKMSKey: "foo/key",
76+
Labels: map[string]string{},
7077
},
7178
},
7279
}

‎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

+152
Original file line numberDiff line numberDiff line change
@@ -295,3 +295,155 @@ 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",
400+
labels: "#k=v",
401+
expectedError: true,
402+
},
403+
{
404+
name: "label key can only contain lowercase chars, digits, _ and -)",
405+
labels: "k*=v",
406+
expectedError: true,
407+
},
408+
{
409+
name: "label key may not have over 63 characters",
410+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
411+
expectedError: true,
412+
},
413+
{
414+
name: "label key can have up to 63 characters",
415+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
416+
expectedError: false,
417+
},
418+
{
419+
name: "label value can only contain lowercase chars, digits, _ and -)",
420+
labels: "k1=###",
421+
expectedError: true,
422+
},
423+
{
424+
name: "label value may not have over 63 characters",
425+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij1234=v",
426+
expectedError: true,
427+
},
428+
{
429+
name: "label value can have up to 63 characters",
430+
labels: "abcdefghijabcdefghijabcdefghijabcdefghijabcdefghijabcdefghij123=v",
431+
expectedError: false,
432+
},
433+
}
434+
435+
for _, tc := range testCases {
436+
t.Logf("test case: %s", tc.name)
437+
_, err := ConvertLabelsStringToMap(tc.labels)
438+
439+
if tc.expectedError && err == nil {
440+
t.Errorf("Expected error but got none")
441+
}
442+
443+
if !tc.expectedError && err != nil {
444+
t.Errorf("Did not expect error but got: %v", err)
445+
}
446+
}
447+
})
448+
449+
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
256256
SelfLink: fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
257257
SourceSnapshotId: snapshotID,
258258
Status: cloud.mockDiskStatus,
259+
Labels: params.Labels,
259260
}
260261
if params.DiskEncryptionKMSKey != "" {
261262
diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
@@ -272,6 +273,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
272273
SelfLink: fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
273274
SourceSnapshotId: snapshotID,
274275
Status: cloud.mockDiskStatus,
276+
Labels: params.Labels,
275277
}
276278
if params.DiskEncryptionKMSKey != "" {
277279
diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{

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

+2
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
284284
SizeGb: common.BytesToGb(capBytes),
285285
Description: "Regional disk created by GCE-PD CSI Driver",
286286
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
287+
Labels: params.Labels,
287288
}
288289
if snapshotID != "" {
289290
diskToCreate.SourceSnapshot = snapshotID
@@ -343,6 +344,7 @@ func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Ke
343344
SizeGb: common.BytesToGb(capBytes),
344345
Description: "Disk created by GCE-PD CSI Driver",
345346
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
347+
Labels: params.Labels,
346348
}
347349

348350
if snapshotID != "" {

‎pkg/gce-pd-csi-driver/controller_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,31 @@ func TestCreateVolumeArguments(t *testing.T) {
668668
AccessibleTopology: stdTopology,
669669
},
670670
},
671+
{
672+
name: "success with labels parameter",
673+
req: &csi.CreateVolumeRequest{
674+
Name: name,
675+
CapacityRange: stdCapRange,
676+
VolumeCapabilities: stdVolCaps,
677+
Parameters: map[string]string{"labels": "key1=value1,key2=value2"},
678+
},
679+
expVol: &csi.Volume{
680+
CapacityBytes: common.GbToBytes(20),
681+
VolumeId: testVolumeID,
682+
VolumeContext: nil,
683+
AccessibleTopology: stdTopology,
684+
},
685+
},
686+
{
687+
name: "fail with malformed labels parameter",
688+
req: &csi.CreateVolumeRequest{
689+
Name: name,
690+
CapacityRange: stdCapRange,
691+
VolumeCapabilities: stdVolCaps,
692+
Parameters: map[string]string{"labels": "key1=value1,#=$;;"},
693+
},
694+
expErrCode: codes.InvalidArgument,
695+
},
671696
}
672697

673698
// Run test cases

0 commit comments

Comments
 (0)