Skip to content

Commit 1364a18

Browse files
christian-roggiachristian-roggia
committed
feat: fixed silent override of project
1 parent c73c98a commit 1364a18

File tree

10 files changed

+216
-207
lines changed

10 files changed

+216
-207
lines changed

‎pkg/common/utils.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const (
4444
// Node ID Expected Format
4545
// "projects/{projectName}/zones/{zoneName}/disks/{diskName}"
4646
nodeIDFmt = "projects/%s/zones/%s/instances/%s"
47+
nodeIDProjectValue = 1
4748
nodeIDZoneValue = 3
4849
nodeIDNameValue = 5
4950
nodeIDTotalElements = 6
@@ -69,17 +70,17 @@ func GbToBytes(Gb int64) int64 {
6970
return Gb * 1024 * 1024 * 1024
7071
}
7172

72-
func VolumeIDToKey(id string) (*meta.Key, error) {
73+
func VolumeIDToKey(id string) (string, *meta.Key, error) {
7374
splitId := strings.Split(id, "/")
7475
if len(splitId) != volIDTotalElements {
75-
return nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
76+
return "", nil, fmt.Errorf("failed to get id components. Expected projects/{project}/zones/{zone}/disks/{name}. Got: %s", id)
7677
}
7778
if splitId[volIDToplogyKey] == "zones" {
78-
return meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
79+
return splitId[nodeIDProjectValue], meta.ZonalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
7980
} else if splitId[volIDToplogyKey] == "regions" {
80-
return meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
81+
return splitId[nodeIDProjectValue], meta.RegionalKey(splitId[volIDDiskNameValue], splitId[volIDToplogyValue]), nil
8182
} else {
82-
return nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
83+
return "", nil, fmt.Errorf("could not get id components, expected either zones or regions, got: %v", splitId[volIDToplogyKey])
8384
}
8485
}
8586

‎pkg/common/utils_test.go‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,33 @@ func TestVolumeIDToKey(t *testing.T) {
146146
testName := "test-name"
147147
testZone := "test-zone"
148148
testProject := "test-project"
149+
testCrossProject := "test-cross-project"
149150
testRegion := "test-region"
150151

151152
testCases := []struct {
152-
name string
153-
volID string
154-
expKey *meta.Key
155-
expErr bool
153+
name string
154+
volID string
155+
expProject string
156+
expKey *meta.Key
157+
expErr bool
156158
}{
157159
{
158-
name: "normal zonal",
159-
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
160-
expKey: meta.ZonalKey(testName, testZone),
160+
name: "normal zonal",
161+
volID: fmt.Sprintf(volIDZoneFmt, testProject, testZone, testName),
162+
expKey: meta.ZonalKey(testName, testZone),
163+
expProject: testProject,
164+
},
165+
{
166+
name: "cross project",
167+
volID: fmt.Sprintf(volIDZoneFmt, testCrossProject, testZone, testName),
168+
expKey: meta.ZonalKey(testName, testZone),
169+
expProject: testCrossProject,
161170
},
162171
{
163-
name: "normal regional",
164-
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
165-
expKey: meta.RegionalKey(testName, testRegion),
172+
name: "normal regional",
173+
volID: fmt.Sprintf(volIDRegionFmt, testProject, testRegion, testName),
174+
expKey: meta.RegionalKey(testName, testRegion),
175+
expProject: testProject,
166176
},
167177
{
168178
name: "malformed",
@@ -177,7 +187,7 @@ func TestVolumeIDToKey(t *testing.T) {
177187
}
178188
for _, tc := range testCases {
179189
t.Logf("test case: %s", tc.name)
180-
gotKey, err := VolumeIDToKey(tc.volID)
190+
project, gotKey, err := VolumeIDToKey(tc.volID)
181191
if err == nil && tc.expErr {
182192
t.Errorf("Expected error but got none")
183193
}
@@ -191,6 +201,10 @@ func TestVolumeIDToKey(t *testing.T) {
191201
if !reflect.DeepEqual(gotKey, tc.expKey) {
192202
t.Errorf("Got key %v, but expected %v, from volume ID %v", gotKey, tc.expKey, tc.volID)
193203
}
204+
205+
if project != tc.expProject {
206+
t.Errorf("Got project %v, but expected %v, from volume ID %v", project, tc.expProject, tc.volID)
207+
}
194208
}
195209

196210
}

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

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (cloud *FakeCloudProvider) GetDefaultZone() string {
8080
return cloud.zone
8181
}
8282

83-
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, volumeKey *meta.Key) (*meta.Key, error) {
83+
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (*meta.Key, error) {
8484
switch volumeKey.Type() {
8585
case meta.Zonal:
8686
if volumeKey.Zone != common.UnspecifiedValue {
@@ -212,7 +212,7 @@ func (cloud *FakeCloudProvider) ListSnapshots(ctx context.Context, filter string
212212
}
213213

214214
// Disk Methods
215-
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
215+
func (cloud *FakeCloudProvider) GetDisk(ctx context.Context, project string, volKey *meta.Key, api GCEAPIVersion) (*CloudDisk, error) {
216216
disk, ok := cloud.disks[volKey.Name]
217217
if !ok {
218218
return nil, notFoundError()
@@ -249,7 +249,7 @@ func (cloud *FakeCloudProvider) ValidateExistingDisk(ctx context.Context, resp *
249249
return ValidateDiskParameters(resp, params)
250250
}
251251

252-
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, multiWriter bool) error {
252+
func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, project string, volKey *meta.Key, params common.DiskParameters, capBytes int64, capacityRange *csi.CapacityRange, replicaZones []string, snapshotID string, multiWriter bool) error {
253253
if disk, ok := cloud.disks[volKey.Name]; ok {
254254
err := cloud.ValidateExistingDisk(ctx, disk, params,
255255
int64(capacityRange.GetRequiredBytes()),
@@ -264,7 +264,7 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
264264
Name: volKey.Name,
265265
SizeGb: common.BytesToGbRoundUp(capBytes),
266266
Description: "Disk created by GCE-PD CSI Driver",
267-
Type: cloud.GetDiskTypeURI(volKey, params.DiskType),
267+
Type: cloud.GetDiskTypeURI(project, volKey, params.DiskType),
268268
SourceSnapshotId: snapshotID,
269269
Status: cloud.mockDiskStatus,
270270
Labels: params.Labels,
@@ -277,10 +277,10 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
277277
switch volKey.Type() {
278278
case meta.Zonal:
279279
computeDisk.Zone = volKey.Zone
280-
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", cloud.project, volKey.Zone, volKey.Name)
280+
computeDisk.SelfLink = fmt.Sprintf("projects/%s/zones/%s/disks/%s", project, volKey.Zone, volKey.Name)
281281
case meta.Regional:
282282
computeDisk.Region = volKey.Region
283-
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", cloud.project, volKey.Region, volKey.Name)
283+
computeDisk.SelfLink = fmt.Sprintf("projects/%s/regions/%s/disks/%s", project, volKey.Region, volKey.Name)
284284
default:
285285
return fmt.Errorf("could not create disk, key was neither zonal nor regional, instead got: %v", volKey.String())
286286
}
@@ -289,16 +289,16 @@ func (cloud *FakeCloudProvider) InsertDisk(ctx context.Context, volKey *meta.Key
289289
return nil
290290
}
291291

292-
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, volKey *meta.Key) error {
292+
func (cloud *FakeCloudProvider) DeleteDisk(ctx context.Context, project string, volKey *meta.Key) error {
293293
if _, ok := cloud.disks[volKey.Name]; !ok {
294294
return notFoundError()
295295
}
296296
delete(cloud.disks, volKey.Name)
297297
return nil
298298
}
299299

300-
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
301-
source := cloud.GetDiskSourceURI(volKey)
300+
func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, project string, volKey *meta.Key, readWrite, diskType, instanceZone, instanceName string) error {
301+
source := cloud.GetDiskSourceURI(project, volKey)
302302

303303
attachedDiskV1 := &computev1.AttachedDisk{
304304
DeviceName: volKey.Name,
@@ -315,7 +315,7 @@ func (cloud *FakeCloudProvider) AttachDisk(ctx context.Context, volKey *meta.Key
315315
return nil
316316
}
317317

318-
func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, deviceName, instanceZone, instanceName string) error {
318+
func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, project, deviceName, instanceZone, instanceName string) error {
319319
instance, ok := cloud.instances[instanceName]
320320
if !ok {
321321
return fmt.Errorf("Failed to get instance %v", instanceName)
@@ -332,31 +332,31 @@ func (cloud *FakeCloudProvider) DetachDisk(ctx context.Context, deviceName, inst
332332
return nil
333333
}
334334

335-
func (cloud *FakeCloudProvider) GetDiskTypeURI(volKey *meta.Key, diskType string) string {
335+
func (cloud *FakeCloudProvider) GetDiskTypeURI(project string, volKey *meta.Key, diskType string) string {
336336
switch volKey.Type() {
337337
case meta.Zonal:
338-
return cloud.getZonalDiskTypeURI(volKey.Zone, diskType)
338+
return cloud.getZonalDiskTypeURI(project, volKey.Zone, diskType)
339339
case meta.Regional:
340-
return cloud.getRegionalDiskTypeURI(volKey.Region, diskType)
340+
return cloud.getRegionalDiskTypeURI(project, volKey.Region, diskType)
341341
default:
342342
return fmt.Sprintf("could not get disk type uri, key was neither zonal nor regional, instead got: %v", volKey.String())
343343
}
344344
}
345345

346-
func (cloud *FakeCloudProvider) getZonalDiskTypeURI(zone, diskType string) string {
347-
return fmt.Sprintf(diskTypeURITemplateSingleZone, cloud.project, zone, diskType)
346+
func (cloud *FakeCloudProvider) getZonalDiskTypeURI(project, zone, diskType string) string {
347+
return fmt.Sprintf(diskTypeURITemplateSingleZone, project, zone, diskType)
348348
}
349349

350-
func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(region, diskType string) string {
351-
return fmt.Sprintf(diskTypeURITemplateRegional, cloud.project, region, diskType)
350+
func (cloud *FakeCloudProvider) getRegionalDiskTypeURI(project, region, diskType string) string {
351+
return fmt.Sprintf(diskTypeURITemplateRegional, project, region, diskType)
352352
}
353353

354-
func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, volKey *meta.Key, instanceZone, instanceName string) error {
354+
func (cloud *FakeCloudProvider) WaitForAttach(ctx context.Context, project string, volKey *meta.Key, instanceZone, instanceName string) error {
355355
return nil
356356
}
357357

358358
// Regional Disk Methods
359-
func (cloud *FakeCloudProvider) GetReplicaZoneURI(zone string) string {
359+
func (cloud *FakeCloudProvider) GetReplicaZoneURI(project, zone string) string {
360360
return ""
361361
}
362362

@@ -384,7 +384,7 @@ func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, project, snapsh
384384
return snapshot, nil
385385
}
386386

387-
func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
387+
func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
388388
if snapshot, ok := cloud.snapshots[snapshotName]; ok {
389389
return snapshot, nil
390390
}
@@ -394,13 +394,13 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
394394
DiskSizeGb: int64(DiskSizeGb),
395395
CreationTimestamp: Timestamp,
396396
Status: "UPLOADING",
397-
SelfLink: cloud.getGlobalSnapshotURI(snapshotName),
397+
SelfLink: cloud.getGlobalSnapshotURI(project, snapshotName),
398398
}
399399
switch volKey.Type() {
400400
case meta.Zonal:
401-
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
401+
snapshotToCreate.SourceDisk = cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
402402
case meta.Regional:
403-
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
403+
snapshotToCreate.SourceDisk = cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
404404
default:
405405
return nil, fmt.Errorf("could not create snapshot, disk key was neither zonal nor regional, instead got: %v", volKey.String())
406406
}
@@ -409,7 +409,7 @@ func (cloud *FakeCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta
409409
return snapshotToCreate, nil
410410
}
411411

412-
func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key, requestBytes int64) (int64, error) {
412+
func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, project string, volKey *meta.Key, requestBytes int64) (int64, error) {
413413
disk, ok := cloud.disks[volKey.Name]
414414
if !ok {
415415
return -1, notFoundError()
@@ -424,7 +424,7 @@ func (cloud *FakeCloudProvider) ResizeDisk(ctx context.Context, volKey *meta.Key
424424
}
425425

426426
// Snapshot Methods
427-
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, snapshotName string) error {
427+
func (cloud *FakeCloudProvider) DeleteSnapshot(ctx context.Context, project, snapshotName string) error {
428428
delete(cloud.snapshots, snapshotName)
429429
return nil
430430
}
@@ -434,7 +434,7 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
434434
return fmt.Errorf("disk does not exist")
435435
}
436436

437-
diskSource := cloud.GetDiskSourceURI(volKey)
437+
diskSource := cloud.GetDiskSourceURI(cloud.project, volKey)
438438
if resp.SourceDisk != diskSource {
439439
return status.Error(codes.AlreadyExists, fmt.Sprintf("snapshot already exists with same name but with a different disk source %s, expected disk source %s", diskSource, resp.SourceDisk))
440440
}
@@ -443,37 +443,37 @@ func (cloud *FakeCloudProvider) ValidateExistingSnapshot(resp *computev1.Snapsho
443443
return nil
444444
}
445445

446-
func (cloud *FakeCloudProvider) GetDiskSourceURI(volKey *meta.Key) string {
446+
func (cloud *FakeCloudProvider) GetDiskSourceURI(project string, volKey *meta.Key) string {
447447
switch volKey.Type() {
448448
case meta.Zonal:
449-
return cloud.getZonalDiskSourceURI(volKey.Name, volKey.Zone)
449+
return cloud.getZonalDiskSourceURI(project, volKey.Name, volKey.Zone)
450450
case meta.Regional:
451-
return cloud.getRegionalDiskSourceURI(volKey.Name, volKey.Region)
451+
return cloud.getRegionalDiskSourceURI(project, volKey.Name, volKey.Region)
452452
default:
453453
return ""
454454
}
455455
}
456456

457-
func (cloud *FakeCloudProvider) getZonalDiskSourceURI(diskName, zone string) string {
457+
func (cloud *FakeCloudProvider) getZonalDiskSourceURI(project, diskName, zone string) string {
458458
return BasePath + fmt.Sprintf(
459459
diskSourceURITemplateSingleZone,
460-
cloud.project,
460+
project,
461461
zone,
462462
diskName)
463463
}
464464

465-
func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(diskName, region string) string {
465+
func (cloud *FakeCloudProvider) getRegionalDiskSourceURI(project, diskName, region string) string {
466466
return BasePath + fmt.Sprintf(
467467
diskSourceURITemplateRegional,
468-
cloud.project,
468+
project,
469469
region,
470470
diskName)
471471
}
472472

473-
func (cloud *FakeCloudProvider) getGlobalSnapshotURI(snapshotName string) string {
473+
func (cloud *FakeCloudProvider) getGlobalSnapshotURI(project, snapshotName string) string {
474474
return BasePath + fmt.Sprintf(
475475
snapshotURITemplateGlobal,
476-
cloud.project,
476+
project,
477477
snapshotName)
478478
}
479479

@@ -490,11 +490,11 @@ type FakeBlockingCloudProvider struct {
490490
// Upon starting a CreateSnapshot, it passes a chan 'executeCreateSnapshot' into readyToExecute, then blocks on executeCreateSnapshot.
491491
// The test calling this function can block on readyToExecute to ensure that the operation has started and
492492
// allowed the CreateSnapshot to continue by passing a struct into executeCreateSnapshot.
493-
func (cloud *FakeBlockingCloudProvider) CreateSnapshot(ctx context.Context, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
493+
func (cloud *FakeBlockingCloudProvider) CreateSnapshot(ctx context.Context, project string, volKey *meta.Key, snapshotName string) (*computev1.Snapshot, error) {
494494
executeCreateSnapshot := make(chan struct{})
495495
cloud.ReadyToExecute <- executeCreateSnapshot
496496
<-executeCreateSnapshot
497-
return cloud.FakeCloudProvider.CreateSnapshot(ctx, volKey, snapshotName)
497+
return cloud.FakeCloudProvider.CreateSnapshot(ctx, project, volKey, snapshotName)
498498
}
499499

500500
func notFoundError() *googleapi.Error {

0 commit comments

Comments
 (0)