Skip to content

Commit f225aee

Browse files
committed
Fix Random CMEK Disk Creation Failure (disk already exists)
1 parent 03f0690 commit f225aee

File tree

2 files changed

+103
-1
lines changed

2 files changed

+103
-1
lines changed

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,9 @@ func ValidateDiskParameters(disk *CloudDisk, params common.DiskParameters) error
256256
return fmt.Errorf("actual disk replication type %v did not match expected param %s", disk.Type(), "regional-pd")
257257
}
258258

259-
if disk.GetKMSKeyName() != params.DiskEncryptionKMSKey {
259+
if !kmsKeyEqual(
260+
disk.GetKMSKeyName(), /* fetchedKMSKey */
261+
params.DiskEncryptionKMSKey /* storageClassKMSKey */) {
260262
return fmt.Errorf("actual disk KMS key name %s did not match expected param %s", disk.GetKMSKeyName(), params.DiskEncryptionKMSKey)
261263
}
262264

@@ -767,3 +769,13 @@ func (cloud *CloudProvider) waitForSnapshotCreation(ctx context.Context, snapsho
767769
}
768770
}
769771
}
772+
773+
// kmsKeyEqual returns true if fetchedKMSKey and storageClassKMSKey refer to the same key.
774+
// fetchedKMSKey - key returned by the server
775+
// example: projects/{0}/locations/{1}/keyRings/{2}/cryptoKeys/{3}/cryptoKeyVersions/{4}
776+
// storageClassKMSKey - key as provided by the client
777+
// example: projects/{0}/locations/{1}/keyRings/{2}/cryptoKeys/{3}
778+
// cryptoKeyVersions should be disregarded if the rest of the key is identical.
779+
func kmsKeyEqual(fetchedKMSKey, storageClassKMSKey string) bool {
780+
return strings.Index(fetchedKMSKey, storageClassKMSKey) == 0
781+
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
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+
http://www.apache.org/licenses/LICENSE-2.0
8+
Unless required by applicable law or agreed to in writing, software
9+
distributed under the License is distributed on an "AS IS" BASIS,
10+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
See the License for the specific language governing permissions and
12+
limitations under the License.
13+
*/
14+
15+
package gcecloudprovider
16+
17+
import (
18+
"testing"
19+
20+
computev1 "google.golang.org/api/compute/v1"
21+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
22+
)
23+
24+
func TestValidateDiskParameters(t *testing.T) {
25+
testCases := []struct {
26+
fetchedKMSKey string
27+
storageClassKMSKey string
28+
expectErr bool
29+
}{
30+
{
31+
fetchedKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key/cryptoKeyVersions/8",
32+
storageClassKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key",
33+
expectErr: false,
34+
},
35+
{
36+
fetchedKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key",
37+
storageClassKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key",
38+
expectErr: false,
39+
},
40+
{
41+
fetchedKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/garbage/cryptoKeyVersions/8",
42+
storageClassKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key",
43+
expectErr: true,
44+
},
45+
{
46+
fetchedKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/garbage",
47+
storageClassKMSKey: "projects/my-project/locations/us-central1/keyRings/TestKeyRing/cryptoKeys/test-key",
48+
expectErr: true,
49+
},
50+
}
51+
52+
for _, tc := range testCases {
53+
// Arrange
54+
existingDisk := &CloudDisk{
55+
ZonalDisk: &computev1.Disk{
56+
Id: 546559531467326555,
57+
CreationTimestamp: "2020-07-24T17:20:06.292-07:00",
58+
Name: "test-disk",
59+
SizeGb: 500,
60+
Zone: "https://www.googleapis.com/compute/v1/projects/my-project/zones/us-central1-c",
61+
Status: "READY",
62+
SelfLink: "https://www.googleapis.com/compute/v1/projects/my-project/zones/us-central1-c/disks/test-disk",
63+
Type: "https://www.googleapis.com/compute/v1/projects/my-project/zones/us-central1-c/diskTypes/pd-standard",
64+
DiskEncryptionKey: &computev1.CustomerEncryptionKey{
65+
KmsKeyName: tc.fetchedKMSKey,
66+
},
67+
LabelFingerprint: "42WmSpB8rSM=",
68+
PhysicalBlockSizeBytes: 4096,
69+
Kind: "compute#disk",
70+
},
71+
}
72+
73+
storageClassParams := common.DiskParameters{
74+
DiskType: "pd-standard",
75+
ReplicationType: "none",
76+
DiskEncryptionKMSKey: tc.storageClassKMSKey,
77+
}
78+
79+
// Act
80+
err := ValidateDiskParameters(existingDisk, storageClassParams)
81+
82+
// Assert
83+
if !tc.expectErr && err != nil {
84+
t.Fatalf("ValidateDiskParameters did not expect error, but got %v", err)
85+
}
86+
if tc.expectErr && err == nil {
87+
t.Fatalf("ValidateDiskParameters expected error, but got no error")
88+
}
89+
}
90+
}

0 commit comments

Comments
 (0)