compute instance helpers / metadata cleanup (#797)

* get rid of getSubnetworkLink and getProjectAndRegionFromSubnetworkLink

* dedupe metadata flatteners

* update flattenMetadata comment

google/compute_instance_helpers.go

@@ -2,7 +2,6 @@ package google
 
 import (
 	"fmt"
-	"regexp"
 
 	"github.com/hashicorp/terraform/helper/schema"
 	computeBeta "google.golang.org/api/compute/v0.beta"
@@ -44,16 +43,6 @@ func flattenScheduling(scheduling *computeBeta.Scheduling) []map[string]interfac
 	return result
 }
 
-func getProjectAndRegionFromSubnetworkLink(subnetwork string) (string, string) {
-	r := regexp.MustCompile(SubnetworkLinkRegex)
-	if !r.MatchString(subnetwork) {
-		return "", ""
-	}
-
-	matches := r.FindStringSubmatch(subnetwork)
-	return matches[1], matches[2]
-}
-
 func flattenAccessConfigs(accessConfigs []*computeBeta.AccessConfig) ([]map[string]interface{}, string) {
 	flattened := make([]map[string]interface{}, len(accessConfigs))
 	natIP := ""
@@ -69,23 +58,26 @@ func flattenAccessConfigs(accessConfigs []*computeBeta.AccessConfig) ([]map[stri
 	return flattened, natIP
 }
 
-func flattenNetworkInterfaces(networkInterfaces []*computeBeta.NetworkInterface) ([]map[string]interface{}, string, string, string) {
+func flattenNetworkInterfaces(d *schema.ResourceData, config *Config, networkInterfaces []*computeBeta.NetworkInterface) ([]map[string]interface{}, string, string, string, error) {
 	flattened := make([]map[string]interface{}, len(networkInterfaces))
 	var region, internalIP, externalIP string
 
 	for i, iface := range networkInterfaces {
 		var ac []map[string]interface{}
 		ac, externalIP = flattenAccessConfigs(iface.AccessConfigs)
 
-		var project string
-		project, region = getProjectAndRegionFromSubnetworkLink(iface.Subnetwork)
+		subnet, err := ParseSubnetworkFieldValue(iface.Subnetwork, d, config)
+		if err != nil {
+			return nil, "", "", "", err
+		}
+		region = subnet.Region
 
 		flattened[i] = map[string]interface{}{
 			"address":            iface.NetworkIP,
 			"network_ip":         iface.NetworkIP,
 			"network":            iface.Network,
 			"subnetwork":         iface.Subnetwork,
-			"subnetwork_project": project,
+			"subnetwork_project": subnet.Project,
 			"access_config":      ac,
 			"alias_ip_range":     flattenAliasIpRange(iface.AliasIpRanges),
 		}
@@ -100,7 +92,7 @@ func flattenNetworkInterfaces(networkInterfaces []*computeBeta.NetworkInterface)
 			internalIP = iface.NetworkIP
 		}
 	}
-	return flattened, region, internalIP, externalIP
+	return flattened, region, internalIP, externalIP, nil
 }
 
 func expandAccessConfigs(configs []interface{}) []*computeBeta.AccessConfig {
@@ -116,15 +108,6 @@ func expandAccessConfigs(configs []interface{}) []*computeBeta.AccessConfig {
 }
 
 func expandNetworkInterfaces(d *schema.ResourceData, config *Config) ([]*computeBeta.NetworkInterface, error) {
-	project, err := getProject(d, config)
-	if err != nil {
-		return nil, err
-	}
-	region, err := getRegion(d, config)
-	if err != nil {
-		return nil, err
-	}
-
 	configs := d.Get("network_interface").([]interface{})
 	ifaces := make([]*computeBeta.NetworkInterface, len(configs))
 	for i, raw := range configs {
@@ -138,19 +121,19 @@ func expandNetworkInterfaces(d *schema.ResourceData, config *Config) ([]*compute
 
 		nf, err := ParseNetworkFieldValue(network, d, config)
 		if err != nil {
-			return nil, fmt.Errorf("cannot determine selflink for subnetwork '%s': %s", subnetwork, err)
+			return nil, fmt.Errorf("cannot determine selflink for network '%s': %s", network, err)
 		}
 
-		subnetworkProject := data["subnetwork_project"].(string)
-		subnetLink, err := getSubnetworkLink(config, project, region, subnetworkProject, subnetwork)
+		subnetProjectField := fmt.Sprintf("network_interface.%d.subnetwork_project", i)
+		sf, err := ParseSubnetworkFieldValueWithProjectField(subnetwork, subnetProjectField, d, config)
 		if err != nil {
 			return nil, fmt.Errorf("cannot determine selflink for subnetwork '%s': %s", subnetwork, err)
 		}
 
 		ifaces[i] = &computeBeta.NetworkInterface{
 			NetworkIP:     data["network_ip"].(string),
 			Network:       nf.RelativeLink(),
-			Subnetwork:    subnetLink,
+			Subnetwork:    sf.RelativeLink(),
 			AccessConfigs: expandAccessConfigs(data["access_config"].([]interface{})),
 			AliasIpRanges: expandAliasIpRanges(data["alias_ip_range"].([]interface{})),
 		}

google/field_helpers.go

@@ -30,6 +30,10 @@ func ParseSubnetworkFieldValue(subnetwork string, d TerraformResourceData, confi
 	return parseRegionalFieldValue("subnetworks", subnetwork, "project", "region", d, config, true)
 }
 
+func ParseSubnetworkFieldValueWithProjectField(subnetwork, projectField string, d TerraformResourceData, config *Config) (*RegionalFieldValue, error) {
+	return parseRegionalFieldValue("subnetworks", subnetwork, projectField, "region", d, config, true)
+}
+
 func ParseSslCertificateFieldValue(sslCertificate string, d TerraformResourceData, config *Config) (*GlobalFieldValue, error) {
 	return parseGlobalFieldValue("sslCertificates", sslCertificate, "project", d, config, false)
 }

google/metadata.go

@@ -2,7 +2,6 @@ package google
 
 import (
 	"fmt"
-	"log"
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
@@ -111,29 +110,6 @@ func BetaMetadataUpdate(oldMDMap map[string]interface{}, newMDMap map[string]int
 	}
 }
 
-// flattenComputeMetadata transforms a list of MetadataItems (as returned via the GCP client) into a simple map from key
-// to value.
-func flattenComputeMetadata(metadata []*compute.MetadataItems) map[string]string {
-	m := map[string]string{}
-
-	for _, item := range metadata {
-		// check for duplicates
-		if item.Value == nil {
-			continue
-		}
-		if val, ok := m[item.Key]; ok {
-			// warn loudly!
-			log.Printf("[WARN] Key '%s' already has value '%s' when flattening - ignoring incoming value '%s'",
-				item.Key,
-				val,
-				*item.Value)
-		}
-		m[item.Key] = *item.Value
-	}
-
-	return m
-}
-
 // expandComputeMetadata transforms a map representing computing metadata into a list of compute.MetadataItems suitable
 // for the GCP client.
 func expandComputeMetadata(m map[string]string) []*compute.MetadataItems {
@@ -151,19 +127,19 @@ func expandComputeMetadata(m map[string]string) []*compute.MetadataItems {
 	return metadata
 }
 
-func flattenMetadata(metadata *computeBeta.Metadata) map[string]string {
+func flattenMetadataBeta(metadata *computeBeta.Metadata) map[string]string {
 	metadataMap := make(map[string]string)
 	for _, item := range metadata.Items {
 		metadataMap[item.Key] = *item.Value
 	}
 	return metadataMap
 }
 
-// This function differs from flattenMetadata only in that it takes
+// This function differs from flattenMetadataBeta only in that it takes
 // compute.metadata rather than computeBeta.metadata as an argument. It should
-// be removed in favour of flattenMetadata if/when this resource gets beta
-// support.
-func flattenCommonInstanceMetadata(metadata *compute.Metadata) map[string]string {
+// be removed in favour of flattenMetadataBeta if/when all resources using it get
+// beta support.
+func flattenMetadata(metadata *compute.Metadata) map[string]string {
 	metadataMap := make(map[string]string)
 	for _, item := range metadata.Items {
 		metadataMap[item.Key] = *item.Value

google/resource_compute_instance.go

@@ -720,7 +720,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 		return err
 	}
 
-	md := flattenMetadata(instance.Metadata)
+	md := flattenMetadataBeta(instance.Metadata)
 
 	if _, scriptExists := d.GetOk("metadata_startup_script"); scriptExists {
 		d.Set("metadata_startup_script", md["startup-script"])
@@ -750,7 +750,10 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
 
 	// Set the networks
 	// Use the first external IP found for the default connection info.
-	networkInterfaces, _, internalIP, externalIP := flattenNetworkInterfaces(instance.NetworkInterfaces)
+	networkInterfaces, _, internalIP, externalIP, err := flattenNetworkInterfaces(d, config, instance.NetworkInterfaces)
+	if err != nil {
+		return err
+	}
 	d.Set("network_interface", networkInterfaces)
 
 	// Fall back on internal ip if there is no external ip.  This makes sense in the situation where

google/resource_compute_instance_template.go

@@ -686,7 +686,7 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 
 		md := instanceTemplate.Properties.Metadata
 
-		_md := flattenMetadata(md)
+		_md := flattenMetadataBeta(md)
 
 		if script, scriptExists := d.GetOk("metadata_startup_script"); scriptExists {
 			if err = d.Set("metadata_startup_script", script); err != nil {
@@ -737,7 +737,10 @@ func resourceComputeInstanceTemplateRead(d *schema.ResourceData, meta interface{
 		return fmt.Errorf("Error setting project: %s", err)
 	}
 	if instanceTemplate.Properties.NetworkInterfaces != nil {
-		networkInterfaces, region, _, _ := flattenNetworkInterfaces(instanceTemplate.Properties.NetworkInterfaces)
+		networkInterfaces, region, _, _, err := flattenNetworkInterfaces(d, config, instanceTemplate.Properties.NetworkInterfaces)
+		if err != nil {
+			return err
+		}
 		if err = d.Set("network_interface", networkInterfaces); err != nil {
 			return fmt.Errorf("Error setting network_interface: %s", err)
 		}

google/resource_compute_project_metadata.go

@@ -106,7 +106,7 @@ func resourceComputeProjectMetadataRead(d *schema.ResourceData, meta interface{}
 		return handleNotFoundError(err, d, fmt.Sprintf("Project metadata for project %q", projectID))
 	}
 
-	md := flattenCommonInstanceMetadata(project.CommonInstanceMetadata)
+	md := flattenMetadata(project.CommonInstanceMetadata)
 	existingMetadata := d.Get("metadata").(map[string]interface{})
 	// Remove all keys not explicitly mentioned in the terraform config
 	for k := range md {

google/resource_compute_project_metadata_item.go

@@ -73,7 +73,7 @@ func resourceComputeProjectMetadataItemRead(d *schema.ResourceData, meta interfa
 		return fmt.Errorf("Error loading project '%s': %s", projectID, err)
 	}
 
-	md := flattenComputeMetadata(project.CommonInstanceMetadata.Items)
+	md := flattenMetadata(project.CommonInstanceMetadata)
 	val, ok := md[d.Id()]
 	if !ok {
 		// Resource no longer exists
@@ -136,7 +136,7 @@ func updateComputeCommonInstanceMetadata(config *Config, projectID string, key s
 			return fmt.Errorf("Error loading project '%s': %s", projectID, err)
 		}
 
-		md := flattenComputeMetadata(project.CommonInstanceMetadata.Items)
+		md := flattenMetadata(project.CommonInstanceMetadata)
 
 		val, ok := md[key]
 

google/resource_compute_project_metadata_item_test.go

@@ -113,7 +113,7 @@ func testAccCheckProjectMetadataItem_hasMetadata(key, value string) resource.Tes
 			return err
 		}
 
-		metadata := flattenComputeMetadata(project.CommonInstanceMetadata.Items)
+		metadata := flattenMetadata(project.CommonInstanceMetadata)
 
 		val, ok := metadata[key]
 		if !ok {
@@ -134,7 +134,7 @@ func testAccCheckProjectMetadataItemDestroy(s *terraform.State) error {
 		return err
 	}
 
-	metadata := flattenComputeMetadata(project.CommonInstanceMetadata.Items)
+	metadata := flattenMetadata(project.CommonInstanceMetadata)
 
 	for _, rs := range s.RootModule().Resources {
 		if rs.Type != "google_compute_project_metadata_item" {

google/utils.go

@@ -5,7 +5,6 @@ package google
 import (
 	"fmt"
 	"log"
-	"regexp"
 	"strings"
 	"time"
 
@@ -124,33 +123,6 @@ func getZonalBetaResourceFromRegion(getResource func(string) (interface{}, error
 	return nil, nil
 }
 
-// getSubnetworkLink takes the "subnetwork" field and if the value is:
-// - a resource URL, returns the string unchanged
-// - a subnetwork name, looks up the resource URL using the google client.
-//
-// If `subnetworkField` is a resource url, `subnetworkProjectField` cannot be set.
-// If `subnetworkField` is a subnetwork name, `subnetworkProjectField` will be used
-// 	as the project if set. If not, we fallback on the default project.
-func getSubnetworkLink(config *Config, defaultProject, region, subnetworkProject, subnetwork string) (string, error) {
-	if subnetwork == "" {
-		return "", nil
-	}
-
-	if regexp.MustCompile(SubnetworkLinkRegex).MatchString(subnetwork) {
-		return subnetwork, nil
-	}
-
-	project := defaultProject
-	if subnetworkProject != "" {
-		project = subnetworkProject
-	}
-	subnetworkData, err := config.clientCompute.Subnetworks.Get(project, region, subnetwork).Do()
-	if err != nil {
-		return "", fmt.Errorf("Error referencing subnetwork '%s' in region '%s': %s", subnetwork, region, err)
-	}
-	return subnetworkData.SelfLink, nil
-}
-
 // getNetworkName reads the "network" field from the given resource data and if the value:
 // - is a resource URL, extracts the network name from the URL and returns it
 // - is the network name only (i.e not prefixed with http://www.googleapis.com/compute/...), is returned unchanged