Merge pull request #5343 from evandbrown/pr-googlecompute-subnetworks

Interpolate network and subnetwork values

builder/googlecompute/config.go

@@ -82,10 +82,14 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) {
 	var errs *packer.MultiError
 
 	// Set defaults.
-	if c.Network == "" {
+	if c.Network == "" && c.Subnetwork == "" {
 		c.Network = "default"
 	}
 
+	if c.NetworkProjectId == "" {
+		c.NetworkProjectId = c.ProjectId
+	}
+
 	if c.DiskSizeGb == 0 {
 		c.DiskSizeGb = 10
 	}

builder/googlecompute/driver_gce.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"log"
 	"net/http"
-	"net/url"
 	"runtime"
 	"strings"
 	"time"
@@ -299,57 +298,10 @@ func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) {
 	}
 	// TODO(mitchellh): deprecation warnings
 
-	networkSelfLink := ""
-	subnetworkSelfLink := ""
-
-	if u, err := url.Parse(c.Network); err == nil && (u.Scheme == "https" || u.Scheme == "http") {
-		// Network is a full server URL
-		// Parse out Network and NetworkProjectId from URL
-		// https://www.googleapis.com/compute/v1/projects/<ProjectId>/global/networks/<Network>
-		networkSelfLink = c.Network
-		parts := strings.Split(u.String(), "/")
-		if len(parts) >= 10 {
-			c.NetworkProjectId = parts[6]
-			c.Network = parts[9]
-		}
-	}
-	if u, err := url.Parse(c.Subnetwork); err == nil && (u.Scheme == "https" || u.Scheme == "http") {
-		// Subnetwork is a full server URL
-		subnetworkSelfLink = c.Subnetwork
-	}
-
-	// If subnetwork is ID's and not full service URL's look them up.
-	if subnetworkSelfLink == "" {
-
-		// Get the network
-		if c.NetworkProjectId == "" {
-			c.NetworkProjectId = d.projectId
-		}
-		d.ui.Message(fmt.Sprintf("Loading network: %s", c.Network))
-		network, err := d.service.Networks.Get(c.NetworkProjectId, c.Network).Do()
+	networkId, subnetworkId, err := getNetworking(c)
 	if err != nil {
 		return nil, err
 	}
-		networkSelfLink = network.SelfLink
-
-		// Subnetwork
-		// Validate Subnetwork config now that we have some info about the network
-		if !network.AutoCreateSubnetworks && len(network.Subnetworks) > 0 {
-			// Network appears to be in "custom" mode, so a subnetwork is required
-			if c.Subnetwork == "" {
-				return nil, fmt.Errorf("a subnetwork must be specified")
-			}
-		}
-		// Get the subnetwork
-		if c.Subnetwork != "" {
-			d.ui.Message(fmt.Sprintf("Loading subnetwork: %s for region: %s", c.Subnetwork, c.Region))
-			subnetwork, err := d.service.Subnetworks.Get(c.NetworkProjectId, c.Region, c.Subnetwork).Do()
-			if err != nil {
-				return nil, err
-			}
-			subnetworkSelfLink = subnetwork.SelfLink
-		}
-	}
 
 	var accessconfig *compute.AccessConfig
 	// Use external IP if OmitExternalIP isn't set
@@ -417,8 +369,8 @@ func (d *driverGCE) RunInstance(c *InstanceConfig) (<-chan error, error) {
 		NetworkInterfaces: []*compute.NetworkInterface{
 			{
 				AccessConfigs: []*compute.AccessConfig{accessconfig},
-				Network:       networkSelfLink,
-				Subnetwork:    subnetworkSelfLink,
+				Network:       networkId,
+				Subnetwork:    subnetworkId,
 			},
 		},
 		Scheduling: &compute.Scheduling{
@@ -611,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)
+		}
+	}
+}

website/source/docs/builders/googlecompute.html.md

@@ -216,7 +216,10 @@ builder.
     instance.
 
 -   `network` (string) - The Google Compute network id or URL to use for the
-    launched instance. Defaults to `"default"`.
+    launched instance. Defaults to `"default"`. If the value is not a URL, it
+    will be interpolated to `projects/((network_project_id))/global/networks/((network))`.
+    This value is not required if a `subnet` is specified.
+
 
 -   `network_project_id` (string) - The project ID for the network and subnetwork
     to use for launched instance. Defaults to `project_id`.
@@ -259,7 +262,9 @@ builder.
 -   `subnetwork` (string) - The Google Compute subnetwork id or URL to use for
     the launched instance. Only required if the `network` has been created with
     custom subnetting. Note, the region of the subnetwork must match the `region`
-    or `zone` in which the VM is launched.
+    or `zone` in which the VM is launched. If the value is not a URL, it
+    will be interpolated to `projects/((network_project_id))/regions/((region))/subnetworks/((subnetwork))`
+
 
 -   `tags` (array of strings)