Create a reusable GlobalFieldValue and support reading project from schema (#550)

google/field_helpers.go

@@ -5,41 +5,85 @@ import (
 	"regexp"
 )
 
-const networkLinkTemplate = "projects/%s/global/networks/%s"
+const (
+	globalLinkTemplate    = "projects/%s/global/%s/%s"
+	globalLinkBasePattern = "projects/(.+)/global/%s/(.+)"
+)
 
-var networkLinkRegex = regexp.MustCompile("projects/(.+)/global/networks/(.+)")
+// ------------------------------------------------------------
+// Field helpers
+// ------------------------------------------------------------
 
-type NetworkFieldValue struct {
+func ParseNetworkFieldValue(network string, d TerraformResourceData, config *Config) (*GlobalFieldValue, error) {
+	return parseGlobalFieldValue("networks", network, "project", d, config, true)
+}
+
+// ------------------------------------------------------------
+// Base helpers used to create helpers for specific fields.
+// ------------------------------------------------------------
+
+type GlobalFieldValue struct {
 	Project string
 	Name    string
+
+	resourceType string
 }
 
-// Parses a `network` supporting 5 different formats:
-// - https://www.googleapis.com/compute/{version}/projects/myproject/global/networks/my-network
-// - projects/myproject/global/networks/my-network
-// - global/networks/my-network (default project is used)
-// - my-network (default project is used)
-// - "" (empty string). RelativeLink() returns empty. For most API, the behavior is to use the default network.
-func ParseNetworkFieldValue(network string, config *Config) *NetworkFieldValue {
-	if networkLinkRegex.MatchString(network) {
-		parts := networkLinkRegex.FindStringSubmatch(network)
-
-		return &NetworkFieldValue{
-			Project: parts[1],
-			Name:    parts[2],
-		}
-	}
-
-	return &NetworkFieldValue{
-		Project: config.Project,
-		Name:    GetResourceNameFromSelfLink(network),
-	}
-}
-
-func (f NetworkFieldValue) RelativeLink() string {
+func (f GlobalFieldValue) RelativeLink() string {
 	if len(f.Name) == 0 {
 		return ""
 	}
 
-	return fmt.Sprintf(networkLinkTemplate, f.Project, f.Name)
+	return fmt.Sprintf(globalLinkTemplate, f.Project, f.resourceType, f.Name)
+}
+
+// Parses a global field supporting 4 different formats:
+// - https://www.googleapis.com/compute/ANY_VERSION/projects/{my-project}/global/{resource_type}/{resource_name}
+// - projects/{my-project}/global/{resource_type}/{resource_name}
+// - global/{resource_type}/{resource_name} (default project is used)
+// - resource_name (default project is used)
+// - "" (empty string). RelativeLink() returns empty if isEmptyValid is true.
+func parseGlobalFieldValue(resourceType, fieldValue, projectSchemaField string, d TerraformResourceData, config *Config, isEmptyValid bool) (*GlobalFieldValue, error) {
+	if len(fieldValue) == 0 {
+		if isEmptyValid {
+			return &GlobalFieldValue{resourceType: resourceType}, nil
+		}
+		return nil, fmt.Errorf("The global field for resource %s cannot be empty", resourceType)
+	}
+
+	r := regexp.MustCompile(fmt.Sprintf(globalLinkBasePattern, resourceType))
+
+	if r.MatchString(fieldValue) {
+		parts := r.FindStringSubmatch(fieldValue)
+
+		return &GlobalFieldValue{
+			Project: parts[1],
+			Name:    parts[2],
+
+			resourceType: resourceType,
+		}, nil
+	}
+
+	project, err := getProjectFromSchema(projectSchemaField, d, config)
+	if err != nil {
+		return nil, err
+	}
+
+	return &GlobalFieldValue{
+		Project: project,
+		Name:    GetResourceNameFromSelfLink(fieldValue),
+
+		resourceType: resourceType,
+	}, nil
+}
+
+func getProjectFromSchema(projectSchemaField string, d TerraformResourceData, config *Config) (string, error) {
+	res, ok := d.GetOk(projectSchemaField)
+	if !ok || len(projectSchemaField) == 0 {
+		if config.Project != "" {
+			return config.Project, nil
+		}
+		return "", fmt.Errorf("project: required field is not set")
+	}
+	return res.(string), nil
 }

google/field_helpers_test.go

@@ -1,36 +1,84 @@
 package google
 
-import "testing"
+import (
+	"testing"
+)
 
-func TestParseNetworkFieldValue(t *testing.T) {
+func TestParseGlobalFieldValue(t *testing.T) {
+	const resourceType = "networks"
 	cases := map[string]struct {
-		Network              string
+		FieldValue           string
 		ExpectedRelativeLink string
+		ExpectedError        bool
+		IsEmptyValid         bool
+		ProjectSchemaField   string
+		ProjectSchemaValue   string
 		Config               *Config
 	}{
 		"network is a full self link": {
-			Network:              "https://www.googleapis.com/compute/v1/projects/myproject/global/networks/my-network",
+			FieldValue:           "https://www.googleapis.com/compute/v1/projects/myproject/global/networks/my-network",
 			ExpectedRelativeLink: "projects/myproject/global/networks/my-network",
 		},
 		"network is a relative self link": {
-			Network:              "projects/myproject/global/networks/my-network",
+			FieldValue:           "projects/myproject/global/networks/my-network",
 			ExpectedRelativeLink: "projects/myproject/global/networks/my-network",
 		},
 		"network is a partial relative self link": {
-			Network:              "global/networks/my-network",
-			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
+			FieldValue:           "global/networks/my-network",
 			Config:               &Config{Project: "default-project"},
+			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
 		},
 		"network is the name only": {
-			Network:              "my-network",
-			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
+			FieldValue:           "my-network",
 			Config:               &Config{Project: "default-project"},
+			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
+		},
+		"network is the name only and has a project set in schema": {
+			FieldValue:           "my-network",
+			ProjectSchemaField:   "project",
+			ProjectSchemaValue:   "schema-project",
+			Config:               &Config{Project: "default-project"},
+			ExpectedRelativeLink: "projects/schema-project/global/networks/my-network",
+		},
+		"network is the name only and has a project set in schema but the field is not specified.": {
+			FieldValue:           "my-network",
+			ProjectSchemaValue:   "schema-project",
+			Config:               &Config{Project: "default-project"},
+			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
+		},
+		"network is empty and it is valid": {
+			FieldValue:           "",
+			IsEmptyValid:         true,
+			ExpectedRelativeLink: "",
+		},
+		"network is empty and it is not valid": {
+			FieldValue:    "",
+			IsEmptyValid:  false,
+			ExpectedError: true,
 		},
 	}
 
 	for tn, tc := range cases {
-		if fieldValue := ParseNetworkFieldValue(tc.Network, tc.Config); fieldValue.RelativeLink() != tc.ExpectedRelativeLink {
-			t.Fatalf("bad: %s, expected relative link to be '%s' but got '%s'", tn, tc.ExpectedRelativeLink, fieldValue.RelativeLink())
+		fieldsInSchema := make(map[string]interface{})
+
+		if len(tc.ProjectSchemaValue) > 0 && len(tc.ProjectSchemaField) > 0 {
+			fieldsInSchema[tc.ProjectSchemaField] = tc.ProjectSchemaValue
+		}
+
+		d := &ResourceDataMock{
+			FieldsInSchema: fieldsInSchema,
+		}
+
+		v, err := parseGlobalFieldValue(resourceType, tc.FieldValue, tc.ProjectSchemaField, d, tc.Config, tc.IsEmptyValid)
+
+		if err != nil {
+			if !tc.ExpectedError {
+				t.Errorf("bad: %s, did not expect an error. Error: %s", tn, err)
+			}
+		} else {
+			if v.RelativeLink() != tc.ExpectedRelativeLink {
+				t.Errorf("bad: %s, expected relative link to be '%s' but got '%s'", tn, tc.ExpectedRelativeLink, v.RelativeLink())
+			}
 		}
 	}
 }

google/resource_compute_firewall.go

@@ -405,6 +405,11 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err
 func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersion ComputeApiVersion) (*computeBeta.Firewall, error) {
 	config := meta.(*Config)
 
+	network, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return nil, err
+	}
+
 	// Build up the list of allowed entries
 	var allowed []*computeBeta.FirewallAllowed
 	if v := d.Get("allow").(*schema.Set); v.Len() > 0 {
@@ -487,7 +492,7 @@ func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersio
 		Name:              d.Get("name").(string),
 		Description:       d.Get("description").(string),
 		Direction:         d.Get("direction").(string),
-		Network:           ParseNetworkFieldValue(d.Get("network").(string), config).RelativeLink(),
+		Network:           network.RelativeLink(),
 		Allowed:           allowed,
 		Denied:            denied,
 		SourceRanges:      sourceRanges,

google/resource_compute_forwarding_rule.go

@@ -125,6 +125,11 @@ func resourceComputeForwardingRule() *schema.Resource {
 func resourceComputeForwardingRuleCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 
+	network, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
+
 	region, err := getRegion(d, config)
 	if err != nil {
 		return err
@@ -148,7 +153,7 @@ func resourceComputeForwardingRuleCreate(d *schema.ResourceData, meta interface{
 		Description:         d.Get("description").(string),
 		LoadBalancingScheme: d.Get("load_balancing_scheme").(string),
 		Name:                d.Get("name").(string),
-		Network:             ParseNetworkFieldValue(d.Get("network").(string), config).RelativeLink(),
+		Network:             network.RelativeLink(),
 		PortRange:           d.Get("port_range").(string),
 		Ports:               ports,
 		Subnetwork:          d.Get("subnetwork").(string),

google/resource_compute_network_peering.go

@@ -59,7 +59,10 @@ func resourceComputeNetworkPeering() *schema.Resource {
 
 func resourceComputeNetworkPeeringCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
-	networkFieldValue := ParseNetworkFieldValue(d.Get("network").(string), config)
+	networkFieldValue, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
 
 	request := &compute.NetworksAddPeeringRequest{
 		Name:             d.Get("name").(string),
@@ -86,7 +89,10 @@ func resourceComputeNetworkPeeringRead(d *schema.ResourceData, meta interface{})
 	config := meta.(*Config)
 
 	peeringName := d.Get("name").(string)
-	networkFieldValue := ParseNetworkFieldValue(d.Get("network").(string), config)
+	networkFieldValue, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
 
 	network, err := config.clientCompute.Networks.Get(networkFieldValue.Project, networkFieldValue.Name).Do()
 	if err != nil {
@@ -113,8 +119,14 @@ func resourceComputeNetworkPeeringDelete(d *schema.ResourceData, meta interface{
 
 	// Remove the `network` to `peer_network` peering
 	name := d.Get("name").(string)
-	networkFieldValue := ParseNetworkFieldValue(d.Get("network").(string), config)
-	peerNetworkFieldValue := ParseNetworkFieldValue(d.Get("peer_network").(string), config)
+	networkFieldValue, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
+	peerNetworkFieldValue, err := ParseNetworkFieldValue(d.Get("peer_network").(string), d, config)
+	if err != nil {
+		return err
+	}
 
 	request := &compute.NetworksRemovePeeringRequest{
 		Name: name,

google/resource_compute_router.go

@@ -99,7 +99,11 @@ func resourceComputeRouterCreate(d *schema.ResourceData, meta interface{}) error
 	mutexKV.Lock(routerLock)
 	defer mutexKV.Unlock(routerLock)
 
-	network := ParseNetworkFieldValue(d.Get("network").(string), config)
+	network, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
+
 	routersService := config.clientCompute.Routers
 
 	router := &compute.Router{

google/resource_compute_router_test.go

@@ -166,7 +166,6 @@ func testAccComputeRouterNoRegion(providerRegion string) string {
 			name = "router-test-subnetwork-%s"
 			network = "${google_compute_network.foobar.name}"
 			ip_cidr_range = "10.0.0.0/16"
-			ip_cidr_range = "10.0.0.0/16"
 			region = "%s"
 		}
 		resource "google_compute_router" "foobar" {

google/resource_compute_subnetwork.go

@@ -99,6 +99,10 @@ func resourceComputeSubnetwork() *schema.Resource {
 
 func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
+	network, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
 
 	region, err := getRegion(d, config)
 	if err != nil {
@@ -117,7 +121,7 @@ func resourceComputeSubnetworkCreate(d *schema.ResourceData, meta interface{}) e
 		IpCidrRange:           d.Get("ip_cidr_range").(string),
 		PrivateIpGoogleAccess: d.Get("private_ip_google_access").(bool),
 		SecondaryIpRanges:     expandSecondaryRanges(d.Get("secondary_ip_range").([]interface{})),
-		Network:               ParseNetworkFieldValue(d.Get("network").(string), config).RelativeLink(),
+		Network:               network.RelativeLink(),
 	}
 
 	log.Printf("[DEBUG] Subnetwork insert request: %#v", subnetwork)

google/resource_compute_vpn_gateway.go

@@ -58,6 +58,10 @@ func resourceComputeVpnGateway() *schema.Resource {
 
 func resourceComputeVpnGatewayCreate(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
+	network, err := ParseNetworkFieldValue(d.Get("network").(string), d, config)
+	if err != nil {
+		return err
+	}
 
 	region, err := getRegion(d, config)
 	if err != nil {
@@ -70,7 +74,6 @@ func resourceComputeVpnGatewayCreate(d *schema.ResourceData, meta interface{}) e
 	}
 
 	name := d.Get("name").(string)
-	network := ParseNetworkFieldValue(d.Get("network").(string), config)
 
 	vpnGatewaysService := compute.NewTargetVpnGatewaysService(config.clientCompute)
 

google/utils.go

@@ -59,14 +59,7 @@ func getRegionFromInstanceState(is *terraform.InstanceState, config *Config) (st
 // back to the provider's value if not given. If the provider's value is not
 // given, an error is returned.
 func getProject(d *schema.ResourceData, config *Config) (string, error) {
-	res, ok := d.GetOk("project")
-	if !ok {
-		if config.Project != "" {
-			return config.Project, nil
-		}
-		return "", fmt.Errorf("project: required field is not set")
-	}
-	return res.(string), nil
+	return getProjectFromSchema("project", d, config)
 }
 
 func getProjectFromInstanceState(is *terraform.InstanceState, config *Config) (string, error) {