Firewall network field now supports self_link in addition of name (#477)

google/field_helpers.go

@@ -0,0 +1,40 @@
+package google
+
+import (
+	"fmt"
+	"regexp"
+)
+
+const networkLinkTemplate = "projects/%s/global/networks/%s"
+
+var networkLinkRegex = regexp.MustCompile("projects/(.+)/global/networks/(.+)")
+
+type NetworkFieldValue struct {
+	Project string
+	Name    string
+}
+
+// Parses a `network` supporting 4 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)
+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 {
+	return fmt.Sprintf(networkLinkTemplate, f.Project, f.Name)
+}

google/field_helpers_test.go

@@ -0,0 +1,36 @@
+package google
+
+import "testing"
+
+func TestParseNetworkFieldValue(t *testing.T) {
+	cases := map[string]struct {
+		Network              string
+		ExpectedRelativeLink string
+		Config               *Config
+	}{
+		"network is a full self link": {
+			Network:              "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",
+			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",
+			Config:               &Config{Project: "default-project"},
+		},
+		"network is the name only": {
+			Network:              "my-network",
+			ExpectedRelativeLink: "projects/default-project/global/networks/my-network",
+			Config:               &Config{Project: "default-project"},
+		},
+	}
+
+	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())
+		}
+	}
+}

google/resource_compute_firewall.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"sort"
-	"strings"
 
 	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/schema"
@@ -44,9 +43,10 @@ func resourceComputeFirewall() *schema.Resource {
 			},
 
 			"network": {
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
+				Type:             schema.TypeString,
+				Required:         true,
+				ForceNew:         true,
+				DiffSuppressFunc: compareSelfLinkOrResourceName,
 			},
 
 			"priority": {
@@ -294,10 +294,9 @@ func resourceComputeFirewallRead(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
-	networkUrl := strings.Split(firewall.Network, "/")
 	d.Set("self_link", ConvertSelfLinkToV1(firewall.SelfLink))
 	d.Set("name", firewall.Name)
-	d.Set("network", networkUrl[len(networkUrl)-1])
+	d.Set("network", ConvertSelfLinkToV1(firewall.Network))
 
 	// Unlike most other Beta properties, direction will always have a value even when
 	// a zero is sent by the client. We'll never revert back to v1 without conditionally reading it.
@@ -405,12 +404,6 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err
 
 func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersion ComputeApiVersion) (*computeBeta.Firewall, error) {
 	config := meta.(*Config)
-	project, _ := getProject(d, config)
-
-	network, err := config.clientCompute.Networks.Get(project, d.Get("network").(string)).Do()
-	if err != nil {
-		return nil, fmt.Errorf("Error reading network: %s", err)
-	}
 
 	// Build up the list of allowed entries
 	var allowed []*computeBeta.FirewallAllowed
@@ -494,7 +487,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:           network.SelfLink,
+		Network:           ParseNetworkFieldValue(d.Get("network").(string), config).RelativeLink(),
 		Allowed:           allowed,
 		Denied:            denied,
 		SourceRanges:      sourceRanges,

google/resource_compute_firewall_test.go

@@ -10,6 +10,7 @@ import (
 
 	computeBeta "google.golang.org/api/compute/v0.beta"
 	"google.golang.org/api/compute/v1"
+	"strings"
 )
 
 func TestAccComputeFirewall_basic(t *testing.T) {
@@ -27,6 +28,7 @@ func TestAccComputeFirewall_basic(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeFirewallExists(
 						"google_compute_firewall.foobar", &firewall),
+					testAccCheckComputeFirewallApiVersion(&firewall),
 				),
 			},
 		},
@@ -48,6 +50,7 @@ func TestAccComputeFirewall_update(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeFirewallExists(
 						"google_compute_firewall.foobar", &firewall),
+					testAccCheckComputeFirewallApiVersion(&firewall),
 				),
 			},
 			resource.TestStep{
@@ -57,6 +60,7 @@ func TestAccComputeFirewall_update(t *testing.T) {
 						"google_compute_firewall.foobar", &firewall),
 					testAccCheckComputeFirewallPorts(
 						&firewall, "80-255"),
+					testAccCheckComputeFirewallApiVersion(&firewall),
 				),
 			},
 		},
@@ -78,6 +82,7 @@ func TestAccComputeFirewall_priority(t *testing.T) {
 				testAccCheckComputeBetaFirewallExists(
 					"google_compute_firewall.foobar", &firewall),
 				testAccCheckComputeFirewallHasPriority(&firewall, 1001),
+				testAccCheckComputeFirewallBetaApiVersion(&firewall),
 			),
 		}},
 	})
@@ -98,6 +103,7 @@ func TestAccComputeFirewall_noSource(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeFirewallExists(
 						"google_compute_firewall.foobar", &firewall),
+					testAccCheckComputeFirewallApiVersion(&firewall),
 				),
 			},
 		},
@@ -119,6 +125,7 @@ func TestAccComputeFirewall_denied(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeBetaFirewallExists("google_compute_firewall.foobar", &firewall),
 					testAccCheckComputeBetaFirewallDenyPorts(&firewall, "22"),
+					testAccCheckComputeFirewallBetaApiVersion(&firewall),
 				),
 			},
 		},
@@ -140,6 +147,7 @@ func TestAccComputeFirewall_egress(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckComputeBetaFirewallExists("google_compute_firewall.foobar", &firewall),
 					testAccCheckComputeBetaFirewallEgress(&firewall),
+					testAccCheckComputeFirewallBetaApiVersion(&firewall),
 				),
 			},
 		},
@@ -270,6 +278,30 @@ func testAccCheckComputeBetaFirewallEgress(firewall *computeBeta.Firewall) resou
 	}
 }
 
+func testAccCheckComputeFirewallBetaApiVersion(firewall *computeBeta.Firewall) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		// The self-link of the network field is used to determine which API was used when fetching
+		// the state from the API.
+		if !strings.Contains(firewall.Network, "compute/beta") {
+			return fmt.Errorf("firewall beta API was not used")
+		}
+
+		return nil
+	}
+}
+
+func testAccCheckComputeFirewallApiVersion(firewall *compute.Firewall) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		// The self-link of the network field is used to determine which API was used when fetching
+		// the state from the API.
+		if !strings.Contains(firewall.Network, "compute/v1") {
+			return fmt.Errorf("firewall beta API was not used")
+		}
+
+		return nil
+	}
+}
+
 func testAccComputeFirewall_basic(network, firewall string) string {
 	return fmt.Sprintf(`
 	resource "google_compute_network" "foobar" {
@@ -299,7 +331,7 @@ func testAccComputeFirewall_update(network, firewall string) string {
 	resource "google_compute_firewall" "foobar" {
 		name = "firewall-test-%s"
 		description = "Resource created for Terraform acceptance testing"
-		network = "${google_compute_network.foobar.name}"
+		network = "${google_compute_network.foobar.self_link}"
 		source_tags = ["foo"]
 
 		allow {

website/docs/r/compute_firewall.html.markdown

@@ -40,7 +40,7 @@ The following arguments are supported:
 * `name` - (Required) A unique name for the resource, required by GCE.
     Changing this forces a new resource to be created.
 
-* `network` - (Required) The name of the network to attach this firewall to.
+* `network` - (Required) The name or self_link of the network to attach this firewall to.
 
 * `allow` - (Required) Can be specified multiple times for each allow
     rule. Each allow block supports fields documented below.