diff --git a/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection.go b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection.go index 7de22784def2..a65197c607ba 100644 --- a/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection.go +++ b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection.go @@ -1,6 +1,7 @@ package servicenetworking import ( + "errors" "fmt" "log" "net/url" @@ -11,6 +12,7 @@ import ( "time" "github.com/hashicorp/terraform-provider-google/google/registry" + tpgcompute "github.com/hashicorp/terraform-provider-google/google/services/compute" rmClient "github.com/hashicorp/terraform-provider-google/google/services/resourcemanager/client" "github.com/hashicorp/terraform-provider-google/google/tpgresource" transport_tpg "github.com/hashicorp/terraform-provider-google/google/transport" @@ -18,7 +20,9 @@ import ( "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "google.golang.org/api/googleapi" "google.golang.org/api/servicenetworking/v1" + "google.golang.org/grpc/codes" ) func isInvalidAuthError(err error) (bool, string) { @@ -28,6 +32,48 @@ func isInvalidAuthError(err error) (bool, string) { return false, "" } +// isProducerServicesInUseError reports whether err is the API refusing to delete a +// connection that service producers still hold. Producers such as Cloud SQL retain +// their resources for a period after the instance itself is deleted. +func isProducerServicesInUseError(err error) bool { + opErr, ok := errors.AsType[*tpgresource.CommonOpError](err) + if !ok { + return false + } + if opErr.Code != int64(codes.FailedPrecondition) { + return false + } + return strings.Contains(opErr.Message, "still using this connection") +} + +// removeServiceNetworkingPeering removes the VPC peering the connection created on +// the consumer network. A network cannot be deleted while a peering references it. +func removeServiceNetworkingPeering(d *schema.ResourceData, config *transport_tpg.Config, userAgent, peering string, network *tpgresource.GlobalFieldValue) error { + // Only one peering operation at a time can be performed for a given network. + mutexKey := fmt.Sprintf("%s/peerings", network.RelativeLink()) + transport_tpg.MutexStore.Lock(mutexKey) + defer transport_tpg.MutexStore.Unlock(mutexKey) + + url := fmt.Sprintf("%sprojects/%s/global/networks/%s/removePeering", transport_tpg.BaseUrl(tpgcompute.Product, config), network.Project, network.Name) + res, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{ + Config: config, + Method: "POST", + Project: network.Project, + RawURL: url, + UserAgent: userAgent, + Body: map[string]interface{}{"name": peering}, + }) + if err != nil { + if gerr, ok := errors.AsType[*googleapi.Error](err); ok && gerr.Code == 404 { + log.Printf("[WARN] Peering %q already removed from network %q", peering, network.Name) + return nil + } + return fmt.Errorf("Error removing peering %q from network %q: %s", peering, network.Name, err) + } + + return tpgcompute.ComputeOperationWaitTime(config, res, network.Project, "Removing Service Networking VPC Peering", userAgent, d.Timeout(schema.TimeoutDelete)) +} + func ResourceServiceNetworkingConnection() *schema.Resource { return &schema.Resource{ Create: resourceServiceNetworkingConnectionCreate, @@ -75,7 +121,7 @@ func ResourceServiceNetworkingConnection() *schema.Resource { Description: `Named IP address range(s) of PEERING type reserved for this service provider. Note that invoking this method with a different range when connection is already established will not reallocate already provisioned service producer subnetworks.`, }, //UDP schema start - "deletion_policy": tpgresource.DeletionPolicySchemaEntry("DELETE"), + "deletion_policy": deletionPolicySchema(), //UDP schema end "peering": { Type: schema.TypeString, @@ -91,6 +137,23 @@ func ResourceServiceNetworkingConnection() *schema.Resource { } } +// deletionPolicySchema returns the universal deletion_policy field with a +// description covering REMOVE_PEERING, which is specific to this resource. +func deletionPolicySchema() *schema.Schema { + s := tpgresource.DeletionPolicySchemaEntry("DELETE") + s.Description = `Whether Terraform will be prevented from destroying the connection. Defaults to "DELETE". +When set to "PREVENT", destroying the resource will fail. +When set to "ABANDON", the resource is removed from Terraform state without +deleting the connection in the API. The VPC peering created by this connection +is left in place, which will block deletion of the network. +When set to "DELETE", the connection is deleted. +When set to "REMOVE_PEERING", the connection is deleted, and if the API refuses +because service producer resources still use it, the VPC peering is removed from +the network instead so that the network can be deleted. Only use this once the +service instances using the connection (such as Cloud SQL) are already deleted.` + return s +} + func resourceServiceNetworkingConnectionCreate(d *schema.ResourceData, meta interface{}) error { config := meta.(*transport_tpg.Config) userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent) @@ -352,7 +415,19 @@ func resourceServiceNetworkingConnectionDelete(d *schema.ResourceData, meta inte } if err := ServiceNetworkingOperationWaitTimeHW(config, op, "Delete Service Networking Connection", userAgent, project, d.Timeout(schema.TimeoutCreate)); err != nil { - return errwrap.Wrapf("Unable to remove Service Networking Connection, err: {{err}}", err) + if d.Get("deletion_policy").(string) != "REMOVE_PEERING" || !isProducerServicesInUseError(err) { + return errwrap.Wrapf("Unable to remove Service Networking Connection, err: {{err}}", err) + } + + peering := d.Get("peering").(string) + if peering == "" { + peering = strings.ReplaceAll(connectionId.Service, ".", "-") + } + + log.Printf("[DEBUG] Connection is still held by service producers; removing peering %q from network %q", peering, networkFieldValue.Name) + if err := removeServiceNetworkingPeering(d, config, userAgent, peering, networkFieldValue); err != nil { + return err + } } d.SetId("") diff --git a/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_internal_test.go b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_internal_test.go new file mode 100644 index 000000000000..25178540f228 --- /dev/null +++ b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_internal_test.go @@ -0,0 +1,72 @@ +package servicenetworking + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-provider-google/google/tpgresource" + cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v1" + "google.golang.org/grpc/codes" +) + +func TestUnitServiceNetworkingConnection_isProducerServicesInUseError(t *testing.T) { + t.Parallel() + + producerErr := &tpgresource.CommonOpError{ + Status: &cloudresourcemanager.Status{ + Code: int64(codes.FailedPrecondition), + Message: "Failed to delete connection; Producer services (e.g. CloudSQL, Cloud Memstore, etc.) are still using this connection.", + }, + } + + cases := map[string]struct { + err error + want bool + }{ + "producer services still hold the connection": { + err: producerErr, + want: true, + }, + "wrapped by the operation waiter": { + // OperationWait wraps the operation error before the delete function + // sees it, so the match has to survive unwrapping. + err: fmt.Errorf("Error waiting for Delete Service Networking Connection: %w", producerErr), + want: true, + }, + "failed precondition for an unrelated reason": { + err: &tpgresource.CommonOpError{ + Status: &cloudresourcemanager.Status{ + Code: int64(codes.FailedPrecondition), + Message: "Failed to delete connection; the connection does not exist.", + }, + }, + want: false, + }, + "matching message under a different status code": { + err: &tpgresource.CommonOpError{ + Status: &cloudresourcemanager.Status{ + Code: int64(codes.PermissionDenied), + Message: "Producer services are still using this connection", + }, + }, + want: false, + }, + "unrelated error type": { + err: fmt.Errorf("connection reset by peer"), + want: false, + }, + "nil error": { + err: nil, + want: false, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + if got := isProducerServicesInUseError(tc.err); got != tc.want { + t.Errorf("isProducerServicesInUseError() = %t, want %t", got, tc.want) + } + }) + } +} diff --git a/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_test.go b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_test.go index 21b7ec5e95d6..e7e0508cce5a 100644 --- a/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_test.go +++ b/mmv1/third_party/terraform/services/servicenetworking/resource_service_networking_connection_test.go @@ -315,6 +315,75 @@ resource "google_service_networking_connection" "foobar" { `, addressRangeName, addressRangeName, org_id, billing_account, networkName, addressRangeName, serviceName) } +// TestAccServiceNetworkingConnection_removePeering covers the case where no service +// producer holds the connection, so the connection is deleted normally and the +// peering removal is never reached. The fallback itself needs a service producer +// that still retains resources, which cannot be set up from a test. +func TestAccServiceNetworkingConnection_removePeering(t *testing.T) { + t.Parallel() + + network := fmt.Sprintf("tf-test-service-networking-connection-remove-peering-%s", acctest.RandString(t, 10)) + addr := fmt.Sprintf("tf-test-%s", acctest.RandString(t, 10)) + service := "servicenetworking.googleapis.com" + org_id := envvar.GetTestOrgFromEnv(t) + billing_account := envvar.GetTestBillingAccountFromEnv(t) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testServiceNetworkingConnectionDestroy(t, service, network), + Steps: []resource.TestStep{ + { + Config: testAccServiceNetworkingConnectionRemovePeering(network, addr, service, org_id, billing_account), + }, + { + ResourceName: "google_service_networking_connection.foobar", + ImportState: true, + ImportStateVerify: false, + }, + }, + }) +} + +func testAccServiceNetworkingConnectionRemovePeering(networkName, addressRangeName, serviceName, org_id, billing_account string) string { + return fmt.Sprintf(` +resource "google_project" "project" { + project_id = "%s" + name = "%s" + org_id = "%s" + billing_account = "%s" + deletion_policy = "DELETE" +} + +resource "google_project_service" "servicenetworking" { + project = google_project.project.project_id + service = "servicenetworking.googleapis.com" +} + +resource "google_compute_network" "servicenet" { + name = "%s" + depends_on = [google_project_service.servicenetworking] +} + +resource "google_compute_global_address" "foobar" { + name = "%s" + purpose = "VPC_PEERING" + address_type = "INTERNAL" + prefix_length = 16 + network = google_compute_network.servicenet.self_link + depends_on = [google_project_service.servicenetworking] +} + +resource "google_service_networking_connection" "foobar" { + network = google_compute_network.servicenet.self_link + service = "%s" + reserved_peering_ranges = [google_compute_global_address.foobar.name] + depends_on = [google_project_service.servicenetworking] + deletion_policy = "REMOVE_PEERING" +} +`, addressRangeName, addressRangeName, org_id, billing_account, networkName, addressRangeName, serviceName) +} + func TestAccServiceNetworkingConnection_reorder(t *testing.T) { t.Parallel() diff --git a/mmv1/third_party/terraform/website/docs/r/service_networking_connection.html.markdown b/mmv1/third_party/terraform/website/docs/r/service_networking_connection.html.markdown index 0f0639fba6c1..0392c20af785 100644 --- a/mmv1/third_party/terraform/website/docs/r/service_networking_connection.html.markdown +++ b/mmv1/third_party/terraform/website/docs/r/service_networking_connection.html.markdown @@ -64,11 +64,50 @@ The following arguments are supported: When a 'terraform destroy' or 'terraform apply' would delete the resource, the command will fail if this field is set to "PREVENT" in Terraform state. When set to "ABANDON", the command will remove the resource from Terraform - management without updating or deleting the resource in the API. + management without updating or deleting the resource in the API. The VPC + peering created by the connection is left in place, which will block deletion + of the network. + When set to "REMOVE_PEERING", the connection is deleted, and if the API + refuses because service producer resources still use it, the VPC peering is + removed from the network so that the network can be deleted. See + [Deleting a connection](#deleting-a-connection) below. When set to "DELETE" or any other value, deleting the resource is allowed. * `update_on_creation_fail` - (Optional) When set to true, enforce an update of the reserved peering ranges on the existing service networking connection in case of a new connection creation failure. +## Deleting a connection + +Creating this resource also creates a VPC network peering on the network, named +after the service. That peering is not a separate Terraform resource, so its +lifecycle is tied to this one. + +Before a connection can be deleted, every service instance reachable through it +must be deleted first, and the service producer must have released the resources +backing those instances. Producers may hold those resources for a period after +the instance itself is deleted. Cloud SQL, for example, retains them so that a +deleted instance can still be restored. Until they are released, deleting the +connection fails with: + +``` +Failed to delete connection; Producer services (e.g. CloudSQL, Cloud Memstore, etc.) +are still using this connection. +``` + +This also blocks `google_compute_network`, because a network cannot be deleted +while a peering references it. Setting `deletion_policy` to `"ABANDON"` drops the +connection from state but leaves the peering in place, so the network still +cannot be deleted. + +Setting `deletion_policy` to `"REMOVE_PEERING"` removes the peering when the +connection cannot be deleted, which allows the network to be deleted. Use it only +once the service instances are already deleted. Removing the peering while an +instance is still running will break that instance's connectivity. Note that this +removes the peering rather than the connection, so the connection may continue to +exist on the service producer side. + +For more detail, see +[Deleting a private connection](https://cloud.google.com/vpc/docs/configure-private-services-access#removing-connection). + ## Attributes Reference In addition to the arguments listed above, the following computed attributes are exported: