Fix permadiff on google_vpc_access_connector resource with breaking c…

…hanges (min_instances, max_instances, min_throughput, max_throughput fields) (#10313)

mmv1/products/vpcaccess/Connector.yaml

@@ -115,34 +115,39 @@ properties:
       Minimum throughput of the connector in Mbps. Default and min is 200. Refers to the expected throughput when using an e2-micro machine type.
       Value must be a multiple of 100 from 200 through 900. Must be lower than the value specified by max_throughput. If both min_throughput and
       min_instances are provided, min_instances takes precedence over min_throughput. The use of min_throughput is discouraged in favor of min_instances.
-    default_value: 200
     validation: !ruby/object:Provider::Terraform::Validation
       function: 'validation.IntBetween(200, 1000)'
+    default_from_api: true
+    conflicts:
+      - min_instances
   - !ruby/object:Api::Type::Integer
     name: minInstances
     description: |
       Minimum value of instances in autoscaling group underlying the connector. Value must be between 2 and 9, inclusive. Must be
       lower than the value specified by max_instances.
+    conflicts:
+      - min_throughput
     default_from_api: true
   - !ruby/object:Api::Type::Integer
     name: maxInstances
     description: |
       Maximum value of instances in autoscaling group underlying the connector. Value must be between 3 and 10, inclusive. Must be
       higher than the value specified by min_instances.
     default_from_api: true
+    conflicts:
+      - max_throughput
   - !ruby/object:Api::Type::Integer
     name: maxThroughput
-    # The API documentation says this will default to 200, but when I tried that I got an error that the minimum
-    # throughput must be lower than the maximum. The console defaults to 1000, so I changed it to that.
-    # API returns 300 if it is not sent
     description: |
       Maximum throughput of the connector in Mbps, must be greater than `min_throughput`. Default is 300. Refers to the expected throughput
       when using an e2-micro machine type. Value must be a multiple of 100 from 300 through 1000. Must be higher than the value specified by
       min_throughput. If both max_throughput and max_instances are provided, max_instances takes precedence over max_throughput. The use of
       max_throughput is discouraged in favor of max_instances.
-    default_value: 300
     validation: !ruby/object:Provider::Terraform::Validation
       function: 'validation.IntBetween(200, 1000)'
+    default_from_api: true
+    conflicts:
+      - max_instances
   - !ruby/object:Api::Type::String
     name: 'selfLink'
     description: |

mmv1/templates/terraform/examples/vpc_access_connector.tf.erb

@@ -2,4 +2,6 @@ resource "google_vpc_access_connector" "connector" {
   name          = "<%= ctx[:vars]['name'] %>"
   ip_cidr_range = "10.8.0.0/28"
   network       = "<%= ctx[:vars]['network_name'] %>"
+  min_instances = 2
+  max_instances = 3
 }

mmv1/templates/terraform/examples/vpc_access_connector_shared_vpc.tf.erb

@@ -4,6 +4,8 @@ resource "google_vpc_access_connector" "connector" {
     name = google_compute_subnetwork.custom_test.name
   }
   machine_type = "e2-standard-4"
+  min_instances = 2
+  max_instances = 3
 }
 
 resource "google_compute_subnetwork" "custom_test" {

mmv1/third_party/terraform/services/appengine/resource_app_engine_standard_app_version_test.go

@@ -189,6 +189,8 @@ resource "google_vpc_access_connector" "bar" {
   region = "us-central1"
   ip_cidr_range = "10.8.0.16/28"
   network = "default"
+  min_throughput  = 200
+  max_throughput = 300
 }
 
 resource "google_app_engine_standard_app_version" "foo" {

mmv1/third_party/terraform/services/cloudfunctions/resource_cloudfunctions_function_test.go.erb

@@ -985,6 +985,8 @@ resource "google_vpc_access_connector" "%s" {
   region        = "us-central1"
   ip_cidr_range = "%s"
   network       = google_compute_network.vpc.name
+  min_throughput  = 200
+  max_throughput = 300
 }
 
 resource "google_storage_bucket" "bucket" {

mmv1/third_party/terraform/services/vpcaccess/data_source_vpc_access_connector_test.go

@@ -37,9 +37,11 @@ func testAccVPCAccessConnectorDatasourceConfig(suffix string) string {
 	return fmt.Sprintf(`
 resource "google_vpc_access_connector" "connector" {
   name          = "tf-test-%s"
-  ip_cidr_range = "10.8.0.32/28"
-  network       = "default"
-  region        = "us-central1"
+  ip_cidr_range  = "10.8.0.32/28"
+  network        = "default"
+  region         = "us-central1"
+  min_throughput  = 200
+  max_throughput = 300
 }
 
 data "google_vpc_access_connector" "connector" {

mmv1/third_party/terraform/services/vpcaccess/resource_vpc_access_connector_test.go

@@ -1,6 +1,7 @@
 package vpcaccess_test
 
 import (
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -31,6 +32,81 @@ func TestAccVPCAccessConnector_vpcAccessConnectorThroughput(t *testing.T) {
 	})
 }
 
+func TestAccVPCAccessConnector_vpcAccessConnectorThroughput_combiningThroughputAndInstancesFields_conflict(t *testing.T) {
+	// Need to skip this test as the expected failure happens before the provider interacts with APIs
+	// In VCR mode this test fails due to lack of cassettes
+	acctest.SkipIfVcr(t)
+	t.Parallel()
+
+	context := map[string]interface{}{
+		"random_suffix": acctest.RandString(t, 10),
+	}
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckVPCAccessConnectorDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccVPCAccessConnector_vpcAccessConnectorThroughput_bothThroughputAndInstances(context),
+				// When all 4 of min_instance/max_instance and min_throughput/max_throughput fields are sent to the API
+				// the API ignores the throughput field values. Instead the API returns values for min and max throughput
+				// based on the value of min and max instances. The mismatch with the config causes a permadiff.
+				// Due to this we make the fields conflict with each other.
+				ExpectError: regexp.MustCompile("conflicts with"),
+			},
+		},
+	})
+}
+
+func TestAccVPCAccessConnector_vpcAccessConnectorThroughput_usingThroughputOrInstancesLimits(t *testing.T) {
+	t.Parallel()
+
+	context := map[string]interface{}{
+		"random_suffix": acctest.RandString(t, 10),
+	}
+
+	acctest.VcrTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckVPCAccessConnectorDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccVPCAccessConnector_vpcAccessConnectorThroughput_justThroughputFields(context),
+				Check: resource.ComposeTestCheckFunc(
+					// These fields are set by the config in this test step
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "min_throughput", "400"),
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "max_throughput", "800"),
+					// These fields aren't set in the config; the API sets and returns values
+					// based on the thoughput values provided
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "min_instances", "4"),
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "max_instances", "8"),
+				),
+			},
+			{
+				Config: testAccVPCAccessConnector_vpcAccessConnectorThroughput_justInstanceFields(context),
+				Check: resource.ComposeTestCheckFunc(
+					// These fields are set by the config in this test step
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "min_instances", "5"),
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "max_instances", "7"),
+					// These fields aren't set in the config; the API sets and returns values
+					// based on the instance limit values provided
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "min_throughput", "500"),
+					resource.TestCheckResourceAttr(
+						"google_vpc_access_connector.connector", "max_throughput", "700"),
+				),
+			},
+		},
+	})
+}
+
 func testAccVPCAccessConnector_vpcAccessConnectorThroughput(context map[string]interface{}) string {
 	return acctest.Nprintf(`
 resource "google_vpc_access_connector" "connector" {
@@ -57,3 +133,86 @@ resource "google_compute_network" "custom_test" {
 }
 `, context)
 }
+
+func testAccVPCAccessConnector_vpcAccessConnectorThroughput_bothThroughputAndInstances(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+resource "google_vpc_access_connector" "connector" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  subnet {
+    name = google_compute_subnetwork.custom_test.name
+  }
+  machine_type = "e2-standard-4"
+  min_instances = 2
+  max_instances = 3
+  min_throughput = 400
+  max_throughput = 1000
+  region        = "us-central1"
+}
+
+resource "google_compute_subnetwork" "custom_test" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  ip_cidr_range = "10.2.0.0/28"
+  region        = "us-central1"
+  network       = google_compute_network.custom_test.id
+}
+
+resource "google_compute_network" "custom_test" {
+  name                    = "tf-test-vpc-con%{random_suffix}"
+  auto_create_subnetworks = false
+}
+`, context)
+}
+
+func testAccVPCAccessConnector_vpcAccessConnectorThroughput_justInstanceFields(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+resource "google_vpc_access_connector" "connector" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  subnet {
+    name = google_compute_subnetwork.custom_test.name
+  }
+  machine_type = "e2-standard-4"
+  min_instances = 5
+  max_instances = 7
+  region        = "us-central1"
+}
+
+resource "google_compute_subnetwork" "custom_test" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  ip_cidr_range = "10.2.0.0/28"
+  region        = "us-central1"
+  network       = google_compute_network.custom_test.id
+}
+
+resource "google_compute_network" "custom_test" {
+  name                    = "tf-test-vpc-con%{random_suffix}"
+  auto_create_subnetworks = false
+}
+`, context)
+}
+
+func testAccVPCAccessConnector_vpcAccessConnectorThroughput_justThroughputFields(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+resource "google_vpc_access_connector" "connector" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  subnet {
+    name = google_compute_subnetwork.custom_test.name
+  }
+  machine_type = "e2-standard-4"
+  min_throughput = 400
+  max_throughput = 800
+  region        = "us-central1"
+}
+
+resource "google_compute_subnetwork" "custom_test" {
+  name          = "tf-test-vpc-con%{random_suffix}"
+  ip_cidr_range = "10.2.0.0/28"
+  region        = "us-central1"
+  network       = google_compute_network.custom_test.id
+}
+
+resource "google_compute_network" "custom_test" {
+  name                    = "tf-test-vpc-con%{random_suffix}"
+  auto_create_subnetworks = false
+}
+`, context)
+}

mmv1/third_party/terraform/website/docs/d/vpc_access_connector.html.markdown

@@ -26,6 +26,8 @@ resource "google_vpc_access_connector" "connector" {
   ip_cidr_range = "10.8.0.0/28"
   network       = "default"
   region        = "us-central1"
+  min_instances = 2
+  max_instances = 3
 }
 ```
 

mmv1/third_party/terraform/website/docs/guides/version_6_upgrade.html.markdown

@@ -118,4 +118,30 @@ Removed in favor of field `settings.ip_configuration.ssl_mode`.
 
 ### `schema_settings` no longer has a default value
 
-An empty value means the setting should be cleared.
+An empty value means the setting should be cleared.
+
+## Resource: `google_vpc_access_connector`
+
+### Fields `min_throughput` and `max_throughput` no longer have default values
+
+The fields `min_throughput` and `max_throughput` no longer have default values 
+set by the provider. This was necessary to add conflicting field validation, also
+described in this guide.
+
+No configuration changes are needed for existing resources as these fields' values
+will default to values present in data returned from the API.
+
+### Conflicting field validation added for `min_throughput` and `min_instances`, and `max_throughput` and `max_instances`
+
+The provider will now enforce that `google_vpc_access_connector` resources can only
+include one of `min_throughput` and `min_instances` and one of `max_throughput`and 
+`max_instances`. Previously if a user included all four fields in a resource block
+they would experience a permadiff. This is a result of how `min_instances` and
+`max_instances` fields' values take precedence in the API, and how the API calculates
+values for `min_throughput` and `max_throughput` that match the number of instances.
+
+Users will need to check their configuration for any `google_vpc_access_connector`
+resource blocks that contain both fields in a conflicting pair, and remove one of those fields.
+The fields that are removed from the configuration will still have Computed values,
+that are derived from the API.
+

mmv1/third_party/tgc/tests/data/example_vpc_access_connector.tf

@@ -31,4 +31,6 @@ resource "google_vpc_access_connector" "connector" {
   ip_cidr_range = "10.8.0.0/28"
   network       = "default"
   region        = "us-central1"
+  min_instances = 2
+  max_instances = 3
 }