hashicorp · tyrannosaurus-becks · Feb 20, 2019 · Feb 13, 2019 · Feb 13, 2019 · Feb 13, 2019
diff --git a/vault/resource_identity_group.go b/vault/resource_identity_group.go
@@ -45,7 +45,7 @@ func identityGroupResource() *schema.Resource {
 			},
 
 			"policies": {
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
@@ -54,7 +54,7 @@ func identityGroupResource() *schema.Resource {
 			},
 
 			"member_group_ids": {
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
@@ -63,12 +63,21 @@ func identityGroupResource() *schema.Resource {
 			},
 
 			"member_entity_ids": {
-				Type:     schema.TypeList,
+				Type:     schema.TypeSet,
 				Optional: true,
+				Computed: true,
 				Elem: &schema.Schema{
 					Type: schema.TypeString,
 				},
 				Description: "Entity IDs to be assigned as group members.",
+				// Suppress the diff if group type is "external" because we cannot manage
+				// group members
+				DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
+					if d.Get("type").(string) == "external" {
+						return true
+					}
+					return false
+				},
 			},
 
 			"id": {
@@ -80,22 +89,24 @@ func identityGroupResource() *schema.Resource {
 	}
 }
 
-func identityGroupUpdateFields(d *schema.ResourceData, data map[string]interface{}) {
+func identityGroupUpdateFields(d *schema.ResourceData, data map[string]interface{}) error {
 	if policies, ok := d.GetOk("policies"); ok {
-		data["policies"] = policies
+		data["policies"] = policies.(*schema.Set).List()
 	}
 
-	if memberEntityIDs, ok := d.GetOk("member_entity_ids"); ok {
-		data["member_entity_ids"] = memberEntityIDs
+	if memberEntityIDs, ok := d.GetOk("member_entity_ids"); ok && d.Get("type").(string) == "internal" {
+		data["member_entity_ids"] = memberEntityIDs.(*schema.Set).List()
 	}
 
 	if memberGroupIDs, ok := d.GetOk("member_group_ids"); ok {
-		data["member_group_ids"] = memberGroupIDs
+		data["member_group_ids"] = memberGroupIDs.(*schema.Set).List()
 	}
 
 	if metadata, ok := d.GetOk("metadata"); ok {
 		data["metadata"] = metadata
 	}
+
+	return nil
 }
 
 func identityGroupCreate(d *schema.ResourceData, meta interface{}) error {
@@ -111,7 +122,9 @@ func identityGroupCreate(d *schema.ResourceData, meta interface{}) error {
 		"type": typeValue,
 	}
 
-	identityGroupUpdateFields(d, data)
+	if err := identityGroupUpdateFields(d, data); err != nil {
+		return fmt.Errorf("error writing IdentityGroup to %q: %s", name, err)
+	}
 
 	resp, err := client.Logical().Write(path, data)
 
@@ -136,7 +149,9 @@ func identityGroupUpdate(d *schema.ResourceData, meta interface{}) error {
 
 	data := map[string]interface{}{}
 
-	identityGroupUpdateFields(d, data)
+	if err := identityGroupUpdateFields(d, data); err != nil {
+		return fmt.Errorf("error updating IdentityGroup %q: %s", id, err)
+	}
 
 	_, err := client.Logical().Write(path, data)
 
@@ -170,7 +185,7 @@ func identityGroupRead(d *schema.ResourceData, meta interface{}) error {
 		return nil
 	}
 
-	for _, k := range []string{"name", "type", "metadata", "member_entity_ids", "member_group_ids"} {
+	for _, k := range []string{"name", "type", "metadata", "policies", "member_entity_ids", "member_group_ids"} {
 		d.Set(k, resp.Data[k])
 	}
 	return nil

diff --git a/vault/resource_identity_group_alias.go b/vault/resource_identity_group_alias.go
@@ -29,13 +29,13 @@ func identityGroupAliasResource() *schema.Resource {
 			"mount_accessor": {
 				Type:        schema.TypeString,
 				Required:    true,
-				Description: "Mount accessor to which this alias belongs toMount accessor to which this alias belongs to.",
+				Description: "Mount accessor to which this alias belongs to.",
 			},
 
 			"canonical_id": {
 				Type:        schema.TypeString,
 				Required:    true,
-				Description: "ID of the group to which this is an alias.uType of the group, internal or external. Defaults to internal.",
+				Description: "ID of the group to which this is an alias.",
 			},
 
 			"id": {

diff --git a/vault/resource_identity_group_test.go b/vault/resource_identity_group_test.go
@@ -3,7 +3,9 @@ package vault
 import (
 	"encoding/json"
 	"fmt"
+	"regexp"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -30,6 +32,7 @@ func TestAccIdentityGroup(t *testing.T) {
 
 func TestAccIdentityGroupUpdate(t *testing.T) {
 	group := acctest.RandomWithPrefix("test-group")
+	entity := acctest.RandomWithPrefix("test-entity")
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -45,10 +48,39 @@ func TestAccIdentityGroupUpdate(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccIdentityGroupCheckAttrs(group),
 					resource.TestCheckResourceAttr("vault_identity_group.group", "metadata.version", "2"),
-					resource.TestCheckResourceAttr("vault_identity_group.group", "policies.0", "dev"),
-					resource.TestCheckResourceAttr("vault_identity_group.group", "policies.1", "test"),
+					resource.TestCheckResourceAttr("vault_identity_group.group", "policies.#", "2"),
+					resource.TestCheckResourceAttr("vault_identity_group.group", "policies.326271447", "dev"),
+					resource.TestCheckResourceAttr("vault_identity_group.group", "policies.1785148924", "test"),
+					resource.TestCheckResourceAttr("vault_identity_group.group", "member_entity_ids.#", "0"),
 				),
 			},
+			{
+				Config: testAccIdentityGroupConfigUpdateMembers(group, entity),
+				Check: resource.ComposeTestCheckFunc(
+					testAccIdentityGroupCheckAttrs(group),
+					resource.TestCheckResourceAttr("vault_identity_group.group", "member_entity_ids.#", "1"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccIdentityGroupExternal(t *testing.T) {
+	group := acctest.RandomWithPrefix("test-group")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testProviders,
+		CheckDestroy: testAccCheckIdentityGroupDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccIdentityGroupConfig(group),
+				Check:  testAccIdentityGroupCheckAttrs(group),
+			},
+			{
+				Config:      testAccIdentityGroupConfigExternalMembers(group),
+				ExpectError: regexp.MustCompile(`cannot set 'member_entity_ids' on external groups`),
+			},
 		},
 	})
 }
@@ -139,10 +171,18 @@ func testAccIdentityGroupCheckAttrs(group string) resource.TestCheckFunc {
 					if count != len(apiData) {
 						return fmt.Errorf("expected %s to have %d entries in state, has %d", stateAttr, len(apiData), count)
 					}
+
 					for i := 0; i < count; i++ {
-						stateData := instanceState.Attributes[stateAttr+"."+strconv.Itoa(i)]
-						if stateData != apiData[i] {
-							return fmt.Errorf("expected item %d of %s (%s in state) of %q to be %q, got %q", i, apiAttr, stateAttr, path, stateData, apiData[i])
+						found := false
+						for stateKey, stateValue := range instanceState.Attributes {
+							if strings.HasPrefix(stateKey, stateAttr) {
+								if apiData[i] == stateValue {
+									found = true
+								}
+							}
+						}
+						if !found {
+							return fmt.Errorf("Expected item %d of %s (%s in state) of %q to be in state but wasn't", i, apiAttr, stateAttr, apiData[i])
 						}
 					}
 					match = true
@@ -181,3 +221,40 @@ resource "vault_identity_group" "group" {
   }
 }`, groupName)
 }
+
+func testAccIdentityGroupConfigUpdateMembers(groupName string, entityName string) string {
+	return fmt.Sprintf(`
+resource "vault_identity_group" "group" {
+  name = "%s"
+  type = "internal"
+  policies = ["dev", "test"]
+  metadata = {
+    version = "2"
+  }
+
+  member_entity_ids = ["${vault_identity_entity.entity.id}"]
+}
+
+resource "vault_identity_entity" "entity" {
+  name = "%s"
+  policies = ["dev", "test"]
+  metadata = {
+    version = "2"
+  }
+}
+`, groupName, entityName)
+}
+
+func testAccIdentityGroupConfigExternalMembers(groupName string) string {
+	return fmt.Sprintf(`
+resource "vault_identity_group" "group" {
+  name = "%s"
+  type = "external"
+  policies = ["test"]
+  metadata = {
+    version = "1"
+  }
+
+  member_entity_ids = ["this will fail"]
+}`, groupName)
+}