From 448d413b3f88ba3f2ea388f62289782074363094 Mon Sep 17 00:00:00 2001 From: Matt Morrison Date: Sun, 4 Sep 2016 18:17:13 +1200 Subject: [PATCH] DRY it out, and reverse fix for #5552. --- builtin/providers/google/provider.go | 50 ++++++++++++++++++ .../google/resource_compute_instance.go | 5 +- .../resource_compute_instance_template.go | 5 +- .../google/resource_compute_route.go | 5 +- .../google/resource_compute_subnetwork.go | 13 ++--- .../google/resource_compute_vpn_gateway.go | 14 ++--- .../google/resource_container_cluster.go | 10 ++-- .../google/resource_container_cluster_test.go | 51 +++++++++++++++++++ .../google/r/compute_instance.html.markdown | 2 +- .../r/compute_instance_template.html.markdown | 2 +- .../google/r/compute_route.html.markdown | 2 +- .../google/r/container_cluster.html.markdown | 2 +- 12 files changed, 124 insertions(+), 37 deletions(-) diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index f04b5b2220c9..28e1b68e1daf 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -223,3 +223,53 @@ func getZonalResourceFromRegion(getResource func(string) (interface{}, error), r // Resource does not exist in this region return nil, nil } + +// getNetworkLink reads the "network" field from the given resource data and if the value: +// - is a resource URL, returns the string unchanged +// - is the network name only, then looks up the resource URL using the google client +func getNetworkLink(d *schema.ResourceData, config *Config, field string) (string, error) { + if v, ok := d.GetOk(field); ok { + network := v.(string) + + project, err := getProject(d, config) + if err != nil { + return "", err + } + + if !strings.HasPrefix(network, "https://www.googleapis.com/compute/") { + // Network value provided is just the name, lookup the network SelfLink + networkData, err := config.clientCompute.Networks.Get( + project, network).Do() + if err != nil { + return "", fmt.Errorf("Error reading network: %s", err) + } + network = networkData.SelfLink + } + + return network, nil + + } else { + return "", nil + } +} + +// getNetworkName reads the "network" field from the given resource data and if the value: +// - is a resource URL, extracts the network name from the URL and returns it +// - is the network name only (i.e not prefixed with http://www.googleapis.com/compute/...), is returned unchanged +func getNetworkName(d *schema.ResourceData, field string) (string, error) { + if v, ok := d.GetOk(field); ok { + network := v.(string) + + if strings.HasPrefix(network, "https://www.googleapis.com/compute/") { + // extract the network name from SelfLink URL + networkName := network[strings.LastIndex(network, "/")+1:] + if networkName == "" { + return "", fmt.Errorf("network url not valid") + } + return networkName, nil + } + + return network, nil + } + return "", nil +} diff --git a/builtin/providers/google/resource_compute_instance.go b/builtin/providers/google/resource_compute_instance.go index 9a4387b5206f..dd4134407222 100644 --- a/builtin/providers/google/resource_compute_instance.go +++ b/builtin/providers/google/resource_compute_instance.go @@ -478,14 +478,13 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err if networkName != "" && subnetworkName != "" { return fmt.Errorf("Cannot specify both network and subnetwork values.") } else if networkName != "" { - network, err := config.clientCompute.Networks.Get( - project, networkName).Do() + networkLink, err = getNetworkLink(d, config, prefix+".network") if err != nil { return fmt.Errorf( "Error referencing network '%s': %s", networkName, err) } - networkLink = network.SelfLink + } else { region := getRegionFromZone(d.Get("zone").(string)) subnetwork, err := config.clientCompute.Subnetworks.Get( diff --git a/builtin/providers/google/resource_compute_instance_template.go b/builtin/providers/google/resource_compute_instance_template.go index 9a5638e29e53..f9d28ec355dc 100644 --- a/builtin/providers/google/resource_compute_instance_template.go +++ b/builtin/providers/google/resource_compute_instance_template.go @@ -417,13 +417,12 @@ func buildNetworks(d *schema.ResourceData, meta interface{}) ([]*compute.Network var networkLink, subnetworkLink string if networkName != "" { - network, err := config.clientCompute.Networks.Get( - project, networkName).Do() + networkLink, err = getNetworkLink(d, config, prefix+".network") if err != nil { return nil, fmt.Errorf("Error referencing network '%s': %s", networkName, err) } - networkLink = network.SelfLink + } else { // lookup subnetwork link using region and subnetwork name region, err := getRegion(d, config) diff --git a/builtin/providers/google/resource_compute_route.go b/builtin/providers/google/resource_compute_route.go index 5808216e172f..6e39a41363e6 100644 --- a/builtin/providers/google/resource_compute_route.go +++ b/builtin/providers/google/resource_compute_route.go @@ -106,8 +106,7 @@ func resourceComputeRouteCreate(d *schema.ResourceData, meta interface{}) error } // Look up the network to attach the route to - network, err := config.clientCompute.Networks.Get( - project, d.Get("network").(string)).Do() + network, err := getNetworkLink(d, config, "network") if err != nil { return fmt.Errorf("Error reading network: %s", err) } @@ -149,7 +148,7 @@ func resourceComputeRouteCreate(d *schema.ResourceData, meta interface{}) error route := &compute.Route{ Name: d.Get("name").(string), DestRange: d.Get("dest_range").(string), - Network: network.SelfLink, + Network: network, NextHopInstance: nextHopInstance, NextHopVpnTunnel: nextHopVpnTunnel, NextHopIp: nextHopIp, diff --git a/builtin/providers/google/resource_compute_subnetwork.go b/builtin/providers/google/resource_compute_subnetwork.go index d4760767f2a7..94c7a9dd8993 100644 --- a/builtin/providers/google/resource_compute_subnetwork.go +++ b/builtin/providers/google/resource_compute_subnetwork.go @@ -91,16 +91,9 @@ func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) e return err } - // Network can be provided as just the name, or the full resource URL - network := d.Get("network").(string) - if !strings.HasPrefix(network, "https://www.googleapis.com/compute/") { - // Network value provided is just the name, lookup the network URL - networkData, err := config.clientCompute.Networks.Get( - project, d.Get("network").(string)).Do() - if err != nil { - return fmt.Errorf("Error reading network: %s", err) - } - network = networkData.SelfLink + network, err := getNetworkLink(d, config, "network") + if err != nil { + return err } // Build the subnetwork parameters diff --git a/builtin/providers/google/resource_compute_vpn_gateway.go b/builtin/providers/google/resource_compute_vpn_gateway.go index 728ccabb0edf..fe716198dde9 100644 --- a/builtin/providers/google/resource_compute_vpn_gateway.go +++ b/builtin/providers/google/resource_compute_vpn_gateway.go @@ -8,7 +8,6 @@ import ( "google.golang.org/api/compute/v1" "google.golang.org/api/googleapi" - "strings" ) func resourceComputeVpnGateway() *schema.Resource { @@ -72,16 +71,9 @@ func resourceComputeVpnGatewayCreate(d *schema.ResourceData, meta interface{}) e } name := d.Get("name").(string) - network := d.Get("network").(string) - - if !strings.HasPrefix(network, "https://www.googleapis.com/compute/") { - // Network value provided is just the name, lookup the network URL - networkData, err := config.clientCompute.Networks.Get( - project, d.Get("network").(string)).Do() - if err != nil { - return fmt.Errorf("Error reading network: %s", err) - } - network = networkData.SelfLink + network, err := getNetworkLink(d, config, "network") + if err != nil { + return err } vpnGatewaysService := compute.NewTargetVpnGatewaysService(config.clientCompute) diff --git a/builtin/providers/google/resource_container_cluster.go b/builtin/providers/google/resource_container_cluster.go index 6954fcfa2c18..8b0397be35bb 100644 --- a/builtin/providers/google/resource_container_cluster.go +++ b/builtin/providers/google/resource_container_cluster.go @@ -289,8 +289,12 @@ func resourceContainerClusterCreate(d *schema.ResourceData, meta interface{}) er cluster.MonitoringService = v.(string) } - if v, ok := d.GetOk("network"); ok { - cluster.Network = v.(string) + if _, ok := d.GetOk("network"); ok { + network, err := getNetworkName(d, "network") + if err != nil { + return err + } + cluster.Network = network } if v, ok := d.GetOk("subnetwork"); ok { @@ -425,7 +429,7 @@ func resourceContainerClusterRead(d *schema.ResourceData, meta interface{}) erro d.Set("description", cluster.Description) d.Set("logging_service", cluster.LoggingService) d.Set("monitoring_service", cluster.MonitoringService) - d.Set("network", cluster.Network) + d.Set("network", d.Get("network").(string)) d.Set("subnetwork", cluster.Subnetwork) d.Set("node_config", flattenClusterNodeConfig(cluster.NodeConfig)) d.Set("instance_group_urls", cluster.InstanceGroupUrls) diff --git a/builtin/providers/google/resource_container_cluster_test.go b/builtin/providers/google/resource_container_cluster_test.go index 11cf1378e725..0bb1f01f2150 100644 --- a/builtin/providers/google/resource_container_cluster_test.go +++ b/builtin/providers/google/resource_container_cluster_test.go @@ -43,6 +43,25 @@ func TestAccContainerCluster_withNodeConfig(t *testing.T) { }) } +func TestAccContainerCluster_network(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckContainerClusterDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccContainerCluster_networkRef, + Check: resource.ComposeTestCheckFunc( + testAccCheckContainerClusterExists( + "google_container_cluster.with_net_ref_by_url"), + testAccCheckContainerClusterExists( + "google_container_cluster.with_net_ref_by_name"), + ), + }, + }, + }) +} + func testAccCheckContainerClusterDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -124,3 +143,35 @@ resource "google_container_cluster" "with_node_config" { ] } }`, acctest.RandString(10)) + +var testAccContainerCluster_networkRef = fmt.Sprintf(` +resource "google_compute_network" "container_network" { + name = "container-net-%s" + auto_create_subnetworks = true +} + +resource "google_container_cluster" "with_net_ref_by_url" { + name = "cluster-test-%s" + zone = "us-central1-a" + initial_node_count = 1 + + master_auth { + username = "mr.yoda" + password = "adoy.rm" + } + + network = "${google_compute_network.container_network.self_link}" +} + +resource "google_container_cluster" "with_net_ref_by_name" { + name = "cluster-test-%s" + zone = "us-central1-a" + initial_node_count = 1 + + master_auth { + username = "mr.yoda" + password = "adoy.rm" + } + + network = "${google_compute_network.container_network.name}" +}`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10)) diff --git a/website/source/docs/providers/google/r/compute_instance.html.markdown b/website/source/docs/providers/google/r/compute_instance.html.markdown index d40618a7675c..ce2308ba5259 100644 --- a/website/source/docs/providers/google/r/compute_instance.html.markdown +++ b/website/source/docs/providers/google/r/compute_instance.html.markdown @@ -133,7 +133,7 @@ the type is "local-ssd", in which case scratch must be true). The `network_interface` block supports: -* `network` - (Optional) The name of the network to attach this interface to. +* `network` - (Optional) The name or self_link of the network to attach this interface to. Either `network` or `subnetwork` must be provided. * `subnetwork` - (Optional) the name of the subnetwork to attach this interface diff --git a/website/source/docs/providers/google/r/compute_instance_template.html.markdown b/website/source/docs/providers/google/r/compute_instance_template.html.markdown index fda3c5563220..80fa363f781d 100644 --- a/website/source/docs/providers/google/r/compute_instance_template.html.markdown +++ b/website/source/docs/providers/google/r/compute_instance_template.html.markdown @@ -192,7 +192,7 @@ The `disk` block supports: The `network_interface` block supports: -* `network` - (Optional) The name of the network to attach this interface to. +* `network` - (Optional) The name or self_link of the network to attach this interface to. Use `network` attribute for Legacy or Auto subnetted networks and `subnetwork` for custom subnetted networks. diff --git a/website/source/docs/providers/google/r/compute_route.html.markdown b/website/source/docs/providers/google/r/compute_route.html.markdown index 0d99fe30dc7b..484fd7db4b11 100644 --- a/website/source/docs/providers/google/r/compute_route.html.markdown +++ b/website/source/docs/providers/google/r/compute_route.html.markdown @@ -37,7 +37,7 @@ The following arguments are supported: * `name` - (Required) A unique name for the resource, required by GCE. Changing this forces a new resource to be created. -* `network` - (Required) The name of the network to attach this route to. +* `network` - (Required) The name or self_link of the network to attach this route to. * `priority` - (Required) The priority of this route, used to break ties. diff --git a/website/source/docs/providers/google/r/container_cluster.html.markdown b/website/source/docs/providers/google/r/container_cluster.html.markdown index 693ed2ac865d..abcca800ad70 100644 --- a/website/source/docs/providers/google/r/container_cluster.html.markdown +++ b/website/source/docs/providers/google/r/container_cluster.html.markdown @@ -66,7 +66,7 @@ resource "google_container_cluster" "primary" { `monitoring.googleapis.com` and `none`. Defaults to `monitoring.googleapis.com` -* `network` - (Optional) The name of the Google Compute Engine network to which +* `network` - (Optional) The name or self_link of the Google Compute Engine network to which the cluster is connected * `node_config` - (Optional) The machine type and image to use for all nodes in