Merge pull request #203 from shapeblue/supportDesc

Support desc and rule_number in create_network_acl_rule

Author: kiranchavala

cloudstack/resource_cloudstack_network_acl_rule.go

@@ -57,6 +57,12 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 				Optional: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
+						"rule_number": {
+							Type:     schema.TypeInt,
+							Optional: true,
+							Computed: true,
+						},
+
 						"action": {
 							Type:     schema.TypeString,
 							Optional: true,
@@ -100,6 +106,11 @@ func resourceCloudStackNetworkACLRule() *schema.Resource {
 							Default:  "ingress",
 						},
 
+						"description": {
+							Type:     schema.TypeString,
+							Optional: true,
+						},
+
 						"uuids": {
 							Type:     schema.TypeMap,
 							Computed: true,
@@ -198,6 +209,11 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str
 	// Create a new parameter struct
 	p := cs.NetworkACL.NewCreateNetworkACLParams(rule["protocol"].(string))
 
+	// If a rule ID is specified, set it
+	if ruleNum, ok := rule["rule_number"].(int); ok && ruleNum > 0 {
+		p.SetNumber(ruleNum)
+	}
+
 	// Set the acl ID
 	p.SetAclid(d.Id())
 
@@ -214,6 +230,11 @@ func createNetworkACLRule(d *schema.ResourceData, meta interface{}, rule map[str
 	// Set the traffic type
 	p.SetTraffictype(rule["traffic_type"].(string))
 
+	// Set the description
+	if desc, ok := rule["description"].(string); ok && desc != "" {
+		p.SetReason(desc)
+	}
+
 	// If the protocol is ICMP set the needed ICMP parameters
 	if rule["protocol"].(string) == "icmp" {
 		p.SetIcmptype(rule["icmp_type"].(int))
@@ -623,6 +644,16 @@ func verifyNetworkACLParams(d *schema.ResourceData) error {
 }
 
 func verifyNetworkACLRuleParams(d *schema.ResourceData, rule map[string]interface{}) error {
+	if ruleNum, ok := rule["rule_number"]; ok && ruleNum != nil {
+		if number, ok := ruleNum.(int); ok && number != 0 {
+			// Validate only if rule_number is explicitly set (non-zero)
+			if number < 1 || number > 65535 {
+				return fmt.Errorf(
+					"%q must be between %d and %d inclusive, got: %d", "rule_number", 1, 65535, number)
+			}
+		}
+	}
+
 	action := rule["action"].(string)
 	if action != "allow" && action != "deny" {
 		return fmt.Errorf("Parameter action only accepts 'allow' or 'deny' as values")

cloudstack/resource_cloudstack_network_acl_rule_test.go

@@ -38,6 +38,7 @@ func TestAccCloudStackNetworkACLRule_basic(t *testing.T) {
 			{
 				Config: testAccCloudStackNetworkACLRule_basic,
 				Check: resource.ComposeTestCheckFunc(
+
 					testAccCheckCloudStackNetworkACLRulesExist("cloudstack_network_acl.foo"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.#", "3"),
@@ -55,6 +56,10 @@ func TestAccCloudStackNetworkACLRule_basic(t *testing.T) {
 						"cloudstack_network_acl_rule.foo", "rule.0.ports.0", "443"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.0.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.0.description", "Allow HTTP and HTTPS"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.1.rule_number", "20"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.1.action", "allow"),
 					resource.TestCheckResourceAttr(
@@ -67,6 +72,12 @@ func TestAccCloudStackNetworkACLRule_basic(t *testing.T) {
 						"cloudstack_network_acl_rule.foo", "rule.1.icmp_type", "-1"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.1.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.1.description", "Allow ICMP traffic"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2.rule_number", "10"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.2.description", "Allow all traffic"),
 				),
 			},
 		},
@@ -99,6 +110,8 @@ func TestAccCloudStackNetworkACLRule_update(t *testing.T) {
 						"cloudstack_network_acl_rule.foo", "rule.0.ports.0", "443"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.0.traffic_type", "ingress"),
+					resource.TestCheckResourceAttr(
+						"cloudstack_network_acl_rule.foo", "rule.1.rule_number", "20"),
 					resource.TestCheckResourceAttr(
 						"cloudstack_network_acl_rule.foo", "rule.1.action", "allow"),
 					resource.TestCheckResourceAttr(
@@ -245,26 +258,31 @@ resource "cloudstack_network_acl_rule" "foo" {
   acl_id = cloudstack_network_acl.foo.id
 
   rule {
+  	rule_number = 10
   	action = "allow"
     cidr_list = ["172.18.100.0/24"]
     protocol = "all"
     traffic_type = "ingress"
+	description = "Allow all traffic"
   }
 
   rule {
+  	rule_number = 20
   	action = "allow"
     cidr_list = ["172.18.100.0/24"]
     protocol = "icmp"
     icmp_type = "-1"
     icmp_code = "-1"
     traffic_type = "ingress"
+	description = "Allow ICMP traffic"
   }
 
   rule {
     cidr_list = ["172.16.100.0/24"]
     protocol = "tcp"
     ports = ["80", "443"]
     traffic_type = "ingress"
+	description = "Allow HTTP and HTTPS"
   }
 }`
 
@@ -294,26 +312,28 @@ resource "cloudstack_network_acl_rule" "foo" {
 
   rule {
   	action = "deny"
-		cidr_list = ["172.18.100.0/24", "172.18.101.0/24"]
+	cidr_list = ["172.18.100.0/24", "172.18.101.0/24"]
     protocol = "icmp"
     icmp_type = "-1"
     icmp_code = "-1"
     traffic_type = "ingress"
+	description = "Deny ICMP traffic"
   }
 
   rule {
-	  action = "allow"
+	action = "allow"
     cidr_list = ["172.18.100.0/24"]
     protocol = "tcp"
     ports = ["80", "443"]
     traffic_type = "ingress"
   }
 
   rule {
-	  action = "deny"
+	action = "deny"
     cidr_list = ["10.0.0.0/24"]
     protocol = "tcp"
     ports = ["80", "1000-2000"]
     traffic_type = "egress"
+	description = "Deny specific TCP ports"
   }
 }`

website/docs/r/network_acl_rule.html.markdown

@@ -48,6 +48,8 @@ The following arguments are supported:
 
 The `rule` block supports:
 
+* `rule_number` - (Optional) The number of the ACL item used to order the ACL rules. The ACL rule with the lowest number has the highest priority. If not specified, the ACL item will be created with a number one greater than the highest numbered rule.
+
 * `action` - (Optional) The action for the rule. Valid options are: `allow` and
     `deny` (defaults allow).
 
@@ -68,6 +70,8 @@ The `rule` block supports:
 * `traffic_type` - (Optional) The traffic type for the rule. Valid options are:
     `ingress` or `egress` (defaults ingress).
 
+* `description` - (Optional) A description indicating why the ACL rule is required.
+
 ## Attributes Reference
 
 The following attributes are exported: