builder/googlecompute: Test networking interpolation

This change pulls the logic that interpolates network and subnetwork
into its own func and adds tests.

builder/googlecompute/driver_gce.go

@@ -298,55 +298,9 @@ func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) {
 	}
 	// TODO(mitchellh): deprecation warnings
 
-	networkId := ""
-	subnetworkId := ""
-
-	// Apply network naming requirements per
-	// https://cloud.google.com/compute/docs/reference/latest/instances#resource
-	switch c.Network {
-	// It is possible to omit the network property as long as a subnet is
-	// specified. That will be validated later.
-	case "":
-		d.ui.Message(fmt.Sprintf("Network: will be inferred from subnetwork"))
-		break
-	// This special short name should be expanded.
-	case "default":
-		networkId = "global/networks/default"
-	// A value other than "default" was provided for the network name.
-	default:
-		// If the value doesn't contain a slash, we assume it's not a full or
-		// partial URL. We will expand it into a partial URL here and avoid
-		// making an API call to discover the network as it's common for the
-		// caller to not have permission against network discovery APIs.
-		if !strings.Contains(c.Network, "/") {
-			networkId = "projects/" + c.NetworkProjectId + "/global/networks/" + c.Network
-			d.ui.Message(fmt.Sprintf("Network name: %q was expanded to the partial URL %q", c.Network, networkId))
-		}
-	}
-
-	// Apply subnetwork naming requirements per
-	// https://cloud.google.com/compute/docs/reference/latest/instances#resource
-	switch c.Subnetwork {
-	case "":
-		// You can't omit both subnetwork and network
-		if networkId == "" {
-			return nil, fmt.Errorf("both network and subnetwork were empty.")
-		}
-		// An empty subnetwork is only valid for networks in legacy mode or
-		// auto-subnet mode. We could make an API call to get that information
-		// about the network, but it's common for the caller to not have
-		// permission to that API. We'll proceed assuming they're correct in
-		// omitting the subnetwork and let the compute.insert API surface an
-		// error about an invalid network configuration if it exists.
-		break
-	default:
-		// If the value doesn't contain a slash, we assume it's not a full or
-		// partial URL. We will expand it into a partial URL here and avoid
-		// making a call to discover the subnetwork.
-		if !strings.Contains(c.Subnetwork, "/") {
-			subnetworkId = "projects/" + c.NetworkProjectId + "/regions/" + c.Region + "/subnetworks/" + c.Subnetwork
-			d.ui.Message(fmt.Sprintf("Subnetwork: %q was expanded to the partial URL %q", c.Subnetwork, subnetworkId))
-		}
+	networkId, subnetworkId, err := getNetworking(c)
+	if err != nil {
+		return nil, err
 	}
 
 	var accessconfig *compute.AccessConfig
@@ -609,7 +563,6 @@ func (d *driverGCE) refreshZoneOp(zone string, op *compute.Operation) stateRefre
 	}
 }
 
-// stateRefreshFunc is used to refresh the state of a thing and is
 // used in conjunction with waitForState.
 type stateRefreshFunc func() (string, error)
 

builder/googlecompute/networking.go

@@ -0,0 +1,59 @@
+package googlecompute
+
+import (
+	"fmt"
+	"strings"
+)
+
+// This method will build a network and subnetwork ID from the provided
+// instance config, and return them in that order.
+func getNetworking(c *InstanceConfig) (string, string, error) {
+	networkId := c.Network
+	subnetworkId := c.Subnetwork
+
+	// Apply network naming requirements per
+	// https://cloud.google.com/compute/docs/reference/latest/instances#resource
+	switch c.Network {
+	// It is possible to omit the network property as long as a subnet is
+	// specified. That will be validated later.
+	case "":
+		break
+	// This special short name should be expanded.
+	case "default":
+		networkId = "global/networks/default"
+	// A value other than "default" was provided for the network name.
+	default:
+		// If the value doesn't contain a slash, we assume it's not a full or
+		// partial URL. We will expand it into a partial URL here and avoid
+		// making an API call to discover the network as it's common for the
+		// caller to not have permission against network discovery APIs.
+		if !strings.Contains(c.Network, "/") {
+			networkId = "projects/" + c.NetworkProjectId + "/global/networks/" + c.Network
+		}
+	}
+
+	// Apply subnetwork naming requirements per
+	// https://cloud.google.com/compute/docs/reference/latest/instances#resource
+	switch c.Subnetwork {
+	case "":
+		// You can't omit both subnetwork and network
+		if networkId == "" {
+			return networkId, subnetworkId, fmt.Errorf("both network and subnetwork were empty.")
+		}
+		// An empty subnetwork is only valid for networks in legacy mode or
+		// auto-subnet mode. We could make an API call to get that information
+		// about the network, but it's common for the caller to not have
+		// permission to that API. We'll proceed assuming they're correct in
+		// omitting the subnetwork and let the compute.insert API surface an
+		// error about an invalid network configuration if it exists.
+		break
+	default:
+		// If the value doesn't contain a slash, we assume it's not a full or
+		// partial URL. We will expand it into a partial URL here and avoid
+		// making a call to discover the subnetwork.
+		if !strings.Contains(c.Subnetwork, "/") {
+			subnetworkId = "projects/" + c.NetworkProjectId + "/regions/" + c.Region + "/subnetworks/" + c.Subnetwork
+		}
+	}
+	return networkId, subnetworkId, nil
+}

builder/googlecompute/networking_test.go

@@ -0,0 +1,72 @@
+package googlecompute
+
+import (
+	"testing"
+)
+
+func TestGetNetworking(t *testing.T) {
+	cases := []struct {
+		c                  *InstanceConfig
+		expectedNetwork    string
+		expectedSubnetwork string
+		error              bool
+	}{
+		{
+			c: &InstanceConfig{
+				Network:          "default",
+				Subnetwork:       "",
+				NetworkProjectId: "project-id",
+				Region:           "region-id",
+			},
+			expectedNetwork:    "global/networks/default",
+			expectedSubnetwork: "",
+			error:              false,
+		},
+		{
+			c: &InstanceConfig{
+				Network:          "",
+				Subnetwork:       "",
+				NetworkProjectId: "project-id",
+				Region:           "region-id",
+			},
+			expectedNetwork:    "",
+			expectedSubnetwork: "",
+			error:              true,
+		},
+		{
+			c: &InstanceConfig{
+				Network:          "some/network/path",
+				Subnetwork:       "some/subnetwork/path",
+				NetworkProjectId: "project-id",
+				Region:           "region-id",
+			},
+			expectedNetwork:    "some/network/path",
+			expectedSubnetwork: "some/subnetwork/path",
+			error:              false,
+		},
+		{
+			c: &InstanceConfig{
+				Network:          "network-value",
+				Subnetwork:       "subnetwork-value",
+				NetworkProjectId: "project-id",
+				Region:           "region-id",
+			},
+			expectedNetwork:    "projects/project-id/global/networks/network-value",
+			expectedSubnetwork: "projects/project-id/regions/region-id/subnetworks/subnetwork-value",
+			error:              false,
+		},
+	}
+
+	for _, tc := range cases {
+		n, sn, err := getNetworking(tc.c)
+		if n != tc.expectedNetwork {
+			t.Errorf("Expected network %q but got network %q", tc.expectedNetwork, n)
+		}
+		if sn != tc.expectedSubnetwork {
+			t.Errorf("Expected subnetwork %q but got subnetwork %q", tc.expectedSubnetwork, sn)
+		}
+		if !tc.error && err != nil {
+			t.Errorf("Did not expect an error but got: %v", err)
+		}
+	}
+}