Skip to content

Commit 5d4784a

Browse files
tas50claude
andcommitted
🐛 gcp: support cross-zone instance scanning via snapshot bridge
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9420283 commit 5d4784a

2 files changed

Lines changed: 110 additions & 0 deletions

File tree

providers/gcp/connection/gcpinstancesnapshot/snapshot.go

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,13 @@ func (sc *SnapshotCreator) createSnapshotDisk(snapshotUrl, projectID, zone, disk
279279

280280
// cloneDisk clones a provided disk
281281
func (sc *SnapshotCreator) cloneDisk(sourceDisk, projectID, zone, diskName string) (string, error) {
282+
// A zonal disk can only be cloned directly within the same zone. If the
283+
// source disk lives in a different zone than the target, bridge through a
284+
// temporary snapshot (snapshots are global) so cross-zone scanning works.
285+
if _, srcZone, _, err := parseDiskUrl(sourceDisk); err == nil && srcZone != "" && srcZone != zone {
286+
return sc.cloneDiskViaSnapshot(sourceDisk, projectID, zone, diskName)
287+
}
288+
282289
// create a new disk clone
283290
disk := &compute.Disk{
284291
Name: diskName,
@@ -288,6 +295,90 @@ func (sc *SnapshotCreator) cloneDisk(sourceDisk, projectID, zone, diskName strin
288295
return sc.createDisk(disk, projectID, zone, diskName)
289296
}
290297

298+
// cloneDiskViaSnapshot clones a disk across zones by bridging through a
299+
// temporary global snapshot. GCP requires the source disk and the target disk
300+
// to be in the same zone for a direct disk clone, so when the scanner zone
301+
// differs from the source disk's zone we first snapshot the source disk
302+
// (snapshots are a global resource), create the scanner disk from that
303+
// snapshot, and best-effort delete the temporary snapshot afterwards.
304+
func (sc *SnapshotCreator) cloneDiskViaSnapshot(sourceDisk, projectID, zone, diskName string) (string, error) {
305+
ctx := context.Background()
306+
307+
computeService, err := sc.computeServiceClient(ctx)
308+
if err != nil {
309+
return "", err
310+
}
311+
312+
srcProject, srcZone, srcDiskName, err := parseDiskUrl(sourceDisk)
313+
if err != nil {
314+
return "", err
315+
}
316+
317+
// build a valid temporary snapshot name. GCP requires names to match
318+
// [a-z]([-a-z0-9]{0,61}[a-z0-9])? and be at most 63 characters. The disk
319+
// name we are handed already follows this convention, so derive from it and
320+
// truncate to stay within the limit.
321+
snapName := diskName
322+
if len(snapName) > 63 {
323+
snapName = snapName[:63]
324+
}
325+
// avoid a trailing hyphen after truncation, which is invalid
326+
snapName = strings.TrimRight(snapName, "-")
327+
328+
// create a snapshot from the source disk in the source project/zone
329+
op, err := computeService.Disks.CreateSnapshot(srcProject, srcZone, srcDiskName, &compute.Snapshot{
330+
Name: snapName,
331+
Labels: sc.labels,
332+
}).Context(ctx).Do()
333+
if err != nil {
334+
return "", err
335+
}
336+
337+
if err := sc.waitForZoneOperation(ctx, computeService, srcProject, srcZone, op.Name, "create snapshot"); err != nil {
338+
return "", err
339+
}
340+
341+
// fetch the snapshot to get its SelfLink, which is required to create a disk
342+
snap, err := computeService.Snapshots.Get(srcProject, snapName).Context(ctx).Do()
343+
if err != nil {
344+
return "", err
345+
}
346+
347+
// create the scanner disk from the snapshot in the target project/zone
348+
clonedDiskUrl, err := sc.createSnapshotDisk(snap.SelfLink, projectID, zone, diskName)
349+
350+
// best-effort cleanup: the scanner disk no longer needs the snapshot once it
351+
// has been created, so delete the temporary snapshot. Do not fail the clone
352+
// if cleanup fails.
353+
if _, delErr := computeService.Snapshots.Delete(srcProject, snapName).Context(ctx).Do(); delErr != nil {
354+
log.Warn().Err(delErr).Str("snapshot", snapName).Msg("could not delete temporary snapshot created for cross-zone clone")
355+
}
356+
357+
return clonedDiskUrl, err
358+
}
359+
360+
// waitForZoneOperation blocks until the given zonal operation reaches the DONE
361+
// state, returning an error if the operation reported one.
362+
func (sc *SnapshotCreator) waitForZoneOperation(ctx context.Context, computeService *compute.Service, projectID, zone, opName, action string) error {
363+
for {
364+
operation, err := computeService.ZoneOperations.Get(projectID, zone, opName).Context(ctx).Do()
365+
if err != nil {
366+
return err
367+
}
368+
if operation.Status == "DONE" {
369+
if operation.Error != nil {
370+
errMessage, _ := operation.Error.MarshalJSON()
371+
log.Debug().Str("error", string(errMessage)).Msg("operation failed")
372+
if len(operation.Error.Errors) > 0 {
373+
errMessage = []byte(operation.Error.Errors[0].Message)
374+
}
375+
return fmt.Errorf("%s failed: %s", action, errMessage)
376+
}
377+
return nil
378+
}
379+
}
380+
}
381+
291382
// attachDisk attaches a disk to an instance
292383
func (sc *SnapshotCreator) attachDisk(projectID, zone, instanceName, sourceDiskUrl, deviceName string) error {
293384
ctx := context.Background()

providers/gcp/connection/gcpinstancesnapshot/snapshot_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,22 @@ func TestParseDiskUrl(t *testing.T) {
1818
assert.Equal(t, "us-central1-a", zone)
1919
assert.Equal(t, "super-dupa-disk", disk)
2020
}
21+
22+
// isCrossZoneClone mirrors the decision cloneDisk makes: a source disk whose
23+
// zone differs from the target zone must be bridged through a snapshot.
24+
func isCrossZoneClone(sourceDisk, targetZone string) bool {
25+
_, srcZone, _, err := parseDiskUrl(sourceDisk)
26+
return err == nil && srcZone != "" && srcZone != targetZone
27+
}
28+
29+
func TestCloneDiskCrossZoneDecision(t *testing.T) {
30+
sourceDisk := "https://www.googleapis.com/compute/v1/projects/my-project-1234/zones/us-central1-a/disks/super-dupa-disk"
31+
32+
// different zones -> must bridge through a snapshot
33+
assert.True(t, isCrossZoneClone(sourceDisk, "us-west1-a"),
34+
"a source disk in us-central1-a with target us-west1-a should be detected as cross-zone")
35+
36+
// same zone -> direct clone, no snapshot bridge
37+
assert.False(t, isCrossZoneClone(sourceDisk, "us-central1-a"),
38+
"a source disk and target in the same zone should not be detected as cross-zone")
39+
}

0 commit comments

Comments
 (0)