From a3eae45b326d24f080b0759038e29d162fafc13c Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Tue, 24 Nov 2015 18:01:12 +0100 Subject: [PATCH] Improve performance all firewall related resources --- .../resource_cloudstack_egress_firewall.go | 92 +++++++-------- .../resource_cloudstack_firewall.go | 92 +++++++-------- .../resource_cloudstack_network_acl_rule.go | 107 ++++++++---------- 3 files changed, 132 insertions(+), 159 deletions(-) diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go index 55209eadf..37979e13f 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go @@ -203,6 +203,22 @@ func resourceCloudStackEgressFirewallCreateRule( func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) + // Get all the rules from the running environment + p := cs.Firewall.NewListEgressFirewallRulesParams() + p.SetNetworkid(d.Id()) + p.SetListall(true) + + l, err := cs.Firewall.ListEgressFirewallRules(p) + if err != nil { + return err + } + + // Make a map of all the rules so we can easily find a rule + ruleMap := make(map[string]*cloudstack.EgressFirewallRule, l.Count) + for _, r := range l.EgressFirewallRules { + ruleMap[r.Id] = r + } + // Create an empty schema.Set to hold all rules rules := &schema.Set{ F: resourceCloudStackEgressFirewallRuleHash, @@ -221,17 +237,15 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface } // Get the rule - r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string)) - // If the count == 0, there is no object found for this ID - if err != nil { - if count == 0 { - delete(uuids, "icmp") - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, "icmp") + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["source_cidr"] = r.Cidrlist rule["protocol"] = r.Protocol @@ -259,16 +273,15 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface } // Get the rule - r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string)) - if err != nil { - if count == 0 { - delete(uuids, port.(string)) - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, port.(string)) + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["source_cidr"] = r.Cidrlist rule["protocol"] = r.Protocol @@ -287,43 +300,22 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface // If this is a managed firewall, add all unknown rules into a single dummy rule managed := d.Get("managed").(bool) - if managed { - // 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 + if managed && len(ruleMap) > 0 { + // Add all UUIDs to a uuids map + uuids := make(map[string]interface{}, len(ruleMap)) + for uuid := range ruleMap { + uuids[uuid] = uuid } - // Add all UUIDs to the uuids map - uuids := make(map[string]interface{}, len(r.EgressFirewallRules)) - for _, r := range r.EgressFirewallRules { - uuids[r.Id] = r.Id + // Make a dummy rule to hold all unknown UUIDs + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": ruleMap, } - // 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) - } + // Add the dummy rule to the rules set + rules.Add(rule) } if rules.Len() > 0 { diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_firewall.go index 1e7ff8e70..3bcced02e 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_firewall.go +++ b/builtin/providers/cloudstack/resource_cloudstack_firewall.go @@ -203,6 +203,22 @@ func resourceCloudStackFirewallCreateRule( func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) + // Get all the rules from the running environment + p := cs.Firewall.NewListFirewallRulesParams() + p.SetIpaddressid(d.Id()) + p.SetListall(true) + + l, err := cs.Firewall.ListFirewallRules(p) + if err != nil { + return err + } + + // Make a map of all the rules so we can easily find a rule + ruleMap := make(map[string]*cloudstack.FirewallRule, l.Count) + for _, r := range l.FirewallRules { + ruleMap[r.Id] = r + } + // Create an empty schema.Set to hold all rules rules := &schema.Set{ F: resourceCloudStackFirewallRuleHash, @@ -221,17 +237,15 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er } // Get the rule - r, count, err := cs.Firewall.GetFirewallRuleByID(id.(string)) - // If the count == 0, there is no object found for this ID - if err != nil { - if count == 0 { - delete(uuids, "icmp") - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, "icmp") + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["source_cidr"] = r.Cidrlist rule["protocol"] = r.Protocol @@ -259,16 +273,15 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er } // Get the rule - r, count, err := cs.Firewall.GetFirewallRuleByID(id.(string)) - if err != nil { - if count == 0 { - delete(uuids, port.(string)) - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, port.(string)) + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["source_cidr"] = r.Cidrlist rule["protocol"] = r.Protocol @@ -287,43 +300,22 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er // If this is a managed firewall, add all unknown rules into a single dummy rule managed := d.Get("managed").(bool) - if managed { - // 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 + if managed && len(ruleMap) > 0 { + // Add all UUIDs to a uuids map + uuids := make(map[string]interface{}, len(ruleMap)) + for uuid := range ruleMap { + uuids[uuid] = uuid } - // Add all UUIDs to the uuids map - uuids := make(map[string]interface{}, len(r.FirewallRules)) - for _, r := range r.FirewallRules { - uuids[r.Id] = r.Id + // Make a dummy rule to hold all unknown UUIDs + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": uuids, } - // 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) - } + // Add the dummy rule to the rules set + rules.Add(rule) } if rules.Len() > 0 { diff --git a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go index ba2650484..18446738a 100644 --- a/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go +++ b/builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go @@ -228,6 +228,22 @@ func resourceCloudStackNetworkACLRuleCreateRule( func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface{}) error { cs := meta.(*cloudstack.CloudStackClient) + // Get all the rules from the running environment + p := cs.NetworkACL.NewListNetworkACLsParams() + p.SetAclid(d.Id()) + p.SetListall(true) + + l, err := cs.NetworkACL.ListNetworkACLs(p) + if err != nil { + return err + } + + // Make a map of all the rules so we can easily find a rule + ruleMap := make(map[string]*cloudstack.NetworkACL, l.Count) + for _, r := range l.NetworkACLs { + ruleMap[r.Id] = r + } + // Create an empty schema.Set to hold all rules rules := &schema.Set{ F: resourceCloudStackNetworkACLRuleHash, @@ -246,17 +262,15 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface } // Get the rule - r, count, err := cs.NetworkACL.GetNetworkACLByID(id.(string)) - // If the count == 0, there is no object found for this ID - if err != nil { - if count == 0 { - delete(uuids, "icmp") - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, "icmp") + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["action"] = strings.ToLower(r.Action) rule["source_cidr"] = r.Cidrlist @@ -274,17 +288,15 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface } // Get the rule - r, count, err := cs.NetworkACL.GetNetworkACLByID(id.(string)) - // If the count == 0, there is no object found for this ID - if err != nil { - if count == 0 { - delete(uuids, "all") - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, "all") + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["action"] = strings.ToLower(r.Action) rule["source_cidr"] = r.Cidrlist @@ -312,16 +324,15 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface } // Get the rule - r, count, err := cs.NetworkACL.GetNetworkACLByID(id.(string)) - if err != nil { - if count == 0 { - delete(uuids, port.(string)) - continue - } - - return err + r, ok := ruleMap[id.(string)] + if !ok { + delete(uuids, port.(string)) + continue } + // Delete the known rule so only unknown rules remain in the ruleMap + delete(ruleMap, id.(string)) + // Update the values rule["action"] = strings.ToLower(r.Action) rule["source_cidr"] = r.Cidrlist @@ -342,43 +353,21 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface // If this is a managed firewall, add all unknown rules into a single dummy rule managed := d.Get("managed").(bool) - if managed { - // 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 + if managed && len(ruleMap) > 0 { + // Add all UUIDs to a uuids map + uuids := make(map[string]interface{}, len(ruleMap)) + for uuid := range ruleMap { + uuids[uuid] = uuid } - // Add all UUIDs to the uuids map - uuids := make(map[string]interface{}, len(r.NetworkACLs)) - for _, r := range r.NetworkACLs { - uuids[r.Id] = r.Id + rule := map[string]interface{}{ + "source_cidr": "N/A", + "protocol": "N/A", + "uuids": uuids, } - // 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) - } + // Add the dummy rule to the rules set + rules.Add(rule) } if rules.Len() > 0 {