diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 9605f2f4c..8ed3897d6 100644 --- a/builder/googlecompute/config.go +++ b/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 } diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 02a230cfe..69af625e9 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -10,7 +10,6 @@ import ( "fmt" "log" "net/http" - "net/url" "runtime" "strings" "time" @@ -299,56 +298,9 @@ 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//global/networks/ - 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() - 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 - } + networkId, subnetworkId, err := getNetworking(c) + if err != nil { + return nil, err } var accessconfig *compute.AccessConfig @@ -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) diff --git a/builder/googlecompute/networking.go b/builder/googlecompute/networking.go new file mode 100644 index 000000000..aa59e77e0 --- /dev/null +++ b/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 +} diff --git a/builder/googlecompute/networking_test.go b/builder/googlecompute/networking_test.go new file mode 100644 index 000000000..85b481df3 --- /dev/null +++ b/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) + } + } +} diff --git a/website/source/docs/builders/googlecompute.html.md b/website/source/docs/builders/googlecompute.html.md index 8e89666bd..4ccfe4cb1 100644 --- a/website/source/docs/builders/googlecompute.html.md +++ b/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)