Merge pull request #4051 from svanharmelen/f-reduce-api-calls

provider/cloudstack: improve performance for all firewall related resources

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,16 +237,14 @@ 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 {
+				r, ok := ruleMap[id.(string)]
+				if !ok {
 					delete(uuids, "icmp")
 					continue
 				}
 
-					return err
-				}
+				// Delete the known rule so only unknown rules remain in the ruleMap
+				delete(ruleMap, id.(string))
 
 				// Update the values
 				rule["source_cidr"] = r.Cidrlist
@@ -259,15 +273,14 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 						}
 
 						// Get the rule
-						r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string))
-						if err != nil {
-							if count == 0 {
+						r, ok := ruleMap[id.(string)]
+						if !ok {
 							delete(uuids, port.(string))
 							continue
 						}
 
-							return err
-						}
+						// Delete the known rule so only unknown rules remain in the ruleMap
+						delete(ruleMap, id.(string))
 
 						// Update the values
 						rule["source_cidr"] = r.Cidrlist
@@ -287,44 +300,23 @@ 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
-		}
-
-		// 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,
+			"uuids":       ruleMap,
 		}
 
 		// Add the dummy rule to the rules set
 		rules.Add(rule)
 	}
-	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)

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,16 +237,14 @@ 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 {
+				r, ok := ruleMap[id.(string)]
+				if !ok {
 					delete(uuids, "icmp")
 					continue
 				}
 
-					return err
-				}
+				// Delete the known rule so only unknown rules remain in the ruleMap
+				delete(ruleMap, id.(string))
 
 				// Update the values
 				rule["source_cidr"] = r.Cidrlist
@@ -259,15 +273,14 @@ 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 {
+						r, ok := ruleMap[id.(string)]
+						if !ok {
 							delete(uuids, port.(string))
 							continue
 						}
 
-							return err
-						}
+						// Delete the known rule so only unknown rules remain in the ruleMap
+						delete(ruleMap, id.(string))
 
 						// Update the values
 						rule["source_cidr"] = r.Cidrlist
@@ -287,33 +300,13 @@ 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
-		}
-
-		// 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",
@@ -324,7 +317,6 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 		// Add the dummy rule to the rules set
 		rules.Add(rule)
 	}
-	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)

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,16 +262,14 @@ 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 {
+				r, ok := ruleMap[id.(string)]
+				if !ok {
 					delete(uuids, "icmp")
 					continue
 				}
 
-					return err
-				}
+				// 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)
@@ -274,16 +288,14 @@ 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 {
+				r, ok := ruleMap[id.(string)]
+				if !ok {
 					delete(uuids, "all")
 					continue
 				}
 
-					return err
-				}
+				// 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)
@@ -312,15 +324,14 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 						}
 
 						// Get the rule
-						r, count, err := cs.NetworkACL.GetNetworkACLByID(id.(string))
-						if err != nil {
-							if count == 0 {
+						r, ok := ruleMap[id.(string)]
+						if !ok {
 							delete(uuids, port.(string))
 							continue
 						}
 
-							return err
-						}
+						// 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)
@@ -342,34 +353,13 @@ 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
-		}
-
-		// 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",
@@ -379,7 +369,6 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 		// Add the dummy rule to the rules set
 		rules.Add(rule)
 	}
-	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)