Skip to content

Commit

Permalink
Implement ValidateVolumeCapabilities and refactor parameter handling …
Browse files Browse the repository at this point in the history 
…for more comprehensive validation of existing disks in all cloud calls
  • Loading branch information
@davidz627
davidz627 committed Feb 5, 2020
1 parent afd7d61 commit 774d938
Show file tree
Hide file tree
Showing 6 changed files with 205 additions and 149 deletions.
5 changes: 0 additions & 5 deletions pkg/common/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ limitations under the License.
package common

const (
// Keys for Storage Class Parameters
ParameterKeyType = "type"
ParameterKeyReplicationType = "replication-type"
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"

// Keys for Topology. This key will be shared amongst drivers from GCP
TopologyKeyZone = "topology.gke.io/zone"

Expand Down
75 changes: 75 additions & 0 deletions pkg/common/parameters.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package common

import (
"fmt"
"strings"
)

const (
ParameterKeyType = "type"
ParameterKeyReplicationType = "replication-type"
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"

replicationTypeNone = "none"
)

// DiskParameters contains normalized and defaulted disk parameters
type DiskParameters struct {
// Values: pd-standard OR pd-ssd
// Default: pd-standard
DiskType string
// Values: "none", regional-pd
// Default: "none"
ReplicationType string
// Values: {string}
// Default: ""
DiskEncryptionKMSKey string
}

// ExtractAndDefaultParameters will take the relevant parameters from a map and
// put them into a well defined struct making sure to default unspecified fields
func ExtractAndDefaultParameters(parameters map[string]string) (DiskParameters, error) {
p := DiskParameters{
DiskType: "pd-standard", // Default
ReplicationType: replicationTypeNone, // Default
DiskEncryptionKMSKey: "", // Default
}
for k, v := range parameters {
if k == "csiProvisionerSecretName" || k == "csiProvisionerSecretNamespace" {
// These are hardcoded secrets keys required to function but not needed by GCE PD
continue
}
switch strings.ToLower(k) {
case ParameterKeyType:
if v != "" {
p.DiskType = v
}
case ParameterKeyReplicationType:
if v != "" {
p.ReplicationType = strings.ToLower(v)
}
case ParameterKeyDiskEncryptionKmsKey:
// Resource names (e.g. "keyRings", "cryptoKeys", etc.) are case sensitive, so do not change case
p.DiskEncryptionKMSKey = v
default:
return p, fmt.Errorf("parameters contains invalid option %q", k)
}
}
return p, nil
}
30 changes: 27 additions & 3 deletions pkg/gce-cloud-provider/compute/cloud-disk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ limitations under the License.
package gcecloudprovider

import (
"strings"

computev1 "google.golang.org/api/compute/v1"
)

Expand Down Expand Up @@ -90,15 +92,21 @@ func (d *CloudDisk) GetKind() string {
}
}

func (d *CloudDisk) GetType() string {
// GetPDType returns the type of the PD as either 'pd-standard' or 'pd-ssd' The
// "Type" field on the compute disk is stored as a url like
// projects/project/zones/zone/diskTypes/pd-standard
func (d *CloudDisk) GetPDType() string {
var pdType string
switch d.Type() {
case Zonal:
return d.ZonalDisk.Type
pdType = d.ZonalDisk.Type
case Regional:
return d.RegionalDisk.Type
pdType = d.RegionalDisk.Type
default:
return ""
}
respType := strings.Split(pdType, "/")
return strings.TrimSpace(respType[len(respType)-1])
}

func (d *CloudDisk) GetSelfLink() string {
Expand Down Expand Up @@ -155,3 +163,19 @@ func (d *CloudDisk) GetSnapshotId() string {
return ""
}
}

func (d *CloudDisk) GetKMSKeyName() string {
var dek *computev1.CustomerEncryptionKey
switch d.Type() {
case Zonal:
dek = d.ZonalDisk.DiskEncryptionKey
case Regional:
dek = d.RegionalDisk.DiskEncryptionKey
default:
return ""
}
if dek == nil {
return ""
}
return dek.KmsKeyName
}
28 changes: 10 additions & 18 deletions pkg/gce-cloud-provider/compute/fake-gce.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key) (
return disk, nil
}

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

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

func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
if disk, ok := cloud.disks[volKey.Name]; ok {
err := cloud.ValidateExistingDisk(ctx, disk, diskType,
err := cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
int64(capacityRange.GetLimitBytes()))
if err != nil {
Expand All @@ -247,13 +239,13 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
Name: volKey.Name,
SizeGb: common.BytesToGb(capBytes),
Description: "Disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(volKey, diskType),
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
SelfLink: fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name),
SourceSnapshotId: snapshotID,
}
if diskEncryptionKmsKey != "" {
if params.DiskEncryptionKMSKey != "" {
diskToCreateGA.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
KmsKeyName: diskEncryptionKmsKey,
KmsKeyName: params.DiskEncryptionKMSKey,
}
}
diskToCreate = ZonalCloudDisk(diskToCreateGA)
Expand All @@ -262,13 +254,13 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
Name: volKey.Name,
SizeGb: common.BytesToGb(capBytes),
Description: "Regional disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(volKey, diskType),
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
SelfLink: fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name),
SourceSnapshotId: snapshotID,
}
if diskEncryptionKmsKey != "" {
if params.DiskEncryptionKMSKey != "" {
diskToCreateV1.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
KmsKeyName: diskEncryptionKmsKey,
KmsKeyName: params.DiskEncryptionKMSKey,
}
}
diskToCreate = RegionalCloudDisk(diskToCreateV1)
Expand Down
65 changes: 40 additions & 25 deletions pkg/gce-cloud-provider/compute/gce-compute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ type GCECompute interface {
// Disk Methods
GetDisk(ctx context.Context, volumeKey *meta.Key) (*CloudDisk, error)
RepairUnderspecifiedVolumeKey(ctx context.Context, volumeKey *meta.Key) (*meta.Key, error)
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, diskType string, reqBytes, limBytes int64) error
InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error
ValidateExistingDisk(ctx context.Context, disk *CloudDisk, params common.DiskParameters, reqBytes, limBytes int64) error
InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error
DeleteDisk(ctx context.Context, volumeKey *meta.Key) error
AttachDisk(ctx context.Context, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error
DetachDisk(ctx context.Context, deviceName string, instanceZone, instanceName string) error
Expand Down Expand Up @@ -212,8 +212,8 @@ func (cloud *CloudProvider) getRegionURI(region string) string {
region)
}

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

respType := strings.Split(resp.GetType(), "/")
typeMatch := strings.TrimSpace(respType[len(respType)-1]) == strings.TrimSpace(diskType)
typeDefault := diskType == "" && strings.TrimSpace(respType[len(respType)-1]) == "pd-standard"
if !typeMatch && !typeDefault {
return fmt.Errorf("disk already exists with incompatible type. Need %v. Got %v",
diskType, respType[len(respType)-1])
return ValidateDiskParameters(resp, params)
}

// ValidateDiskParameters takes a CloudDisk and returns true if the parameters
// specified validly describe the disk provided, and false otherwise.
func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error {
if disk.GetPDType() != params.DiskType {
return fmt.Errorf("actual pd type %s did not match the expected param %s", disk.GetPDType(), params.DiskType)
}

if params.ReplicationType == "none" && disk.Type() != Zonal {
return fmt.Errorf("actual disk replication type %v did not match expected param %s", disk.Type(), params.ReplicationType)
}

if params.ReplicationType == "regional-pd" && disk.Type() != Regional {
return fmt.Errorf("actual disk replication type %v did not match expected param %s", disk.Type(), "regional-pd")
}

if disk.GetKMSKeyName() != params.DiskEncryptionKMSKey {
return fmt.Errorf("actual disk KMS key name %s did not match expected param %s", disk.GetKMSKeyName(), params.DiskEncryptionKMSKey)
}

return nil
}

func (cloud *CloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
func (cloud *CloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
klog.V(5).Infof("Inserting disk %v", volKey)
switch volKey.Type() {
case meta.Zonal:
return cloud.insertZonalDisk(ctx, volKey, diskType, capBytes, capacityRange, snapshotID, diskEncryptionKmsKey)
return cloud.insertZonalDisk(ctx, volKey, params, capBytes, capacityRange, snapshotID)
case meta.Regional:
return cloud.insertRegionalDisk(ctx, volKey, diskType, capBytes, capacityRange, replicaZones, snapshotID, diskEncryptionKmsKey)
return cloud.insertRegionalDisk(ctx, volKey, params, capBytes, capacityRange, replicaZones, snapshotID)
default:
return fmt.Errorf("could not insert disk, key was neither zonal nor regional, instead got: %v", volKey.String())
}
}

func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta.Key, diskType string, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID, diskEncryptionKmsKey string) error {
func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string) error {
diskToCreate := &computev1.Disk{
Name: volKey.Name,
SizeGb: common.BytesToGb(capBytes),
Description: "Regional disk created by GCE-PD CSI Driver",
Type: cloud.GetDiskTypeURI(volKey, diskType),
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
}
if snapshotID != "" {
diskToCreate.SourceSnapshot = snapshotID
}
if len(replicaZones) != 0 {
diskToCreate.ReplicaZones = replicaZones
}
if diskEncryptionKmsKey != "" {
if params.DiskEncryptionKMSKey != "" {
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
KmsKeyName: diskEncryptionKmsKey,
KmsKeyName: params.DiskEncryptionKMSKey,
}
}

Expand All @@ -273,7 +288,7 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
if err != nil {
return err
}
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
err = cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
int64(capacityRange.GetLimitBytes()))
if err != nil {
Expand All @@ -292,7 +307,7 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
if err != nil {
return err
}
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
err = cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
int64(capacityRange.GetLimitBytes()))
if err != nil {
Expand All @@ -306,21 +321,21 @@ func (cloud *CloudProvider) insertRegionalDisk(ctx context.Context, volKey *meta
return nil
}

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

if snapshotID != "" {
diskToCreate.SourceSnapshot = snapshotID
}

if diskEncryptionKmsKey != "" {
if params.DiskEncryptionKMSKey != "" {
diskToCreate.DiskEncryptionKey = &computev1.CustomerEncryptionKey{
KmsKeyName: diskEncryptionKmsKey,
KmsKeyName: params.DiskEncryptionKMSKey,
}
}

Expand All @@ -332,7 +347,7 @@ func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Ke
if err != nil {
return err
}
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
err = cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
int64(capacityRange.GetLimitBytes()))
if err != nil {
Expand All @@ -352,7 +367,7 @@ func (cloud *CloudProvider) insertZonalDisk(ctx context.Context, volKey *meta.Ke
if err != nil {
return err
}
err = cloud.ValidateExistingDisk(ctx, disk, diskType,
err = cloud.ValidateExistingDisk(ctx, disk, params,
int64(capacityRange.GetRequiredBytes()),
int64(capacityRange.GetLimitBytes()))
if err != nil {
Expand Down
Loading

0 comments on commit 774d938

Please sign in to comment.