From a2415211364668d3e45feba0a40fe1754cb5035a Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Fri, 15 Mar 2024 09:14:09 +0000 Subject: [PATCH 01/11] Adding ipv6 support for bgp router peer, router interface and router. --- mmv1/products/compute/Router.yaml | 10 + ...ource_compute_router_bgp_peer_test.go.erb} | 244 ++++++++++++++++++ .../resource_compute_router_interface.go.erb | 21 ++ .../resource_compute_router_peer.go.erb | 105 ++++++++ 4 files changed, 380 insertions(+) rename mmv1/third_party/terraform/services/compute/{resource_compute_router_bgp_peer_test.go => resource_compute_router_bgp_peer_test.go.erb} (85%) diff --git a/mmv1/products/compute/Router.yaml b/mmv1/products/compute/Router.yaml index 7f868b4ed1a0..499a577d3147 100644 --- a/mmv1/products/compute/Router.yaml +++ b/mmv1/products/compute/Router.yaml @@ -187,6 +187,16 @@ properties: between the two peers. If set, this value must be between 20 and 60. The default is 20. default_value: 20 + - !ruby/object:Api::Type::String + name: identifierRange + default_from_api: true + min_version: beta + description: | + Explicitly specifies a range of valid BGP Identifiers for this Router. + It is provided as a link-local IPv4 range (from 169.254.0.0/16), of + size at least /30, even if the BGP sessions are over IPv6. It must + not overlap with any IPv4 BGP session ranges.Other vendors commonly + call this router ID. - !ruby/object:Api::Type::Boolean name: encryptedInterconnectRouter immutable: true diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb similarity index 85% rename from mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go rename to mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb index 37d15c254e31..3bf32222272f 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb @@ -1,3 +1,4 @@ +<% autogen_exception -%> package compute_test import ( @@ -206,6 +207,75 @@ func TestAccComputeRouterPeer_Ipv6Basic(t *testing.T) { }) } +<% unless version == 'ga' -%> +func TestAccComputeRouterPeer_Ipv4Basic(t *testing.T) { + t.Parallel() + + routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10)) + resourceName := "google_compute_router_peer.foobar" + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterPeerDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouterPeerIpv4(routerName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeRouterPeerExists( + t, resourceName), + resource.TestCheckResourceAttr(resourceName, "enable_ipv4", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccComputeRouterPeer_UpdateIpv4(t *testing.T) { + t.Parallel() + + routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10)) + resourceName := "google_compute_router_peer.foobar" + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterPeerDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouterPeerIpv4(routerName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeRouterPeerExists( + t, resourceName), + resource.TestCheckResourceAttr(resourceName, "enable_ipv6", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeRouterPeerUpdateIpv4Address(routerName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeRouterPeerExists( + t, resourceName), + resource.TestCheckResourceAttr(resourceName, "enable_ipv4", "true"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} +<% end -%> + func TestAccComputeRouterPeer_UpdateIpv6Address(t *testing.T) { t.Parallel() @@ -1480,3 +1550,177 @@ resource "google_compute_router_peer" "foobar" { } `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, enableIpv6) } + +<% unless version == 'ga' -%> +func testAccComputeRouterPeerIpv4(routerName string) string { + return fmt.Sprintf(`resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" + auto_create_subnetworks = false + } + + resource "google_compute_subnetwork" "foobar" { + provider = google-beta + name = "%s-subnet" + network = google_compute_network.foobar.self_link + ip_cidr_range = "10.0.0.0/16" + region = "us-central1" + stack_type = "IPV4_IPV6" + ipv6_access_type = "EXTERNAL" + } + + resource "google_compute_ha_vpn_gateway" "foobar" { + provider = google-beta + name = "%s-gateway" + network = google_compute_network.foobar.self_link + region = google_compute_subnetwork.foobar.region + stack_type = "IPV4_IPV6" + } + + resource "google_compute_external_vpn_gateway" "external_gateway" { + provider = google-beta + name = "%s-external-gateway" + redundancy_type = "SINGLE_IP_INTERNALLY_REDUNDANT" + description = "An externally managed VPN gateway" + interface { + id = 0 + ip_address = "8.8.8.8" + } + } + + resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + region = google_compute_subnetwork.foobar.region + network = google_compute_network.foobar.self_link + bgp { + asn = 64514 + } + } + + resource "google_compute_vpn_tunnel" "foobar" { + provider = google-beta + name = "%s-tunnel" + region = google_compute_subnetwork.foobar.region + vpn_gateway = google_compute_ha_vpn_gateway.foobar.id + peer_external_gateway = google_compute_external_vpn_gateway.external_gateway.id + peer_external_gateway_interface = 0 + shared_secret = "unguessable" + router = google_compute_router.foobar.name + vpn_gateway_interface = 0 + } + + resource "google_compute_router_interface" "foobar" { + provider = google-beta + name = "%s-interface" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + vpn_tunnel = google_compute_vpn_tunnel.foobar.name + ip_range = "fdff:1::1:1/126" + } + + resource "google_compute_router_peer" "foobar" { + provider = google-beta + name = "%s-peer" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + peer_asn = 65515 + advertised_route_priority = 100 + interface = google_compute_router_interface.foobar.name + ip_address = "fdff:1::1:1" + peer_ip_address = "fdff:1::1:2" + + enable_ipv4 = true + enable_ipv6 = true + ipv4_nexthop_address = "169.254.1.1" + peer_ipv4_nexthop_address = "169.254.1.2" + } + `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName) +} + +func testAccComputeRouterPeerUpdateIpv4Address(routerName string) string { + return fmt.Sprintf(`resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" + auto_create_subnetworks = false + } + + resource "google_compute_subnetwork" "foobar" { + provider = google-beta + name = "%s-subnet" + network = google_compute_network.foobar.self_link + ip_cidr_range = "10.0.0.0/16" + region = "us-central1" + stack_type = "IPV4_IPV6" + ipv6_access_type = "EXTERNAL" + } + + resource "google_compute_ha_vpn_gateway" "foobar" { + provider = google-beta + name = "%s-gateway" + network = google_compute_network.foobar.self_link + region = google_compute_subnetwork.foobar.region + stack_type = "IPV4_IPV6" + } + + resource "google_compute_external_vpn_gateway" "external_gateway" { + provider = google-beta + name = "%s-external-gateway" + redundancy_type = "SINGLE_IP_INTERNALLY_REDUNDANT" + description = "An externally managed VPN gateway" + interface { + id = 0 + ip_address = "8.8.8.8" + } + } + + resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + region = google_compute_subnetwork.foobar.region + network = google_compute_network.foobar.self_link + bgp { + asn = 64514 + } + } + + resource "google_compute_vpn_tunnel" "foobar" { + provider = google-beta + name = "%s-tunnel" + region = google_compute_subnetwork.foobar.region + vpn_gateway = google_compute_ha_vpn_gateway.foobar.id + peer_external_gateway = google_compute_external_vpn_gateway.external_gateway.id + peer_external_gateway_interface = 0 + shared_secret = "unguessable" + router = google_compute_router.foobar.name + vpn_gateway_interface = 0 + } + + resource "google_compute_router_interface" "foobar" { + provider = google-beta + name = "%s-interface" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + vpn_tunnel = google_compute_vpn_tunnel.foobar.name + ip_range = "fdff:1::1:1/126" + } + + resource "google_compute_router_peer" "foobar" { + provider = google-beta + name = "%s-peer" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + peer_asn = 65515 + advertised_route_priority = 100 + interface = google_compute_router_interface.foobar.name + ip_address = "fdff:1::1:1" + peer_ip_address = "fdff:1::1:2" + + enable_ipv4 = true + enable_ipv6 = true + ipv4_nexthop_address = "169.254.1.2" + peer_ipv4_nexthop_address = "169.254.1.1" + } + `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName) +} +<% end -%> diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb index 807a9c443d4d..fb7f345e80cb 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-google/google/verify" "google.golang.org/api/googleapi" <% if version == "ga" -%> @@ -80,6 +81,15 @@ func ResourceComputeRouterInterface() *schema.Resource { AtLeastOneOf: []string{"ip_range", "interconnect_attachment", "subnetwork", "vpn_tunnel"}, Description: `The IP address and range of the interface. The IP range must be in the RFC3927 link-local IP space. Changing this forces a new interface to be created.`, }, + <% unless version == 'ga' -%> + "ip_version": { + Type: schema.TypeString, + Optional: true, + ForceNew: true, + ValidateFunc: verify.ValidateEnum([]string{"IPV4", "IPV6"}), + Description: `IP version of this interface.`, + }, + <% end -%> "private_ip_address": { Type: schema.TypeString, Optional: true, @@ -178,6 +188,12 @@ func resourceComputeRouterInterfaceCreate(d *schema.ResourceData, meta interface iface.IpRange = ipRangeVal.(string) } + <% unless version == 'ga' -%> + if ipVersionVal, ok := d.GetOk("ip_version"); ok { + iface.IpVersion = ipVersionVal.(string) + } + <% end -%> + if privateIpVal, ok := d.GetOk("private_ip_address"); ok { iface.PrivateIpAddress = privateIpVal.(string) } @@ -269,6 +285,11 @@ func resourceComputeRouterInterfaceRead(d *schema.ResourceData, meta interface{} if err := d.Set("ip_range", iface.IpRange); err != nil { return fmt.Errorf("Error setting ip_range: %s", err) } + <% unless version == 'ga' -%> + if err := d.Set("ip_version", iface.IpVersion); err != nil { + return fmt.Errorf("Error setting ip_version: %s", err) + } + <% end -%> if err := d.Set("private_ip_address", iface.PrivateIpAddress); err != nil { return fmt.Errorf("Error setting private_ip_address: %s", err) } diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb index 76d3e7c5e4bf..9a633d8bbe0d 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb @@ -207,6 +207,14 @@ The default is true.`, Description: `Enable IPv6 traffic over BGP Peer. If not specified, it is disabled by default.`, Default: false, }, + <% unless version == 'ga' -%> + "enable_ipv4": { + Type: schema.TypeBool, + Optional: true, + Description: `Enable IPv4 traffic over BGP Peer. It is enabled by default if the peerIpAddress is version 4.`, + Default: false, + }, + <% end -%> "ip_address": { Type: schema.TypeString, Computed: true, @@ -225,6 +233,15 @@ The address must be in the range 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64. If you do not specify the next hop addresses, Google Cloud automatically assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range for you.`, }, + <% unless version == 'ga' -%> + "ipv4_nexthop_address": { + Type: schema.TypeString, + Computed: true, + Optional: true, + ValidateFunc: verify.ValidateIpAddress, + Description: `IPv4 address of the interface inside Google Cloud Platform.`, + }, + <% end -%> "peer_ip_address": { Type: schema.TypeString, Computed: true, @@ -243,6 +260,15 @@ The address must be in the range 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64. If you do not specify the next hop addresses, Google Cloud automatically assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range for you.`, }, + <% unless version == 'ga' -%> + "peer_ipv4_nexthop_address": { + Type: schema.TypeString, + Computed: true, + Optional: true, + ValidateFunc: verify.ValidateIpAddress, + Description: `IPv4 address of the BGP interface outside Google Cloud Platform.`, + }, + <% end -%> "region": { Type: schema.TypeString, Computed: true, @@ -395,6 +421,26 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("enable_ipv6"); ok || !reflect.DeepEqual(v, enableIpv6Prop) { obj["enableIpv6"] = enableIpv6Prop } + <% unless version == 'ga' -%> + enableIpv4Prop, err := expandNestedComputeRouterBgpPeerEnableIpv4(d.Get("enable_ipv4"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { + obj["enableIpv4"] = enableIpv4Prop + } + ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("ipv4_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(ipv4NexthopAddressProp)) && (ok || !reflect.DeepEqual(v, ipv4NexthopAddressProp)) { + obj["ipv4NexthopAddress"] = ipv4NexthopAddressProp + } + peerIpv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(d.Get("peer_ipv4_nexthop_address"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("peer_ipv6_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(peerIpv4NexthopAddressProp)) && (ok || !reflect.DeepEqual(v, peerIpv4NexthopAddressProp)) { + obj["peerIpv4NexthopAddress"] = peerIpv4NexthopAddressProp + } + <% end -%> ipv6NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv6NexthopAddress(d.Get("ipv6_nexthop_address"), d, config) if err != nil { return err @@ -586,6 +632,17 @@ func resourceComputeRouterBgpPeerRead(d *schema.ResourceData, meta interface{}) if err := d.Set("enable_ipv6", flattenNestedComputeRouterBgpPeerEnableIpv6(res["enableIpv6"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } + <% unless version == 'ga' -%> + if err := d.Set("enable_ipv4", flattenNestedComputeRouterBgpPeerEnableIpv4(res["enableIpv4"], d, config)); err != nil { + return fmt.Errorf("Error reading RouterBgpPeer: %s", err) + } + if err := d.Set("ipv4_nexthop_address", flattenNestedComputeRouterBgpPeerIpv4NexthopAddress(res["ipv4NexthopAddress"], d, config)); err != nil { + return fmt.Errorf("Error reading RouterBgpPeer: %s", err) + } + if err := d.Set("peer_ipv4_nexthop_address", flattenNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(res["peerIpv4NexthopAddress"], d, config)); err != nil { + return fmt.Errorf("Error reading RouterBgpPeer: %s", err) + } + <% end -%> if err := d.Set("ipv6_nexthop_address", flattenNestedComputeRouterBgpPeerIpv6NexthopAddress(res["ipv6NexthopAddress"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } @@ -681,6 +738,26 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("enable_ipv6"); ok || !reflect.DeepEqual(v, enableIpv6Prop) { obj["enableIpv6"] = enableIpv6Prop } + <% unless version == 'ga' -%> + enableIpv4Prop, err := expandNestedComputeRouterBgpPeerEnableIpv4(d.Get("enable_ipv4"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { + obj["enableIpv4"] = enableIpv4Prop + } + ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("ipv4_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(ipv4NexthopAddressProp)) && (ok || !reflect.DeepEqual(v, ipv4NexthopAddressProp)) { + obj["ipv4NexthopAddress"] = ipv4NexthopAddressProp + } + peerIpv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(d.Get("peer_ipv4_nexthop_address"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("peer_ipv4_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, peerIpv4NexthopAddressProp)) { + obj["peerIpv4NexthopAddress"] = peerIpv4NexthopAddressProp + } + <% end -%> ipv6NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv6NexthopAddress(d.Get("ipv6_nexthop_address"), d, config) if err != nil { return err @@ -1054,6 +1131,20 @@ func flattenNestedComputeRouterBgpPeerEnableIpv6(v interface{}, d *schema.Resour return v } +<% unless version == 'ga' -%> +func flattenNestedComputeRouterBgpPeerEnableIpv4(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { + return v +} + +func flattenNestedComputeRouterBgpPeerIpv4NexthopAddress(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { + return v +} + +func flattenNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { + return v +} +<% end -%> + func flattenNestedComputeRouterBgpPeerIpv6NexthopAddress(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { return v } @@ -1241,6 +1332,20 @@ func expandNestedComputeRouterBgpPeerEnableIpv6(v interface{}, d tpgresource.Ter return v, nil } +<% unless version == 'ga' -%> +func expandNestedComputeRouterBgpPeerEnableIpv4(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { + return v, nil +} + +func expandNestedComputeRouterBgpPeerIpv4NexthopAddress(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { + return v, nil +} + +func expandNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { + return v, nil +} +<% end-%> + func expandNestedComputeRouterBgpPeerIpv6NexthopAddress(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { return v, nil } From 026256b3fc97c693f02f3e0e67e32dc2353ccd57 Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Tue, 19 Mar 2024 12:56:44 +0000 Subject: [PATCH 02/11] Added tests for interface and router new fields --- ...urce_compute_router_interface_test.go.erb} | 1 + ...go => resource_compute_router_test.go.erb} | 123 ++++++++++++++++++ 2 files changed, 124 insertions(+) rename mmv1/third_party/terraform/services/compute/{resource_compute_router_interface_test.go => resource_compute_router_interface_test.go.erb} (99%) rename mmv1/third_party/terraform/services/compute/{resource_compute_router_test.go => resource_compute_router_test.go.erb} (66%) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb similarity index 99% rename from mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go rename to mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb index 036d8685119c..1021bfffd225 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb @@ -1,3 +1,4 @@ +<% autogen_exception -%> package compute_test import ( diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go b/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb similarity index 66% rename from mmv1/third_party/terraform/services/compute/resource_compute_router_test.go rename to mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb index 267cdbe42d18..24a5f4e55d40 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb @@ -1,3 +1,6 @@ +<% autogen_exception -%> +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 package compute_test import ( @@ -155,6 +158,66 @@ func TestAccComputeRouter_updateAddRemoveBGP(t *testing.T) { }) } +<% unless version == 'ga' -%> +func TestAccCompouteRouter_addIdentifierRangeBgp(t *testing T){ + t.Parallel() + + testId := acctest.RandString(t, 10) + routerName := fmt.Sprintf("tf-test-router-%s", testId) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouter_addIdentifierRangeBgp(routerName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("google_compute_router.foobar", "bgp.0.identifier_range", "169.254.8.8/29"), + ), + }, + { + ResourceName: "google_compute_router.foobar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccComputeRouter_updateIdentifierRangeBgp(t *testing.T) { + t.Parallel() + + testId := acctest.RandString(t, 10) + routerName := fmt.Sprintf("tf-test-router-%s", testId) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouter_addIdentifierRangeBgp(routerName), + }, + { + ResourceName: "google_compute_router.foobar", + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeRouter_updateIdentifierRangeBgp(routerName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("google_compute_router.foobar", "bgp.0.identifier_range", "169.254.8.8/30"), + ), + }, + { + ResourceName: "google_compute_router.foobar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} +<% end -%> + func testAccComputeRouterBasic(routerName, resourceRegion string) string { return fmt.Sprintf(` resource "google_compute_network" "foobar" { @@ -251,3 +314,63 @@ resource "google_compute_router" "foobar" { } `, routerName, routerName, resourceRegion, routerName) } + +<% unless version == 'ga' -%> +func testAccComputeRouter_addIdentifierRangeBgp(routerName string) string { + return fmt.Sprintf(` +resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" + auto_create_subnetworks = false +} + +resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + network = google_compute_network.foobar.name + bgp { + asn = 64514 + advertise_mode = "CUSTOM" + advertised_groups = ["ALL_SUBNETS"] + advertised_ip_ranges { + range = "1.2.3.4" + } + advertised_ip_ranges { + range = "6.7.0.0/16" + } + identifier_range = "169.254.8.8/29" + keepalive_interval = 25 + } +} +`, routerName, routerName) +} + +func testAccComputeRouter_updateIdentifierRangeBgp(routerName string) string { + return fmt.Sprintf(` +resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" + auto_create_subnetworks = false +} + +resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + network = google_compute_network.foobar.name + bgp { + asn = 64514 + advertise_mode = "CUSTOM" + advertised_groups = ["ALL_SUBNETS"] + advertised_ip_ranges { + range = "1.2.3.4" + } + advertised_ip_ranges { + range = "6.7.0.0/16" + } + identifier_range = "169.254.8.8/30" + keepalive_interval = 25 + } +} +`, routerName, routerName) +} +<% end -%> \ No newline at end of file From 22a03a3dec6dae6e7ae603d7651aede6fb502a13 Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Thu, 21 Mar 2024 17:57:48 +0000 Subject: [PATCH 03/11] Added tests for field ip_version in google_compute_router_interface resource --- ...ource_compute_router_interface_test.go.erb | 118 ++++++++++++++++++ .../resource_compute_router_test.go.erb | 4 +- 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb index 1021bfffd225..041918cc5ce6 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface_test.go.erb @@ -120,6 +120,54 @@ func TestAccComputeRouterInterface_withPrivateIpAddress(t *testing.T) { }) } +<% unless version == 'ga' -%> +func TestAccComputeRouterInterface_withIPVersionV4(t *testing.T) { + t.Parallel() + + routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10)) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterInterfaceDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouterInterfaceWithIpVersionIPV4(routerName), + Check: testAccCheckComputeRouterInterfaceExists( + t, "google_compute_router_interface.foobar"), + }, + { + ResourceName: "google_compute_router_interface.foobar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccComputeRouterInterface_withIPVersionV6(t *testing.T) { + t.Parallel() + + routerName := fmt.Sprintf("tf-test-router-%s", acctest.RandString(t, 10)) + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderBetaFactories(t), + CheckDestroy: testAccCheckComputeRouterInterfaceDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouterInterfaceWithIpVersionIPV6(routerName), + Check: testAccCheckComputeRouterInterfaceExists( + t, "google_compute_router_interface.foobar"), + }, + { + ResourceName: "google_compute_router_interface.foobar", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} +<% end -%> + func testAccCheckComputeRouterInterfaceDestroyProducer(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { config := acctest.GoogleProviderConfig(t) @@ -512,3 +560,73 @@ resource "google_compute_router_interface" "foobar" { } `, routerName, routerName, routerName, routerName, routerName) } + +<% unless version == 'ga' -%> +func testAccComputeRouterInterfaceWithIpVersionIPV6(routerName string) string { + return fmt.Sprintf(` +resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" +} + +resource "google_compute_subnetwork" "foobar" { + provider = google-beta + name = "%s-subnet" + network = google_compute_network.foobar.self_link + ip_cidr_range = "10.0.0.0/16" +} + +resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + network = google_compute_network.foobar.self_link + bgp { + asn = 64514 + } +} + +resource "google_compute_router_interface" "foobar" { + provider = google-beta + name = "%s-interface" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + ip_range = "fdff:1::1:1/126" + ip_version = "IPV6" +} +`, routerName, routerName, routerName, routerName) +} + +func testAccComputeRouterInterfaceWithIpVersionIPV4(routerName string) string { + return fmt.Sprintf(` +resource "google_compute_network" "foobar" { + provider = google-beta + name = "%s-net" +} + +resource "google_compute_subnetwork" "foobar" { + provider = google-beta + name = "%s-subnet" + network = google_compute_network.foobar.self_link + ip_cidr_range = "10.0.0.0/16" +} + +resource "google_compute_router" "foobar" { + provider = google-beta + name = "%s" + network = google_compute_network.foobar.self_link + bgp { + asn = 64514 + } +} + +resource "google_compute_router_interface" "foobar" { + provider = google-beta + name = "%s-interface" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + ip_range = "169.254.3.1/30" + ip_version = "IPV4" +} +`, routerName, routerName, routerName, routerName) +} +<% end -%> \ No newline at end of file diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb index 24a5f4e55d40..a1ff3dad0230 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_test.go.erb @@ -1,6 +1,4 @@ <% autogen_exception -%> -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 package compute_test import ( @@ -159,7 +157,7 @@ func TestAccComputeRouter_updateAddRemoveBGP(t *testing.T) { } <% unless version == 'ga' -%> -func TestAccCompouteRouter_addIdentifierRangeBgp(t *testing T){ +func TestAccCompouteRouter_addIdentifierRangeBgp(t *testing.T){ t.Parallel() testId := acctest.RandString(t, 10) From 5dc0990a6cef27e968c4532fc23a8f825eea76d4 Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Mon, 1 Apr 2024 09:19:33 +0000 Subject: [PATCH 04/11] Fixing build issue --- .../services/compute/resource_compute_router_interface.go.erb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb index fb7f345e80cb..b0022c73bd94 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb @@ -13,7 +13,10 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + <% if version == "ga" -%> + <% else -%> "github.com/hashicorp/terraform-provider-google/google/verify" + <% end -%> "google.golang.org/api/googleapi" <% if version == "ga" -%> From e463d66a7dec42619344ac75e85eff31f7ba3dd0 Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Wed, 3 Apr 2024 12:35:03 +0000 Subject: [PATCH 05/11] fixing tests --- ...source_compute_router_bgp_peer_test.go.erb | 50 ++++++++++++++++-- .../resource_compute_router_interface.go.erb | 3 +- .../resource_compute_router_peer.go.erb | 51 ++++++++++++++----- 3 files changed, 84 insertions(+), 20 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb index 3bf32222272f..b82db04d7980 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb @@ -674,6 +674,17 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { router = google_compute_router.foobar.name vpn_gateway_interface = 0 } + + resource "google_compute_vpn_tunnel" "foobar1" { + name = "%s1" + region = google_compute_subnetwork.foobar.region + vpn_gateway = google_compute_ha_vpn_gateway.foobar.id + peer_external_gateway = google_compute_external_vpn_gateway.external_gateway.id + peer_external_gateway_interface = 0 + shared_secret = "unguessable" + router = google_compute_router.foobar.name + vpn_gateway_interface = 1 + } resource "google_compute_router_interface" "foobar" { name = "%s" @@ -687,7 +698,7 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { name = "%s1" router = google_compute_router.foobar.name region = google_compute_router.foobar.region - vpn_tunnel = google_compute_vpn_tunnel.foobar.name + vpn_tunnel = google_compute_vpn_tunnel.foobar1.name ip_range = "169.254.4.1/30" depends_on = [ google_compute_router_interface.foobar @@ -702,6 +713,9 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name peer_ip_address = "169.254.3.2" + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> md5_authentication_key { name = "%s-peer-key" key = "%s-peer-key-value" @@ -716,6 +730,9 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar1.name peer_ip_address = "169.254.4.2" + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> md5_authentication_key { name = "%s-peer1-key" key = "%s-peer1-key-value" @@ -725,7 +742,7 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { ] } `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, - routerName) + routerName, routerName) } func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { @@ -782,6 +799,17 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { router = google_compute_router.foobar.name vpn_gateway_interface = 0 } + + resource "google_compute_vpn_tunnel" "foobar1" { + name = "%s1" + region = google_compute_subnetwork.foobar.region + vpn_gateway = google_compute_ha_vpn_gateway.foobar.id + peer_external_gateway = google_compute_external_vpn_gateway.external_gateway.id + peer_external_gateway_interface = 0 + shared_secret = "unguessable" + router = google_compute_router.foobar.name + vpn_gateway_interface = 1 + } resource "google_compute_router_interface" "foobar" { name = "%s" @@ -795,7 +823,7 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { name = "%s1" router = google_compute_router.foobar.name region = google_compute_router.foobar.region - vpn_tunnel = google_compute_vpn_tunnel.foobar.name + vpn_tunnel = google_compute_vpn_tunnel.foobar1.name ip_range = "169.254.4.1/30" depends_on = [ google_compute_router_interface.foobar @@ -810,6 +838,9 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name peer_ip_address = "169.254.3.2" + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> md5_authentication_key { name = "%s-peer-key" key = "%s-peer-key-value" @@ -824,6 +855,9 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar1.name peer_ip_address = "169.254.4.2" + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> md5_authentication_key { name = "%s-peer1-key" key = "%s-peer1-key-value-changed" @@ -833,7 +867,7 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { ] } `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, - routerName) + routerName, routerName) } func testAccComputeRouterPeerKeepRouter(routerName string) string { @@ -1467,8 +1501,12 @@ resource "google_compute_router_peer" "foobar" { peer_asn = 65515 advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> enable_ipv6 = %v + } `, routerName, routerName, routerName, routerName, routerName, routerName, routerName, routerName, enableIpv6) } @@ -1543,7 +1581,9 @@ resource "google_compute_router_peer" "foobar" { peer_asn = 65515 advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name - + <% unless version == 'ga' -%> + enable_ipv4 = true + <% end -%> enable_ipv6 = %v ipv6_nexthop_address = "2600:2d00:0000:0002:0000:0000:0000:0001" peer_ipv6_nexthop_address = "2600:2d00:0:2::2" diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb index b0022c73bd94..e9d6b9d8baf5 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_interface.go.erb @@ -88,7 +88,8 @@ func ResourceComputeRouterInterface() *schema.Resource { "ip_version": { Type: schema.TypeString, Optional: true, - ForceNew: true, + ForceNew: true, + Computed: true, ValidateFunc: verify.ValidateEnum([]string{"IPV4", "IPV6"}), Description: `IP version of this interface.`, }, diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb index 9a633d8bbe0d..f94529a784cc 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb @@ -32,6 +32,15 @@ func ipv6RepresentationDiffSuppress(_, old, new string, d *schema.ResourceData) return oldIp.Equal(newIp) } +func isIPv4Address(input string) bool { + ip := net.ParseIP(input) + return ip != nil && ip.To4() != nil +} + +func interfaceToString(value interface{}) string { + return fmt.Sprintf("%v", value) +} + func ResourceComputeRouterBgpPeer() *schema.Resource { return &schema.Resource{ Create: resourceComputeRouterBgpPeerCreate, @@ -207,14 +216,14 @@ The default is true.`, Description: `Enable IPv6 traffic over BGP Peer. If not specified, it is disabled by default.`, Default: false, }, - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> "enable_ipv4": { Type: schema.TypeBool, Optional: true, Description: `Enable IPv4 traffic over BGP Peer. It is enabled by default if the peerIpAddress is version 4.`, Default: false, }, - <% end -%> +<% end -%> "ip_address": { Type: schema.TypeString, Computed: true, @@ -233,7 +242,7 @@ The address must be in the range 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64. If you do not specify the next hop addresses, Google Cloud automatically assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range for you.`, }, - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> "ipv4_nexthop_address": { Type: schema.TypeString, Computed: true, @@ -241,7 +250,7 @@ assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range ValidateFunc: verify.ValidateIpAddress, Description: `IPv4 address of the interface inside Google Cloud Platform.`, }, - <% end -%> +<% end -%> "peer_ip_address": { Type: schema.TypeString, Computed: true, @@ -260,7 +269,7 @@ The address must be in the range 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64. If you do not specify the next hop addresses, Google Cloud automatically assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range for you.`, }, - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> "peer_ipv4_nexthop_address": { Type: schema.TypeString, Computed: true, @@ -268,7 +277,7 @@ assigns unused addresses from the 2600:2d00:0:2::/64 or 2600:2d00:0:3::/64 range ValidateFunc: verify.ValidateIpAddress, Description: `IPv4 address of the BGP interface outside Google Cloud Platform.`, }, - <% end -%> +<% end -%> "region": { Type: schema.TypeString, Computed: true, @@ -421,12 +430,19 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("enable_ipv6"); ok || !reflect.DeepEqual(v, enableIpv6Prop) { obj["enableIpv6"] = enableIpv6Prop } - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> enableIpv4Prop, err := expandNestedComputeRouterBgpPeerEnableIpv4(d.Get("enable_ipv4"), d, config) if err != nil { return err } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { - obj["enableIpv4"] = enableIpv4Prop + //If peerIpaddress is ipv4, then enable_ipv4 should be true + peerIpAddressString := interfaceToString(peerIpAddressProp) + isIpV4 := isIPv4Address(peerIpAddressString) + if isIpV4 { + obj["enableIpv4"] = interface{}(true) + } else { + obj["enableIpv4"] = enableIpv4Prop + } } ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) if err != nil { @@ -440,7 +456,7 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("peer_ipv6_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(peerIpv4NexthopAddressProp)) && (ok || !reflect.DeepEqual(v, peerIpv4NexthopAddressProp)) { obj["peerIpv4NexthopAddress"] = peerIpv4NexthopAddressProp } - <% end -%> +<% end -%> ipv6NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv6NexthopAddress(d.Get("ipv6_nexthop_address"), d, config) if err != nil { return err @@ -632,7 +648,7 @@ func resourceComputeRouterBgpPeerRead(d *schema.ResourceData, meta interface{}) if err := d.Set("enable_ipv6", flattenNestedComputeRouterBgpPeerEnableIpv6(res["enableIpv6"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> if err := d.Set("enable_ipv4", flattenNestedComputeRouterBgpPeerEnableIpv4(res["enableIpv4"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } @@ -642,7 +658,7 @@ func resourceComputeRouterBgpPeerRead(d *schema.ResourceData, meta interface{}) if err := d.Set("peer_ipv4_nexthop_address", flattenNestedComputeRouterBgpPeerPeerIpv4NexthopAddress(res["peerIpv4NexthopAddress"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } - <% end -%> +<% end -%> if err := d.Set("ipv6_nexthop_address", flattenNestedComputeRouterBgpPeerIpv6NexthopAddress(res["ipv6NexthopAddress"], d, config)); err != nil { return fmt.Errorf("Error reading RouterBgpPeer: %s", err) } @@ -738,12 +754,19 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("enable_ipv6"); ok || !reflect.DeepEqual(v, enableIpv6Prop) { obj["enableIpv6"] = enableIpv6Prop } - <% unless version == 'ga' -%> +<% unless version == 'ga' -%> enableIpv4Prop, err := expandNestedComputeRouterBgpPeerEnableIpv4(d.Get("enable_ipv4"), d, config) if err != nil { return err } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { - obj["enableIpv4"] = enableIpv4Prop + //If peerIpaddress is ipv4, then enable_ipv4 should be true + peerIpAddressString := interfaceToString(peerIpAddressProp) + isIpV4 := isIPv4Address(peerIpAddressString) + if isIpV4 { + obj["enableIpv4"] = interface{}(true) + } else { + obj["enableIpv4"] = enableIpv4Prop + } } ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) if err != nil { @@ -757,7 +780,7 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{} } else if v, ok := d.GetOkExists("peer_ipv4_nexthop_address"); !tpgresource.IsEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, peerIpv4NexthopAddressProp)) { obj["peerIpv4NexthopAddress"] = peerIpv4NexthopAddressProp } - <% end -%> +<% end -%> ipv6NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv6NexthopAddress(d.Get("ipv6_nexthop_address"), d, config) if err != nil { return err From 12f24286835b581b77fcf268bfdd617434d4b320 Mon Sep 17 00:00:00 2001 From: Shivang Dixit Date: Thu, 4 Apr 2024 10:03:29 +0000 Subject: [PATCH 06/11] Enabling computed flag for enable_ipv4 field in router peer --- ...source_compute_router_bgp_peer_test.go.erb | 19 --------------- .../resource_compute_router_peer.go.erb | 23 ++++--------------- 2 files changed, 5 insertions(+), 37 deletions(-) diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb index b82db04d7980..09f7754bfc49 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_bgp_peer_test.go.erb @@ -713,9 +713,6 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name peer_ip_address = "169.254.3.2" - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> md5_authentication_key { name = "%s-peer-key" key = "%s-peer-key-value" @@ -730,9 +727,6 @@ func testAccComputeRouterPeerWithMd5AuthKey(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar1.name peer_ip_address = "169.254.4.2" - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> md5_authentication_key { name = "%s-peer1-key" key = "%s-peer1-key-value" @@ -838,9 +832,6 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name peer_ip_address = "169.254.3.2" - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> md5_authentication_key { name = "%s-peer-key" key = "%s-peer-key-value" @@ -855,9 +846,6 @@ func testAccComputeRouterPeerWithMd5AuthKeyUpdate(routerName string) string { advertised_route_priority = 100 interface = google_compute_router_interface.foobar1.name peer_ip_address = "169.254.4.2" - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> md5_authentication_key { name = "%s-peer1-key" key = "%s-peer1-key-value-changed" @@ -1501,10 +1489,6 @@ resource "google_compute_router_peer" "foobar" { peer_asn = 65515 advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> - enable_ipv6 = %v } @@ -1581,9 +1565,6 @@ resource "google_compute_router_peer" "foobar" { peer_asn = 65515 advertised_route_priority = 100 interface = google_compute_router_interface.foobar.name - <% unless version == 'ga' -%> - enable_ipv4 = true - <% end -%> enable_ipv6 = %v ipv6_nexthop_address = "2600:2d00:0000:0002:0000:0000:0000:0001" peer_ipv6_nexthop_address = "2600:2d00:0:2::2" diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb index f94529a784cc..fccdb8eafe07 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_peer.go.erb @@ -221,7 +221,7 @@ The default is true.`, Type: schema.TypeBool, Optional: true, Description: `Enable IPv4 traffic over BGP Peer. It is enabled by default if the peerIpAddress is version 4.`, - Default: false, + Computed: true, }, <% end -%> "ip_address": { @@ -435,14 +435,7 @@ func resourceComputeRouterBgpPeerCreate(d *schema.ResourceData, meta interface{} if err != nil { return err } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { - //If peerIpaddress is ipv4, then enable_ipv4 should be true - peerIpAddressString := interfaceToString(peerIpAddressProp) - isIpV4 := isIPv4Address(peerIpAddressString) - if isIpV4 { - obj["enableIpv4"] = interface{}(true) - } else { - obj["enableIpv4"] = enableIpv4Prop - } + obj["enableIpv4"] = enableIpv4Prop } ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) if err != nil { @@ -759,14 +752,7 @@ func resourceComputeRouterBgpPeerUpdate(d *schema.ResourceData, meta interface{} if err != nil { return err } else if v, ok := d.GetOkExists("enable_ipv4"); ok || !reflect.DeepEqual(v, enableIpv4Prop) { - //If peerIpaddress is ipv4, then enable_ipv4 should be true - peerIpAddressString := interfaceToString(peerIpAddressProp) - isIpV4 := isIPv4Address(peerIpAddressString) - if isIpV4 { - obj["enableIpv4"] = interface{}(true) - } else { - obj["enableIpv4"] = enableIpv4Prop - } + obj["enableIpv4"] = enableIpv4Prop } ipv4NexthopAddressProp, err := expandNestedComputeRouterBgpPeerIpv4NexthopAddress(d.Get("ipv4_nexthop_address"), d, config) if err != nil { @@ -1156,6 +1142,7 @@ func flattenNestedComputeRouterBgpPeerEnableIpv6(v interface{}, d *schema.Resour <% unless version == 'ga' -%> func flattenNestedComputeRouterBgpPeerEnableIpv4(v interface{}, d *schema.ResourceData, config *transport_tpg.Config) interface{} { + log.Printf("[DEBUG] printing value for enable_ipv4 that we are going to set %v)", v) return v } @@ -1637,4 +1624,4 @@ func resourceComputeRouterBgpPeerListForPatch(d *schema.ResourceData, meta inter } else { return nil, nil, nil } -} +} \ No newline at end of file From c1a0366bb578774d091a381c119e3d94f6d81d48 Mon Sep 17 00:00:00 2001 From: Patrick Costello Date: Thu, 4 Apr 2024 10:55:10 -0700 Subject: [PATCH 07/11] test: Fix missing database for datastore_index test. (#10316) --- mmv1/products/datastore/Index.yaml | 2 ++ .../terraform/examples/datastore_index.tf.erb | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/mmv1/products/datastore/Index.yaml b/mmv1/products/datastore/Index.yaml index cd2baffce40f..a88b121f13b9 100644 --- a/mmv1/products/datastore/Index.yaml +++ b/mmv1/products/datastore/Index.yaml @@ -42,6 +42,8 @@ examples: - !ruby/object:Provider::Terraform::Examples name: 'datastore_index' primary_resource_id: 'default' + test_env_vars: + project_id: :PROJECT_NAME vars: property_name_1: 'property_a' property_name_2: 'property_b' diff --git a/mmv1/templates/terraform/examples/datastore_index.tf.erb b/mmv1/templates/terraform/examples/datastore_index.tf.erb index 0d9e5dcc016f..8e1917ffad40 100644 --- a/mmv1/templates/terraform/examples/datastore_index.tf.erb +++ b/mmv1/templates/terraform/examples/datastore_index.tf.erb @@ -1,3 +1,16 @@ +resource "google_firestore_database" "database" { + project = "<%= ctx[:test_env_vars]['project_id'] %>" + # google_datastore_index resources only support the (default) database. + # However, google_firestore_index can express any Datastore Mode index + # and should be preferred in all cases. + name = "(default)" + location_id = "nam5" + type = "DATASTORE_MODE" + + delete_protection_state = "DELETE_PROTECTION_DISABLED" + deletion_policy = "DELETE" +} + resource "google_datastore_index" "<%= ctx[:primary_resource_id] %>" { kind = "foo" properties { @@ -8,4 +21,6 @@ resource "google_datastore_index" "<%= ctx[:primary_resource_id] %>" { name = "<%= ctx[:vars]['property_name_2'] %>" direction = "ASCENDING" } + + depends_on = [google_firestore_database.database] } From e3a58578fdf9347fdbe648750268ee1349a896a4 Mon Sep 17 00:00:00 2001 From: matheusaleixo-cit <82680416+matheusaleixo-cit@users.noreply.github.com> Date: Thu, 4 Apr 2024 14:58:10 -0300 Subject: [PATCH 08/11] Added "endpointTypes" support to compute router nat resource (#10338) --- mmv1/products/compute/RouterNat.yaml | 11 +++ .../resource_compute_router_nat_test.go.erb | 94 +++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/mmv1/products/compute/RouterNat.yaml b/mmv1/products/compute/RouterNat.yaml index fef5eb3398c2..f108e041366d 100644 --- a/mmv1/products/compute/RouterNat.yaml +++ b/mmv1/products/compute/RouterNat.yaml @@ -308,6 +308,17 @@ properties: - :ERRORS_ONLY - :TRANSLATIONS_ONLY - :ALL + - !ruby/object:Api::Type::Array + name: 'endpointTypes' + immutable: true + min_size: 1 + description: | + Specifies the endpoint Types supported by the NAT Gateway. + Supported values include: + `ENDPOINT_TYPE_VM`, `ENDPOINT_TYPE_SWG`, + `ENDPOINT_TYPE_MANAGED_PROXY_LB`. + default_from_api: true + item_type: Api::Type::String - !ruby/object:Api::Type::Array name: rules description: 'A list of rules associated with this NAT.' diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_router_nat_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_router_nat_test.go.erb index 7436c9ccfa45..aad9ce68aeda 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_router_nat_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_router_nat_test.go.erb @@ -410,6 +410,66 @@ func TestAccComputeRouterNat_withNatRules(t *testing.T) { }) } +func TestAccComputeRouterNat_withEndpointTypes(t *testing.T) { + t.Parallel() + + testId := acctest.RandString(t, 10) + routerName := fmt.Sprintf("tf-test-router-nat-%s", testId) + testResourceName := "google_compute_router_nat.foobar" + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckComputeRouterNatDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccComputeRouterNatBasic(routerName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(testResourceName, "endpoint_types.0", "ENDPOINT_TYPE_VM"), + ), + }, + { + ResourceName: testResourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeRouterNatUpdateEndpointType(routerName, "ENDPOINT_TYPE_SWG"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(testResourceName, "endpoint_types.0", "ENDPOINT_TYPE_SWG"), + ), + }, + { + ResourceName: testResourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeRouterNatUpdateEndpointType(routerName, "ENDPOINT_TYPE_VM"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(testResourceName, "endpoint_types.0", "ENDPOINT_TYPE_VM"), + ), + }, + { + ResourceName: testResourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeRouterNatUpdateEndpointType(routerName, "ENDPOINT_TYPE_MANAGED_PROXY_LB"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(testResourceName, "endpoint_types.0", "ENDPOINT_TYPE_MANAGED_PROXY_LB"), + ), + }, + { + ResourceName: testResourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + <% unless version == 'ga' -%> func TestAccComputeRouterNat_withPrivateNat(t *testing.T) { t.Parallel() @@ -817,6 +877,40 @@ resource "google_compute_router_nat" "foobar" { `, routerName, routerName, routerName, routerName, routerName) } +func testAccComputeRouterNatUpdateEndpointType(routerName string, endpointType string) string { + return fmt.Sprintf(` +resource "google_compute_network" "foobar" { + name = "%[1]s-net" +} + +resource "google_compute_subnetwork" "foobar" { + name = "%[1]s-subnet" + network = google_compute_network.foobar.self_link + ip_cidr_range = "10.0.0.0/16" + region = "us-central1" +} + +resource "google_compute_router" "foobar" { + name = "%[1]s" + region = google_compute_subnetwork.foobar.region + network = google_compute_network.foobar.self_link +} + +resource "google_compute_router_nat" "foobar" { + name = "%[1]s" + router = google_compute_router.foobar.name + region = google_compute_router.foobar.region + nat_ip_allocate_option = "AUTO_ONLY" + source_subnetwork_ip_ranges_to_nat = "ALL_SUBNETWORKS_ALL_IP_RANGES" + endpoint_types = [ "%[2]s" ] + log_config { + enable = true + filter = "ERRORS_ONLY" + } +} +`, routerName, endpointType) +} + func testAccComputeRouterNatUpdateToNatIPsId(routerName string) string { return fmt.Sprintf(` resource "google_compute_router" "foobar" { From e5f98eab95ddd537dfcd4fe23197965fd06c1b1a Mon Sep 17 00:00:00 2001 From: Shuya Ma <87669292+shuyama1@users.noreply.github.com> Date: Thu, 4 Apr 2024 13:33:25 -0700 Subject: [PATCH 09/11] make vcr_merge wait until all downstream pushes finish (#10369) --- .ci/gcb-push-downstream.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/gcb-push-downstream.yml b/.ci/gcb-push-downstream.yml index 9b0b58493ab5..9188e749a00d 100644 --- a/.ci/gcb-push-downstream.yml +++ b/.ci/gcb-push-downstream.yml @@ -181,7 +181,7 @@ steps: entrypoint: '/workspace/.ci/scripts/go-plus/vcr-cassette-merger/vcr_merge.sh' secretEnv: ["GITHUB_TOKEN_CLASSIC", "GOOGLE_PROJECT"] id: vcr-merge - waitFor: ["tpg-push"] + waitFor: ["tpg-push", "tpgb-push", "tgc-push", "tf-oics-push"] env: - BASE_BRANCH=$BRANCH_NAME args: From c493aef6ee58c835c0851c049de03e5c4e709d30 Mon Sep 17 00:00:00 2001 From: "Stephen Lewis (Burrows)" Date: Thu, 4 Apr 2024 15:57:34 -0700 Subject: [PATCH 10/11] Don't request reviewers if there are none to request. (#10371) --- .ci/magician/cmd/request_reviewer.go | 10 ++++++---- .ci/magician/cmd/request_reviewer_test.go | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.ci/magician/cmd/request_reviewer.go b/.ci/magician/cmd/request_reviewer.go index 0f3f16e51805..f4d2122a6424 100644 --- a/.ci/magician/cmd/request_reviewer.go +++ b/.ci/magician/cmd/request_reviewer.go @@ -83,10 +83,12 @@ func execRequestReviewer(prNumber string, gh GithubClient) { reviewersToRequest, newPrimaryReviewer := github.ChooseCoreReviewers(requestedReviewers, previousReviewers) - err = gh.RequestPullRequestReviewers(prNumber, reviewersToRequest) - if err != nil { - fmt.Println(err) - os.Exit(1) + if len(reviewersToRequest) > 0 { + err = gh.RequestPullRequestReviewers(prNumber, reviewersToRequest) + if err != nil { + fmt.Println(err) + os.Exit(1) + } } if newPrimaryReviewer != "" { diff --git a/.ci/magician/cmd/request_reviewer_test.go b/.ci/magician/cmd/request_reviewer_test.go index 25fdb8eddbb4..a150c5dbd2cf 100644 --- a/.ci/magician/cmd/request_reviewer_test.go +++ b/.ci/magician/cmd/request_reviewer_test.go @@ -92,6 +92,9 @@ func TestExecRequestReviewer(t *testing.T) { if tc.expectSpecificReviewers != nil { assert.ElementsMatch(t, tc.expectSpecificReviewers, actualReviewers) + if len(tc.expectSpecificReviewers) == 0 { + assert.Len(t, gh.calledMethods["RequestPullRequestReviewers"], 0) + } } if tc.expectReviewersFromList != nil { for _, reviewer := range actualReviewers { From 323ac2c8c44a74c0f1d772c0386b1220fe0dc81b Mon Sep 17 00:00:00 2001 From: Obada Alabbadi <76101898+obada-ab@users.noreply.github.com> Date: Fri, 5 Apr 2024 01:54:04 +0200 Subject: [PATCH 11/11] Allow in-place column drop for bigquery table (#10170) --- .../bigquery/resource_bigquery_table.go | 90 ++++++++++++++++--- .../resource_bigquery_table_internal_test.go | 65 +++++++++++--- .../bigquery/resource_bigquery_table_test.go | 89 ++++++++++++++++++ 3 files changed, 222 insertions(+), 22 deletions(-) diff --git a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go index f72162a93267..b39be4116a79 100644 --- a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go +++ b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table.go @@ -256,19 +256,17 @@ func bigQueryTableNormalizePolicyTags(val interface{}) interface{} { // Compares two existing schema implementations and decides if // it is changeable.. pairs with a force new on not changeable -func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) { +func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTable bool, topLevel bool) (bool, error) { switch old.(type) { case []interface{}: arrayOld := old.([]interface{}) arrayNew, ok := new.([]interface{}) + sameNameColumns := 0 + droppedColumns := 0 if !ok { // if not both arrays not changeable return false, nil } - if len(arrayOld) > len(arrayNew) { - // if not growing not changeable - return false, nil - } if err := bigQueryTablecheckNameExists(arrayOld); err != nil { return false, err } @@ -289,16 +287,28 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) } } for key := range mapOld { - // all old keys should be represented in the new config + // dropping top level columns can happen in-place + // but this doesn't apply to external tables if _, ok := mapNew[key]; !ok { - return false, nil + if !topLevel || isExternalTable { + return false, nil + } + droppedColumns += 1 + continue } - if isChangable, err := - resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key]); err != nil || !isChangable { + + isChangable, err := resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key], isExternalTable, false) + if err != nil || !isChangable { return false, err + } else if isChangable && topLevel { + // top level column that exists in the new schema + sameNameColumns += 1 } } - return true, nil + // in-place column dropping alongside column additions is not allowed + // as of now because user intention can be ambiguous (e.g. column renaming) + newColumns := len(arrayNew) - sameNameColumns + return (droppedColumns == 0) || (newColumns == 0), nil case map[string]interface{}: objectOld := old.(map[string]interface{}) objectNew, ok := new.(map[string]interface{}) @@ -337,7 +347,7 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) return false, nil } case "fields": - return resourceBigQueryTableSchemaIsChangeable(valOld, valNew) + return resourceBigQueryTableSchemaIsChangeable(valOld, valNew, isExternalTable, false) // other parameters: description, policyTags and // policyTags.names[] are changeable @@ -376,7 +386,8 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourc // same as above log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err) } - isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) + _, isExternalTable := d.GetOk("external_data_configuration") + isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, isExternalTable, true) if err != nil { return err } @@ -1710,6 +1721,12 @@ func resourceBigQueryTableRead(d *schema.ResourceData, meta interface{}) error { return nil } +type TableReference struct { + project string + datasetID string + tableID string +} + func resourceBigQueryTableUpdate(d *schema.ResourceData, meta interface{}) error { config := meta.(*transport_tpg.Config) userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent) @@ -1732,6 +1749,16 @@ func resourceBigQueryTableUpdate(d *schema.ResourceData, meta interface{}) error datasetID := d.Get("dataset_id").(string) tableID := d.Get("table_id").(string) + tableReference := &TableReference{ + project: project, + datasetID: datasetID, + tableID: tableID, + } + + if err = resourceBigQueryTableColumnDrop(config, userAgent, table, tableReference); err != nil { + return err + } + if _, err = config.NewBigQueryClient(userAgent).Tables.Update(project, datasetID, tableID, table).Do(); err != nil { return err } @@ -1739,6 +1766,45 @@ func resourceBigQueryTableUpdate(d *schema.ResourceData, meta interface{}) error return resourceBigQueryTableRead(d, meta) } +func resourceBigQueryTableColumnDrop(config *transport_tpg.Config, userAgent string, table *bigquery.Table, tableReference *TableReference) error { + oldTable, err := config.NewBigQueryClient(userAgent).Tables.Get(tableReference.project, tableReference.datasetID, tableReference.tableID).Do() + if err != nil { + return err + } + + newTableFields := map[string]bool{} + for _, field := range table.Schema.Fields { + newTableFields[field.Name] = true + } + + droppedColumns := []string{} + for _, field := range oldTable.Schema.Fields { + if !newTableFields[field.Name] { + droppedColumns = append(droppedColumns, field.Name) + } + } + + if len(droppedColumns) > 0 { + droppedColumnsString := strings.Join(droppedColumns, ", DROP COLUMN ") + + dropColumnsDDL := fmt.Sprintf("ALTER TABLE `%s.%s.%s` DROP COLUMN %s", tableReference.project, tableReference.datasetID, tableReference.tableID, droppedColumnsString) + log.Printf("[INFO] Dropping columns in-place: %s", dropColumnsDDL) + + useLegacySQL := false + req := &bigquery.QueryRequest{ + Query: dropColumnsDDL, + UseLegacySql: &useLegacySQL, + } + + _, err = config.NewBigQueryClient(userAgent).Jobs.Query(tableReference.project, req).Do() + if err != nil { + return err + } + } + + return nil +} + func resourceBigQueryTableDelete(d *schema.ResourceData, meta interface{}) error { if d.Get("deletion_protection").(bool) { return fmt.Errorf("cannot destroy instance without setting deletion_protection=false and running `terraform apply`") diff --git a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_internal_test.go b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_internal_test.go index 82bad6650a6b..b8747ac327e4 100644 --- a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_internal_test.go +++ b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_internal_test.go @@ -389,10 +389,11 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) { } type testUnitBigQueryDataTableJSONChangeableTestCase struct { - name string - jsonOld string - jsonNew string - changeable bool + name string + jsonOld string + jsonNew string + isExternalTable bool + changeable bool } func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) { @@ -403,7 +404,7 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil { t.Fatalf("unable to unmarshal json - %v", err) } - changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new) + changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, testcase.isExternalTable, true) if err != nil { t.Errorf("%s failed unexpectedly: %s", testcase.name, err) } @@ -419,6 +420,11 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin d.Before["schema"] = testcase.jsonOld d.After["schema"] = testcase.jsonNew + if testcase.isExternalTable { + d.Before["external_data_configuration"] = "" + d.After["external_data_configuration"] = "" + } + err = resourceBigQueryTableSchemaCustomizeDiffFunc(d) if err != nil { t.Errorf("error on testcase %s - %v", testcase.name, err) @@ -428,7 +434,7 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin } } -var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{ +var testUnitBigQueryDataTableIsChangeableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{ { name: "defaultEquality", jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", @@ -445,7 +451,14 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ name: "arraySizeDecreases", jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"asomeValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", - changeable: false, + changeable: true, + }, + { + name: "externalArraySizeDecreases", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"asomeValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + isExternalTable: true, + changeable: false, }, { name: "descriptionChanges", @@ -523,6 +536,24 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ jsonNew: "[{\"name\": \"value3\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"newVal\" }, {\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", changeable: false, }, + { + name: "renameRequiredColumn", + jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"value3\", \"type\" : \"INTEGER\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]", + changeable: false, + }, + { + name: "renameNullableColumn", + jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"value3\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + changeable: false, + }, + { + name: "typeModeReqToNullAndColumnDropped", + jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }, {\"name\": \"someValue2\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]", + jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]", + changeable: true, + }, { name: "policyTags", jsonOld: `[ @@ -548,15 +579,29 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ }, } -func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) { +func TestUnitBigQueryDataTable_schemaIsChangeable(t *testing.T) { t.Parallel() - for _, testcase := range testUnitBigQueryDataTableIsChangableTestCases { + for _, testcase := range testUnitBigQueryDataTableIsChangeableTestCases { testcase.check(t) + } +} + +func TestUnitBigQueryDataTable_schemaIsChangeableNested(t *testing.T) { + t.Parallel() + // Only top level column drops are changeable + customNestedValues := map[string]bool{"arraySizeDecreases": false, "typeModeReqToNullAndColumnDropped": false} + for _, testcase := range testUnitBigQueryDataTableIsChangeableTestCases { + changeable := testcase.changeable + if overrideValue, ok := customNestedValues[testcase.name]; ok { + changeable = overrideValue + } + testcaseNested := &testUnitBigQueryDataTableJSONChangeableTestCase{ testcase.name + "Nested", fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"fields\" : %s }]", testcase.jsonOld), fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INT64\", \"fields\" : %s }]", testcase.jsonNew), - testcase.changeable, + testcase.isExternalTable, + changeable, } testcaseNested.check(t) } diff --git a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_test.go b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_test.go index 5cab9a250f87..778ecf40b3d5 100644 --- a/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_test.go +++ b/mmv1/third_party/terraform/services/bigquery/resource_bigquery_table_test.go @@ -45,6 +45,39 @@ func TestAccBigQueryTable_Basic(t *testing.T) { }) } +func TestAccBigQueryTable_DropColumns(t *testing.T) { + t.Parallel() + + datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10)) + tableID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccBigQueryTableTimePartitioningDropColumns(datasetID, tableID), + }, + { + ResourceName: "google_bigquery_table.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_protection"}, + }, + { + Config: testAccBigQueryTableTimePartitioningDropColumnsUpdate(datasetID, tableID), + }, + { + ResourceName: "google_bigquery_table.test", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_protection"}, + }, + }, + }) +} + func TestAccBigQueryTable_Kms(t *testing.T) { t.Parallel() resourceName := "google_bigquery_table.test" @@ -1759,6 +1792,62 @@ EOH `, datasetID, tableID, partitioningType) } +func testAccBigQueryTableTimePartitioningDropColumns(datasetID, tableID string) string { + return fmt.Sprintf(` +resource "google_bigquery_dataset" "test" { + dataset_id = "%s" +} + +resource "google_bigquery_table" "test" { + deletion_protection = false + table_id = "%s" + dataset_id = google_bigquery_dataset.test.dataset_id + + schema = <