Skip to content

Commit da333d9

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2198 from hajiler/gv-clean-up-branch
Parameters Cleanup
2 parents 157abf7 + b3f746e commit da333d9

File tree

5 files changed

+188
-162
lines changed

5 files changed

+188
-162
lines changed

‎pkg/parameters/constants.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package parameters
2+
3+
const (
4+
// Disk Params
5+
ParameterAccessMode = "access-mode"
6+
7+
// Parameters for StorageClass
8+
ParameterKeyType = "type"
9+
ParameterKeyReplicationType = "replication-type"
10+
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
11+
ParameterKeyLabels = "labels"
12+
ParameterKeyProvisionedIOPSOnCreate = "provisioned-iops-on-create"
13+
ParameterKeyProvisionedThroughputOnCreate = "provisioned-throughput-on-create"
14+
ParameterAvailabilityClass = "availability-class"
15+
ParameterKeyEnableConfidentialCompute = "enable-confidential-storage"
16+
ParameterKeyStoragePools = "storage-pools"
17+
ParameterKeyUseAllowedDiskTopology = "use-allowed-disk-topology"
18+
19+
// Parameters for Data Cache
20+
ParameterKeyDataCacheSize = "data-cache-size"
21+
ParameterKeyDataCacheMode = "data-cache-mode"
22+
ParameterKeyResourceTags = "resource-tags"
23+
ParameterKeyEnableMultiZoneProvisioning = "enable-multi-zone-provisioning"
24+
25+
// Parameters for VolumeSnapshotClass
26+
ParameterKeyStorageLocations = "storage-locations"
27+
ParameterKeySnapshotType = "snapshot-type"
28+
ParameterKeyImageFamily = "image-family"
29+
replicationTypeNone = "none"
30+
31+
// Keys for PV and PVC parameters as reported by external-provisioner
32+
ParameterKeyPVCName = "csi.storage.k8s.io/pvc/name"
33+
ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
34+
ParameterKeyPVName = "csi.storage.k8s.io/pv/name"
35+
36+
// Keys for tags to put in the provisioned disk description
37+
tagKeyCreatedForClaimNamespace = "kubernetes.io/created-for/pvc/namespace"
38+
tagKeyCreatedForClaimName = "kubernetes.io/created-for/pvc/name"
39+
tagKeyCreatedForVolumeName = "kubernetes.io/created-for/pv/name"
40+
tagKeyCreatedBy = "storage.gke.io/created-by"
41+
42+
// Keys for Snapshot and SnapshotContent parameters as reported by external-snapshotter
43+
ParameterKeyVolumeSnapshotName = "csi.storage.k8s.io/volumesnapshot/name"
44+
ParameterKeyVolumeSnapshotNamespace = "csi.storage.k8s.io/volumesnapshot/namespace"
45+
ParameterKeyVolumeSnapshotContentName = "csi.storage.k8s.io/volumesnapshotcontent/name"
46+
47+
// Parameters for AvailabilityClass
48+
ParameterNoAvailabilityClass = "none"
49+
ParameterRegionalHardFailoverClass = "regional-hard-failover"
50+
51+
// Keys for tags to put in the provisioned snapshot description
52+
tagKeyCreatedForSnapshotName = "kubernetes.io/created-for/volumesnapshot/name"
53+
tagKeyCreatedForSnapshotNamespace = "kubernetes.io/created-for/volumesnapshot/namespace"
54+
tagKeyCreatedForSnapshotContentName = "kubernetes.io/created-for/volumesnapshotcontent/name"
55+
56+
// Hyperdisk disk types
57+
DiskTypeHdHA = "hyperdisk-balanced-high-availability"
58+
DiskTypeHdT = "hyperdisk-throughput"
59+
DiskTypeHdE = "hyperdisk-extreme"
60+
DiskTypeHdML = "hyperdisk-ml"
61+
62+
// Parameters for VolumeSnapshotClass
63+
DiskSnapshotType = "snapshots"
64+
DiskImageType = "images"
65+
)

‎pkg/parameters/parameters.go‎

Lines changed: 5 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -27,164 +27,11 @@ import (
2727
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/convert"
2828
)
2929

30-
const (
31-
// Disk Params
32-
ParameterAccessMode = "access-mode"
33-
34-
// Parameters for StorageClass
35-
ParameterKeyType = "type"
36-
ParameterKeyReplicationType = "replication-type"
37-
ParameterKeyDiskEncryptionKmsKey = "disk-encryption-kms-key"
38-
ParameterKeyLabels = "labels"
39-
ParameterKeyProvisionedIOPSOnCreate = "provisioned-iops-on-create"
40-
ParameterKeyProvisionedThroughputOnCreate = "provisioned-throughput-on-create"
41-
ParameterAvailabilityClass = "availability-class"
42-
ParameterKeyEnableConfidentialCompute = "enable-confidential-storage"
43-
ParameterKeyStoragePools = "storage-pools"
44-
ParameterKeyUseAllowedDiskTopology = "use-allowed-disk-topology"
45-
46-
// Parameters for Data Cache
47-
ParameterKeyDataCacheSize = "data-cache-size"
48-
ParameterKeyDataCacheMode = "data-cache-mode"
49-
ParameterKeyResourceTags = "resource-tags"
50-
ParameterKeyEnableMultiZoneProvisioning = "enable-multi-zone-provisioning"
51-
52-
// Parameters for VolumeSnapshotClass
53-
ParameterKeyStorageLocations = "storage-locations"
54-
ParameterKeySnapshotType = "snapshot-type"
55-
ParameterKeyImageFamily = "image-family"
56-
replicationTypeNone = "none"
57-
58-
// Keys for PV and PVC parameters as reported by external-provisioner
59-
ParameterKeyPVCName = "csi.storage.k8s.io/pvc/name"
60-
ParameterKeyPVCNamespace = "csi.storage.k8s.io/pvc/namespace"
61-
ParameterKeyPVName = "csi.storage.k8s.io/pv/name"
62-
63-
// Keys for tags to put in the provisioned disk description
64-
tagKeyCreatedForClaimNamespace = "kubernetes.io/created-for/pvc/namespace"
65-
tagKeyCreatedForClaimName = "kubernetes.io/created-for/pvc/name"
66-
tagKeyCreatedForVolumeName = "kubernetes.io/created-for/pv/name"
67-
tagKeyCreatedBy = "storage.gke.io/created-by"
68-
69-
// Keys for Snapshot and SnapshotContent parameters as reported by external-snapshotter
70-
ParameterKeyVolumeSnapshotName = "csi.storage.k8s.io/volumesnapshot/name"
71-
ParameterKeyVolumeSnapshotNamespace = "csi.storage.k8s.io/volumesnapshot/namespace"
72-
ParameterKeyVolumeSnapshotContentName = "csi.storage.k8s.io/volumesnapshotcontent/name"
73-
74-
// Parameters for AvailabilityClass
75-
ParameterNoAvailabilityClass = "none"
76-
ParameterRegionalHardFailoverClass = "regional-hard-failover"
77-
78-
// Keys for tags to put in the provisioned snapshot description
79-
tagKeyCreatedForSnapshotName = "kubernetes.io/created-for/volumesnapshot/name"
80-
tagKeyCreatedForSnapshotNamespace = "kubernetes.io/created-for/volumesnapshot/namespace"
81-
tagKeyCreatedForSnapshotContentName = "kubernetes.io/created-for/volumesnapshotcontent/name"
82-
83-
// Hyperdisk disk types
84-
DiskTypeHdHA = "hyperdisk-balanced-high-availability"
85-
DiskTypeHdT = "hyperdisk-throughput"
86-
DiskTypeHdE = "hyperdisk-extreme"
87-
DiskTypeHdML = "hyperdisk-ml"
88-
89-
// Parameters for VolumeSnapshotClass
90-
DiskSnapshotType = "snapshots"
91-
DiskImageType = "images"
92-
)
93-
94-
type StoragePool struct {
95-
Project string
96-
Zone string
97-
Name string
98-
ResourceName string
99-
}
100-
101-
type DataCacheParameters struct {
102-
// Values: {string} in int64 form
103-
// Default: ""
104-
DataCacheSize string
105-
// Values: writethrough, writeback
106-
// Default: writethrough
107-
DataCacheMode string
108-
}
109-
110-
// DiskParameters contains normalized and defaulted disk parameters
111-
type DiskParameters struct {
112-
// Values: pd-standard, pd-balanced, pd-ssd, or any other PD disk type. Not validated.
113-
// Default: pd-standard
114-
DiskType string
115-
// Values: "none", regional-pd
116-
// Default: "none"
117-
ReplicationType string
118-
// Values: {string}
119-
// Default: ""
120-
DiskEncryptionKMSKey string
121-
// Values: {map[string]string}
122-
// Default: ""
123-
Tags map[string]string
124-
// Values: {map[string]string}
125-
// Default: ""
126-
Labels map[string]string
127-
// Values: {int64}
128-
// Default: none
129-
ProvisionedIOPSOnCreate int64
130-
// Values: {int64}
131-
// Default: none
132-
ProvisionedThroughputOnCreate int64
133-
// Values: {bool}
134-
// Default: false
135-
EnableConfidentialCompute bool
136-
// Default: false
137-
ForceAttach bool
138-
// Values: {[]string}
139-
// Default: ""
140-
StoragePools []StoragePool
141-
// Values: {map[string]string}
142-
// Default: ""
143-
ResourceTags map[string]string
144-
// Values: {bool}
145-
// Default: false
146-
MultiZoneProvisioning bool
147-
// Values: READ_WRITE_SINGLE, READ_ONLY_MANY, READ_WRITE_MANY
148-
// Default: READ_WRITE_SINGLE
149-
AccessMode string
150-
// Values {}
151-
// Default: false
152-
UseAllowedDiskTopology bool
153-
}
154-
155-
func (dp *DiskParameters) IsRegional() bool {
156-
return dp.ReplicationType == "regional-pd" || dp.DiskType == DiskTypeHdHA
157-
}
158-
159-
// SnapshotParameters contains normalized and defaulted parameters for snapshots
160-
type SnapshotParameters struct {
161-
StorageLocations []string
162-
SnapshotType string
163-
ImageFamily string
164-
Tags map[string]string
165-
Labels map[string]string
166-
ResourceTags map[string]string
167-
}
168-
169-
type ParameterProcessor struct {
170-
DriverName string
171-
EnableStoragePools bool
172-
EnableMultiZone bool
173-
EnableHdHA bool
174-
EnableDiskTopology bool
175-
}
176-
177-
type ModifyVolumeParameters struct {
178-
IOPS *int64
179-
Throughput *int64
180-
}
181-
18230
// ExtractAndDefaultParameters will take the relevant parameters from a map and
18331
// put them into a well defined struct making sure to default unspecified fields.
18432
// extraVolumeLabels are added as labels; if there are also labels specified in
18533
// parameters, any matching extraVolumeLabels will be overridden.
18634
func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]string, extraVolumeLabels map[string]string, enableDataCache bool, extraTags map[string]string) (DiskParameters, DataCacheParameters, error) {
187-
18835
p := DiskParameters{
18936
DiskType: "pd-standard", // Default
19037
ReplicationType: replicationTypeNone, // Default
@@ -275,7 +122,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
275122
if paramEnableConfidentialCompute {
276123
// DiskEncryptionKmsKey is needed to enable confidentialStorage
277124
if val, ok := parameters[ParameterKeyDiskEncryptionKmsKey]; !ok || !isValidDiskEncryptionKmsKey(val) {
278-
return p, d, fmt.Errorf("Valid %v is required to enable ConfidentialStorage", ParameterKeyDiskEncryptionKmsKey)
125+
return p, d, fmt.Errorf("valid %v is required to enable ConfidentialStorage", ParameterKeyDiskEncryptionKmsKey)
279126
}
280127
}
281128

@@ -304,7 +151,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
304151
d.DataCacheSize = strconv.FormatInt(paramDataCacheSize, 10)
305152
case ParameterKeyDataCacheMode:
306153
if !enableDataCache {
307-
return p, d, fmt.Errorf("data caching enabled %v; parameters contains invalid option %q", enableDataCache, ParameterKeyDataCacheSize)
154+
return p, d, fmt.Errorf("data caching enabled %v; parameters contains invalid option %q", enableDataCache, ParameterKeyDataCacheMode)
308155
}
309156
if err := ValidateDataCacheMode(v); err != nil {
310157
return p, d, fmt.Errorf("parameters contains invalid option: %s: %w", ParameterKeyDataCacheMode, err)
@@ -425,7 +272,6 @@ func extractResourceTagsParameter(tagsString string, resourceTags map[string]str
425272
}
426273

427274
func ExtractModifyVolumeParameters(parameters map[string]string) (ModifyVolumeParameters, error) {
428-
429275
modifyVolumeParams := ModifyVolumeParameters{}
430276

431277
for key, value := range parameters {
@@ -446,6 +292,7 @@ func ExtractModifyVolumeParameters(parameters map[string]string) (ModifyVolumePa
446292
return ModifyVolumeParameters{}, fmt.Errorf("parameters contain unknown parameter: %s", key)
447293
}
448294
}
295+
449296
return modifyVolumeParams, nil
450297
}
451298

@@ -457,13 +304,13 @@ func convertStringToAvailabilityClass(str string) (string, error) {
457304
case ParameterRegionalHardFailoverClass:
458305
return ParameterRegionalHardFailoverClass, nil
459306
}
460-
return "", fmt.Errorf("Unexpected boolean string %s", str)
307+
return "", fmt.Errorf("unexpected boolean string %s", str)
461308
}
462309

463310
// StoragePoolZones returns the unique zones of the given storage pool resource names.
464311
// Returns an error if multiple storage pools in 1 zone are found.
465312
func StoragePoolZones(storagePools []StoragePool) ([]string, error) {
466-
zonesSet := sets.String{}
313+
zonesSet := sets.New[string]()
467314
var zones []string
468315
for _, sp := range storagePools {
469316
if zonesSet.Has(sp.Zone) {

‎pkg/parameters/parameters_test.go‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"testing"
2222

2323
"github.com/google/go-cmp/cmp"
24+
"github.com/google/go-cmp/cmp/cmpopts"
2425
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
2526
)
2627

@@ -281,6 +282,24 @@ func TestExtractAndDefaultParameters(t *testing.T) {
281282
ResourceTags: map[string]string{},
282283
},
283284
},
285+
{
286+
name: "confidential compute enabled with valid KMS key",
287+
parameters: map[string]string{ParameterKeyEnableConfidentialCompute: "true", ParameterKeyDiskEncryptionKmsKey: validKmsKeyRegional},
288+
expectParams: DiskParameters{
289+
DiskType: "pd-standard",
290+
ReplicationType: "none",
291+
DiskEncryptionKMSKey: validKmsKeyRegional,
292+
EnableConfidentialCompute: true,
293+
Tags: map[string]string{},
294+
Labels: map[string]string{},
295+
ResourceTags: map[string]string{},
296+
},
297+
},
298+
{
299+
name: "confidential compute enabled with invalid KMS key",
300+
parameters: map[string]string{ParameterKeyEnableConfidentialCompute: "true", ParameterKeyDiskEncryptionKmsKey: invalidKmsKey},
301+
expectErr: true,
302+
},
284303
{
285304
name: "storage pool parameters",
286305
enableStoragePools: true,
@@ -533,7 +552,7 @@ func TestExtractAndDefaultParameters(t *testing.T) {
533552
return
534553
}
535554

536-
if diff := cmp.Diff(tc.expectParams, p); diff != "" {
555+
if diff := cmp.Diff(tc.expectParams, p, cmpopts.IgnoreUnexported(DiskParameters{})); diff != "" {
537556
t.Errorf("ExtractAndDefaultParameters(%+v): -want, +got \n%s", tc.parameters, diff)
538557
}
539558

0 commit comments

Comments
 (0)