diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index e767ff6ce..69af625e9 100644 --- a/builder/googlecompute/driver_gce.go +++ b/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) 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) + } + } +}