From 31b12a007f18c594f7a623abd8a49cd360a43e46 Mon Sep 17 00:00:00 2001 From: Tilen Faganel Date: Thu, 30 Aug 2018 19:58:37 +0100 Subject: [PATCH] Refactored the firewall resource in order to better handle the various possible changes in configuration --- .../resource_digitalocean_firewall.go | 629 ++++++++---------- .../resource_digitalocean_firewall_test.go | 100 ++- website/docs/r/firewall.html.markdown | 18 +- 3 files changed, 335 insertions(+), 412 deletions(-) diff --git a/digitalocean/resource_digitalocean_firewall.go b/digitalocean/resource_digitalocean_firewall.go index 2da609f1..dbe203b9 100644 --- a/digitalocean/resource_digitalocean_firewall.go +++ b/digitalocean/resource_digitalocean_firewall.go @@ -1,16 +1,14 @@ package digitalocean import ( - "bytes" "context" "fmt" "log" - "strconv" "strings" "github.com/digitalocean/godo" - "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/schema" + "github.com/hashicorp/terraform/helper/validation" ) func resourceDigitalOceanFirewall() *schema.Resource { @@ -19,12 +17,113 @@ func resourceDigitalOceanFirewall() *schema.Resource { Read: resourceDigitalOceanFirewallRead, Update: resourceDigitalOceanFirewallUpdate, Delete: resourceDigitalOceanFirewallDelete, - Exists: resourceDigitalOceanFirewallExists, Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.NoZeroValues, + }, + + "droplet_ids": { + Type: schema.TypeSet, + Elem: &schema.Schema{Type: schema.TypeInt}, + Optional: true, + }, + + "inbound_rule": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "protocol": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{ + "tcp", + "udp", + "icmp", + }, false), + }, + "port_range": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.NoZeroValues, + }, + "source_addresses": { + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + Optional: true, + }, + "source_droplet_ids": { + Type: schema.TypeSet, + Elem: &schema.Schema{Type: schema.TypeInt}, + Optional: true, + }, + "source_load_balancer_uids": { + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + Optional: true, + }, + "source_tags": tagsSchema(), + }, + }, + }, + + "outbound_rule": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "protocol": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringInSlice([]string{ + "tcp", + "udp", + "icmp", + }, false), + }, + "port_range": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.NoZeroValues, + }, + "destination_addresses": { + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + Optional: true, + }, + "destination_droplet_ids": { + Type: schema.TypeSet, + Elem: &schema.Schema{Type: schema.TypeInt}, + Optional: true, + }, + "destination_load_balancer_uids": { + Type: schema.TypeSet, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.NoZeroValues, + }, + Optional: true, + }, + "destination_tags": tagsSchema(), + }, + }, + }, + "status": { Type: schema.TypeString, Computed: true, @@ -56,105 +155,40 @@ func resourceDigitalOceanFirewall() *schema.Resource { }, }, - "name": { - Type: schema.TypeString, - Required: true, - }, - - "droplet_ids": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - - "inbound_rule": { - Type: schema.TypeList, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "protocol": { - Type: schema.TypeString, - Optional: true, - }, - "port_range": { - Type: schema.TypeString, - Optional: true, - DiffSuppressFunc: func(k, oldV, newV string, d *schema.ResourceData) bool { - if oldV == "0" && newV == "all" { - return true - } - return (oldV == newV) - }, - }, - "source_addresses": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - "source_tags": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - "source_droplet_ids": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeInt}, - Optional: true, - }, - "source_load_balancer_uids": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - }, - }, - }, - - "outbound_rule": { - Type: schema.TypeList, - Optional: true, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "protocol": { - Type: schema.TypeString, - Optional: true, - }, - "port_range": { - Type: schema.TypeString, - Optional: true, - DiffSuppressFunc: func(k, oldV, newV string, d *schema.ResourceData) bool { - if oldV == "0" && newV == "all" { - return true - } - return (oldV == newV) - }, - }, - "destination_addresses": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - "destination_tags": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - "destination_droplet_ids": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeInt}, - Optional: true, - }, - "destination_load_balancer_uids": { - Type: schema.TypeList, - Elem: &schema.Schema{Type: schema.TypeString}, - Optional: true, - }, - }, - }, - }, - "tags": tagsSchema(), }, + + CustomizeDiff: func(diff *schema.ResourceDiff, v interface{}) error { + + inboundRules, hasInbound := diff.GetOk("inbound_rule") + outboundRules, hasOutbound := diff.GetOk("outbound_rule") + + if !hasInbound && !hasOutbound { + return fmt.Errorf("At least one rule must be specified") + } + + for _, v := range inboundRules.(*schema.Set).List() { + inbound := v.(map[string]interface{}) + protocol := inbound["protocol"] + + port := inbound["port_range"] + if protocol != "icmp" && port == "" { + return fmt.Errorf("`port_range` of inbound rules is required if protocol is `tcp` or `udp`") + } + } + + for _, v := range outboundRules.(*schema.Set).List() { + inbound := v.(map[string]interface{}) + protocol := inbound["protocol"] + + port := inbound["port_range"] + if protocol != "icmp" && port == "" { + return fmt.Errorf("`port_range` of outbound rules is required if protocol is `tcp` or `udp`") + } + } + + return nil + }, } } @@ -201,18 +235,21 @@ func resourceDigitalOceanFirewallRead(d *schema.ResourceData, meta interface{}) d.Set("create_at", firewall.Created) d.Set("pending_changes", firewallPendingChanges(d, firewall)) d.Set("name", firewall.Name) - d.Set("droplet_ids", firewall.DropletIDs) - if err := d.Set("inbound_rule", flattenFirewallInboundRules(d, firewall.InboundRules)); err != nil { + if err := d.Set("droplet_ids", flattenFirewallDropletIds(firewall.DropletIDs)); err != nil { + return fmt.Errorf("[DEBUG] Error setting `droplet_ids`: %+v", err) + } + + if err := d.Set("inbound_rule", flattenFirewallInboundRules(firewall.InboundRules)); err != nil { return fmt.Errorf("[DEBUG] Error setting Firewall inbound_rule error: %#v", err) } - if err := d.Set("outbound_rule", flattenFirewallOutboundRules(d, firewall.OutboundRules)); err != nil { + if err := d.Set("outbound_rule", flattenFirewallOutboundRules(firewall.OutboundRules)); err != nil { return fmt.Errorf("[DEBUG] Error setting Firewall outbound_rule error: %#v", err) } if err := d.Set("tags", flattenTags(firewall.Tags)); err != nil { - return fmt.Errorf("Error setting `tags`: %+v", err) + return fmt.Errorf("[DEBUG] Error setting `tags`: %+v", err) } return nil @@ -256,27 +293,6 @@ func resourceDigitalOceanFirewallDelete(d *schema.ResourceData, meta interface{} return nil } -func resourceDigitalOceanFirewallExists(d *schema.ResourceData, meta interface{}) (bool, error) { - client := meta.(*godo.Client) - - log.Printf("[INFO] Exists firewall: %s", d.Id()) - - // Retrieve the firewall properties for updating the state - _, resp, err := client.Firewalls.Get(context.Background(), d.Id()) - if err != nil { - // check if the firewall no longer exists. - if resp != nil && resp.StatusCode == 404 { - log.Printf("[WARN] DigitalOcean Firewall (%s) not found", d.Id()) - d.SetId("") - return false, nil - } - - return false, fmt.Errorf("Error retrieving firewall: %s", err) - } - - return true, nil -} - func firewallRequest(d *schema.ResourceData, client *godo.Client) (*godo.FirewallRequest, error) { // Build up our firewall request opts := &godo.FirewallRequest{ @@ -284,22 +300,18 @@ func firewallRequest(d *schema.ResourceData, client *godo.Client) (*godo.Firewal } if v, ok := d.GetOk("droplet_ids"); ok { - var droplets []int - for _, id := range v.([]interface{}) { - i, err := strconv.Atoi(id.(string)) - if err != nil { - return nil, err - } - droplets = append(droplets, i) - } - opts.DropletIDs = droplets + opts.DropletIDs = expandFirewallDropletIds(v.(*schema.Set).List()) } // Get inbound_rules - opts.InboundRules = expandFirewallInboundRules(d) + if v, ok := d.GetOk("inbound_rule"); ok { + opts.InboundRules = expandFirewallInboundRules(v.(*schema.Set).List()) + } // Get outbound_rules - opts.OutboundRules = expandFirewallOutboundRules(d) + if v, ok := d.GetOk("outbound_rule"); ok { + opts.OutboundRules = expandFirewallOutboundRules(v.(*schema.Set).List()) + } // Get tags opts.Tags = expandTags(d.Get("tags").(*schema.Set).List()) @@ -307,78 +319,74 @@ func firewallRequest(d *schema.ResourceData, client *godo.Client) (*godo.Firewal return opts, nil } -func expandFirewallInboundRules(d *schema.ResourceData) []godo.InboundRule { - rules := make([]godo.InboundRule, 0, len(d.Get("inbound_rule").([]interface{}))) - for _, rawRule := range d.Get("inbound_rule").([]interface{}) { +func expandFirewallDropletIds(droplets []interface{}) []int { + expandedDroplets := make([]int, len(droplets)) + for i, v := range droplets { + expandedDroplets[i] = v.(int) + } + + return expandedDroplets +} + +func expandFirewallRuleStringSet(strings []interface{}) []string { + expandedStrings := make([]string, len(strings)) + for i, v := range strings { + expandedStrings[i] = v.(string) + } + + return expandedStrings +} + +func expandFirewallInboundRules(rules []interface{}) []godo.InboundRule { + expandedRules := make([]godo.InboundRule, 0, len(rules)) + for _, rawRule := range rules { var src godo.Sources rule := rawRule.(map[string]interface{}) - sourceAddresses := rule["source_addresses"].([]interface{}) - for _, address := range sourceAddresses { - src.Addresses = append(src.Addresses, address.(string)) - } + src.DropletIDs = expandFirewallDropletIds(rule["source_droplet_ids"].(*schema.Set).List()) - sourceTags := rule["source_tags"].([]interface{}) - for _, tag := range sourceTags { - src.Tags = append(src.Tags, tag.(string)) - } + src.Addresses = expandFirewallRuleStringSet(rule["source_addresses"].(*schema.Set).List()) - dropletIds := rule["source_droplet_ids"].([]interface{}) - for _, dropletId := range dropletIds { - src.DropletIDs = append(src.DropletIDs, dropletId.(int)) - } + src.LoadBalancerUIDs = expandFirewallRuleStringSet(rule["source_load_balancer_uids"].(*schema.Set).List()) - lbIds := rule["source_load_balancer_uids"].([]interface{}) - for _, lbId := range lbIds { - src.LoadBalancerUIDs = append(src.LoadBalancerUIDs, lbId.(string)) - } + src.Tags = expandTags(rule["source_tags"].(*schema.Set).List()) r := godo.InboundRule{ Protocol: rule["protocol"].(string), PortRange: rule["port_range"].(string), Sources: &src, } - rules = append(rules, r) + + expandedRules = append(expandedRules, r) } - return rules + return expandedRules } -func expandFirewallOutboundRules(d *schema.ResourceData) []godo.OutboundRule { - rules := make([]godo.OutboundRule, 0, len(d.Get("outbound_rule").([]interface{}))) - for _, rawRule := range d.Get("outbound_rule").([]interface{}) { +func expandFirewallOutboundRules(rules []interface{}) []godo.OutboundRule { + expandedRules := make([]godo.OutboundRule, 0, len(rules)) + for _, rawRule := range rules { var dest godo.Destinations rule := rawRule.(map[string]interface{}) - destinationAddresses := rule["destination_addresses"].([]interface{}) - for _, address := range destinationAddresses { - dest.Addresses = append(dest.Addresses, address.(string)) - } + dest.DropletIDs = expandFirewallDropletIds(rule["destination_droplet_ids"].(*schema.Set).List()) - destinationTags := rule["destination_tags"].([]interface{}) - for _, tag := range destinationTags { - dest.Tags = append(dest.Tags, tag.(string)) - } + dest.Addresses = expandFirewallRuleStringSet(rule["destination_addresses"].(*schema.Set).List()) - dropletIds := rule["destination_droplet_ids"].([]interface{}) - for _, dropletId := range dropletIds { - dest.DropletIDs = append(dest.DropletIDs, dropletId.(int)) - } + dest.LoadBalancerUIDs = expandFirewallRuleStringSet(rule["destination_load_balancer_uids"].(*schema.Set).List()) - lbIds := rule["destination_load_balancer_uids"].([]interface{}) - for _, lbId := range lbIds { - dest.LoadBalancerUIDs = append(dest.LoadBalancerUIDs, lbId.(string)) - } + dest.Tags = expandTags(rule["destination_tags"].(*schema.Set).List()) r := godo.OutboundRule{ Protocol: rule["protocol"].(string), PortRange: rule["port_range"].(string), Destinations: &dest, } - rules = append(rules, r) + + expandedRules = append(expandedRules, r) } - return rules + return expandedRules } func firewallPendingChanges(d *schema.ResourceData, firewall *godo.Firewall) []interface{} { @@ -394,191 +402,120 @@ func firewallPendingChanges(d *schema.ResourceData, firewall *godo.Firewall) []i return remote } -func flattenFirewallInboundRules(d *schema.ResourceData, rules []godo.InboundRule) []interface{} { +func flattenFirewallDropletIds(droplets []int) *schema.Set { + if droplets == nil { + return nil + } + + flattenedDroplets := schema.NewSet(schema.HashInt, []interface{}{}) + for _, v := range droplets { + flattenedDroplets.Add(v) + } + + return flattenedDroplets +} + +func flattenFirewallRuleStringSet(strings []string) *schema.Set { + flattenedStrings := schema.NewSet(schema.HashString, []interface{}{}) + for _, v := range strings { + flattenedStrings.Add(v) + } + + return flattenedStrings +} + +func flattenFirewallInboundRules(rules []godo.InboundRule) []interface{} { if rules == nil { return nil } - // Prepare the data. - local := d.Get("inbound_rule").([]interface{}) - remote := make([]interface{}, 0, len(rules)) - remoteMap := make(map[int]map[string]interface{}) - for _, rule := range rules { + flattenedRules := make([]interface{}, len(rules)) + for i, rule := range rules { + sources := rule.Sources + protocol := rule.Protocol + portRange := rule.PortRange + rawRule := map[string]interface{}{ - "protocol": rule.Protocol, - "port_range": rule.PortRange, - "source_droplet_ids": rule.Sources.DropletIDs, - "source_tags": rule.Sources.Tags, - "source_addresses": rule.Sources.Addresses, - "source_load_balancer_uids": rule.Sources.LoadBalancerUIDs, - } - remote = append(remote, rawRule) - hash := hashFirewallRule(rule.Protocol, rule.PortRange) - remoteMap[hash] = rawRule - } - - // Handle special cases, both using the remote rules. - if len(remote) == 0 || len(local) == 0 { - return remote - } - - // Update the local rules to only contains rules match - // to the remote rules. - match := make([]interface{}, 0, len(rules)) - for _, rawRule := range local { - local := rawRule.(map[string]interface{}) - protocol := local["protocol"].(string) - portRange := local["port_range"].(string) - hash := hashFirewallRule(protocol, portRange) - remote, ok := remoteMap[hash] - if !ok { - // No entry in the remote, remove it. - continue + "protocol": protocol, } - // matches source lists. - key := "source_droplet_ids" - local[key] = matchFirewallIntLists(key, local, remote) - keys := []string{ - "source_tags", - "source_addresses", - "source_load_balancer_uids", - } - for _, key := range keys { - local[key] = matchFirewallStringLists(key, local, remote) + // The API returns 0 when the port range was specified as all. + // If protocol is `icmp` the API returns 0 for when port was + // not specified. + if portRange == "0" { + if protocol != "icmp" { + rawRule["port_range"] = "all" + } + } else { + rawRule["port_range"] = portRange } - match = append(match, local) - delete(remoteMap, hash) + if sources.Tags != nil { + rawRule["source_tags"] = flattenTags(sources.Tags) + } + + if sources.DropletIDs != nil { + rawRule["source_droplet_ids"] = flattenFirewallDropletIds(sources.DropletIDs) + } + + if sources.Addresses != nil { + rawRule["source_addresses"] = flattenFirewallRuleStringSet(sources.Addresses) + } + + if sources.LoadBalancerUIDs != nil { + rawRule["source_load_balancer_uids"] = flattenFirewallRuleStringSet(sources.LoadBalancerUIDs) + } + + flattenedRules[i] = rawRule } - // Append the remaining remote rules. - for _, rawRule := range remoteMap { - match = append(match, rawRule) - } - - return match + return flattenedRules } -func flattenFirewallOutboundRules(d *schema.ResourceData, rules []godo.OutboundRule) []interface{} { - // Prepare the data. - local := d.Get("outbound_rule").([]interface{}) - remote := make([]interface{}, 0, len(rules)) - remoteMap := make(map[int]map[string]interface{}) - for _, rule := range rules { +func flattenFirewallOutboundRules(rules []godo.OutboundRule) []interface{} { + if rules == nil { + return nil + } + + flattenedRules := make([]interface{}, len(rules)) + for i, rule := range rules { + destinations := rule.Destinations + protocol := rule.Protocol + portRange := rule.PortRange + rawRule := map[string]interface{}{ - "protocol": rule.Protocol, - "port_range": rule.PortRange, - "destination_droplet_ids": rule.Destinations.DropletIDs, - "destination_tags": rule.Destinations.Tags, - "destination_addresses": rule.Destinations.Addresses, - "destination_load_balancer_uids": rule.Destinations.LoadBalancerUIDs, - } - remote = append(remote, rawRule) - hash := hashFirewallRule(rule.Protocol, rule.PortRange) - remoteMap[hash] = rawRule - } - - // Handle special cases, both using the remote rules. - if len(remote) == 0 || len(local) == 0 { - return remote - } - - // Update the local rules to only contains rules match - // to the remote rules. - match := make([]interface{}, 0, len(rules)) - for _, rawRule := range local { - local := rawRule.(map[string]interface{}) - protocol := local["protocol"].(string) - portRange := local["port_range"].(string) - hash := hashFirewallRule(protocol, portRange) - remote, ok := remoteMap[hash] - if !ok { - // No entry in the remote, remove it. - continue + "protocol": protocol, } - // matches destination lists. - key := "destination_droplet_ids" - local[key] = matchFirewallIntLists(key, local, remote) - keys := []string{ - "destination_tags", - "destination_addresses", - "destination_load_balancer_uids", - } - for _, key := range keys { - local[key] = matchFirewallStringLists(key, local, remote) + // The API returns 0 when the port range was specified as all. + // If protocol is `icmp` the API returns 0 for when port was + // not specified. + if portRange == "0" { + if protocol != "icmp" { + rawRule["port_range"] = "all" + } + } else { + rawRule["port_range"] = portRange } - match = append(match, local) - delete(remoteMap, hash) + if destinations.Tags != nil { + rawRule["destination_tags"] = flattenTags(destinations.Tags) + } + + if destinations.DropletIDs != nil { + rawRule["destination_droplet_ids"] = flattenFirewallDropletIds(destinations.DropletIDs) + } + + if destinations.Addresses != nil { + rawRule["destination_addresses"] = flattenFirewallRuleStringSet(destinations.Addresses) + } + + if destinations.LoadBalancerUIDs != nil { + rawRule["destination_load_balancer_uids"] = flattenFirewallRuleStringSet(destinations.LoadBalancerUIDs) + } + + flattenedRules[i] = rawRule } - // Append the remaining remote rules. - for _, rawRule := range remoteMap { - match = append(match, rawRule) - } - - return match -} - -func matchFirewallIntLists(key string, local, remote map[string]interface{}) []interface{} { - remoteSize := len(remote[key].([]int)) - remoteSet := make(map[int]bool) - matchedList := make([]interface{}, 0, remoteSize) - - // Create a remote set out of the list for the quick comparison. - for _, i := range remote[key].([]int) { - remoteSet[i] = true - } - - // Add only the item which exists in the remote list. - for _, i := range local[key].([]interface{}) { - if _, ok := remoteSet[i.(int)]; !ok { - continue - } - matchedList = append(matchedList, i) - delete(remoteSet, i.(int)) - } - - // Append items only exists in the remote list. - for i := range remoteSet { - matchedList = append(matchedList, i) - } - - return matchedList -} - -func matchFirewallStringLists(key string, local, remote map[string]interface{}) []interface{} { - remoteSize := len(remote[key].([]string)) - remoteList := make([]interface{}, 0, remoteSize) - matchedList := make([]interface{}, 0, remoteSize) - - // Create a remote set out of the list for the quick comparison. - for _, s := range remote[key].([]string) { - remoteList = append(remoteList, s) - } - remoteSet := schema.NewSet(schema.HashString, remoteList) - - // Add only the item which exists in the remote list. - for _, s := range local[key].([]interface{}) { - if !remoteSet.Contains(s.(string)) { - continue - } - matchedList = append(matchedList, s) - remoteSet.Remove(s) - } - - // Append items only exists in the remote list. - for _, s := range remoteSet.List() { - matchedList = append(matchedList, s) - } - - return matchedList -} - -func hashFirewallRule(protocol, portRange string) int { - var buf bytes.Buffer - buf.WriteString(fmt.Sprintf("%s-%s", protocol, portRange)) - return hashcode.String(buf.String()) + return flattenedRules } diff --git a/digitalocean/resource_digitalocean_firewall_test.go b/digitalocean/resource_digitalocean_firewall_test.go index 8e9f0a2e..29a7109c 100644 --- a/digitalocean/resource_digitalocean_firewall_test.go +++ b/digitalocean/resource_digitalocean_firewall_test.go @@ -25,11 +25,6 @@ func TestAccDigitalOceanFirewall_AllowOnlyInbound(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.port_range", "22"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.0", "0.0.0.0/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.1", "::/0"), ), }, }, @@ -50,16 +45,6 @@ func TestAccDigitalOceanFirewall_AllowMultipleInbound(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.port_range", "22"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.0", "0.0.0.0/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.1", "::/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.port_range", "80"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.0", "1.2.3.0/24"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.1", "2002::/16"), ), }, }, @@ -80,11 +65,6 @@ func TestAccDigitalOceanFirewall_AllowOnlyOutbound(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.port_range", "22"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.0", "0.0.0.0/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.1", "::/0"), ), }, }, @@ -105,16 +85,6 @@ func TestAccDigitalOceanFirewall_AllowMultipleOutbound(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.port_range", "22"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.0", "192.168.1.0/24"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.1", "2002:1001::/48"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.port_range", "53"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.protocol", "udp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.0", "1.2.3.0/24"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.1", "2002::/16"), ), }, }, @@ -136,31 +106,7 @@ func TestAccDigitalOceanFirewall_MultipleInboundAndOutbound(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.port_range", "443"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.0", "192.168.1.0/24"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_addresses.1", "2002:1001:1:2::/64"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_tags.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.1.source_tags.0", tagName), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.port_range", "22"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.0", "0.0.0.0/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.1", "::/0"), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.port_range", "443"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.0", "192.168.1.0/24"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.1", "2002:1001:1:2::/64"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_tags.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_tags.0", tagName), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.port_range", "53"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.protocol", "udp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.#", "2"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.0", "0.0.0.0/0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.1.destination_addresses.1", "::/0"), ), }, }, @@ -181,15 +127,28 @@ func TestAccDigitalOceanFirewall_fullPortRange(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.port_range", "0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.0.source_addresses.0", "192.168.1.1/32"), resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.port_range", "0"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.protocol", "tcp"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.#", "1"), - resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.0.destination_addresses.0", "192.168.1.2/32"), + ), + }, + }, + }) +} + +func TestAccDigitalOceanFirewall_icmp(t *testing.T) { + rName := acctest.RandString(10) + var firewall godo.Firewall + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckDigitalOceanFirewallDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDigitalOceanFirewallConfig_icmp(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckDigitalOceanFirewallExists("digitalocean_firewall.foobar", &firewall), + resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "inbound_rule.#", "1"), + resource.TestCheckResourceAttr("digitalocean_firewall.foobar", "outbound_rule.#", "1"), ), }, }, @@ -337,6 +296,23 @@ resource "digitalocean_firewall" "foobar" { `, rName) } +func testAccDigitalOceanFirewallConfig_icmp(rName string) string { + return fmt.Sprintf(` +resource "digitalocean_firewall" "foobar" { + name = "%s" + inbound_rule { + protocol = "icmp" + source_addresses = ["192.168.1.1/32"] + } + outbound_rule { + protocol = "icmp" + port_range = "1-65535" + destination_addresses = ["192.168.1.2/32"] + } +} +`, rName) +} + func testAccCheckDigitalOceanFirewallDestroy(s *terraform.State) error { client := testAccProvider.Meta().(*godo.Client) diff --git a/website/docs/r/firewall.html.markdown b/website/docs/r/firewall.html.markdown index dce9ca0c..ecbd8789 100644 --- a/website/docs/r/firewall.html.markdown +++ b/website/docs/r/firewall.html.markdown @@ -42,6 +42,10 @@ resource "digitalocean_firewall" "web" { port_range = "443" source_addresses = ["0.0.0.0/0", "::/0"] }, + { + protocol = "icmp" + source_addresses = ["0.0.0.0/0", "::/0"] + }, ] outbound_rule = [ @@ -55,6 +59,10 @@ resource "digitalocean_firewall" "web" { port_range = "53" destination_addresses = ["0.0.0.0/0", "::/0"] }, + { + protocol = "icmp" + destination_addresses = ["0.0.0.0/0", "::/0"] + }, ] } ``` @@ -74,11 +82,12 @@ The following arguments are supported: `inbound_rule` supports the following: -* `protocol` - (Optional) The type of traffic to be allowed. +* `protocol` - (Required) The type of traffic to be allowed. This may be one of "tcp", "udp", or "icmp". * `port_range` - (Optional) The ports on which traffic will be allowed specified as a string containing a single port, a range (e.g. "8000-9000"), - or "1-65535" to open all ports for a protocol. + or "1-65535" to open all ports for a protocol. Required for when protocol is + `tcp` or `udp`. * `source_addresses` - (Optional) An array of strings containing the IPv4 addresses, IPv6 addresses, IPv4 CIDRs, and/or IPv6 CIDRs from which the inbound traffic will be accepted. @@ -92,11 +101,12 @@ The following arguments are supported: `outbound_rule` supports the following: -* `protocol` - (Optional) The type of traffic to be allowed. +* `protocol` - (Required) The type of traffic to be allowed. This may be one of "tcp", "udp", or "icmp". * `port_range` - (Optional) The ports on which traffic will be allowed specified as a string containing a single port, a range (e.g. "8000-9000"), - or "1-65535" to open all ports for a protocol. + or "1-65535" to open all ports for a protocol. Required for when protocol is + `tcp` or `udp`. * `destination_addresses` - (Optional) An array of strings containing the IPv4 addresses, IPv6 addresses, IPv4 CIDRs, and/or IPv6 CIDRs to which the outbound traffic will be allowed.