Extending the managed firewall option

It’s now also possible to don’t give any rules, when the firewall is
configured with `managed = true`. This will in effect mean; make sure
no rules exist at all for the firewall.

builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go

@@ -35,7 +35,7 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
 
 			"rule": &schema.Schema{
 				Type:     schema.TypeSet,
-				Required: true,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"source_cidr": &schema.Schema{
@@ -84,6 +84,11 @@ func resourceCloudStackEgressFirewall() *schema.Resource {
 func resourceCloudStackEgressFirewallCreate(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
+	// Make sure all required parameters are there
+	if err := verifyEgressFirewallParams(d); err != nil {
+		return err
+	}
+
 	// Retrieve the network UUID
 	networkid, e := retrieveUUID(cs, "network", d.Get("network").(string))
 	if e != nil {
@@ -203,10 +208,6 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 		F: resourceCloudStackEgressFirewallRuleHash,
 	}
 
-	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() {
@@ -285,7 +286,8 @@ func resourceCloudStackEgressFirewallRead(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) {
+	managed := d.Get("managed").(bool)
+	if managed {
 		// Get all the rules from the running environment
 		p := cs.Firewall.NewListEgressFirewallRulesParams()
 		p.SetNetworkid(d.Id())
@@ -297,7 +299,7 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 		}
 
 		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{})
+		uuids := make(map[string]interface{}, len(r.EgressFirewallRules))
 		for _, r := range r.EgressFirewallRules {
 			uuids[r.Id] = r.Id
 		}
@@ -326,7 +328,7 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)
-	} else {
+	} else if !managed {
 		d.SetId("")
 	}
 
@@ -334,6 +336,11 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 }
 
 func resourceCloudStackEgressFirewallUpdate(d *schema.ResourceData, meta interface{}) error {
+	// Make sure all required parameters are there
+	if err := verifyEgressFirewallParams(d); err != nil {
+		return err
+	}
+
 	// Check if the rule set as a whole has changed
 	if d.HasChange("rule") {
 		o, n := d.GetChange("rule")
@@ -462,6 +469,18 @@ func resourceCloudStackEgressFirewallRuleHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }
 
+func verifyEgressFirewallParams(d *schema.ResourceData) error {
+	managed := d.Get("managed").(bool)
+	_, rules := d.GetOk("rule")
+
+	if !rules && !managed {
+		return fmt.Errorf(
+			"You must supply at least one 'rule' when not using the 'managed' firewall feature")
+	}
+
+	return nil
+}
+
 func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
 	protocol := rule["protocol"].(string)
 	if protocol != "tcp" && protocol != "udp" && protocol != "icmp" {

builtin/providers/cloudstack/resource_cloudstack_firewall.go

@@ -35,7 +35,7 @@ func resourceCloudStackFirewall() *schema.Resource {
 
 			"rule": &schema.Schema{
 				Type:     schema.TypeSet,
-				Required: true,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"source_cidr": &schema.Schema{
@@ -84,6 +84,11 @@ func resourceCloudStackFirewall() *schema.Resource {
 func resourceCloudStackFirewallCreate(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
+	// Make sure all required parameters are there
+	if err := verifyFirewallParams(d); err != nil {
+		return err
+	}
+
 	// Retrieve the ipaddress UUID
 	ipaddressid, e := retrieveUUID(cs, "ipaddress", d.Get("ipaddress").(string))
 	if e != nil {
@@ -281,7 +286,8 @@ 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) {
+	managed := d.Get("managed").(bool)
+	if managed {
 		// Get all the rules from the running environment
 		p := cs.Firewall.NewListFirewallRulesParams()
 		p.SetIpaddressid(d.Id())
@@ -293,7 +299,7 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 		}
 
 		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{})
+		uuids := make(map[string]interface{}, len(r.FirewallRules))
 		for _, r := range r.FirewallRules {
 			uuids[r.Id] = r.Id
 		}
@@ -322,7 +328,7 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)
-	} else {
+	} else if !managed {
 		d.SetId("")
 	}
 
@@ -330,6 +336,11 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 }
 
 func resourceCloudStackFirewallUpdate(d *schema.ResourceData, meta interface{}) error {
+	// Make sure all required parameters are there
+	if err := verifyFirewallParams(d); err != nil {
+		return err
+	}
+
 	// Check if the rule set as a whole has changed
 	if d.HasChange("rule") {
 		o, n := d.GetChange("rule")
@@ -458,6 +469,18 @@ func resourceCloudStackFirewallRuleHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }
 
+func verifyFirewallParams(d *schema.ResourceData) error {
+	managed := d.Get("managed").(bool)
+	_, rules := d.GetOk("rule")
+
+	if !rules && !managed {
+		return fmt.Errorf(
+			"You must supply at least one 'rule' when not using the 'managed' firewall feature")
+	}
+
+	return nil
+}
+
 func verifyFirewallRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
 	protocol := rule["protocol"].(string)
 	if protocol != "tcp" && protocol != "udp" && protocol != "icmp" {

builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go

@@ -35,7 +35,7 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 
 			"rule": &schema.Schema{
 				Type:     schema.TypeSet,
-				Required: true,
+				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"action": &schema.Schema{
@@ -94,11 +94,13 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 }
 
 func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interface{}) error {
-	// Get the acl UUID
+	// Make sure all required parameters are there
-	aclid := d.Get("aclid").(string)
+	if err := verifyNetworkACLParams(d); err != nil {
+		return err
+	}
 
 	// We need to set this upfront in order to be able to save a partial state
-	d.SetId(aclid)
+	d.SetId(d.Get("aclid").(string))
 
 	// Create all rules that are configured
 	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
@@ -110,8 +112,7 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 
 		for _, rule := range rs.List() {
 			// Create a single rule
-			err := resourceCloudStackNetworkACLRuleCreateRule(
+			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule.(map[string]interface{}))
-				d, meta, aclid, rule.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
 			rules.Add(rule)
@@ -127,7 +128,7 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 }
 
 func resourceCloudStackNetworkACLRuleCreateRule(
-	d *schema.ResourceData, meta interface{}, aclid 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{})
 
@@ -140,7 +141,7 @@ func resourceCloudStackNetworkACLRuleCreateRule(
 	p := cs.NetworkACL.NewCreateNetworkACLParams(rule["protocol"].(string))
 
 	// Set the acl ID
-	p.SetAclid(aclid)
+	p.SetAclid(d.Id())
 
 	// Set the action
 	p.SetAction(rule["action"].(string))
@@ -302,7 +303,8 @@ 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) {
+	managed := d.Get("managed").(bool)
+	if managed {
 		// Get all the rules from the running environment
 		p := cs.NetworkACL.NewListNetworkACLsParams()
 		p.SetAclid(d.Id())
@@ -314,7 +316,7 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 		}
 
 		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{})
+		uuids := make(map[string]interface{}, len(r.NetworkACLs))
 		for _, r := range r.NetworkACLs {
 			uuids[r.Id] = r.Id
 		}
@@ -343,7 +345,7 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)
-	} else {
+	} else if !managed {
 		d.SetId("")
 	}
 
@@ -351,8 +353,10 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 }
 
 func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interface{}) error {
-	// Get the acl UUID
+	// Make sure all required parameters are there
-	aclid := d.Get("aclid").(string)
+	if err := verifyNetworkACLParams(d); err != nil {
+		return err
+	}
 
 	// Check if the rule set as a whole has changed
 	if d.HasChange("rule") {
@@ -376,8 +380,7 @@ func resourceCloudStackNetworkACLRuleUpdate(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 := resourceCloudStackNetworkACLRuleCreateRule(
+			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule.(map[string]interface{}))
-				d, meta, aclid, rule.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
 			rules.Add(rule)
@@ -486,6 +489,18 @@ func resourceCloudStackNetworkACLRuleHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }
 
+func verifyNetworkACLParams(d *schema.ResourceData) error {
+	managed := d.Get("managed").(bool)
+	_, rules := d.GetOk("rule")
+
+	if !rules && !managed {
+		return fmt.Errorf(
+			"You must supply at least one 'rule' when not using the 'managed' firewall feature")
+	}
+
+	return nil
+}
+
 func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
 	action := rule["action"].(string)
 	if action != "allow" && action != "deny" {