provider/openstack: Security Group Rule fixes

This commit fixes an issue with security group rules where the rules
were not being correctly computed due to a typo in the rule map.

Once rules were successfully computed, the rules then needed to be
converted into a Set so they can be correctly ordered.

builtin/providers/openstack/resource_openstack_compute_secgroup_v2.go

@@ -38,8 +38,9 @@ func resourceComputeSecGroupV2() *schema.Resource {
 				ForceNew: false,
 			},
 			"rule": &schema.Schema{
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
+				Computed: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"id": &schema.Schema{
@@ -79,6 +80,7 @@ func resourceComputeSecGroupV2() *schema.Resource {
 						},
 					},
 				},
+				Set: secgroupRuleV2Hash,
 			},
 		},
 	}
@@ -131,10 +133,10 @@ func resourceComputeSecGroupV2Read(d *schema.ResourceData, meta interface{}) err
 	d.Set("description", sg.Description)
 	rtm := rulesToMap(sg.Rules)
 	for _, v := range rtm {
-		if v["group"] == d.Get("name") {
-			v["self"] = "1"
+		if v["from_group_id"] == d.Get("name") {
+			v["self"] = true
 		} else {
-			v["self"] = "0"
+			v["self"] = false
 		}
 	}
 	log.Printf("[DEBUG] rulesToMap(sg.Rules): %+v", rtm)
@@ -164,9 +166,7 @@ func resourceComputeSecGroupV2Update(d *schema.ResourceData, meta interface{}) e
 
 	if d.HasChange("rule") {
 		oldSGRaw, newSGRaw := d.GetChange("rule")
-		oldSGRSlice, newSGRSlice := oldSGRaw.([]interface{}), newSGRaw.([]interface{})
-		oldSGRSet := schema.NewSet(secgroupRuleV2Hash, oldSGRSlice)
-		newSGRSet := schema.NewSet(secgroupRuleV2Hash, newSGRSlice)
+		oldSGRSet, newSGRSet := oldSGRaw.(*schema.Set), newSGRaw.(*schema.Set)
 		secgrouprulesToAdd := newSGRSet.Difference(oldSGRSet)
 		secgrouprulesToRemove := oldSGRSet.Difference(newSGRSet)
 
@@ -231,7 +231,7 @@ func resourceComputeSecGroupV2Delete(d *schema.ResourceData, meta interface{}) e
 }
 
 func resourceSecGroupRulesV2(d *schema.ResourceData) []secgroups.CreateRuleOpts {
-	rawRules := d.Get("rule").([]interface{})
+	rawRules := d.Get("rule").(*schema.Set).List()
 	createRuleOptsList := make([]secgroups.CreateRuleOpts, len(rawRules))
 	for i, raw := range rawRules {
 		rawMap := raw.(map[string]interface{})
@@ -283,12 +283,12 @@ func rulesToMap(sgrs []secgroups.Rule) []map[string]interface{} {
 	sgrMap := make([]map[string]interface{}, len(sgrs))
 	for i, sgr := range sgrs {
 		sgrMap[i] = map[string]interface{}{
-			"id":          sgr.ID,
-			"from_port":   sgr.FromPort,
-			"to_port":     sgr.ToPort,
-			"ip_protocol": sgr.IPProtocol,
-			"cidr":        sgr.IPRange.CIDR,
-			"group":       sgr.Group.Name,
+			"id":            sgr.ID,
+			"from_port":     sgr.FromPort,
+			"to_port":       sgr.ToPort,
+			"ip_protocol":   sgr.IPProtocol,
+			"cidr":          sgr.IPRange.CIDR,
+			"from_group_id": sgr.Group.Name,
 		}
 	}
 	return sgrMap

builtin/providers/openstack/resource_openstack_compute_secgroup_v2_test.go

@@ -28,6 +28,31 @@ func TestAccComputeV2SecGroup_basic(t *testing.T) {
 	})
 }
 
+func TestAccComputeV2SecGroup_update(t *testing.T) {
+	var secgroup secgroups.SecurityGroup
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeV2SecGroupDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeV2SecGroup_basic,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.foo", &secgroup),
+				),
+			},
+			resource.TestStep{
+				Config: testAccComputeV2SecGroup_update,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.foo", &secgroup),
+					testAccCheckComputeV2SecGroupRuleCount(t, "openstack_compute_secgroup_v2.foo", 2),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckComputeV2SecGroupDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 	computeClient, err := config.computeV2Client(OS_REGION_NAME)
@@ -81,10 +106,78 @@ func testAccCheckComputeV2SecGroupExists(t *testing.T, n string, secgroup *secgr
 	}
 }
 
+func testAccCheckComputeV2SecGroupRuleCount(t *testing.T, n string, count int) 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 ID is set")
+		}
+
+		config := testAccProvider.Meta().(*Config)
+		computeClient, err := config.computeV2Client(OS_REGION_NAME)
+		if err != nil {
+			return fmt.Errorf("(testAccCheckComputeV2SecGroupExists) Error creating OpenStack compute client: %s", err)
+		}
+
+		found, err := secgroups.Get(computeClient, rs.Primary.ID).Extract()
+		if err != nil {
+			return err
+		}
+
+		if found.ID != rs.Primary.ID {
+			return fmt.Errorf("Security group not found")
+		}
+
+		if len(found.Rules) != count {
+			return fmt.Errorf("Security group rule count does not match. Expected %d, got %d", count, len(found.Rules))
+		}
+
+		return nil
+	}
+}
+
 var testAccComputeV2SecGroup_basic = fmt.Sprintf(`
   resource "openstack_compute_secgroup_v2" "foo" {
-    region = "%s"
     name = "test_group_1"
     description = "first test security group"
-    }`,
-	OS_REGION_NAME)
+		rule {
+			from_port = 22
+			to_port = 22
+			ip_protocol = "tcp"
+			cidr = "0.0.0.0/0"
+		}
+		rule {
+			from_port = 1
+			to_port = 65535
+			ip_protocol = "udp"
+			cidr = "0.0.0.0/0"
+		}
+		rule {
+			from_port = -1
+			to_port = -1
+			ip_protocol = "icmp"
+			cidr = "0.0.0.0/0"
+		}
+  }`)
+
+var testAccComputeV2SecGroup_update = fmt.Sprintf(`
+  resource "openstack_compute_secgroup_v2" "foo" {
+    name = "test_group_1"
+    description = "first test security group"
+		rule {
+			from_port = 2200
+			to_port = 2200
+			ip_protocol = "tcp"
+			cidr = "0.0.0.0/0"
+		}
+		rule {
+			from_port = -1
+			to_port = -1
+			ip_protocol = "icmp"
+			cidr = "0.0.0.0/0"
+		}
+  }`)