Skip to content

Commit b855729

Browse files
davidz627davidz627
committed
Implement ValidateVolumeCapabilities and refactor parameter handling for more comprehensive validation of existing disks in all cloud calls
1 parent afd7d61 commit b855729

File tree

6 files changed

+205
-150
lines changed

6 files changed

+205
-150
lines changed

‎pkg/common/constants.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ limitations under the License.
1717
package common
1818

1919
const (
20-
// Keys for Storage Class Parameters
21-
ParameterKeyType = "type"
22-
ParameterKeyReplicationType = "replication-type"
23-
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
24-
2520
// Keys for Topology. This key will be shared amongst drivers from GCP
2621
TopologyKeyZone = "topology.gke.io/zone"
2722

‎pkg/common/parameters.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
Copyright 2020 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package common
18+
19+
import (
20+
"fmt"
21+
"strings"
22+
)
23+
24+
const (
25+
ParameterKeyType = "type"
26+
ParameterKeyReplicationType = "replication-type"
27+
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
28+
29+
replicationTypeNone = "none"
30+
)
31+
32+
// DiskParameters contains normalized and defaulted disk parameters
33+
type DiskParameters struct {
34+
// Values: pd-standard OR pd-ssd
35+
// Default: pd-standard
36+
DiskType string
37+
// Values: "none", regional-pd
38+
// Default: "none"
39+
ReplicationType string
40+
// Values: {string}
41+
// Default: ""
42+
DiskEncryptionKMSKey string
43+
}
44+
45+
// ExtractAndDefaultParameters will take the relevant parameters from a map and
46+
// put them into a well defined struct making sure to default unspecified fields
47+
func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters, error) {
48+
p := DiskParameters{
49+
DiskType: "pd-standard", // Default
50+
ReplicationType: replicationTypeNone, // Default
51+
DiskEncryptionKMSKey: "", // Default
52+
}
53+
for k, v := range parameters {
54+
if k == "csiProvisionerSecretName" || k == "csiProvisionerSecretNamespace" {
55+
// These are hardcoded secrets keys required to function but not needed by GCE PD
56+
continue
57+
}
58+
switch strings.ToLower(k) {
59+
case ParameterKeyType:
60+
if v != "" {
61+
p.DiskType = strings.ToLower(v)
62+
}
63+
case ParameterKeyReplicationType:
64+
if v != "" {
65+
p.ReplicationType = strings.ToLower(v)
66+
}
67+
case ParameterKeyDiskEncryptionKmsKey:
68+
// Resource names (e.g. "keyRings", "cryptoKeys", etc.) are case sensitive, so do not change case
69+
p.DiskEncryptionKMSKey = v
70+
default:
71+
return p, fmt.Errorf("parameters contains invalid option %q", k)
72+
}
73+
}
74+
return p, nil
75+
}

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

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ limitations under the License.
1515
package gcecloudprovider
1616

1717
import (
18+
"strings"
19+
1820
computev1 "google.golang.org/api/compute/v1"
1921
)
2022

@@ -90,15 +92,21 @@ func (d *CloudDisk) GetKind() string {
9092
}
9193
}
9294

93-
func (d *CloudDisk) GetType() string {
95+
// GetPDType returns the type of the PD as either 'pd-standard' or 'pd-ssd' The
96+
// "Type" field on the compute disk is stored as a url like
97+
// projects/project/zones/zone/diskTypes/pd-standard
98+
func (d *CloudDisk) GetPDType() string {
99+
var pdType string
94100
switch d.Type() {
95101
case Zonal:
96-
return d.ZonalDisk.Type
102+
pdType = d.ZonalDisk.Type
97103
case Regional:
98-
return d.RegionalDisk.Type
104+
pdType = d.RegionalDisk.Type
99105
default:
100106
return ""
101107
}
108+
respType := strings.Split(pdType, "/")
109+
return strings.TrimSpace(respType[len(respType)-1])
102110
}
103111

104112
func (d *CloudDisk) GetSelfLink() string {
@@ -155,3 +163,19 @@ func (d *CloudDisk) GetSnapshotId() string {
155163
return ""
156164
}
157165
}
166+
167+
func (d *CloudDisk) GetKMSKeyName() string {
168+
var dek *computev1.CustomerEncryptionKey
169+
switch d.Type() {
170+
case Zonal:
171+
dek = d.ZonalDisk.DiskEncryptionKey
172+
case Regional:
173+
dek = d.RegionalDisk.DiskEncryptionKey
174+
default:
175+
return ""
176+
}
177+
if dek == nil {
178+
return ""
179+
}
180+
return dek.KmsKeyName
181+
}

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

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key) (
207207
return disk, nil
208208
}
209209

210-
func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, diskType string, reqBytes, limBytes int64) error {
210+
func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64) error {
211211
if resp == nil {
212212
return fmt.Errorf("disk does not exist")
213213
}
@@ -219,20 +219,12 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *
219219
reqBytes, common.GbToBytes(resp.GetSizeGb()), limBytes)
220220
}
221221

222-
respType := strings.Split(resp.GetType(), "/")
223-
typeMatch := strings.TrimSpace(respType[len(respType)-1]) == strings.TrimSpace(diskType)
224-
typeDefault := diskType == "" && strings.TrimSpace(respType[len(respType)-1]) == "pd-standard"
225-
if !typeMatch && !typeDefault {
226-
return fmt.Errorf("disk already exists with incompatible type. Need %v. Got %v",
227-
diskType, respType[len(respType)-1])
228-
}
229-
klog.V(4).Infof("Compatible disk already exists")
230-
return nil
222+
return ValidateDiskParameters(resp, params)
231223
}
232224

233-
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
225+
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
234226
if disk, ok := cloud.disks[volKey.Name]; ok {
235-
err := cloud.ValidateExistingDisk(ctx, disk, diskType,
227+
err := cloud.ValidateExistingDisk(ctx, disk, params,
236228
int64(capacityRange.GetRequiredBytes()),
237229
int64(capacityRange.GetLimitBytes()))
238230
if err != nil {
@@ -247,13 +239,13 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
247239
Name: volKey.Name,
248240
SizeGb: common.BytesToGb(capBytes),
249241
Description: "Disk created by GCE-PD CSI Driver",
250-
Type: cloud.GetDiskTypeURI(volKey, diskType),
242+
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
251243
SelfLink: fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
252244
SourceSnapshotId: snapshotID,
253245
}
254-
if diskEncryptionKmsKey != "" {
246+
if params.DiskEncryptionKMSKey != "" {
255247
diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
256-
KmsKeyName: diskEncryptionKmsKey,
248+
KmsKeyName: params.DiskEncryptionKMSKey,
257249
}
258250
}
259251
diskToCreate = ZonalCloudDisk(diskToCreateGA)
@@ -262,13 +254,13 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
262254
Name: volKey.Name,
263255
SizeGb: common.BytesToGb(capBytes),
264256
Description: "Regional disk created by GCE-PD CSI Driver",
265-
Type: cloud.GetDiskTypeURI(volKey, diskType),
257+
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
266258
SelfLink: fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
267259
SourceSnapshotId: snapshotID,
268260
}
269-
if diskEncryptionKmsKey != "" {
261+
if params.DiskEncryptionKMSKey != "" {
270262
diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
271-
KmsKeyName: diskEncryptionKmsKey,
263+
KmsKeyName: params.DiskEncryptionKMSKey,
272264
}
273265
}
274266
diskToCreate = RegionalCloudDisk(diskToCreateV1)

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

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ type GCECompute interface {
4141
// Disk Methods
4242
GetDisk(ctx context.Context, volumeKey *meta.Key) (*CloudDisk, error)
4343
RepairUnderspecifiedVolumeKey(ctx context.Context, volumeKey *meta.Key) (*meta.Key, error)
44-
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, diskType string, reqBytes, limBytes int64) error
45-
InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error
44+
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64) error
45+
InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error
4646
DeleteDisk(ctx context.Context, volumeKey *meta.Key) error
4747
AttachDisk(ctx context.Context, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error
4848
DetachDisk(ctx context.Context, deviceName string, instanceZone, instanceName string) error
@@ -212,8 +212,8 @@ func (cloud *CloudProvider) getRegionURI(region string) string {
212212
region)
213213
}
214214

215-
func (cloud *CloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, diskType string, reqBytes, limBytes int64) error {
216-
klog.V(5).Infof("Validating existing disk %v with diskType: %s, reqested bytes: %v, limit bytes: %v", resp, diskType, reqBytes, limBytes)
215+
func (cloud *CloudProvider) ValidateExistingDisk(ctx context.Context, resp *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64) error {
216+
klog.V(5).Infof("Validating existing disk %v with diskType: %s, reqested bytes: %v, limit bytes: %v", resp, params.DiskType, reqBytes, limBytes)
217217
if resp == nil {
218218
return fmt.Errorf("disk does not exist")
219219
}
@@ -225,44 +225,59 @@ func (cloud *CloudProvider) ValidateExistingDisk(ctx context.Context, resp *Clou
225225
reqBytes, common.GbToBytes(resp.GetSizeGb()), limBytes)
226226
}
227227

228-
respType := strings.Split(resp.GetType(), "/")
229-
typeMatch := strings.TrimSpace(respType[len(respType)-1]) == strings.TrimSpace(diskType)
230-
typeDefault := diskType == "" && strings.TrimSpace(respType[len(respType)-1]) == "pd-standard"
231-
if !typeMatch && !typeDefault {
232-
return fmt.Errorf("disk already exists with incompatible type. Need %v. Got %v",
233-
diskType, respType[len(respType)-1])
228+
return ValidateDiskParameters(resp, params)
229+
}
230+
231+
// ValidateDiskParameters takes a CloudDisk and returns true if the parameters
232+
// specified validly describe the disk provided, and false otherwise.
233+
func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error {
234+
if disk.GetPDType() != params.DiskType {
235+
return fmt.Errorf("actual pd type %s did not match the expected param %s", disk.GetPDType(), params.DiskType)
234236
}
237+
238+
if params.ReplicationType == "none" && disk.Type() != Zonal {
239+
return fmt.Errorf("actual disk replication type %v did not match expected param %s", disk.Type(), params.ReplicationType)
240+
}
241+
242+
if params.ReplicationType == "regional-pd" && disk.Type() != Regional {
243+
return fmt.Errorf("actual disk replication type %v did not match expected param %s", disk.Type(), "regional-pd")
244+
}
245+
246+
if disk.GetKMSKeyName() != params.DiskEncryptionKMSKey {
247+
return fmt.Errorf("actual disk KMS key name %s did not match expected param %s", disk.GetKMSKeyName(), params.DiskEncryptionKMSKey)
248+
}
249+
235250
return nil
236251
}
237252

238-
func (cloud *CloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
253+
func (cloud *CloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
239254
klog.V(5).Infof("Inserting disk %v", volKey)
240255
switch volKey.Type() {
241256
case meta.Zonal:
242-
return cloud.insertZonalDisk(ctx, volKey, diskType, capBytes, capacityRange, snapshotID, diskEncryptionKmsKey)
257+
return cloud.insertZonalDisk(ctx, volKey, params, capBytes, capacityRange, snapshotID)
243258
case meta.Regional:
244-
return cloud.insertRegionalDisk(ctx, volKey, diskType, capBytes, capacityRange, replicaZones, snapshotID, diskEncryptionKmsKey)
259+
return cloud.insertRegionalDisk(ctx, volKey, params, capBytes, capacityRange, replicaZones, snapshotID)
245260
default:
246261
return fmt.Errorf("could not insert disk, key was neither zonal nor regional, instead got: %v", volKey.String())
247262
}
248263
}
249264

250-
func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
265+
func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
251266
diskToCreate := &computev1.Disk{
252267
Name: volKey.Name,
253268
SizeGb: common.BytesToGb(capBytes),
254269
Description: "Regional disk created by GCE-PD CSI Driver",
255-
Type: cloud.GetDiskTypeURI(volKey, diskType),
270+
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
256271
}
257272
if snapshotID != "" {
258273
diskToCreate.SourceSnapshot = snapshotID
259274
}
260275
if len(replicaZones) != 0 {
261276
diskToCreate.ReplicaZones = replicaZones
262277
}
263-
if diskEncryptionKmsKey != "" {
278+
if params.DiskEncryptionKMSKey != "" {
264279
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
265-
KmsKeyName: diskEncryptionKmsKey,
280+
KmsKeyName: params.DiskEncryptionKMSKey,
266281
}
267282
}
268283

@@ -273,7 +288,7 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
273288
if err != nil {
274289
return err
275290
}
276-
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
291+
err = cloud.ValidateExistingDisk(ctx, disk, params,
277292
int64(capacityRange.GetRequiredBytes()),
278293
int64(capacityRange.GetLimitBytes()))
279294
if err != nil {
@@ -292,7 +307,7 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
292307
if err != nil {
293308
return err
294309
}
295-
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
310+
err = cloud.ValidateExistingDisk(ctx, disk, params,
296311
int64(capacityRange.GetRequiredBytes()),
297312
int64(capacityRange.GetLimitBytes()))
298313
if err != nil {
@@ -306,21 +321,21 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
306321
return nil
307322
}
308323

309-
func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, snapshotID, diskEncryptionKmsKey string) error {
324+
func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, snapshotID string) error {
310325
diskToCreate := &computev1.Disk{
311326
Name: volKey.Name,
312327
SizeGb: common.BytesToGb(capBytes),
313328
Description: "Disk created by GCE-PD CSI Driver",
314-
Type: cloud.GetDiskTypeURI(volKey, diskType),
329+
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
315330
}
316331

317332
if snapshotID != "" {
318333
diskToCreate.SourceSnapshot = snapshotID
319334
}
320335

321-
if diskEncryptionKmsKey != "" {
336+
if params.DiskEncryptionKMSKey != "" {
322337
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
323-
KmsKeyName: diskEncryptionKmsKey,
338+
KmsKeyName: params.DiskEncryptionKMSKey,
324339
}
325340
}
326341

@@ -332,7 +347,7 @@ func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Ke
332347
if err != nil {
333348
return err
334349
}
335-
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
350+
err = cloud.ValidateExistingDisk(ctx, disk, params,
336351
int64(capacityRange.GetRequiredBytes()),
337352
int64(capacityRange.GetLimitBytes()))
338353
if err != nil {
@@ -352,7 +367,7 @@ func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Ke
352367
if err != nil {
353368
return err
354369
}
355-
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
370+
err = cloud.ValidateExistingDisk(ctx, disk, params,
356371
int64(capacityRange.GetRequiredBytes()),
357372
int64(capacityRange.GetLimitBytes()))
358373
if err != nil {

0 commit comments

Comments
 (0)