From 2d0d8bdcc04c134bdc1c10ffb0aee27d54923c32 Mon Sep 17 00:00:00 2001 From: Riley Karson Date: Mon, 7 Aug 2017 13:14:35 -0700 Subject: [PATCH] Add Beta support & Beta feature deny to google_compute_firewall (#282) * Add versioned Beta support to google_compute_firewall. * Add Beta support for deny to google_compute_firewall. * remove extra line: * make fmt * Add missing ForceNew fields. * Respond to review comments testing functionality + reducing network GET to v1 --- google/resource_compute_firewall.go | 209 ++++++++++++++---- google/resource_compute_firewall_test.go | 86 +++++++ website/docs/r/compute_firewall.html.markdown | 13 ++ 3 files changed, 270 insertions(+), 38 deletions(-) diff --git a/google/resource_compute_firewall.go b/google/resource_compute_firewall.go index 07fb4225eab..13ee91be621 100644 --- a/google/resource_compute_firewall.go +++ b/google/resource_compute_firewall.go @@ -8,9 +8,14 @@ import ( "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/schema" + + computeBeta "google.golang.org/api/compute/v0.beta" "google.golang.org/api/compute/v1" ) +var FirewallBaseApiVersion = v1 +var FirewallVersionedFeatures = []Feature{Feature{Version: v0beta, Item: "deny"}} + func resourceComputeFirewall() *schema.Resource { return &schema.Resource{ Create: resourceComputeFirewallCreate, @@ -37,8 +42,9 @@ func resourceComputeFirewall() *schema.Resource { }, "allow": { - Type: schema.TypeSet, - Required: true, + Type: schema.TypeSet, + Optional: true, + ConflictsWith: []string{"deny"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "protocol": { @@ -53,7 +59,33 @@ func resourceComputeFirewall() *schema.Resource { }, }, }, - Set: resourceComputeFirewallAllowHash, + Set: resourceComputeFirewallRuleHash, + }, + + "deny": { + Type: schema.TypeSet, + Optional: true, + ConflictsWith: []string{"allow"}, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "protocol": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "ports": { + Type: schema.TypeList, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + ForceNew: true, + }, + }, + }, + Set: resourceComputeFirewallRuleHash, + + // Unlike allow, deny can't be updated upstream + ForceNew: true, }, "description": { @@ -98,7 +130,7 @@ func resourceComputeFirewall() *schema.Resource { } } -func resourceComputeFirewallAllowHash(v interface{}) int { +func resourceComputeFirewallRuleHash(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) buf.WriteString(fmt.Sprintf("%s-", m["protocol"].(string))) @@ -118,6 +150,7 @@ func resourceComputeFirewallAllowHash(v interface{}) int { } func resourceComputeFirewallCreate(d *schema.ResourceData, meta interface{}) error { + computeApiVersion := getComputeApiVersion(d, FirewallBaseApiVersion, FirewallVersionedFeatures) config := meta.(*Config) project, err := getProject(d, config) @@ -125,21 +158,41 @@ func resourceComputeFirewallCreate(d *schema.ResourceData, meta interface{}) err return err } - firewall, err := resourceFirewall(d, meta) + firewall, err := resourceFirewall(d, meta, computeApiVersion) if err != nil { return err } - op, err := config.clientCompute.Firewalls.Insert( - project, firewall).Do() - if err != nil { - return fmt.Errorf("Error creating firewall: %s", err) + var op interface{} + switch computeApiVersion { + case v1: + firewallV1 := &compute.Firewall{} + err := Convert(firewall, firewallV1) + if err != nil { + return err + } + + op, err = config.clientCompute.Firewalls.Insert(project, firewallV1).Do() + if err != nil { + return fmt.Errorf("Error creating firewall: %s", err) + } + case v0beta: + firewallV0Beta := &computeBeta.Firewall{} + err := Convert(firewall, firewallV0Beta) + if err != nil { + return err + } + + op, err = config.clientComputeBeta.Firewalls.Insert(project, firewallV0Beta).Do() + if err != nil { + return fmt.Errorf("Error creating firewall: %s", err) + } } // It probably maybe worked, so store the ID now d.SetId(firewall.Name) - err = computeOperationWait(config, op, project, "Creating Firewall") + err = computeSharedOperationWait(config, op, project, "Creating Firewall") if err != nil { return err } @@ -147,7 +200,7 @@ func resourceComputeFirewallCreate(d *schema.ResourceData, meta interface{}) err return resourceComputeFirewallRead(d, meta) } -func flattenAllowed(allowed []*compute.FirewallAllowed) []map[string]interface{} { +func flattenAllowed(allowed []*computeBeta.FirewallAllowed) []map[string]interface{} { result := make([]map[string]interface{}, 0, len(allowed)) for _, allow := range allowed { allowMap := make(map[string]interface{}) @@ -159,7 +212,20 @@ func flattenAllowed(allowed []*compute.FirewallAllowed) []map[string]interface{} return result } +func flattenDenied(denied []*computeBeta.FirewallDenied) []map[string]interface{} { + result := make([]map[string]interface{}, 0, len(denied)) + for _, deny := range denied { + denyMap := make(map[string]interface{}) + denyMap["protocol"] = deny.IPProtocol + denyMap["ports"] = deny.Ports + + result = append(result, denyMap) + } + return result +} + func resourceComputeFirewallRead(d *schema.ResourceData, meta interface{}) error { + computeApiVersion := getComputeApiVersion(d, FirewallBaseApiVersion, FirewallVersionedFeatures) config := meta.(*Config) project, err := getProject(d, config) @@ -167,14 +233,32 @@ func resourceComputeFirewallRead(d *schema.ResourceData, meta interface{}) error return err } - firewall, err := config.clientCompute.Firewalls.Get( - project, d.Id()).Do() - if err != nil { - return handleNotFoundError(err, d, fmt.Sprintf("Firewall %q", d.Get("name").(string))) + firewall := &computeBeta.Firewall{} + switch computeApiVersion { + case v1: + firewallV1, err := config.clientCompute.Firewalls.Get(project, d.Id()).Do() + if err != nil { + return handleNotFoundError(err, d, fmt.Sprintf("Firewall %q", d.Get("name").(string))) + } + + err = Convert(firewallV1, firewall) + if err != nil { + return err + } + case v0beta: + firewallV0Beta, err := config.clientComputeBeta.Firewalls.Get(project, d.Id()).Do() + if err != nil { + return handleNotFoundError(err, d, fmt.Sprintf("Firewall %q", d.Get("name").(string))) + } + + err = Convert(firewallV0Beta, firewall) + if err != nil { + return err + } } networkUrl := strings.Split(firewall.Network, "/") - d.Set("self_link", firewall.SelfLink) + d.Set("self_link", ConvertSelfLinkToV1(firewall.SelfLink)) d.Set("name", firewall.Name) d.Set("network", networkUrl[len(networkUrl)-1]) d.Set("description", firewall.Description) @@ -183,10 +267,12 @@ func resourceComputeFirewallRead(d *schema.ResourceData, meta interface{}) error d.Set("source_tags", firewall.SourceTags) d.Set("target_tags", firewall.TargetTags) d.Set("allow", flattenAllowed(firewall.Allowed)) + d.Set("deny", flattenDenied(firewall.Denied)) return nil } func resourceComputeFirewallUpdate(d *schema.ResourceData, meta interface{}) error { + computeApiVersion := getComputeApiVersionUpdate(d, FirewallBaseApiVersion, FirewallVersionedFeatures, []Feature{}) config := meta.(*Config) project, err := getProject(d, config) @@ -196,18 +282,38 @@ func resourceComputeFirewallUpdate(d *schema.ResourceData, meta interface{}) err d.Partial(true) - firewall, err := resourceFirewall(d, meta) + firewall, err := resourceFirewall(d, meta, computeApiVersion) if err != nil { return err } - op, err := config.clientCompute.Firewalls.Update( - project, d.Id(), firewall).Do() - if err != nil { - return fmt.Errorf("Error updating firewall: %s", err) + var op interface{} + switch computeApiVersion { + case v1: + firewallV1 := &compute.Firewall{} + err := Convert(firewall, firewallV1) + if err != nil { + return err + } + + op, err = config.clientCompute.Firewalls.Update(project, d.Id(), firewallV1).Do() + if err != nil { + return fmt.Errorf("Error updating firewall: %s", err) + } + case v0beta: + firewallV0Beta := &computeBeta.Firewall{} + err := Convert(firewall, firewallV0Beta) + if err != nil { + return err + } + + op, err = config.clientComputeBeta.Firewalls.Update(project, d.Id(), firewallV0Beta).Do() + if err != nil { + return fmt.Errorf("Error updating firewall: %s", err) + } } - err = computeOperationWait(config, op, project, "Updating Firewall") + err = computeSharedOperationWait(config, op, project, "Updating Firewall") if err != nil { return err } @@ -218,6 +324,7 @@ func resourceComputeFirewallUpdate(d *schema.ResourceData, meta interface{}) err } func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) error { + computeApiVersion := getComputeApiVersion(d, FirewallBaseApiVersion, FirewallVersionedFeatures) config := meta.(*Config) project, err := getProject(d, config) @@ -226,13 +333,21 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err } // Delete the firewall - op, err := config.clientCompute.Firewalls.Delete( - project, d.Id()).Do() - if err != nil { - return fmt.Errorf("Error deleting firewall: %s", err) + var op interface{} + switch computeApiVersion { + case v1: + op, err = config.clientCompute.Firewalls.Delete(project, d.Id()).Do() + if err != nil { + return fmt.Errorf("Error deleting firewall: %s", err) + } + case v0beta: + op, err = config.clientComputeBeta.Firewalls.Delete(project, d.Id()).Do() + if err != nil { + return fmt.Errorf("Error deleting firewall: %s", err) + } } - err = computeOperationWait(config, op, project, "Deleting Firewall") + err = computeSharedOperationWait(config, op, project, "Deleting Firewall") if err != nil { return err } @@ -241,24 +356,41 @@ func resourceComputeFirewallDelete(d *schema.ResourceData, meta interface{}) err return nil } -func resourceFirewall( - d *schema.ResourceData, - meta interface{}) (*compute.Firewall, error) { +func resourceFirewall(d *schema.ResourceData, meta interface{}, computeApiVersion ComputeApiVersion) (*computeBeta.Firewall, error) { config := meta.(*Config) - project, _ := getProject(d, config) - // Look up the network to attach the firewall to - network, err := config.clientCompute.Networks.Get( - project, d.Get("network").(string)).Do() + 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 []*compute.FirewallAllowed + var allowed []*computeBeta.FirewallAllowed if v := d.Get("allow").(*schema.Set); v.Len() > 0 { - allowed = make([]*compute.FirewallAllowed, 0, v.Len()) + allowed = make([]*computeBeta.FirewallAllowed, 0, v.Len()) + for _, v := range v.List() { + m := v.(map[string]interface{}) + + var ports []string + if v := convertStringArr(m["ports"].([]interface{})); len(v) > 0 { + ports = make([]string, len(v)) + for i, v := range v { + ports[i] = v + } + } + + allowed = append(allowed, &computeBeta.FirewallAllowed{ + IPProtocol: m["protocol"].(string), + Ports: ports, + }) + } + } + + // Build up the list of denied entries + var denied []*computeBeta.FirewallDenied + if v := d.Get("deny").(*schema.Set); v.Len() > 0 { + denied = make([]*computeBeta.FirewallDenied, 0, v.Len()) for _, v := range v.List() { m := v.(map[string]interface{}) @@ -270,7 +402,7 @@ func resourceFirewall( } } - allowed = append(allowed, &compute.FirewallAllowed{ + denied = append(denied, &computeBeta.FirewallDenied{ IPProtocol: m["protocol"].(string), Ports: ports, }) @@ -302,11 +434,12 @@ func resourceFirewall( } // Build the firewall parameter - return &compute.Firewall{ + return &computeBeta.Firewall{ Name: d.Get("name").(string), Description: d.Get("description").(string), Network: network.SelfLink, Allowed: allowed, + Denied: denied, SourceRanges: sourceRanges, SourceTags: sourceTags, TargetTags: targetTags, diff --git a/google/resource_compute_firewall_test.go b/google/resource_compute_firewall_test.go index 01bafcbb2a3..2ea6bf97fe6 100644 --- a/google/resource_compute_firewall_test.go +++ b/google/resource_compute_firewall_test.go @@ -7,6 +7,8 @@ import ( "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + + computeBeta "google.golang.org/api/compute/v0.beta" "google.golang.org/api/compute/v1" ) @@ -82,6 +84,27 @@ func TestAccComputeFirewall_noSource(t *testing.T) { }) } +func TestAccComputeFirewall_denied(t *testing.T) { + var firewall computeBeta.Firewall + networkName := fmt.Sprintf("firewall-test-%s", acctest.RandString(10)) + firewallName := fmt.Sprintf("firewall-test-%s", acctest.RandString(10)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeFirewallDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeFirewall_denied(networkName, firewallName), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeBetaFirewallExists("google_compute_firewall.foobar", &firewall), + testAccCheckComputeBetaFirewallDenyPorts(&firewall, "22"), + ), + }, + }, + }) +} + func testAccCheckComputeFirewallDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -129,6 +152,35 @@ func testAccCheckComputeFirewallExists(n string, firewall *compute.Firewall) res } } +func testAccCheckComputeBetaFirewallExists(n string, firewall *computeBeta.Firewall) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No ID is set") + } + + config := testAccProvider.Meta().(*Config) + + found, err := config.clientComputeBeta.Firewalls.Get( + config.Project, rs.Primary.ID).Do() + if err != nil { + return err + } + + if found.Name != rs.Primary.ID { + return fmt.Errorf("Firewall not found") + } + + *firewall = *found + + return nil + } +} + func testAccCheckComputeFirewallPorts( firewall *compute.Firewall, ports string) resource.TestCheckFunc { return func(s *terraform.State) error { @@ -144,6 +196,20 @@ func testAccCheckComputeFirewallPorts( } } +func testAccCheckComputeBetaFirewallDenyPorts(firewall *computeBeta.Firewall, ports string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if len(firewall.Denied) == 0 { + return fmt.Errorf("no denied rules") + } + + if firewall.Denied[0].Ports[0] != ports { + return fmt.Errorf("bad: %#v", firewall.Denied[0].Ports) + } + + return nil + } +} + func testAccComputeFirewall_basic(network, firewall string) string { return fmt.Sprintf(` resource "google_compute_network" "foobar" { @@ -201,3 +267,23 @@ func testAccComputeFirewall_noSource(network, firewall string) string { } }`, network, firewall) } + +func testAccComputeFirewall_denied(network, firewall string) string { + return fmt.Sprintf(` + resource "google_compute_network" "foobar" { + name = "%s" + ipv4_range = "10.0.0.0/16" + } + + resource "google_compute_firewall" "foobar" { + name = "firewall-test-%s" + description = "Resource created for Terraform acceptance testing" + network = "${google_compute_network.foobar.name}" + source_tags = ["foo"] + + deny { + protocol = "tcp" + ports = [22] + } + }`, network, firewall) +} diff --git a/website/docs/r/compute_firewall.html.markdown b/website/docs/r/compute_firewall.html.markdown index fb6a423ac39..a7e3bdbb6f7 100644 --- a/website/docs/r/compute_firewall.html.markdown +++ b/website/docs/r/compute_firewall.html.markdown @@ -59,10 +59,23 @@ The following arguments are supported: * `target_tags` - (Optional) A list of target tags for this firewall. +- - - + +* `deny` - (Optional, Beta) Can be specified multiple times for each deny + rule. Each deny block supports fields documented below. Can be specified + instead of allow. + The `allow` block supports: * `protocol` - (Required) The name of the protocol to allow. +* `ports` - (Optional) List of ports and/or port ranges to allow. This can + only be specified if the protocol is TCP or UDP. + +The `deny` block supports: + +* `protocol` - (Required) The name of the protocol to allow. + * `ports` - (Optional) List of ports and/or port ranges to allow. This can only be specified if the protocol is TCP or UDP.