fix(diff): VPC/Droplet/AppPlatform Diff now compares region (#70)

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to workflow-plugin-digitalocean are documented here.
 
 ## [Unreleased]
 
+### Fixed
+
+- **VPCDriver / DropletDriver / AppPlatformDriver `Diff` now compares `region`** (#70) — previously only `VolumeDriver.Diff` checked region drift; the other three drivers silently no-op'd on region changes despite DO having no in-place region update on those resources. Region change now correctly emits `FieldChange{ForceNew: true}` and sets `NeedsReplace=true`. Surfaced by core-dump's TC2 cutover plan-shape assertion which expected a 5-resource cascade replace but got 1 (Volume only); without the assertion gate the partial cutover would have left VPC + Droplet in nyc3 while the App + Volume moved to nyc1, producing a half-migrated state requiring manual cleanup. Includes upgrade-safe guard: `vpcOutput` and `dropletOutput` already populated `region`, and this PR adds `region` to `appOutput.Outputs` so AppPlatformDriver.Diff has a current-side value to compare; the new check then skips when `current.Outputs["region"]` is empty so state from earlier plugin versions doesn't false-positive — the next Read populates it without spurious drift. Regression tests cover both the change-detection and empty-current-skip paths for all three drivers.
+
 ## [v0.10.0]
 
 ### Added

internal/drivers/app_platform.go

@@ -209,7 +209,30 @@ func (d *AppPlatformDriver) Diff(_ context.Context, desired interfaces.ResourceS
 	if desiredExpose != curExpose {
 		changes = append(changes, interfaces.FieldChange{Path: "expose", Old: curExpose, New: desiredExpose})
 	}
-	return &interfaces.DiffResult{NeedsUpdate: len(changes) > 0, Changes: changes}, nil
+
+	// region: App Platform's UpdateApp does not accept region changes —
+	// region is a Create-only field on godo.AppSpec, so any drift forces
+	// replace. Mirror VolumeDriver.Diff's region pattern. Guard
+	// curRegion != "" so apps in state from earlier plugin versions
+	// (when appOutput didn't include region) don't false-positive on
+	// the first plan after upgrade — they'll Read on next apply to
+	// populate the field.
+	var needsReplace bool
+	if region := strFromConfig(desired.Config, "region", ""); region != "" {
+		curRegion, _ := current.Outputs["region"].(string)
+		if curRegion != "" && curRegion != region {
+			changes = append(changes, interfaces.FieldChange{
+				Path: "region", Old: curRegion, New: region, ForceNew: true,
+			})
+			needsReplace = true
+		}
+	}
+
+	return &interfaces.DiffResult{
+		NeedsUpdate:  len(changes) > 0,
+		NeedsReplace: needsReplace,
+		Changes:      changes,
+	}, nil
 }
 
 // canonicalExpose returns the canonical `expose` value for a desired-spec
@@ -768,6 +791,11 @@ func appOutput(app *godo.App) *interfaces.ResourceOutput {
 			// desired `cfg["image"]` and avoid emitting spurious image
 			// FieldChanges on no-op reconciles — F4 round-3 Finding A.
 			"image": deriveImageFromAppSpec(app.Spec),
+			// `region` is the App Platform region from AppSpec.Region.
+			// Stored on Outputs so Diff can detect region drift and
+			// surface it as ForceNew (App Platform's UpdateApp does not
+			// accept region changes — region is a Create-only field).
+			"region": app.Spec.Region,
 		},
 		Status: "running",
 	}

internal/drivers/app_platform_test.go

@@ -371,6 +371,77 @@ func TestAppPlatformDriver_Diff_NoChanges(t *testing.T) {
 	}
 }
 
+// TestAppPlatformDriver_Diff_RegionChangeForcesReplace covers issue #70:
+// App Platform region is a Create-only field on godo.AppSpec — UpdateApp
+// does not accept region changes. Region drift must surface as ForceNew so
+// dependents (vpc_ref to a region-locked VPC) get correctly recreated too.
+func TestAppPlatformDriver_Diff_RegionChangeForcesReplace(t *testing.T) {
+	mock := &mockAppClient{}
+	d := drivers.NewAppPlatformDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{
+			"image":  "registry.digitalocean.com/myrepo/myapp:v1",
+			"region": "nyc3",
+		},
+	}
+	result, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{
+			"image":  "registry.digitalocean.com/myrepo/myapp:v1",
+			"region": "nyc1",
+		},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if !result.NeedsReplace {
+		t.Fatal("region change must force replace; NeedsReplace=false")
+	}
+	var found bool
+	for _, c := range result.Changes {
+		if c.Path == "region" && c.Old == "nyc3" && c.New == "nyc1" && c.ForceNew {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected FieldChange{Path:region, Old:nyc3, New:nyc1, ForceNew:true}; got %+v", result.Changes)
+	}
+}
+
+// TestAppPlatformDriver_Diff_RegionEmptyCurrentSkipped covers the
+// upgrade-safe guard: Apps in state from earlier plugin versions (when
+// appOutput didn't include region) must not false-positive on the first
+// plan after upgrade — they'll Read on next apply to populate the field.
+func TestAppPlatformDriver_Diff_RegionEmptyCurrentSkipped(t *testing.T) {
+	mock := &mockAppClient{}
+	d := drivers.NewAppPlatformDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{
+			"image": "registry.digitalocean.com/myrepo/myapp:v1",
+			// no region — represents pre-region-output state
+		},
+	}
+	result, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{
+			"image":  "registry.digitalocean.com/myrepo/myapp:v1",
+			"region": "nyc1",
+		},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if result.NeedsReplace {
+		t.Error("empty curRegion should not force replace; got NeedsReplace=true")
+	}
+	for _, c := range result.Changes {
+		if c.Path == "region" {
+			t.Errorf("empty curRegion should not emit a region change; got %+v", c)
+		}
+	}
+}
+
 // TestAppPlatformDriver_Diff_DetectsExposeChange covers quality-review Finding
 // 1: changing `expose` (a security-relevant toggle) on an existing service
 // must produce a Plan action — Diff cannot silently no-op the way the

internal/drivers/droplet.go

@@ -223,6 +223,21 @@ func (d *DropletDriver) Diff(_ context.Context, desired interfaces.ResourceSpec,
 	_ = boolFromConfig(desired.Config, "ipv6", false)       // see TODO above
 	// ssh_keys: parsed in Create; not re-parsed here to avoid wasted work.
 
+	// region: Droplets are regional and DO does not support region change
+	// in Update. Mirror VolumeDriver.Diff's region pattern: any region
+	// drift forces replace. Guard curRegion != "" so Droplets in state
+	// from earlier plugin versions (when dropletOutput didn't include
+	// region) don't false-positive on the first plan after upgrade —
+	// they'll Read on next apply to populate the field.
+	if region := strFromConfig(desired.Config, "region", ""); region != "" {
+		curRegion, _ := current.Outputs["region"].(string)
+		if curRegion != "" && curRegion != region {
+			changes = append(changes, interfaces.FieldChange{
+				Path: "region", Old: curRegion, New: region, ForceNew: true,
+			})
+		}
+	}
+
 	return &interfaces.DiffResult{
 		NeedsUpdate:  len(changes) > 0,
 		NeedsReplace: len(changes) > 0,

internal/drivers/droplet_test.go

@@ -965,6 +965,78 @@ func TestDropletDriver_Create_Volumes_EmptyStringRejected(t *testing.T) {
 	}
 }
 
+// TestDropletDriver_Diff_RegionChangeForcesReplace covers issue #70:
+// Droplets are regional and DO does not support region change in Update
+// (godo Droplet PUT only resizes). Region drift must surface as ForceNew
+// so cascade dependents (Volume mount, Firewall tags) get correctly
+// re-coordinated against the replaced Droplet.
+func TestDropletDriver_Diff_RegionChangeForcesReplace(t *testing.T) {
+	mock := &mockDropletClient{}
+	d := drivers.NewDropletDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{
+			"size":   "s-1vcpu-2gb",
+			"region": "nyc3",
+		},
+	}
+	r, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{
+			"size":   "s-1vcpu-2gb",
+			"region": "nyc1",
+		},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if !r.NeedsReplace {
+		t.Fatal("region change must force replace; NeedsReplace=false")
+	}
+	var found bool
+	for _, c := range r.Changes {
+		if c.Path == "region" && c.Old == "nyc3" && c.New == "nyc1" && c.ForceNew {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected FieldChange{Path:region, Old:nyc3, New:nyc1, ForceNew:true}; got %+v", r.Changes)
+	}
+}
+
+// TestDropletDriver_Diff_RegionEmptyCurrentSkipped covers the upgrade-safe
+// guard: Droplets in state from earlier plugin versions (when dropletOutput
+// didn't include region) must not false-positive on the first plan after
+// upgrade — they'll Read on next apply to populate the field.
+func TestDropletDriver_Diff_RegionEmptyCurrentSkipped(t *testing.T) {
+	mock := &mockDropletClient{}
+	d := drivers.NewDropletDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{
+			"size": "s-1vcpu-2gb",
+			// no region — represents pre-region-output state
+		},
+	}
+	r, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{
+			"size":   "s-1vcpu-2gb",
+			"region": "nyc1",
+		},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if r.NeedsReplace {
+		t.Error("empty curRegion should not force replace; got NeedsReplace=true")
+	}
+	for _, c := range r.Changes {
+		if c.Path == "region" {
+			t.Errorf("empty curRegion should not emit a region change; got %+v", c)
+		}
+	}
+}
+
 func contains(s, sub string) bool {
 	if len(sub) == 0 {
 		return true

internal/drivers/vpc.go

@@ -151,6 +151,22 @@ func (d *VPCDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, cur
 			})
 		}
 	}
+
+	// region: VPCs are regional and DO does not support region change in
+	// Update. Mirror VolumeDriver.Diff's region pattern: any region drift
+	// forces replace. Guard curRegion != "" so VPCs in state from earlier
+	// plugin versions (when vpcOutput didn't include region) don't false-
+	// positive on the first plan after upgrade — they'll Read on next
+	// apply to populate the field.
+	if region := strFromConfig(desired.Config, "region", ""); region != "" {
+		curRegion, _ := current.Outputs["region"].(string)
+		if curRegion != "" && curRegion != region {
+			changes = append(changes, interfaces.FieldChange{
+				Path: "region", Old: curRegion, New: region, ForceNew: true,
+			})
+		}
+	}
+
 	return &interfaces.DiffResult{
 		NeedsUpdate:  len(changes) > 0,
 		NeedsReplace: len(changes) > 0,

internal/drivers/vpc_test.go

@@ -217,6 +217,65 @@ func TestVPCDriver_Diff_IPRangeChange(t *testing.T) {
 	}
 }
 
+// TestVPCDriver_Diff_RegionChangeForcesReplace covers issue #70: VPCs are
+// regional and DO does not support region change in Update. Region drift
+// must surface as ForceNew so cascade dependents (Droplet vpc_uuid, App
+// vpc_ref) are correctly replaced too.
+func TestVPCDriver_Diff_RegionChangeForcesReplace(t *testing.T) {
+	mock := &mockVPCClient{vpc: testVPC()}
+	d := drivers.NewVPCDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{"ip_range": "10.20.0.0/16", "region": "nyc3"},
+	}
+	result, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{"ip_range": "10.20.0.0/16", "region": "nyc1"},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if !result.NeedsReplace {
+		t.Fatal("expected NeedsReplace for region change")
+	}
+	var found bool
+	for _, c := range result.Changes {
+		if c.Path == "region" && c.Old == "nyc3" && c.New == "nyc1" && c.ForceNew {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("expected FieldChange{Path:region, Old:nyc3, New:nyc1, ForceNew:true}; got %+v", result.Changes)
+	}
+}
+
+// TestVPCDriver_Diff_RegionEmptyCurrentSkipped covers the upgrade-safe
+// guard: VPCs in state from earlier plugin versions (when vpcOutput
+// didn't include region) must not false-positive on the first plan
+// after upgrade — they'll Read on next apply to populate the field.
+func TestVPCDriver_Diff_RegionEmptyCurrentSkipped(t *testing.T) {
+	mock := &mockVPCClient{vpc: testVPC()}
+	d := drivers.NewVPCDriverWithClient(mock, "nyc3")
+
+	current := &interfaces.ResourceOutput{
+		Outputs: map[string]any{"ip_range": "10.20.0.0/16"}, // no region
+	}
+	result, err := d.Diff(context.Background(), interfaces.ResourceSpec{
+		Config: map[string]any{"ip_range": "10.20.0.0/16", "region": "nyc1"},
+	}, current)
+	if err != nil {
+		t.Fatalf("Diff: %v", err)
+	}
+	if result.NeedsReplace {
+		t.Error("empty curRegion should not force replace; got NeedsReplace=true")
+	}
+	for _, c := range result.Changes {
+		if c.Path == "region" {
+			t.Errorf("empty curRegion should not emit a region change; got %+v", c)
+		}
+	}
+}
+
 func TestVPCDriver_SupportsUpsert(t *testing.T) {
 	d := drivers.NewVPCDriverWithClient(&mockVPCClient{}, "nyc3")
 	if !d.SupportsUpsert() {