Make network attribute more consistent.

Some google resources required network be refernced by resource URL (aka self_link), while others required network name.
This change allows either to be supplied.

DRY it out, and add a fix for #5552.
This commit is contained in:
Matt Morrison 2016-09-03 21:51:20 +12:00
parent 251a0e73ab
commit 6ca21ec009
16 changed files with 158 additions and 27 deletions

View File

@ -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
}

View File

@ -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(

View File

@ -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)

View File

@ -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,

View File

@ -91,12 +91,17 @@ func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) e
return err
}
network, err := getNetworkLink(d, config, "network")
if err != nil {
return err
}
// Build the subnetwork parameters
subnetwork := &compute.Subnetwork{
Name: d.Get("name").(string),
Description: d.Get("description").(string),
IpCidrRange: d.Get("ip_cidr_range").(string),
Network: d.Get("network").(string),
Network: network,
}
log.Printf("[DEBUG] Subnetwork insert request: %#v", subnetwork)

View File

@ -11,7 +11,8 @@ import (
)
func TestAccComputeSubnetwork_basic(t *testing.T) {
var subnetwork compute.Subnetwork
var subnetwork1 compute.Subnetwork
var subnetwork2 compute.Subnetwork
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
@ -22,7 +23,9 @@ func TestAccComputeSubnetwork_basic(t *testing.T) {
Config: testAccComputeSubnetwork_basic,
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeSubnetworkExists(
"google_compute_subnetwork.foobar", &subnetwork),
"google_compute_subnetwork.network-ref-by-url", &subnetwork1),
testAccCheckComputeSubnetworkExists(
"google_compute_subnetwork.network-ref-by-name", &subnetwork2),
),
},
},
@ -84,9 +87,19 @@ resource "google_compute_network" "custom-test" {
auto_create_subnetworks = false
}
resource "google_compute_subnetwork" "foobar" {
resource "google_compute_subnetwork" "network-ref-by-url" {
name = "subnetwork-test-%s"
ip_cidr_range = "10.0.0.0/16"
region = "us-central1"
network = "${google_compute_network.custom-test.self_link}"
}`, acctest.RandString(10), acctest.RandString(10))
}
resource "google_compute_subnetwork" "network-ref-by-name" {
name = "subnetwork-test-%s"
ip_cidr_range = "10.1.0.0/16"
region = "us-central1"
network = "${google_compute_network.custom-test.name}"
}
`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10))

View File

@ -71,7 +71,10 @@ func resourceComputeVpnGatewayCreate(d *schema.ResourceData, meta interface{}) e
}
name := d.Get("name").(string)
network := d.Get("network").(string)
network, err := getNetworkLink(d, config, "network")
if err != nil {
return err
}
vpnGatewaysService := compute.NewTargetVpnGatewaysService(config.clientCompute)

View File

@ -22,6 +22,8 @@ func TestAccComputeVpnGateway_basic(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeVpnGatewayExists(
"google_compute_vpn_gateway.foobar"),
testAccCheckComputeVpnGatewayExists(
"google_compute_vpn_gateway.baz"),
),
},
},
@ -89,4 +91,9 @@ resource "google_compute_vpn_gateway" "foobar" {
name = "gateway-test-%s"
network = "${google_compute_network.foobar.self_link}"
region = "us-central1"
}`, acctest.RandString(10), acctest.RandString(10))
}
resource "google_compute_vpn_gateway" "baz" {
name = "gateway-test-%s"
network = "${google_compute_network.foobar.name}"
region = "us-central1"
}`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10))

View File

@ -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)

View File

@ -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))

View File

@ -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

View File

@ -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.

View File

@ -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.

View File

@ -31,8 +31,9 @@ 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) A link to the parent network of this subnetwork.
The parent network must have been created in custom subnet mode.
* `network` - (Required) The network name or resource link to the parent
network of this subnetwork. The parent network must have been created
in custom subnet mode.
- - -

View File

@ -89,8 +89,8 @@ 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) A link to the network this VPN gateway is accepting
traffic for. Changing this forces a new resource to be created.
* `network` - (Required) The name or resource link to the network this VPN gateway
is accepting traffic for. Changing this forces a new resource to be created.
- - -

View File

@ -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