From 3898098c7824e32ec22d9defa16eaca41c3cc3f7 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 15 Jan 2015 10:04:09 +0100 Subject: [PATCH 1/3] Adding the ability to manage the whole firewall This goes for the normal firewall, the egress firewall and the network ACL. USE WITH CAUTION! When setting `managed = true` in your config, it means it will delete all firewall rules that are not in your config, so unknown to TF. Also adding the new `cloudstack_egress_firewall` resource with this commit and updating go-cloudstack to the latest API version (v4.4) --- builtin/providers/cloudstack/provider.go | 1 + builtin/providers/cloudstack/provider_test.go | 124 +---- .../cloudstack/resource_cloudstack_disk.go | 5 +- .../resource_cloudstack_egress_firewall.go | 484 ++++++++++++++++++ ...esource_cloudstack_egress_firewall_test.go | 189 +++++++ .../resource_cloudstack_firewall.go | 70 ++- .../resource_cloudstack_ipaddress.go | 3 +- .../resource_cloudstack_network_acl_rule.go | 50 +- .../cloudstack/resource_cloudstack_vpc.go | 2 +- 9 files changed, 781 insertions(+), 147 deletions(-) create mode 100644 builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go create mode 100644 builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go diff --git a/builtin/providers/cloudstack/provider.go b/builtin/providers/cloudstack/provider.go index fa9bc3e26b..03c8768f62 100644 --- a/builtin/providers/cloudstack/provider.go +++ b/builtin/providers/cloudstack/provider.go @@ -38,6 +38,7 @@ func Provider() terraform.ResourceProvider { ResourcesMap: map[string]*schema.Resource{ "cloudstack_disk": resourceCloudStackDisk(), + "cloudstack_egress_firewall": resourceCloudStackEgressFirewall(), "cloudstack_firewall": resourceCloudStackFirewall(), "cloudstack_instance": resourceCloudStackInstance(), "cloudstack_ipaddress": resourceCloudStackIPAddress(), diff --git a/builtin/providers/cloudstack/provider_test.go b/builtin/providers/cloudstack/provider_test.go index 91761c3443..a13839177e 100644 --- a/builtin/providers/cloudstack/provider_test.go +++ b/builtin/providers/cloudstack/provider_test.go @@ -38,131 +38,9 @@ func testAccPreCheck(t *testing.T) { if v := os.Getenv("CLOUDSTACK_SECRET_KEY"); v == "" { t.Fatal("CLOUDSTACK_SECRET_KEY must be set for acceptance tests") } - - // Testing all environment/installation specific variables which are needed - // to run all the acceptance tests - if CLOUDSTACK_DISK_OFFERING_1 == "" { - if v := os.Getenv("CLOUDSTACK_DISK_OFFERING_1"); v == "" { - t.Fatal("CLOUDSTACK_DISK_OFFERING_1 must be set for acceptance tests") - } else { - CLOUDSTACK_DISK_OFFERING_1 = v - } - } - if CLOUDSTACK_DISK_OFFERING_2 == "" { - if v := os.Getenv("CLOUDSTACK_DISK_OFFERING_2"); v == "" { - t.Fatal("CLOUDSTACK_DISK_OFFERING_2 must be set for acceptance tests") - } else { - CLOUDSTACK_DISK_OFFERING_2 = v - } - } - if CLOUDSTACK_SERVICE_OFFERING_1 == "" { - if v := os.Getenv("CLOUDSTACK_SERVICE_OFFERING_1"); v == "" { - t.Fatal("CLOUDSTACK_SERVICE_OFFERING_1 must be set for acceptance tests") - } else { - CLOUDSTACK_SERVICE_OFFERING_1 = v - } - } - if CLOUDSTACK_SERVICE_OFFERING_2 == "" { - if v := os.Getenv("CLOUDSTACK_SERVICE_OFFERING_2"); v == "" { - t.Fatal("CLOUDSTACK_SERVICE_OFFERING_2 must be set for acceptance tests") - } else { - CLOUDSTACK_SERVICE_OFFERING_2 = v - } - } - if CLOUDSTACK_NETWORK_1 == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_1"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_1 must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_1 = v - } - } - if CLOUDSTACK_NETWORK_1_CIDR == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_1_CIDR"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_1_CIDR must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_1_CIDR = v - } - } - if CLOUDSTACK_NETWORK_1_OFFERING == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_1_OFFERING"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_1_OFFERING must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_1_OFFERING = v - } - } - if CLOUDSTACK_NETWORK_1_IPADDRESS == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_1_IPADDRESS"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_1_IPADDRESS must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_1_IPADDRESS = v - } - } - if CLOUDSTACK_NETWORK_2 == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_2"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_2 must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_2 = v - } - } - if CLOUDSTACK_NETWORK_2_IPADDRESS == "" { - if v := os.Getenv("CLOUDSTACK_NETWORK_2_IPADDRESS"); v == "" { - t.Fatal("CLOUDSTACK_NETWORK_2_IPADDRESS must be set for acceptance tests") - } else { - CLOUDSTACK_NETWORK_2_IPADDRESS = v - } - } - if CLOUDSTACK_VPC_CIDR == "" { - if v := os.Getenv("CLOUDSTACK_VPC_CIDR"); v == "" { - t.Fatal("CLOUDSTACK_VPC_CIDR must be set for acceptance tests") - } else { - CLOUDSTACK_VPC_CIDR = v - } - } - if CLOUDSTACK_VPC_OFFERING == "" { - if v := os.Getenv("CLOUDSTACK_VPC_OFFERING"); v == "" { - t.Fatal("CLOUDSTACK_VPC_OFFERING must be set for acceptance tests") - } else { - CLOUDSTACK_VPC_OFFERING = v - } - } - if CLOUDSTACK_VPC_NETWORK_CIDR == "" { - if v := os.Getenv("CLOUDSTACK_VPC_NETWORK_CIDR"); v == "" { - t.Fatal("CLOUDSTACK_VPC_NETWORK_CIDR must be set for acceptance tests") - } else { - CLOUDSTACK_VPC_NETWORK_CIDR = v - } - } - if CLOUDSTACK_VPC_NETWORK_OFFERING == "" { - if v := os.Getenv("CLOUDSTACK_VPC_NETWORK_OFFERING"); v == "" { - t.Fatal("CLOUDSTACK_VPC_NETWORK_OFFERING must be set for acceptance tests") - } else { - CLOUDSTACK_VPC_NETWORK_OFFERING = v - } - } - if CLOUDSTACK_PUBLIC_IPADDRESS == "" { - if v := os.Getenv("CLOUDSTACK_PUBLIC_IPADDRESS"); v == "" { - t.Fatal("CLOUDSTACK_PUBLIC_IPADDRESS must be set for acceptance tests") - } else { - CLOUDSTACK_PUBLIC_IPADDRESS = v - } - } - if CLOUDSTACK_TEMPLATE == "" { - if v := os.Getenv("CLOUDSTACK_TEMPLATE"); v == "" { - t.Fatal("CLOUDSTACK_TEMPLATE must be set for acceptance tests") - } else { - CLOUDSTACK_TEMPLATE = v - } - } - if CLOUDSTACK_ZONE == "" { - if v := os.Getenv("CLOUDSTACK_ZONE"); v == "" { - t.Fatal("CLOUDSTACK_ZONE must be set for acceptance tests") - } else { - CLOUDSTACK_ZONE = v - } - } } -// EITHER SET THESE, OR ADD THE VALUES TO YOUR ENV!! +// SET THESE VALUES IN ORDER TO RUN THE ACC TESTS!! var CLOUDSTACK_DISK_OFFERING_1 = "" var CLOUDSTACK_DISK_OFFERING_2 = "" var CLOUDSTACK_SERVICE_OFFERING_1 = "" diff --git a/builtin/providers/cloudstack/resource_cloudstack_disk.go b/builtin/providers/cloudstack/resource_cloudstack_disk.go index 23665f9c0a..88ddff59ee 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_disk.go +++ b/builtin/providers/cloudstack/resource_cloudstack_disk.go @@ -183,10 +183,7 @@ func resourceCloudStackDiskUpdate(d *schema.ResourceData, meta interface{}) erro } // Create a new parameter struct - p := cs.Volume.NewResizeVolumeParams() - - // Set the volume UUID - p.SetId(d.Id()) + p := cs.Volume.NewResizeVolumeParams(d.Id()) // Retrieve the disk_offering UUID diskofferingid, e := retrieveUUID(cs, "disk_offering", d.Get("disk_offering").(string)) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go new file mode 100644 index 0000000000..3eef7be149 --- /dev/null +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go @@ -0,0 +1,484 @@ +package cloudstack + +import ( + "bytes" + "fmt" + "regexp" + "sort" + "strconv" + "strings" + + "github.com/hashicorp/terraform/helper/hashcode" + "github.com/hashicorp/terraform/helper/schema" + "github.com/xanzy/go-cloudstack/cloudstack" +) + +func resourceCloudStackEgressFirewall() *schema.Resource { + return &schema.Resource{ + Create: resourceCloudStackEgressFirewallCreate, + Read: resourceCloudStackEgressFirewallRead, + Update: resourceCloudStackEgressFirewallUpdate, + Delete: resourceCloudStackEgressFirewallDelete, + + Schema: map[string]*schema.Schema{ + "network": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + }, + + "managed": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + + "rule": &schema.Schema{ + Type: schema.TypeSet, + Required: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "source_cidr": &schema.Schema{ + Type: schema.TypeString, + Required: true, + }, + + "protocol": &schema.Schema{ + Type: schema.TypeString, + Required: true, + }, + + "icmp_type": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Computed: true, + }, + + "icmp_code": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Computed: true, + }, + + "ports": &schema.Schema{ + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + Set: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + }, + + "uuids": &schema.Schema{ + Type: schema.TypeMap, + Computed: true, + }, + }, + }, + Set: resourceCloudStackFirewallRuleHash, + }, + }, + } +} + +func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Retrieve the network UUID + networkid, e := retrieveUUID(cs, "network", d.Get("network").(string)) + if e != nil { + return e.Error() + } + + // We need to set this upfront in order to be able to save a partial state + d.SetId(networkid) + + // Create all rules that are configured + if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 { + + // Create an empty schema.Set to hold all rules + rules := &schema.Set{ + F: resourceCloudStackFirewallRuleHash, + } + + for _, rule := range rs.List() { + // Create a single rule + err := resourceCloudStackEgressFirewallCreateRule(d, meta, rule.(map[string]interface{})) + + // We need to update this first to preserve the correct state + rules.Add(rule) + d.Set("rule", rules) + + if err != nil { + return err + } + } + } + + return resourceCloudStackEgressFirewallRead(d, meta) +} + +func resourceCloudStackEgressFirewallCreateRule( + d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + uuids := rule["uuids"].(map[string]interface{}) + + // Make sure all required rule parameters are there + if err := verifyEgressFirewallRuleParams(d, rule); err != nil { + return err + } + + // Create a new parameter struct + p := cs.Firewall.NewCreateEgressFirewallRuleParams(d.Id(), rule["protocol"].(string)) + + // Set the CIDR list + p.SetCidrlist([]string{rule["source_cidr"].(string)}) + + // If the protocol is ICMP set the needed ICMP parameters + if rule["protocol"].(string) == "icmp" { + p.SetIcmptype(rule["icmp_type"].(int)) + p.SetIcmpcode(rule["icmp_code"].(int)) + + r, err := cs.Firewall.CreateEgressFirewallRule(p) + if err != nil { + return err + } + uuids["icmp"] = r.Id + rule["uuids"] = uuids + } + + // If protocol is not ICMP, loop through all ports + if rule["protocol"].(string) != "icmp" { + if ps := rule["ports"].(*schema.Set); ps.Len() > 0 { + + // Create an empty schema.Set to hold all processed ports + ports := &schema.Set{ + F: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + } + + for _, port := range ps.List() { + re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`) + m := re.FindStringSubmatch(port.(string)) + + startPort, err := strconv.Atoi(m[1]) + if err != nil { + return err + } + + endPort := startPort + if m[2] != "" { + endPort, err = strconv.Atoi(m[2]) + if err != nil { + return err + } + } + + p.SetStartport(startPort) + p.SetEndport(endPort) + + r, err := cs.Firewall.CreateEgressFirewallRule(p) + if err != nil { + return err + } + + ports.Add(port) + rule["ports"] = ports + + uuids[port.(string)] = r.Id + rule["uuids"] = uuids + } + } + } + + return nil +} + +func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + + // Create an empty schema.Set to hold all rules + rules := &schema.Set{ + F: resourceCloudStackFirewallRuleHash, + } + + if d.Get("managed").(bool) { + // Read all rules... + } + + // Read all rules that are configured + if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 { + for _, rule := range rs.List() { + rule := rule.(map[string]interface{}) + uuids := rule["uuids"].(map[string]interface{}) + + if rule["protocol"].(string) == "icmp" { + id, ok := uuids["icmp"] + if !ok { + continue + } + + // Get the rule + r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string)) + // If the count == 0, there is no object found for this UUID + if err != nil { + if count == 0 { + delete(uuids, "icmp") + continue + } + + return err + } + + // Update the values + rule["source_cidr"] = r.Cidrlist + rule["protocol"] = r.Protocol + rule["icmp_type"] = r.Icmptype + rule["icmp_code"] = r.Icmpcode + rules.Add(rule) + } + + // If protocol is not ICMP, loop through all ports + if rule["protocol"].(string) != "icmp" { + if ps := rule["ports"].(*schema.Set); ps.Len() > 0 { + + // Create an empty schema.Set to hold all ports + ports := &schema.Set{ + F: func(v interface{}) int { + return hashcode.String(v.(string)) + }, + } + + // Loop through all ports and retrieve their info + for _, port := range ps.List() { + id, ok := uuids[port.(string)] + if !ok { + continue + } + + // Get the rule + r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string)) + if err != nil { + if count == 0 { + delete(uuids, port.(string)) + continue + } + + return err + } + + // Update the values + rule["source_cidr"] = r.Cidrlist + rule["protocol"] = r.Protocol + ports.Add(port) + } + + // If there is at least one port found, add this rule to the rules set + if ports.Len() > 0 { + rule["ports"] = ports + rules.Add(rule) + } + } + } + } + } + + // If this is a managed firewall, add all unknown rules into a single dummy rule + if d.Get("managed").(bool) { + // Get all the rules from the running environment + p := cs.Firewall.NewListEgressFirewallRulesParams() + p.SetNetworkid(d.Id()) + p.SetListall(true) + + r, err := cs.Firewall.ListEgressFirewallRules(p) + if err != nil { + return err + } + + // Add all UUIDs to the uuids map + uuids := make(map[string]interface{}) + for _, r := range r.EgressFirewallRules { + uuids[r.Id] = r.Id + } + + // Delete all expected UUIDs from the uuids map + for _, rule := range rules.List() { + rule := rule.(map[string]interface{}) + + for _, id := range rule["uuids"].(map[string]interface{}) { + delete(uuids, id.(string)) + } + } + + if len(uuids) > 0 { + // Make a dummy rule to hold all unknown UUIDs + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": uuids, + } + + // Add the dummy rule to the rules set + rules.Add(rule) + } + } + + if rules.Len() > 0 { + d.Set("rule", rules) + } else { + d.SetId("") + } + + return nil +} + +func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interface{}) error { + // Check if the rule set as a whole has changed + if d.HasChange("rule") { + o, n := d.GetChange("rule") + ors := o.(*schema.Set).Difference(n.(*schema.Set)) + nrs := n.(*schema.Set).Difference(o.(*schema.Set)) + + // Now first loop through all the old rules and delete any obsolete ones + for _, rule := range ors.List() { + // Delete the rule as it no longer exists in the config + err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{})) + if err != nil { + return err + } + } + + // Make sure we save the state of the currently configured rules + rules := o.(*schema.Set).Intersection(n.(*schema.Set)) + d.Set("rule", rules) + + // Then loop through al the currently configured rules and create the new ones + for _, rule := range nrs.List() { + // When succesfully deleted, re-create it again if it still exists + err := resourceCloudStackFirewallCreateRule( + d, meta, rule.(map[string]interface{})) + + // We need to update this first to preserve the correct state + rules.Add(rule) + d.Set("rule", rules) + + if err != nil { + return err + } + } + } + + return resourceCloudStackEgressFirewallRead(d, meta) +} + +func resourceCloudStackEgressFirewallDelete(d *schema.ResourceData, meta interface{}) error { + // Delete all rules + if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 { + for _, rule := range rs.List() { + // Delete a single rule + err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{})) + + // We need to update this first to preserve the correct state + d.Set("rule", rs) + + if err != nil { + return err + } + } + } + + return nil +} + +func resourceCloudStackEgressFirewallDeleteRule( + d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { + cs := meta.(*cloudstack.CloudStackClient) + uuids := rule["uuids"].(map[string]interface{}) + + for k, id := range uuids { + // Create the parameter struct + p := cs.Firewall.NewDeleteEgressFirewallRuleParams(id.(string)) + + // Delete the rule + if _, err := cs.Firewall.DeleteEgressFirewallRule(p); err != nil { + + // This is a very poor way to be told the UUID does no longer exist :( + if strings.Contains(err.Error(), fmt.Sprintf( + "Invalid parameter id value=%s due to incorrect long value format, "+ + "or entity does not exist", id.(string))) { + delete(uuids, k) + continue + } + + return err + } + + // Delete the UUID of this rule + delete(uuids, k) + } + + // Update the UUIDs + rule["uuids"] = uuids + + return nil +} + +func resourceCloudStackEgressFirewallRuleHash(v interface{}) int { + var buf bytes.Buffer + m := v.(map[string]interface{}) + buf.WriteString(fmt.Sprintf( + "%s-%s-", m["source_cidr"].(string), m["protocol"].(string))) + + if v, ok := m["icmp_type"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + + if v, ok := m["icmp_code"]; ok { + buf.WriteString(fmt.Sprintf("%d-", v.(int))) + } + + // We need to make sure to sort the strings below so that we always + // generate the same hash code no matter what is in the set. + if v, ok := m["ports"]; ok { + vs := v.(*schema.Set).List() + s := make([]string, len(vs)) + + for i, raw := range vs { + s[i] = raw.(string) + } + sort.Strings(s) + + for _, v := range s { + buf.WriteString(fmt.Sprintf("%s-", v)) + } + } + + return hashcode.String(buf.String()) +} + +func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error { + protocol := rule["protocol"].(string) + if protocol != "tcp" && protocol != "udp" && protocol != "icmp" { + return fmt.Errorf( + "%s is not a valid protocol. Valid options are 'tcp', 'udp' and 'icmp'", protocol) + } + + if protocol == "icmp" { + if _, ok := rule["icmp_type"]; !ok { + return fmt.Errorf( + "Parameter icmp_type is a required parameter when using protocol 'icmp'") + } + if _, ok := rule["icmp_code"]; !ok { + return fmt.Errorf( + "Parameter icmp_code is a required parameter when using protocol 'icmp'") + } + } else { + if _, ok := rule["ports"]; !ok { + return fmt.Errorf( + "Parameter port is a required parameter when using protocol 'tcp' or 'udp'") + } + } + + return nil +} diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go new file mode 100644 index 0000000000..e68397a0db --- /dev/null +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go @@ -0,0 +1,189 @@ +package cloudstack + +import ( + "fmt" + "strings" + "testing" + + "github.com/hashicorp/terraform/helper/resource" + "github.com/hashicorp/terraform/terraform" + "github.com/xanzy/go-cloudstack/cloudstack" +) + +func TestAccCloudStackEgressFirewall_basic(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccCloudStackEgressFirewall_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + ), + }, + }, + }) +} + +func TestAccCloudStackEgressFirewall_update(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudStackEgressFirewallDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccCloudStackEgressFirewall_basic, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "ipaddress", CLOUDSTACK_NETWORK_1), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.#", "1"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + ), + }, + + resource.TestStep{ + Config: testAccCloudStackEgressFirewall_update, + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "ipaddress", CLOUDSTACK_NETWORK_1), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.#", "2"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.3779782959.source_cidr", "172.16.100.0/24"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.3779782959.protocol", "tcp"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.3779782959.ports.#", "2"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.3779782959.ports.1889509032", "80"), + resource.TestCheckResourceAttr( + "cloudstack_egress_firewall.foo", "rule.3779782959.ports.3638101695", "443"), + ), + }, + }, + }) +} + +func testAccCheckCloudStackEgressFirewallRulesExist(n string) 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 firewall ID is set") + } + + for k, uuid := range rs.Primary.Attributes { + if !strings.Contains(k, "uuids") { + continue + } + + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + _, count, err := cs.Firewall.GetEgressFirewallRuleByID(uuid) + + if err != nil { + return err + } + + if count == 0 { + return fmt.Errorf("Firewall rule for %s not found", k) + } + } + + return nil + } +} + +func testAccCheckCloudStackEgressFirewallDestroy(s *terraform.State) error { + cs := testAccProvider.Meta().(*cloudstack.CloudStackClient) + + for _, rs := range s.RootModule().Resources { + if rs.Type != "cloudstack_egress_firewall" { + continue + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No instance ID is set") + } + + for k, uuid := range rs.Primary.Attributes { + if !strings.Contains(k, "uuids") { + continue + } + + p := cs.Firewall.NewDeleteEgressFirewallRuleParams(uuid) + _, err := cs.Firewall.DeleteEgressFirewallRule(p) + + if err != nil { + return err + } + } + } + + return nil +} + +var testAccCloudStackEgressFirewall_basic = fmt.Sprintf(` +resource "cloudstack_egress_firewall" "foo" { + ipaddress = "%s" + + rule { + source_cidr = "10.0.0.0/24" + protocol = "tcp" + ports = ["80", "1000-2000"] + } +}`, CLOUDSTACK_NETWORK_1) + +var testAccCloudStackEgressFirewall_update = fmt.Sprintf(` +resource "cloudstack_egress_firewall" "foo" { + ipaddress = "%s" + + rule { + source_cidr = "10.0.0.0/24" + protocol = "tcp" + ports = ["80", "1000-2000"] + } + + rule { + source_cidr = "172.16.100.0/24" + protocol = "tcp" + ports = ["80", "443"] + } +}`, CLOUDSTACK_NETWORK_1) diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_firewall.go index 50083999f3..7de4da2a6e 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_firewall.go @@ -27,6 +27,12 @@ func resourceCloudStackFirewall() *schema.Resource { ForceNew: true, }, + "managed": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "rule": &schema.Schema{ Type: schema.TypeSet, Required: true, @@ -85,7 +91,7 @@ func resourceCloudStackFirewallCreate(d *schema.ResourceData, meta interface{}) } // We need to set this upfront in order to be able to save a partial state - d.SetId(d.Get("ipaddress").(string)) + d.SetId(ipaddressid) // Create all rules that are configured if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 { @@ -97,7 +103,7 @@ func resourceCloudStackFirewallCreate(d *schema.ResourceData, meta interface{}) for _, rule := range rs.List() { // Create a single rule - err := resourceCloudStackFirewallCreateRule(d, meta, ipaddressid, rule.(map[string]interface{})) + err := resourceCloudStackFirewallCreateRule(d, meta, rule.(map[string]interface{})) // We need to update this first to preserve the correct state rules.Add(rule) @@ -113,17 +119,17 @@ func resourceCloudStackFirewallCreate(d *schema.ResourceData, meta interface{}) } func resourceCloudStackFirewallCreateRule( - d *schema.ResourceData, meta interface{}, ipaddressid string, rule map[string]interface{}) error { + d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error { cs := meta.(*cloudstack.CloudStackClient) uuids := rule["uuids"].(map[string]interface{}) - // Make sure all required parameters are there - if err := verifyFirewallParams(d, rule); err != nil { + // Make sure all required rule parameters are there + if err := verifyFirewallRuleParams(d, rule); err != nil { return err } // Create a new parameter struct - p := cs.Firewall.NewCreateFirewallRuleParams(ipaddressid, rule["protocol"].(string)) + p := cs.Firewall.NewCreateFirewallRuleParams(d.Id(), rule["protocol"].(string)) // Set the CIDR list p.SetCidrlist([]string{rule["source_cidr"].(string)}) @@ -274,6 +280,46 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er } } + // If this is a managed firewall, add all unknown rules into a single dummy rule + if d.Get("managed").(bool) { + // Get all the rules from the running environment + p := cs.Firewall.NewListFirewallRulesParams() + p.SetIpaddressid(d.Id()) + p.SetListall(true) + + r, err := cs.Firewall.ListFirewallRules(p) + if err != nil { + return err + } + + // Add all UUIDs to the uuids map + uuids := make(map[string]interface{}) + for _, r := range r.FirewallRules { + uuids[r.Id] = r.Id + } + + // Delete all expected UUIDs from the uuids map + for _, rule := range rules.List() { + rule := rule.(map[string]interface{}) + + for _, id := range rule["uuids"].(map[string]interface{}) { + delete(uuids, id.(string)) + } + } + + if len(uuids) > 0 { + // Make a dummy rule to hold all unknown UUIDs + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": uuids, + } + + // Add the dummy rule to the rules set + rules.Add(rule) + } + } + if rules.Len() > 0 { d.Set("rule", rules) } else { @@ -284,14 +330,6 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er } func resourceCloudStackFirewallUpdate(d *schema.ResourceData, meta interface{}) error { - cs := meta.(*cloudstack.CloudStackClient) - - // Retrieve the ipaddress UUID - ipaddressid, e := retrieveUUID(cs, "ipaddress", d.Get("ipaddress").(string)) - if e != nil { - return e.Error() - } - // Check if the rule set as a whole has changed if d.HasChange("rule") { o, n := d.GetChange("rule") @@ -315,7 +353,7 @@ func resourceCloudStackFirewallUpdate(d *schema.ResourceData, meta interface{}) for _, rule := range nrs.List() { // When succesfully deleted, re-create it again if it still exists err := resourceCloudStackFirewallCreateRule( - d, meta, ipaddressid, rule.(map[string]interface{})) + d, meta, rule.(map[string]interface{})) // We need to update this first to preserve the correct state rules.Add(rule) @@ -415,7 +453,7 @@ func resourceCloudStackFirewallRuleHash(v interface{}) int { return hashcode.String(buf.String()) } -func verifyFirewallParams(d *schema.ResourceData, rule map[string]interface{}) error { +func verifyFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error { protocol := rule["protocol"].(string) if protocol != "tcp" && protocol != "udp" && protocol != "icmp" { return fmt.Errorf( diff --git a/builtin/providers/cloudstack/resource_cloudstack_ipaddress.go b/builtin/providers/cloudstack/resource_cloudstack_ipaddress.go index ac6ed58a73..59d942b632 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_ipaddress.go +++ b/builtin/providers/cloudstack/resource_cloudstack_ipaddress.go @@ -147,7 +147,8 @@ func verifyIPAddressParams(d *schema.ResourceData) error { _, vpc := d.GetOk("vpc") if (network && vpc) || (!network && !vpc) { - return fmt.Errorf("You must supply a value for either (so not both) the 'network' or 'vpc' argument") + return fmt.Errorf( + "You must supply a value for either (so not both) the 'network' or 'vpc' parameter") } return nil diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go index 95c91682ac..d2c9b56fde 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go @@ -27,6 +27,12 @@ func resourceCloudStackNetworkACLRule() *schema.Resource { ForceNew: true, }, + "managed": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "rule": &schema.Schema{ Type: schema.TypeSet, Required: true, @@ -295,6 +301,46 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface } } + // If this is a managed firewall, add all unknown rules into a single dummy rule + if d.Get("managed").(bool) { + // Get all the rules from the running environment + p := cs.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(d.Id()) + p.SetListall(true) + + r, err := cs.NetworkACL.ListNetworkACLs(p) + if err != nil { + return err + } + + // Add all UUIDs to the uuids map + uuids := make(map[string]interface{}) + for _, r := range r.NetworkACLs { + uuids[r.Id] = r.Id + } + + // Delete all expected UUIDs from the uuids map + for _, rule := range rules.List() { + rule := rule.(map[string]interface{}) + + for _, id := range rule["uuids"].(map[string]interface{}) { + delete(uuids, id.(string)) + } + } + + if len(uuids) > 0 { + // Make a dummy rule to hold all unknown UUIDs + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": uuids, + } + + // Add the dummy rule to the rules set + rules.Add(rule) + } + } + if rules.Len() > 0 { d.Set("rule", rules) } else { @@ -438,7 +484,7 @@ func resourceCloudStackNetworkACLRuleHash(v interface{}) int { func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interface{}) error { action := rule["action"].(string) if action != "allow" && action != "deny" { - return fmt.Errorf("Parameter action only excepts 'allow' or 'deny' as values") + return fmt.Errorf("Parameter action only accepts 'allow' or 'deny' as values") } protocol := rule["protocol"].(string) @@ -469,7 +515,7 @@ func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interfac traffic := rule["traffic_type"].(string) if traffic != "ingress" && traffic != "egress" { return fmt.Errorf( - "Parameter traffic_type only excepts 'ingress' or 'egress' as values") + "Parameter traffic_type only accepts 'ingress' or 'egress' as values") } return nil diff --git a/builtin/providers/cloudstack/resource_cloudstack_vpc.go b/builtin/providers/cloudstack/resource_cloudstack_vpc.go index fc76b6071f..46f64827c5 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_vpc.go +++ b/builtin/providers/cloudstack/resource_cloudstack_vpc.go @@ -124,7 +124,7 @@ func resourceCloudStackVPCUpdate(d *schema.ResourceData, meta interface{}) error // Check if the name or display text is changed if d.HasChange("name") || d.HasChange("display_text") { // Create a new parameter struct - p := cs.VPC.NewUpdateVPCParams(d.Id(), d.Get("name").(string)) + p := cs.VPC.NewUpdateVPCParams(d.Id()) // Set the display text displaytext, ok := d.GetOk("display_text") From 19776ba402f0485b7996d3d17503f31c3e44ce72 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 15 Jan 2015 11:29:04 +0100 Subject: [PATCH 2/3] Updating some logic and tests These fixes are needed to make the provider work with master again. These are still some issues, but they seem not to be related to the provider, but the changes in `helper/schema`. --- .../resource_cloudstack_egress_firewall.go | 5 ++++ ...esource_cloudstack_egress_firewall_test.go | 24 +++++++++---------- .../resource_cloudstack_firewall.go | 5 ++++ .../resource_cloudstack_firewall_test.go | 4 ++-- .../resource_cloudstack_network_acl_rule.go | 5 ++++ ...source_cloudstack_network_acl_rule_test.go | 4 ++-- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go index 3eef7be149..d5b9d80bdc 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go @@ -397,6 +397,11 @@ func resourceCloudStackEgressFirewallDeleteRule( uuids := rule["uuids"].(map[string]interface{}) for k, id := range uuids { + // We don't care about the count here, so just continue + if k == "#" { + continue + } + // Create the parameter struct p := cs.Firewall.NewDeleteEgressFirewallRuleParams(id.(string)) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go index e68397a0db..8e70901d94 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go @@ -111,7 +111,7 @@ func testAccCheckCloudStackEgressFirewallRulesExist(n string) resource.TestCheck } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } @@ -144,7 +144,7 @@ func testAccCheckCloudStackEgressFirewallDestroy(s *terraform.State) error { } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } @@ -165,25 +165,23 @@ resource "cloudstack_egress_firewall" "foo" { ipaddress = "%s" rule { - source_cidr = "10.0.0.0/24" + source_cidr = "%s/32" protocol = "tcp" ports = ["80", "1000-2000"] } -}`, CLOUDSTACK_NETWORK_1) +}`, + CLOUDSTACK_NETWORK_1, + CLOUDSTACK_NETWORK_1_IPADDRESS) var testAccCloudStackEgressFirewall_update = fmt.Sprintf(` resource "cloudstack_egress_firewall" "foo" { ipaddress = "%s" rule { - source_cidr = "10.0.0.0/24" + source_cidr = "%s/32" protocol = "tcp" - ports = ["80", "1000-2000"] + ports = ["80", "443", 1000-2000"] } - - rule { - source_cidr = "172.16.100.0/24" - protocol = "tcp" - ports = ["80", "443"] - } -}`, CLOUDSTACK_NETWORK_1) +}`, + CLOUDSTACK_NETWORK_1, + CLOUDSTACK_NETWORK_1_IPADDRESS) diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_firewall.go index 7de4da2a6e..75f0410e6e 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_firewall.go @@ -393,6 +393,11 @@ func resourceCloudStackFirewallDeleteRule( uuids := rule["uuids"].(map[string]interface{}) for k, id := range uuids { + // We don't care about the count here, so just continue + if k == "#" { + continue + } + // Create the parameter struct p := cs.Firewall.NewDeleteFirewallRuleParams(id.(string)) diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall_test.go b/builtin/providers/cloudstack/resource_cloudstack_firewall_test.go index f2fa8a78b8..bb629cfab9 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_firewall_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_firewall_test.go @@ -111,7 +111,7 @@ func testAccCheckCloudStackFirewallRulesExist(n string) resource.TestCheckFunc { } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } @@ -144,7 +144,7 @@ func testAccCheckCloudStackFirewallDestroy(s *terraform.State) error { } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go index d2c9b56fde..6e13e90c97 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go @@ -417,6 +417,11 @@ func resourceCloudStackNetworkACLRuleDeleteRule( uuids := rule["uuids"].(map[string]interface{}) for k, id := range uuids { + // We don't care about the count here, so just continue + if k == "#" { + continue + } + // Create the parameter struct p := cs.NetworkACL.NewDeleteNetworkACLParams(id.(string)) diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go index 5b71ec5de0..1a577ac7f3 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go @@ -123,7 +123,7 @@ func testAccCheckCloudStackNetworkACLRulesExist(n string) resource.TestCheckFunc } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } @@ -156,7 +156,7 @@ func testAccCheckCloudStackNetworkACLRuleDestroy(s *terraform.State) error { } for k, uuid := range rs.Primary.Attributes { - if !strings.Contains(k, "uuids") { + if !strings.Contains(k, ".uuids.") || strings.HasSuffix(k, ".uuids.#") { continue } From ab8977eed6477d7caefbcc3f999ff27f0d3dafb4 Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 15 Jan 2015 21:46:06 +0100 Subject: [PATCH 3/3] Last tweaks needed to get through all acc tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Needed to tweak a few things in order get all tests running OK and to be able to handle the latest changes in master. All is good again now… --- .../resource_cloudstack_egress_firewall.go | 12 ++-- ...esource_cloudstack_egress_firewall_test.go | 65 +++++++++++-------- .../cloudstack/resource_cloudstack_network.go | 8 +-- ...source_cloudstack_network_acl_rule_test.go | 2 +- .../cloudstack/resource_cloudstack_vpc.go | 4 +- 5 files changed, 52 insertions(+), 39 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go index d5b9d80bdc..9f672c7a2a 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go @@ -75,7 +75,7 @@ func resourceCloudStackEgressFirewall() *schema.Resource { }, }, }, - Set: resourceCloudStackFirewallRuleHash, + Set: resourceCloudStackEgressFirewallRuleHash, }, }, } @@ -98,7 +98,7 @@ func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interfa // Create an empty schema.Set to hold all rules rules := &schema.Set{ - F: resourceCloudStackFirewallRuleHash, + F: resourceCloudStackEgressFirewallRuleHash, } for _, rule := range rs.List() { @@ -200,7 +200,7 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface // Create an empty schema.Set to hold all rules rules := &schema.Set{ - F: resourceCloudStackFirewallRuleHash, + F: resourceCloudStackEgressFirewallRuleHash, } if d.Get("managed").(bool) { @@ -343,7 +343,7 @@ func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interfa // Now first loop through all the old rules and delete any obsolete ones for _, rule := range ors.List() { // Delete the rule as it no longer exists in the config - err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{})) + err := resourceCloudStackEgressFirewallDeleteRule(d, meta, rule.(map[string]interface{})) if err != nil { return err } @@ -356,7 +356,7 @@ func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interfa // Then loop through al the currently configured rules and create the new ones for _, rule := range nrs.List() { // When succesfully deleted, re-create it again if it still exists - err := resourceCloudStackFirewallCreateRule( + err := resourceCloudStackEgressFirewallCreateRule( d, meta, rule.(map[string]interface{})) // We need to update this first to preserve the correct state @@ -377,7 +377,7 @@ func resourceCloudStackEgressFirewallDelete(d *schema.ResourceData, meta interfa if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 { for _, rule := range rs.List() { // Delete a single rule - err := resourceCloudStackFirewallDeleteRule(d, meta, rule.(map[string]interface{})) + err := resourceCloudStackEgressFirewallDeleteRule(d, meta, rule.(map[string]interface{})) // We need to update this first to preserve the correct state d.Set("rule", rs) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go index 8e70901d94..9211ba34e4 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall_test.go @@ -23,15 +23,17 @@ func TestAccCloudStackEgressFirewall_basic(t *testing.T) { resource.TestCheckResourceAttr( "cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + "cloudstack_egress_firewall.foo", + "rule.411689741.source_cidr", + CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + "cloudstack_egress_firewall.foo", "rule.411689741.protocol", "tcp"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.#", "2"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1209010669", "1000-2000"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1889509032", "80"), ), }, }, @@ -49,19 +51,21 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "ipaddress", CLOUDSTACK_NETWORK_1), + "cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1), resource.TestCheckResourceAttr( "cloudstack_egress_firewall.foo", "rule.#", "1"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + "cloudstack_egress_firewall.foo", + "rule.411689741.source_cidr", + CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + "cloudstack_egress_firewall.foo", "rule.411689741.protocol", "tcp"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.#", "2"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1209010669", "1000-2000"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1889509032", "80"), ), }, @@ -70,29 +74,31 @@ func TestAccCloudStackEgressFirewall_update(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckCloudStackEgressFirewallRulesExist("cloudstack_egress_firewall.foo"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "ipaddress", CLOUDSTACK_NETWORK_1), + "cloudstack_egress_firewall.foo", "network", CLOUDSTACK_NETWORK_1), resource.TestCheckResourceAttr( "cloudstack_egress_firewall.foo", "rule.#", "2"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.source_cidr", "10.0.0.0/24"), + "cloudstack_egress_firewall.foo", + "rule.411689741.source_cidr", + CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.protocol", "tcp"), + "cloudstack_egress_firewall.foo", "rule.411689741.protocol", "tcp"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.#", "2"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.#", "2"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1209010669", "1000-2000"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1209010669", "1000-2000"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.1702320581.ports.1889509032", "80"), + "cloudstack_egress_firewall.foo", "rule.411689741.ports.1889509032", "80"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.3779782959.source_cidr", "172.16.100.0/24"), + "cloudstack_egress_firewall.foo", + "rule.845479598.source_cidr", + CLOUDSTACK_NETWORK_1_IPADDRESS+"/32"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.3779782959.protocol", "tcp"), + "cloudstack_egress_firewall.foo", "rule.845479598.protocol", "tcp"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.3779782959.ports.#", "2"), + "cloudstack_egress_firewall.foo", "rule.845479598.ports.#", "1"), resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.3779782959.ports.1889509032", "80"), - resource.TestCheckResourceAttr( - "cloudstack_egress_firewall.foo", "rule.3779782959.ports.3638101695", "443"), + "cloudstack_egress_firewall.foo", "rule.845479598.ports.3638101695", "443"), ), }, }, @@ -162,7 +168,7 @@ func testAccCheckCloudStackEgressFirewallDestroy(s *terraform.State) error { var testAccCloudStackEgressFirewall_basic = fmt.Sprintf(` resource "cloudstack_egress_firewall" "foo" { - ipaddress = "%s" + network = "%s" rule { source_cidr = "%s/32" @@ -175,13 +181,20 @@ resource "cloudstack_egress_firewall" "foo" { var testAccCloudStackEgressFirewall_update = fmt.Sprintf(` resource "cloudstack_egress_firewall" "foo" { - ipaddress = "%s" + network = "%s" rule { source_cidr = "%s/32" protocol = "tcp" - ports = ["80", "443", 1000-2000"] + ports = ["80", "1000-2000"] + } + + rule { + source_cidr = "%s/32" + protocol = "tcp" + ports = ["443"] } }`, CLOUDSTACK_NETWORK_1, + CLOUDSTACK_NETWORK_1_IPADDRESS, CLOUDSTACK_NETWORK_1_IPADDRESS) diff --git a/builtin/providers/cloudstack/resource_cloudstack_network.go b/builtin/providers/cloudstack/resource_cloudstack_network.go index 161378fe89..51dc7d9e02 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network.go @@ -79,13 +79,13 @@ func resourceCloudStackNetworkCreate(d *schema.ResourceData, meta interface{}) e } // Compute/set the display text - displaytext := d.Get("display_text").(string) - if displaytext == "" { + displaytext, ok := d.GetOk("display_text") + if !ok { displaytext = name } // Create a new parameter struct - p := cs.Network.NewCreateNetworkParams(displaytext, name, networkofferingid, zoneid) + p := cs.Network.NewCreateNetworkParams(displaytext.(string), name, networkofferingid, zoneid) // Get the network details from the CIDR m, err := parseCIDR(d.Get("cidr").(string)) @@ -147,7 +147,7 @@ func resourceCloudStackNetworkRead(d *schema.ResourceData, meta interface{}) err } d.Set("name", n.Name) - d.Set("display_test", n.Displaytext) + d.Set("display_text", n.Displaytext) d.Set("cidr", n.Cidr) d.Set("network_offering", n.Networkofferingname) d.Set("zone", n.Zonename) diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go index 1a577ac7f3..6e8f0d2076 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule_test.go @@ -104,7 +104,7 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) { resource.TestCheckResourceAttr( "cloudstack_network_acl_rule.foo", "rule.4267872693.ports.1889509032", "80"), resource.TestCheckResourceAttr( - "cloudstack_network_acl_rule.foo", "rule.4267872693.traffic_type", "engress"), + "cloudstack_network_acl_rule.foo", "rule.4267872693.traffic_type", "egress"), ), }, }, diff --git a/builtin/providers/cloudstack/resource_cloudstack_vpc.go b/builtin/providers/cloudstack/resource_cloudstack_vpc.go index 46f64827c5..5b884d0314 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_vpc.go +++ b/builtin/providers/cloudstack/resource_cloudstack_vpc.go @@ -69,7 +69,7 @@ func resourceCloudStackVPCCreate(d *schema.ResourceData, meta interface{}) error // Set the display text displaytext, ok := d.GetOk("display_text") if !ok { - displaytext = d.Get("name") + displaytext = name } // Create a new parameter struct @@ -103,7 +103,7 @@ func resourceCloudStackVPCRead(d *schema.ResourceData, meta interface{}) error { } d.Set("name", v.Name) - d.Set("display_test", v.Displaytext) + d.Set("display_text", v.Displaytext) d.Set("cidr", v.Cidr) d.Set("zone", v.Zonename)