Validate that subnetwork_project should match with the project in subnetwork field in google_compute_instance resource (#11537)

Author: karolgorc

mmv1/third_party/terraform/services/compute/resource_compute_instance.go.erb

@@ -99,6 +99,36 @@ var (
 	}
 )
 
+// This checks if the project provided in subnetwork's self_link matches
+// the project provided in subnetwork_project not to produce a confusing plan diff.
+func validateSubnetworkProject(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
+	// separate func to allow unit testing
+	return ValidateSubnetworkProjectFunc(d)
+}
+
+func ValidateSubnetworkProjectFunc(d tpgresource.TerraformResourceDiff) error {
+	oldCount, newCount := d.GetChange("network_interface.#")
+	if oldCount.(int) != newCount.(int) {
+		return nil
+	}
+	for i := 0; i < newCount.(int); i++ {
+		prefix := fmt.Sprintf("network_interface.%d", i)
+		subnetworkProject := d.Get(prefix + ".subnetwork_project")
+		subnetwork := d.Get(prefix + ".subnetwork")
+
+		_, err := tpgresource.GetRelativePath(subnetwork.(string))
+		if err != nil {
+			log.Printf("[DEBUG] Subnetwork %q is not a selflink", subnetwork)
+			return nil
+		}
+
+		if tpgresource.GetProjectFromRegionalSelfLink(subnetwork.(string)) != subnetworkProject.(string) {
+			return fmt.Errorf("project in subnetwork's self_link %q must match subnetwork_project %q", subnetwork, subnetworkProject)
+		}
+	}
+	return nil
+}
+
 // network_interface.[d].network_ip can only change when subnet/network
 // is also changing. Validate that if network_ip is changing this scenario
 // holds up to par.
@@ -1224,6 +1254,7 @@ be from 0 to 999,999,999 inclusive.`,
 				suppressEmptyGuestAcceleratorDiff,
 			),
 			desiredStatusDiff,
+			validateSubnetworkProject,
 			forceNewIfNetworkIPNotUpdatable,
 			tpgresource.SetLabelsDiff,
 		),

mmv1/third_party/terraform/services/compute/resource_compute_instance_test.go.erb

@@ -2687,6 +2687,23 @@ func TestAccComputeInstance_subnetworkUpdate(t *testing.T) {
 	})
 }
 
+func TestAccComputeInstance_subnetworkProjectMustMatchError(t *testing.T) {
+	t.Parallel()
+	instanceName := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10))
+	suffix := fmt.Sprintf("%s", acctest.RandString(t, 10))
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:     func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy: testAccCheckComputeInstanceDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccComputeInstance_subnetworkProjectExpectError(suffix, instanceName),
+				ExpectError: regexp.MustCompile("must match subnetwork_project"),
+			},
+		},
+	})
+}
+
 func TestAccComputeInstance_networkIpUpdate(t *testing.T) {
 	t.Parallel()
 
@@ -8695,6 +8712,45 @@ func testAccComputeInstance_subnetworkUpdateTwo(suffix, instance string) string
 `, suffix, suffix, suffix, suffix, instance)
 }
 
+func testAccComputeInstance_subnetworkProjectExpectError(suffix, instance string) string {
+	return fmt.Sprintf(`
+	data "google_compute_image" "my_image" {
+		family  = "debian-11"
+		project = "debian-cloud"
+	}
+
+	resource "google_compute_network" "inst-test-network" {
+		name = "tf-test-network-%s"
+		auto_create_subnetworks = false
+	}
+
+	resource "google_compute_subnetwork" "inst-test-subnetwork" {
+		name          = "tf-test-compute-subnet-%s"
+		ip_cidr_range = "10.0.0.0/16"
+		region        = "us-east1"
+		network       = google_compute_network.inst-test-network.id
+	}
+
+	resource "google_compute_instance" "foobar" {
+		name         = "%s"
+		machine_type = "e2-medium"
+		zone         = "us-east1-d"
+		allow_stopping_for_update = true
+
+		boot_disk {
+			initialize_params {
+				image = data.google_compute_image.my_image.id
+			}
+		}
+
+		network_interface {
+			subnetwork = google_compute_subnetwork.inst-test-subnetwork.id
+			subnetwork_project = "placeholder"
+		}
+	}
+`, suffix, suffix, instance)
+}
+
 func testAccComputeInstance_networkIpUpdate(suffix, instance string) string {
 	return fmt.Sprintf(`
 	data "google_compute_image" "my_image" {

mmv1/third_party/terraform/tpgresource/self_link_helpers.go

@@ -170,3 +170,14 @@ func GetRegionFromRegionalSelfLink(selfLink string) string {
 	}
 	return selfLink
 }
+
+func GetProjectFromRegionalSelfLink(selfLink string) string {
+	re := regexp.MustCompile("projects/([a-zA-Z0-9-]*)/(?:locations|regions)/[a-zA-Z0-9-]*")
+	switch {
+	case re.MatchString(selfLink):
+		if res := re.FindStringSubmatch(selfLink); len(res) == 2 && res[1] != "" {
+			return res[1]
+		}
+	}
+	return selfLink
+}

mmv1/third_party/terraform/tpgresource/self_link_helpers_test.go

@@ -122,3 +122,15 @@ func TestGetRegionFromRegionalSelfLink(t *testing.T) {
 		}
 	}
 }
+
+func TestGetProjectFromRegionalSelfLink(t *testing.T) {
+	cases := map[string]string{
+		"projects/foo/locations/europe-north1/datasets/bar/operations/foobar":        "foo",
+		"projects/REDACTED/regions/europe-north1/subnetworks/tf-test-net-xbwhsmlfm8": "REDACTED",
+	}
+	for input, expected := range cases {
+		if result := GetProjectFromRegionalSelfLink(input); result != expected {
+			t.Errorf("expected to get %q from %q, got %q", expected, input, result)
+		}
+	}
+}

mmv1/third_party/terraform/website/docs/r/compute_instance.html.markdown

@@ -372,7 +372,7 @@ is desired, you will need to modify your state file manually using
 
 
 *  `subnetwork_project` - (Optional) The project in which the subnetwork belongs.
-   If the `subnetwork` is a self_link, this field is ignored in favor of the project
+   If the `subnetwork` is a self_link, this field is set to the project
    defined in the subnetwork self_link. If the `subnetwork` is a name and this
    field is not provided, the provider project is used.