From b4c3f833d10455b02d549e3cd98087916b1b916c Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Tue, 11 Oct 2022 09:50:11 -0700 Subject: [PATCH 1/7] add initial implementation of externally managed member group IDs --- internal/identity/group/group.go | 209 ++++++++++++++++++ vault/provider.go | 4 + vault/resource_identity_group.go | 22 +- ...source_identity_group_member_entity_ids.go | 100 +-------- ...e_identity_group_member_entity_ids_test.go | 62 +----- ...esource_identity_group_member_group_ids.go | 148 +++++++++++++ ...ce_identity_group_member_group_ids_test.go | 130 +++++++++++ 7 files changed, 523 insertions(+), 152 deletions(-) create mode 100644 vault/resource_identity_group_member_group_ids.go create mode 100644 vault/resource_identity_group_member_group_ids_test.go diff --git a/internal/identity/group/group.go b/internal/identity/group/group.go index eecb9de73..34fbcf2ca 100644 --- a/internal/identity/group/group.go +++ b/internal/identity/group/group.go @@ -1,5 +1,214 @@ package group +import ( + "fmt" + "strconv" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/vault/api" +) + const ( LookupPath = "identity/lookup/group" ) + +func GetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) (map[string]interface{}, error) { + data := map[string]interface{}{} + + switch memberField { + case "member_group_ids", "member_entity_ids": + default: + return nil, fmt.Errorf("invalid value for member field") + } + var curIDS []interface{} + if t, ok := resp.Data["type"]; ok && t.(string) != "external" { + if v, ok := resp.Data[memberField]; ok && v != nil { + curIDS = v.([]interface{}) + } + + if d.Get("exclusive").(bool) || len(curIDS) == 0 { + data[memberField] = d.Get(memberField).(*schema.Set).List() + } else { + set := map[interface{}]bool{} + for _, v := range curIDS { + set[v] = true + } + + o, _ := d.GetChange(memberField) + if !d.IsNewResource() && o != nil { + // set.delete() + for _, i := range o.(*schema.Set).List() { + delete(set, i) + } + } + + if ids, ok := d.GetOk(memberField); ok { + for _, id := range ids.(*schema.Set).List() { + // set.add() + set[id] = true + } + } + + // set.keys() + var result []interface{} + for k := range set { + result = append(result, k) + } + data[memberField] = result + } + } + + return data, nil +} + +func SetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) error { + curIDS := resp.Data[memberField] + if d.Get("exclusive").(bool) { + if err := d.Set(memberField, curIDS); err != nil { + return err + } + } else { + set := map[interface{}]bool{} + if curIDS != nil { + for _, v := range curIDS.([]interface{}) { + set[v] = true + } + } + + var result []interface{} + // set.intersection() + if i, ok := d.GetOk(memberField); ok { + for _, v := range i.(*schema.Set).List() { + if _, ok := set[v]; ok { + result = append(result, v) + } + } + } + if err := d.Set(memberField, result); err != nil { + return err + } + } + + return nil +} + +func DeleteGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) (map[string]interface{}, error) { + data := map[string]interface{}{} + + switch memberField { + case "member_group_ids", "member_entity_ids": + default: + return nil, fmt.Errorf("invalid value for member field") + } + + t, ok := resp.Data["type"] + if ok && t != "external" { + if d.Get("exclusive").(bool) { + data[memberField] = make([]string, 0) + } else { + set := map[interface{}]bool{} + if v, ok := resp.Data[memberField]; ok && v != nil { + for _, id := range v.([]interface{}) { + set[id] = true + } + } + + result := []interface{}{} + if len(set) > 0 { + if v, ok := d.GetOk(memberField); ok { + for _, id := range v.(*schema.Set).List() { + delete(set, id) + } + } + + for k := range set { + result = append(result, k) + } + } + data[memberField] = result + } + } + + return data, nil +} + +// needed for testing +type GroupMemberTester struct { + EntityIDS []string + GroupIDS []string +} + +func (r *GroupMemberTester) SetMemberEntities(resource string) resource.TestCheckFunc { + return func(s *terraform.State) error { + result, err := r.getGroupMemberResourceData(s, resource, "member_entity_ids") + if err != nil { + return err + } + r.EntityIDS = result + return nil + } +} + +func (r *GroupMemberTester) SetMemberGroups(resource string) resource.TestCheckFunc { + return func(s *terraform.State) error { + result, err := r.getGroupMemberResourceData(s, resource, "member_group_ids") + if err != nil { + return err + } + r.GroupIDS = result + return nil + } +} + +func (r *GroupMemberTester) getGroupMemberResourceData(s *terraform.State, resource, memberField string) ([]string, error) { + var result []string + resourceState := s.Modules[0].Resources[resource] + if resourceState == nil { + return result, fmt.Errorf("resource not found in state") + } + + instanceState := resourceState.Primary + if instanceState == nil { + return result, fmt.Errorf("resource not found in state") + } + + count, err := strconv.Atoi(instanceState.Attributes[fmt.Sprintf("%s.#", memberField)]) + if err != nil { + return nil, err + } + + for i := 0; i < count; i++ { + k := fmt.Sprintf("%s.%d", memberField, i) + result = append(result, instanceState.Attributes[k]) + } + + return result, nil +} + +func (r *GroupMemberTester) CheckMemberEntities(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + for i, v := range r.EntityIDS { + k := fmt.Sprintf("member_entity_ids.%d", i) + f := resource.TestCheckResourceAttr(resourceName, k, v) + if err := f(s); err != nil { + return err + } + } + return nil + } +} + +func (r *GroupMemberTester) CheckMemberGroups(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + for i, v := range r.GroupIDS { + k := fmt.Sprintf("member_group_ids.%d", i) + f := resource.TestCheckResourceAttr(resourceName, k, v) + if err := f(s); err != nil { + return err + } + } + return nil + } +} diff --git a/vault/provider.go b/vault/provider.go index 5d8ca6d34..2b6d4c448 100644 --- a/vault/provider.go +++ b/vault/provider.go @@ -615,6 +615,10 @@ var ( Resource: UpdateSchemaResource(identityGroupMemberEntityIdsResource()), PathInventory: []string{"/identity/group/id/{id}"}, }, + "vault_identity_group_member_group_ids": { + Resource: UpdateSchemaResource(identityGroupMemberGroupIdsResource()), + PathInventory: []string{"/identity/group/id/{id}"}, + }, "vault_identity_group_policies": { Resource: UpdateSchemaResource(identityGroupPoliciesResource()), PathInventory: []string{"/identity/lookup/group"}, diff --git a/vault/resource_identity_group.go b/vault/resource_identity_group.go index d57428acd..6e35e52ea 100644 --- a/vault/resource_identity_group.go +++ b/vault/resource_identity_group.go @@ -75,7 +75,7 @@ func identityGroupResource() *schema.Resource { // 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" { + if d.Get("type").(string) == "external" || d.Get("external_member_group_ids").(bool) == true { return true } return false @@ -102,7 +102,14 @@ func identityGroupResource() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: false, - Description: "Manage member entities externally through `vault_identity_group_policies_member_entity_ids`", + Description: "Manage member entities externally through `vault_identity_group_member_entity_ids`", + }, + + "external_member_group_ids": { + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "Manage member groups externally through `vault_identity_group_member_group_ids`", }, }, } @@ -120,11 +127,13 @@ func identityGroupUpdateFields(d *schema.ResourceData, data map[string]interface // Member groups and entities can't be set for external groups if d.Get("type").(string) == "internal" { - data["member_group_ids"] = d.Get("member_group_ids").(*schema.Set).List() - if externalMemberEntityIds, ok := d.GetOk("external_member_entity_ids"); !(ok && externalMemberEntityIds.(bool)) { data["member_entity_ids"] = d.Get("member_entity_ids").(*schema.Set).List() } + + if externalMemberGroupIds, ok := d.GetOk("external_member_group_ids"); !(ok && externalMemberGroupIds.(bool)) { + data["member_group_ids"] = d.Get("member_group_ids").(*schema.Set).List() + } } if metadata, ok := d.GetOk(consts.FieldMetadata); ok { @@ -137,10 +146,13 @@ func identityGroupUpdateFields(d *schema.ResourceData, data map[string]interface data["policies"] = d.Get("policies").(*schema.Set).List() // Member groups and entities can't be set for external groups if d.Get("type").(string) == "internal" { - data["member_group_ids"] = d.Get("member_group_ids").(*schema.Set).List() if !d.Get("external_member_entity_ids").(bool) { data["member_entity_ids"] = d.Get("member_entity_ids").(*schema.Set).List() } + + if !d.Get("external_member_group_ids").(bool) { + data["member_group_ids"] = d.Get("member_group_ids").(*schema.Set).List() + } } // Edge case where if external_policies is true, no policies diff --git a/vault/resource_identity_group_member_entity_ids.go b/vault/resource_identity_group_member_entity_ids.go index ccf4afd91..20ddc5c31 100644 --- a/vault/resource_identity_group_member_entity_ids.go +++ b/vault/resource_identity_group_member_entity_ids.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" ) @@ -66,48 +67,14 @@ func identityGroupMemberEntityIdsUpdate(d *schema.ResourceData, meta interface{} o, n := d.GetChange("group_id") log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) } - data := make(map[string]interface{}) resp, err := readIdentityGroup(client, gid, d.IsNewResource()) if err != nil { return err } - var curIDS []interface{} - if t, ok := resp.Data["type"]; ok && t.(string) != "external" { - if v, ok := resp.Data["member_entity_ids"]; ok && v != nil { - curIDS = v.([]interface{}) - } - - if d.Get("exclusive").(bool) || len(curIDS) == 0 { - data["member_entity_ids"] = d.Get("member_entity_ids").(*schema.Set).List() - } else { - set := map[interface{}]bool{} - for _, v := range curIDS { - set[v] = true - } - - o, _ := d.GetChange("member_entity_ids") - if !d.IsNewResource() && o != nil { - // set.delete() - for _, i := range o.(*schema.Set).List() { - delete(set, i) - } - } - - if ids, ok := d.GetOk("member_entity_ids"); ok { - for _, id := range ids.(*schema.Set).List() { - // set.add() - set[id] = true - } - } - - // set.keys() - var result []interface{} - for k := range set { - result = append(result, k) - } - data["member_entity_ids"] = result - } + data, err := group.GetGroupMember(d, resp, "member_entity_ids") + if err != nil { + return err } _, err = client.Logical().Write(path, data) @@ -147,32 +114,10 @@ func identityGroupMemberEntityIdsRead(d *schema.ResourceData, meta interface{}) return err } - curIDS := resp.Data["member_entity_ids"] - if d.Get("exclusive").(bool) { - if err = d.Set("member_entity_ids", curIDS); err != nil { - return err - } - } else { - set := map[interface{}]bool{} - if curIDS != nil { - for _, v := range curIDS.([]interface{}) { - set[v] = true - } - } - - var result []interface{} - // set.intersection() - if i, ok := d.GetOk("member_entity_ids"); ok { - for _, v := range i.(*schema.Set).List() { - if _, ok := set[v]; ok { - result = append(result, v) - } - } - } - if err = d.Set("member_entity_ids", result); err != nil { - return err - } + if err := group.SetGroupMember(d, resp, "member_entity_ids"); err != nil { + return err } + return nil } @@ -189,8 +134,6 @@ func identityGroupMemberEntityIdsDelete(d *schema.ResourceData, meta interface{} log.Printf("[DEBUG] Deleting IdentityGroupMemberEntityIds %q", id) - data := make(map[string]interface{}) - resp, err := readIdentityGroup(client, id, false) if err != nil { if isIdentityNotFoundError(err) { @@ -199,32 +142,9 @@ func identityGroupMemberEntityIdsDelete(d *schema.ResourceData, meta interface{} return err } - t, ok := resp.Data["type"] - if ok && t != "external" { - if d.Get("exclusive").(bool) { - data["member_entity_ids"] = make([]string, 0) - } else { - set := map[interface{}]bool{} - if v, ok := resp.Data["member_entity_ids"]; ok && v != nil { - for _, id := range v.([]interface{}) { - set[id] = true - } - } - - result := []interface{}{} - if len(set) > 0 { - if v, ok := d.GetOk("member_entity_ids"); ok { - for _, id := range v.(*schema.Set).List() { - delete(set, id) - } - } - - for k := range set { - result = append(result, k) - } - } - data["member_entity_ids"] = result - } + data, err := group.DeleteGroupMember(d, resp, "member_entity_ids") + if err != nil { + return err } _, err = client.Logical().Write(path, data) diff --git a/vault/resource_identity_group_member_entity_ids_test.go b/vault/resource_identity_group_member_entity_ids_test.go index 4559516b5..50fe5774b 100644 --- a/vault/resource_identity_group_member_entity_ids_test.go +++ b/vault/resource_identity_group_member_entity_ids_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/testutil" "github.com/hashicorp/terraform-provider-vault/util" @@ -76,7 +77,7 @@ func TestAccIdentityGroupMemberEntityIdsExclusive(t *testing.T) { func TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty(t *testing.T) { devEntity := acctest.RandomWithPrefix("dev-entity") testEntity := acctest.RandomWithPrefix("test-entity") - var devEntityTester memberEntityTester + var devEntityTester group.GroupMemberTester resourceNameDev := "vault_identity_group_member_entity_ids.dev" resourceNameTest := "vault_identity_group_member_entity_ids.test" resource.Test(t, resource.TestCase{ @@ -125,14 +126,14 @@ type identityGMETest struct { } func TestAccIdentityGroupMemberEntityIdsNonExclusive(t *testing.T) { - var tester1 memberEntityTester + var tester1 group.GroupMemberTester entity1 := acctest.RandomWithPrefix("entity") entity2 := acctest.RandomWithPrefix("entity") - var tester2 memberEntityTester + var tester2 group.GroupMemberTester entity3 := acctest.RandomWithPrefix("entity") - var tester3 memberEntityTester + var tester3 group.GroupMemberTester resourceNameDev := "vault_identity_group_member_entity_ids.dev" resourceNameTest := "vault_identity_group_member_entity_ids.test" @@ -297,59 +298,6 @@ resource "vault_identity_group_member_entity_ids" "%s" { return config } -type memberEntityTester struct { - EntityIDS []string -} - -func (r *memberEntityTester) SetMemberEntities(resource string) resource.TestCheckFunc { - return func(s *terraform.State) error { - result, err := r.getMemberEntities(s, resource) - if err != nil { - return err - } - r.EntityIDS = result - return nil - } -} - -func (r *memberEntityTester) getMemberEntities(s *terraform.State, resource string) ([]string, error) { - var result []string - resourceState := s.Modules[0].Resources[resource] - if resourceState == nil { - return result, fmt.Errorf("resource not found in state") - } - - instanceState := resourceState.Primary - if instanceState == nil { - return result, fmt.Errorf("resource not found in state") - } - - count, err := strconv.Atoi(instanceState.Attributes["member_entity_ids.#"]) - if err != nil { - return nil, err - } - - for i := 0; i < count; i++ { - k := fmt.Sprintf("member_entity_ids.%d", i) - result = append(result, instanceState.Attributes[k]) - } - - return result, nil -} - -func (r *memberEntityTester) CheckMemberEntities(resourceName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - for i, v := range r.EntityIDS { - k := fmt.Sprintf("member_entity_ids.%d", i) - f := resource.TestCheckResourceAttr(resourceName, k, v) - if err := f(s); err != nil { - return err - } - } - return nil - } -} - func testAccCheckidentityGroupMemberEntityIdsDestroy(s *terraform.State) error { for _, rs := range s.RootModule().Resources { if rs.Type != "vault_identity_group_member_entity_ids" { diff --git a/vault/resource_identity_group_member_group_ids.go b/vault/resource_identity_group_member_group_ids.go new file mode 100644 index 000000000..e31ec6904 --- /dev/null +++ b/vault/resource_identity_group_member_group_ids.go @@ -0,0 +1,148 @@ +package vault + +import ( + "fmt" + "log" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" + "github.com/hashicorp/terraform-provider-vault/internal/provider" +) + +func identityGroupMemberGroupIdsResource() *schema.Resource { + return &schema.Resource{ + Create: identityGroupMemberGroupIdsUpdate, + Update: identityGroupMemberGroupIdsUpdate, + Read: ReadWrapper(identityGroupMemberGroupIdsRead), + Delete: identityGroupMemberGroupIdsDelete, + + Schema: map[string]*schema.Schema{ + "member_group_ids": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Description: "Group IDs to be assigned as group members.", + }, + "exclusive": { + Type: schema.TypeBool, + Optional: true, + Default: true, + Description: `Should the resource manage member group ids +exclusively? Beware of race conditions when disabling exclusive management`, + }, + "group_id": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "ID of the group.", + }, + }, + } +} + +func identityGroupMemberGroupIdsUpdate(d *schema.ResourceData, meta interface{}) error { + gid := d.Get("group_id").(string) + path := identityGroupIDPath(gid) + vaultMutexKV.Lock(path) + defer vaultMutexKV.Unlock(path) + + client, e := provider.GetClient(d, meta) + if e != nil { + return e + } + + log.Printf("[DEBUG] Updating IdentityGroupMemberGroupIds %q", gid) + + if d.HasChange("group_id") { + o, n := d.GetChange("group_id") + log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) + } + + resp, err := readIdentityGroup(client, gid, d.IsNewResource()) + if err != nil { + return err + } + + data, err := group.GetGroupMember(d, resp, "member_group_ids") + if err != nil { + return err + } + + _, err = client.Logical().Write(path, data) + if err != nil { + return fmt.Errorf("error updating IdentityGroupMemberGroupIds %q: %s", gid, err) + } + log.Printf("[DEBUG] Updated IdentityGroupMemberGroupIds %q", gid) + + d.SetId(gid) + + return identityGroupMemberGroupIdsRead(d, meta) +} + +func identityGroupMemberGroupIdsRead(d *schema.ResourceData, meta interface{}) error { + client, e := provider.GetClient(d, meta) + if e != nil { + return e + } + + id := d.Id() + + log.Printf("[DEBUG] Read IdentityGroupMemberGroupIds %s", id) + resp, err := readIdentityGroup(client, id, d.IsNewResource()) + if err != nil { + if isIdentityNotFoundError(err) { + log.Printf("[WARN] IdentityGroupMemberGroupIds %q not found, removing from state", id) + d.SetId("") + return nil + } + return err + } + + if err := d.Set("group_id", id); err != nil { + return err + } + + if err := group.SetGroupMember(d, resp, "member_group_ids"); err != nil { + return err + } + + return nil +} + +func identityGroupMemberGroupIdsDelete(d *schema.ResourceData, meta interface{}) error { + id := d.Get("group_id").(string) + path := identityGroupIDPath(id) + vaultMutexKV.Lock(path) + defer vaultMutexKV.Unlock(path) + + client, e := provider.GetClient(d, meta) + if e != nil { + return e + } + + log.Printf("[DEBUG] Deleting IdentityGroupMemberGroupIds %q", id) + + resp, err := readIdentityGroup(client, id, false) + if err != nil { + if isIdentityNotFoundError(err) { + return nil + } + return err + } + + data, err := group.DeleteGroupMember(d, resp, "member_group_ids") + if err != nil { + return err + } + + _, err = client.Logical().Write(path, data) + if err != nil { + return fmt.Errorf("error updating IdentityGroupMemberGroupIds %q: %s", id, err) + } + log.Printf("[DEBUG] Updated IdentityGroupMemberGroupIds %q", id) + + return nil +} diff --git a/vault/resource_identity_group_member_group_ids_test.go b/vault/resource_identity_group_member_group_ids_test.go new file mode 100644 index 000000000..b598677f6 --- /dev/null +++ b/vault/resource_identity_group_member_group_ids_test.go @@ -0,0 +1,130 @@ +package vault + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" + "github.com/hashicorp/terraform-provider-vault/testutil" +) + +func TestAccIdentityGroupMemberGroupIdsNonExclusive(t *testing.T) { + group1 := acctest.RandomWithPrefix("group") + var tester1 group.GroupMemberTester + + group2 := acctest.RandomWithPrefix("group") + var tester2 group.GroupMemberTester + + group3 := acctest.RandomWithPrefix("group") + var tester3 group.GroupMemberTester + + resourceNameDev := "vault_identity_group_member_group_ids.dev" + resourceNameTest := "vault_identity_group_member_group_ids.test" + resource.Test(t, resource.TestCase{ + PreCheck: func() { testutil.TestAccPreCheck(t) }, + Providers: testProviders, + Steps: []resource.TestStep{ + { + Config: testAccIdentityGroupMemberGroupIdsConfigNonExclusive(group1, group2), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceNameDev, "member_group_ids.#", "1"), + tester1.SetMemberGroups(resourceNameDev), + resource.TestCheckResourceAttr(resourceNameTest, "member_group_ids.#", "1"), + tester2.SetMemberGroups(resourceNameTest), + ), + }, + { + Config: testAccIdentityGroupMemberGroupIdsConfigNonExclusiveUpdate(group1, group3), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceNameDev, "member_group_ids.#", "1"), + tester1.CheckMemberGroups(resourceNameDev), + resource.TestCheckResourceAttr(resourceNameTest, "member_group_ids.#", "1"), + tester2.SetMemberGroups(resourceNameTest), + tester3.SetMemberGroups(resourceNameTest), + ), + }, + { + Config: testAccIdentityGroupMemberGroupIdsConfigNonExclusiveUpdate(group1, group3), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceNameDev, "member_group_ids.#", "1"), + tester1.CheckMemberGroups(resourceNameDev), + resource.TestCheckResourceAttr(resourceNameTest, "member_group_ids.#", "1"), + tester2.CheckMemberGroups(resourceNameTest), + tester3.CheckMemberGroups(resourceNameTest), + ), + }, + }, + }) +} + +func testAccIdentityGroupMemberGroupIdsConfigNonExclusive(devGroupName, testGroupName string) string { + return fmt.Sprintf(` +resource "vault_identity_group" "group" { + external_member_group_ids = true +} + +resource "vault_identity_group" "dev_group" { + name = "%s" + metadata = { + version = "2" + } +} + +resource "vault_identity_group" "test_group" { + name = "%s" + metadata = { + version = "2" + } +} + +resource "vault_identity_group_member_group_ids" "dev" { + group_id = vault_identity_group.group.id + exclusive = false + member_group_ids = [vault_identity_group.dev_group.id] +} + + +resource "vault_identity_group_member_group_ids" "test" { + group_id = vault_identity_group.group.id + exclusive = false + member_group_ids = [vault_identity_group.test_group.id] +} +`, devGroupName, testGroupName) +} + +func testAccIdentityGroupMemberGroupIdsConfigNonExclusiveUpdate(devGroupName, fooGroupName string) string { + return fmt.Sprintf(` +resource "vault_identity_group" "group" { + external_member_group_ids = true +} + +resource "vault_identity_group" "dev_group" { + name = "%s" + metadata = { + version = "2" + } +} + +resource "vault_identity_group" "foo_group" { + name = "%s" + metadata = { + version = "2" + } +} + +resource "vault_identity_group_member_group_ids" "dev" { + group_id = vault_identity_group.group.id + exclusive = false + member_group_ids = [vault_identity_group.dev_group.id] +} + +resource "vault_identity_group_member_group_ids" "test" { + group_id = vault_identity_group.group.id + exclusive = false + member_group_ids = [vault_identity_group.foo_group.id] +} +`, devGroupName, fooGroupName) +} From 5811303ee8dc5008726e0838917254ea4003d56c Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Tue, 11 Oct 2022 09:57:25 -0700 Subject: [PATCH 2/7] add new constants for fields --- internal/consts/consts.go | 5 ++++ ...source_identity_group_member_entity_ids.go | 27 ++++++++++--------- ...esource_identity_group_member_group_ids.go | 23 ++++++++-------- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/internal/consts/consts.go b/internal/consts/consts.go index 5343c4a4d..359328626 100644 --- a/internal/consts/consts.go +++ b/internal/consts/consts.go @@ -124,6 +124,11 @@ const ( FieldResourceGroupName = "resource_group_name" FieldVMName = "vm_name" FieldVMSSName = "vmss_name" + FieldMemberEntityIDs = "member_entity_ids" + FieldMemberGroupIDs = "member_group_ids" + FieldExclusive = "exclusive" + FieldGroupID = "group_id" + FieldGroupName = "group_name" /* common environment variables diff --git a/vault/resource_identity_group_member_entity_ids.go b/vault/resource_identity_group_member_entity_ids.go index 20ddc5c31..37e425bc6 100644 --- a/vault/resource_identity_group_member_entity_ids.go +++ b/vault/resource_identity_group_member_entity_ids.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" ) @@ -18,7 +19,7 @@ func identityGroupMemberEntityIdsResource() *schema.Resource { Delete: identityGroupMemberEntityIdsDelete, Schema: map[string]*schema.Schema{ - "member_entity_ids": { + consts.FieldMemberEntityIDs: { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{ @@ -26,20 +27,20 @@ func identityGroupMemberEntityIdsResource() *schema.Resource { }, Description: "Entity IDs to be assigned as group members.", }, - "exclusive": { + consts.FieldExclusive: { Type: schema.TypeBool, Optional: true, Default: true, Description: `Should the resource manage member entity ids exclusively? Beware of race conditions when disabling exclusive management`, }, - "group_id": { + consts.FieldGroupID: { Type: schema.TypeString, Required: true, ForceNew: true, Description: "ID of the group.", }, - "group_name": { + consts.FieldGroupName: { Type: schema.TypeString, Computed: true, Description: "Name of the group.", @@ -51,7 +52,7 @@ use "data.vault_identity_group.*.group_name", "vault_identity_group.*.group_name } func identityGroupMemberEntityIdsUpdate(d *schema.ResourceData, meta interface{}) error { - gid := d.Get("group_id").(string) + gid := d.Get(consts.FieldGroupID).(string) path := identityGroupIDPath(gid) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -63,8 +64,8 @@ func identityGroupMemberEntityIdsUpdate(d *schema.ResourceData, meta interface{} log.Printf("[DEBUG] Updating IdentityGroupMemberEntityIds %q", gid) - if d.HasChange("group_id") { - o, n := d.GetChange("group_id") + if d.HasChange(consts.FieldGroupID) { + o, n := d.GetChange(consts.FieldGroupID) log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) } resp, err := readIdentityGroup(client, gid, d.IsNewResource()) @@ -72,7 +73,7 @@ func identityGroupMemberEntityIdsUpdate(d *schema.ResourceData, meta interface{} return err } - data, err := group.GetGroupMember(d, resp, "member_entity_ids") + data, err := group.GetGroupMember(d, resp, consts.FieldMemberEntityIDs) if err != nil { return err } @@ -107,14 +108,14 @@ func identityGroupMemberEntityIdsRead(d *schema.ResourceData, meta interface{}) return err } - if err := d.Set("group_id", id); err != nil { + if err := d.Set(consts.FieldGroupID, id); err != nil { return err } - if err := d.Set("group_name", resp.Data["name"]); err != nil { + if err := d.Set(consts.FieldGroupName, resp.Data["name"]); err != nil { return err } - if err := group.SetGroupMember(d, resp, "member_entity_ids"); err != nil { + if err := group.SetGroupMember(d, resp, consts.FieldMemberEntityIDs); err != nil { return err } @@ -122,7 +123,7 @@ func identityGroupMemberEntityIdsRead(d *schema.ResourceData, meta interface{}) } func identityGroupMemberEntityIdsDelete(d *schema.ResourceData, meta interface{}) error { - id := d.Get("group_id").(string) + id := d.Get(consts.FieldGroupID).(string) path := identityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -142,7 +143,7 @@ func identityGroupMemberEntityIdsDelete(d *schema.ResourceData, meta interface{} return err } - data, err := group.DeleteGroupMember(d, resp, "member_entity_ids") + data, err := group.DeleteGroupMember(d, resp, consts.FieldMemberEntityIDs) if err != nil { return err } diff --git a/vault/resource_identity_group_member_group_ids.go b/vault/resource_identity_group_member_group_ids.go index e31ec6904..39b68767d 100644 --- a/vault/resource_identity_group_member_group_ids.go +++ b/vault/resource_identity_group_member_group_ids.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" ) @@ -18,7 +19,7 @@ func identityGroupMemberGroupIdsResource() *schema.Resource { Delete: identityGroupMemberGroupIdsDelete, Schema: map[string]*schema.Schema{ - "member_group_ids": { + consts.FieldMemberGroupIDs: { Type: schema.TypeSet, Optional: true, Elem: &schema.Schema{ @@ -26,14 +27,14 @@ func identityGroupMemberGroupIdsResource() *schema.Resource { }, Description: "Group IDs to be assigned as group members.", }, - "exclusive": { + consts.FieldExclusive: { Type: schema.TypeBool, Optional: true, Default: true, Description: `Should the resource manage member group ids exclusively? Beware of race conditions when disabling exclusive management`, }, - "group_id": { + consts.FieldGroupID: { Type: schema.TypeString, Required: true, ForceNew: true, @@ -44,7 +45,7 @@ exclusively? Beware of race conditions when disabling exclusive management`, } func identityGroupMemberGroupIdsUpdate(d *schema.ResourceData, meta interface{}) error { - gid := d.Get("group_id").(string) + gid := d.Get(consts.FieldGroupID).(string) path := identityGroupIDPath(gid) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -56,8 +57,8 @@ func identityGroupMemberGroupIdsUpdate(d *schema.ResourceData, meta interface{}) log.Printf("[DEBUG] Updating IdentityGroupMemberGroupIds %q", gid) - if d.HasChange("group_id") { - o, n := d.GetChange("group_id") + if d.HasChange(consts.FieldGroupID) { + o, n := d.GetChange(consts.FieldGroupID) log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) } @@ -66,7 +67,7 @@ func identityGroupMemberGroupIdsUpdate(d *schema.ResourceData, meta interface{}) return err } - data, err := group.GetGroupMember(d, resp, "member_group_ids") + data, err := group.GetGroupMember(d, resp, consts.FieldMemberGroupIDs) if err != nil { return err } @@ -101,11 +102,11 @@ func identityGroupMemberGroupIdsRead(d *schema.ResourceData, meta interface{}) e return err } - if err := d.Set("group_id", id); err != nil { + if err := d.Set(consts.FieldGroupID, id); err != nil { return err } - if err := group.SetGroupMember(d, resp, "member_group_ids"); err != nil { + if err := group.SetGroupMember(d, resp, consts.FieldMemberGroupIDs); err != nil { return err } @@ -113,7 +114,7 @@ func identityGroupMemberGroupIdsRead(d *schema.ResourceData, meta interface{}) e } func identityGroupMemberGroupIdsDelete(d *schema.ResourceData, meta interface{}) error { - id := d.Get("group_id").(string) + id := d.Get(consts.FieldGroupID).(string) path := identityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -133,7 +134,7 @@ func identityGroupMemberGroupIdsDelete(d *schema.ResourceData, meta interface{}) return err } - data, err := group.DeleteGroupMember(d, resp, "member_group_ids") + data, err := group.DeleteGroupMember(d, resp, consts.FieldMemberGroupIDs) if err != nil { return err } From f4f6f036c19583daca839e354143f91c6773b27d Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Mon, 17 Oct 2022 10:55:07 -0700 Subject: [PATCH 3/7] factor common context functions for group members --- internal/consts/consts.go | 2 + internal/identity/entity/entity.go | 31 ++++ internal/identity/group/group.go | 138 ++++++++++++++++-- vault/resource_identity_entity.go | 36 +---- vault/resource_identity_entity_alias.go | 5 +- vault/resource_identity_entity_policies.go | 5 +- .../resource_identity_entity_policies_test.go | 3 +- vault/resource_identity_entity_test.go | 19 +-- vault/resource_identity_group.go | 31 ++-- ...source_identity_group_member_entity_ids.go | 107 +++----------- ...e_identity_group_member_entity_ids_test.go | 6 +- ...esource_identity_group_member_group_ids.go | 105 +++---------- vault/resource_identity_group_policies.go | 9 +- .../resource_identity_group_policies_test.go | 7 +- vault/resource_identity_group_test.go | 5 +- 15 files changed, 246 insertions(+), 263 deletions(-) diff --git a/internal/consts/consts.go b/internal/consts/consts.go index 16c6a0117..dc0a6b6da 100644 --- a/internal/consts/consts.go +++ b/internal/consts/consts.go @@ -177,6 +177,8 @@ const ( FieldExclusive = "exclusive" FieldGroupID = "group_id" FieldGroupName = "group_name" + FieldExternal = "external" + FieldInternal = "internal" /* common environment variables diff --git a/internal/identity/entity/entity.go b/internal/identity/entity/entity.go index c8cbb30e7..f1c654363 100644 --- a/internal/identity/entity/entity.go +++ b/internal/identity/entity/entity.go @@ -1,10 +1,15 @@ package entity import ( + "errors" "fmt" + "log" "github.com/hashicorp/vault/api" "github.com/mitchellh/mapstructure" + + "github.com/hashicorp/terraform-provider-vault/internal/provider" + "github.com/hashicorp/terraform-provider-vault/util" ) const ( @@ -15,6 +20,8 @@ const ( LookupPath = "identity/lookup/entity" ) +var ErrEntityNotFound = errors.New("entity not found") + // Entity represents a Vault identity entity type Entity struct { Aliases []*Alias `mapstructure:"aliases" json:"aliases,omitempty"` @@ -152,3 +159,27 @@ func LookupEntityAlias(client *api.Client, params *FindAliasParams) (*Alias, err return nil, nil } + +func ReadEntity(client *api.Client, path string, retry bool) (*api.Secret, error) { + log.Printf("[DEBUG] Reading Entity from %q", path) + + var err error + if retry { + client, err = client.Clone() + if err != nil { + return nil, fmt.Errorf("error cloning client: %w", err) + } + util.SetupCCCRetryClient(client, provider.MaxHTTPRetriesCCC) + } + + resp, err := client.Logical().Read(path) + if err != nil { + return resp, fmt.Errorf("failed reading %q", path) + } + + if resp == nil { + return nil, fmt.Errorf("%w: %q", ErrEntityNotFound, path) + } + + return resp, nil +} diff --git a/internal/identity/group/group.go b/internal/identity/group/group.go index 34fbcf2ca..e2b676b46 100644 --- a/internal/identity/group/group.go +++ b/internal/identity/group/group.go @@ -1,34 +1,148 @@ package group import ( + "errors" "fmt" + "log" "strconv" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/vault/api" + + "github.com/hashicorp/terraform-provider-vault/internal/consts" + "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" ) const ( - LookupPath = "identity/lookup/group" + LookupPath = "identity/lookup/group" + IdentityGroupPath = "/identity/group" ) +func IdentityGroupIDPath(id string) string { + return fmt.Sprintf("%s/id/%s", IdentityGroupPath, id) +} + +// This function may return `nil` for the IdentityGroup if it does not exist +func ReadIdentityGroup(client *api.Client, groupID string, retry bool) (*api.Secret, error) { + path := IdentityGroupIDPath(groupID) + log.Printf("[DEBUG] Reading IdentityGroup %s from %q", groupID, path) + + return entity.ReadEntity(client, path, retry) +} + +func IsIdentityNotFoundError(err error) bool { + return err != nil && errors.Is(err, entity.ErrEntityNotFound) +} + +func UpdateGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { + gid := d.Get(consts.FieldGroupID).(string) + path := IdentityGroupIDPath(gid) + + log.Printf("[DEBUG] Updating field %q on Identity Group %q", memberField, gid) + + if d.HasChange(consts.FieldGroupID) { + o, n := d.GetChange(consts.FieldGroupID) + log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) + } + + resp, err := ReadIdentityGroup(client, gid, d.IsNewResource()) + if err != nil { + return diag.FromErr(err) + } + + data, err := GetGroupMember(d, resp, memberField) + if err != nil { + return diag.FromErr(err) + } + + _, err = client.Logical().Write(path, data) + if err != nil { + return diag.Errorf("error updating field %q on Identity Group %s: err=%s", memberField, gid, err) + } + log.Printf("[DEBUG] Updated field %q on Identity Group %s", memberField, gid) + + d.SetId(gid) + + return nil +} + +func ReadGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string, setGroupName bool) diag.Diagnostics { + id := d.Id() + + log.Printf("[DEBUG] Reading Identity Group %s with field %q", id, memberField) + resp, err := ReadIdentityGroup(client, id, d.IsNewResource()) + if err != nil { + if IsIdentityNotFoundError(err) { + log.Printf("[WARN] Identity Group %s not found, removing from state", id) + d.SetId("") + return nil + } + return diag.FromErr(err) + } + + if err := d.Set(consts.FieldGroupID, id); err != nil { + return diag.FromErr(err) + } + + if setGroupName { + if err := d.Set(consts.FieldGroupName, resp.Data[consts.FieldName]); err != nil { + return diag.FromErr(err) + } + } + + if err := SetGroupMember(d, resp, memberField); err != nil { + return diag.FromErr(err) + } + + return nil +} + +func DeleteGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { + id := d.Get(consts.FieldGroupID).(string) + path := IdentityGroupIDPath(id) + + log.Printf("[DEBUG] Deleting Identity Group %q with field %q", memberField, id) + + resp, err := ReadIdentityGroup(client, id, false) + if err != nil { + if IsIdentityNotFoundError(err) { + return nil + } + return diag.FromErr(err) + } + + data, err := DeleteGroupMember(d, resp, memberField) + if err != nil { + return diag.FromErr(err) + } + + _, err = client.Logical().Write(path, data) + if err != nil { + return diag.Errorf("error deleting Identity Group %q with field %q; err=%s", id, memberField, err) + } + log.Printf("[DEBUG] Deleted Identity Group %q with field %q", memberField, id) + + return nil +} + func GetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) (map[string]interface{}, error) { data := map[string]interface{}{} switch memberField { - case "member_group_ids", "member_entity_ids": + case consts.FieldMemberGroupIDs, consts.FieldMemberEntityIDs: default: return nil, fmt.Errorf("invalid value for member field") } var curIDS []interface{} - if t, ok := resp.Data["type"]; ok && t.(string) != "external" { + if t, ok := resp.Data[consts.FieldType]; ok && t.(string) != consts.FieldExternal { if v, ok := resp.Data[memberField]; ok && v != nil { curIDS = v.([]interface{}) } - if d.Get("exclusive").(bool) || len(curIDS) == 0 { + if d.Get(consts.FieldExclusive).(bool) || len(curIDS) == 0 { data[memberField] = d.Get(memberField).(*schema.Set).List() } else { set := map[interface{}]bool{} @@ -65,7 +179,7 @@ func GetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string func SetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) error { curIDS := resp.Data[memberField] - if d.Get("exclusive").(bool) { + if d.Get(consts.FieldExclusive).(bool) { if err := d.Set(memberField, curIDS); err != nil { return err } @@ -79,7 +193,7 @@ func SetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string var result []interface{} // set.intersection() - if i, ok := d.GetOk(memberField); ok { + if i, ok := d.GetOk(memberField); ok && i != nil { for _, v := range i.(*schema.Set).List() { if _, ok := set[v]; ok { result = append(result, v) @@ -98,14 +212,14 @@ func DeleteGroupMember(d *schema.ResourceData, resp *api.Secret, memberField str data := map[string]interface{}{} switch memberField { - case "member_group_ids", "member_entity_ids": + case consts.FieldMemberGroupIDs, consts.FieldMemberEntityIDs: default: return nil, fmt.Errorf("invalid value for member field") } - t, ok := resp.Data["type"] - if ok && t != "external" { - if d.Get("exclusive").(bool) { + t, ok := resp.Data[consts.FieldType] + if ok && t != consts.FieldExternal { + if d.Get(consts.FieldExclusive).(bool) { data[memberField] = make([]string, 0) } else { set := map[interface{}]bool{} @@ -142,7 +256,7 @@ type GroupMemberTester struct { func (r *GroupMemberTester) SetMemberEntities(resource string) resource.TestCheckFunc { return func(s *terraform.State) error { - result, err := r.getGroupMemberResourceData(s, resource, "member_entity_ids") + result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberEntityIDs) if err != nil { return err } @@ -153,7 +267,7 @@ func (r *GroupMemberTester) SetMemberEntities(resource string) resource.TestChec func (r *GroupMemberTester) SetMemberGroups(resource string) resource.TestCheckFunc { return func(s *terraform.State) error { - result, err := r.getGroupMemberResourceData(s, resource, "member_group_ids") + result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberGroupIDs) if err != nil { return err } diff --git a/vault/resource_identity_entity.go b/vault/resource_identity_entity.go index 4e97461e1..79f34724e 100644 --- a/vault/resource_identity_entity.go +++ b/vault/resource_identity_entity.go @@ -1,7 +1,6 @@ package vault import ( - "errors" "fmt" "log" @@ -10,12 +9,11 @@ import ( "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/util" ) -var errEntityNotFound = errors.New("entity not found") - func identityEntityResource() *schema.Resource { return &schema.Resource{ Create: identityEntityCreate, @@ -190,7 +188,7 @@ func identityEntityRead(d *schema.ResourceData, meta interface{}) error { return nil } - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { log.Printf("[WARN] IdentityEntity %q not found, removing from state", id) d.SetId("") return nil @@ -276,33 +274,5 @@ func readIdentityEntity(client *api.Client, entityID string, retry bool) (*api.S path := entity.JoinEntityID(entityID) log.Printf("[DEBUG] Reading Entity %q from %q", entityID, path) - return readEntity(client, path, retry) -} - -func readEntity(client *api.Client, path string, retry bool) (*api.Secret, error) { - log.Printf("[DEBUG] Reading Entity from %q", path) - - var err error - if retry { - client, err = client.Clone() - if err != nil { - return nil, fmt.Errorf("error cloning client: %w", err) - } - util.SetupCCCRetryClient(client, provider.MaxHTTPRetriesCCC) - } - - resp, err := client.Logical().Read(path) - if err != nil { - return resp, fmt.Errorf("failed reading %q", path) - } - - if resp == nil { - return nil, fmt.Errorf("%w: %q", errEntityNotFound, path) - } - - return resp, nil -} - -func isIdentityNotFoundError(err error) bool { - return err != nil && errors.Is(err, errEntityNotFound) + return entity.ReadEntity(client, path, retry) } diff --git a/vault/resource_identity_entity_alias.go b/vault/resource_identity_entity_alias.go index 0dafe8eb9..cea70a322 100644 --- a/vault/resource_identity_entity_alias.go +++ b/vault/resource_identity_entity_alias.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/util" ) @@ -184,9 +185,9 @@ func identityEntityAliasRead(ctx context.Context, d *schema.ResourceData, meta i diags := diag.Diagnostics{} log.Printf("[DEBUG] Reading entity alias %q from %q", id, path) - resp, err := readEntity(client, path, d.IsNewResource()) + resp, err := entity.ReadEntity(client, path, d.IsNewResource()) if err != nil { - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { log.Printf("[WARN] entity alias %q not found, removing from state", id) d.SetId("") return diags diff --git a/vault/resource_identity_entity_policies.go b/vault/resource_identity_entity_policies.go index 338a3d030..08a304909 100644 --- a/vault/resource_identity_entity_policies.go +++ b/vault/resource_identity_entity_policies.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/util" ) @@ -109,7 +110,7 @@ func identityEntityPoliciesRead(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] Read IdentityEntityPolicies %s", id) resp, err := readIdentityEntity(client, id, d.IsNewResource()) if err != nil { - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { log.Printf("[WARN] IdentityEntityPolicies %q not found, removing from state", id) d.SetId("") return nil @@ -162,7 +163,7 @@ func identityEntityPoliciesDelete(d *schema.ResourceData, meta interface{}) erro } else { apiPolicies, err := readIdentityEntityPolicies(client, id) if err != nil { - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { return nil } return err diff --git a/vault/resource_identity_entity_policies_test.go b/vault/resource_identity_entity_policies_test.go index bd1925d06..1fa24244b 100644 --- a/vault/resource_identity_entity_policies_test.go +++ b/vault/resource_identity_entity_policies_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/testutil" "github.com/hashicorp/terraform-provider-vault/util" @@ -81,7 +82,7 @@ func testAccCheckidentityEntityPoliciesDestroy(s *terraform.State) error { } if _, err := readIdentityEntity(client, rs.Primary.ID, false); err != nil { - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { continue } return err diff --git a/vault/resource_identity_entity_test.go b/vault/resource_identity_entity_test.go index b6a21256d..ec895e0d4 100644 --- a/vault/resource_identity_entity_test.go +++ b/vault/resource_identity_entity_test.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/vault/api" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/testutil" ) @@ -260,7 +261,7 @@ func TestReadEntity(t *testing.T) { }, maxRetries: DefaultMaxHTTPRetriesCCC, expectedRetries: DefaultMaxHTTPRetriesCCC, - wantError: fmt.Errorf(`%w: %q`, errEntityNotFound, + wantError: fmt.Errorf(`%w: %q`, entity.ErrEntityNotFound, entity.JoinEntityID("retry-exhausted-default-max-404")), }, { @@ -284,7 +285,7 @@ func TestReadEntity(t *testing.T) { }, maxRetries: 5, expectedRetries: 5, - wantError: fmt.Errorf(`%w: %q`, errEntityNotFound, + wantError: fmt.Errorf(`%w: %q`, entity.ErrEntityNotFound, entity.JoinEntityID("retry-exhausted-custom-max-404")), }, { @@ -324,7 +325,7 @@ func TestReadEntity(t *testing.T) { path = tt.name } - actualResp, err := readEntity(c, path, true) + actualResp, err := entity.ReadEntity(c, path, true) if tt.wantError != nil { if err == nil { @@ -336,8 +337,8 @@ func TestReadEntity(t *testing.T) { } if tt.retryHandler.retryStatus == http.StatusNotFound { - if !isIdentityNotFoundError(err) { - t.Errorf("expected an errEntityNotFound err %q, actual %q", errEntityNotFound, err) + if !group.IsIdentityNotFoundError(err) { + t.Errorf("expected an errEntityNotFound err %q, actual %q", entity.ErrEntityNotFound, err) } } } else { @@ -375,23 +376,23 @@ func TestIsEntityNotFoundError(t *testing.T) { }{ { name: "default", - err: errEntityNotFound, + err: entity.ErrEntityNotFound, expected: true, }, { name: "wrapped", - err: fmt.Errorf("%w: foo", errEntityNotFound), + err: fmt.Errorf("%w: foo", entity.ErrEntityNotFound), expected: true, }, { name: "not", - err: fmt.Errorf("%s: foo", errEntityNotFound), + err: fmt.Errorf("%s: foo", entity.ErrEntityNotFound), expected: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual := isIdentityNotFoundError(tt.err) + actual := group.IsIdentityNotFoundError(tt.err) if actual != tt.expected { t.Fatalf("isIdentityNotFoundError(): expected %v, actual %v", tt.expected, actual) } diff --git a/vault/resource_identity_group.go b/vault/resource_identity_group.go index e8faa4a91..bdac474e3 100644 --- a/vault/resource_identity_group.go +++ b/vault/resource_identity_group.go @@ -9,12 +9,11 @@ import ( "github.com/hashicorp/vault/api" "github.com/hashicorp/terraform-provider-vault/internal/consts" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/util" ) -const identityGroupPath = "/identity/group" - func identityGroupResource() *schema.Resource { return &schema.Resource{ Create: identityGroupCreate, @@ -175,7 +174,7 @@ func identityGroupCreate(d *schema.ResourceData, meta interface{}) error { name := d.Get("name").(string) typeValue := d.Get("type").(string) - path := identityGroupPath + path := group.IdentityGroupPath data := map[string]interface{}{ "type": typeValue, @@ -218,7 +217,7 @@ func identityGroupUpdate(d *schema.ResourceData, meta interface{}) error { id := d.Id() log.Printf("[DEBUG] Updating IdentityGroup %q", id) - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -247,14 +246,14 @@ func identityGroupRead(d *schema.ResourceData, meta interface{}) error { id := d.Id() log.Printf("[DEBUG] Read IdentityGroup %s", id) - resp, err := readIdentityGroup(client, id, d.IsNewResource()) + resp, err := group.ReadIdentityGroup(client, id, d.IsNewResource()) if err != nil { // We need to check if the secret_id has expired if util.IsExpiredTokenErr(err) { return nil } - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { log.Printf("[WARN] IdentityGroup %q not found, removing from state", id) d.SetId("") return nil @@ -280,7 +279,7 @@ func identityGroupDelete(d *schema.ResourceData, meta interface{}) error { id := d.Id() - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -296,15 +295,11 @@ func identityGroupDelete(d *schema.ResourceData, meta interface{}) error { } func identityGroupNamePath(name string) string { - return fmt.Sprintf("%s/name/%s", identityGroupPath, name) -} - -func identityGroupIDPath(id string) string { - return fmt.Sprintf("%s/id/%s", identityGroupPath, id) + return fmt.Sprintf("%s/name/%s", group.IdentityGroupPath, name) } func readIdentityGroupPolicies(client *api.Client, groupID string, retry bool) ([]interface{}, error) { - resp, err := readIdentityGroup(client, groupID, retry) + resp, err := group.ReadIdentityGroup(client, groupID, retry) if err != nil { return nil, err } @@ -316,7 +311,7 @@ func readIdentityGroupPolicies(client *api.Client, groupID string, retry bool) ( } func readIdentityGroupMemberEntityIds(client *api.Client, groupID string, retry bool) ([]interface{}, error) { - resp, err := readIdentityGroup(client, groupID, retry) + resp, err := group.ReadIdentityGroup(client, groupID, retry) if err != nil { return nil, err } @@ -326,11 +321,3 @@ func readIdentityGroupMemberEntityIds(client *api.Client, groupID string, retry } return make([]interface{}, 0), nil } - -// This function may return `nil` for the IdentityGroup if it does not exist -func readIdentityGroup(client *api.Client, groupID string, retry bool) (*api.Secret, error) { - path := identityGroupIDPath(groupID) - log.Printf("[DEBUG] Reading IdentityGroup %s from %q", groupID, path) - - return readEntity(client, path, retry) -} diff --git a/vault/resource_identity_group_member_entity_ids.go b/vault/resource_identity_group_member_entity_ids.go index 37e425bc6..24a4cf65e 100644 --- a/vault/resource_identity_group_member_entity_ids.go +++ b/vault/resource_identity_group_member_entity_ids.go @@ -1,9 +1,9 @@ package vault import ( - "fmt" - "log" + "context" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-vault/internal/consts" @@ -13,10 +13,10 @@ import ( func identityGroupMemberEntityIdsResource() *schema.Resource { return &schema.Resource{ - Create: identityGroupMemberEntityIdsUpdate, - Update: identityGroupMemberEntityIdsUpdate, - Read: ReadWrapper(identityGroupMemberEntityIdsRead), - Delete: identityGroupMemberEntityIdsDelete, + CreateContext: identityGroupMemberEntityIdsUpdate, + UpdateContext: identityGroupMemberEntityIdsUpdate, + ReadContext: ReadContextWrapper(identityGroupMemberEntityIdsRead), + DeleteContext: identityGroupMemberEntityIdsDelete, Schema: map[string]*schema.Schema{ consts.FieldMemberEntityIDs: { @@ -31,8 +31,8 @@ func identityGroupMemberEntityIdsResource() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: true, - Description: `Should the resource manage member entity ids -exclusively? Beware of race conditions when disabling exclusive management`, + Description: `If set to true, allows the resource to manage member entity ids +exclusively. Beware of race conditions when disabling exclusive management`, }, consts.FieldGroupID: { Type: schema.TypeString, @@ -51,108 +51,43 @@ use "data.vault_identity_group.*.group_name", "vault_identity_group.*.group_name } } -func identityGroupMemberEntityIdsUpdate(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberEntityIdsUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { gid := d.Get(consts.FieldGroupID).(string) - path := identityGroupIDPath(gid) + path := group.IdentityGroupIDPath(gid) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) client, e := provider.GetClient(d, meta) if e != nil { - return e + return diag.FromErr(e) } - log.Printf("[DEBUG] Updating IdentityGroupMemberEntityIds %q", gid) - - if d.HasChange(consts.FieldGroupID) { - o, n := d.GetChange(consts.FieldGroupID) - log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) - } - resp, err := readIdentityGroup(client, gid, d.IsNewResource()) - if err != nil { - return err - } - - data, err := group.GetGroupMember(d, resp, consts.FieldMemberEntityIDs) - if err != nil { - return err - } - - _, err = client.Logical().Write(path, data) - if err != nil { - return fmt.Errorf("error updating IdentityGroupMemberEntityIds %q: %s", gid, err) + if diag := group.UpdateGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs); diag != nil { + return diag } - log.Printf("[DEBUG] Updated IdentityGroupMemberEntityIds %q", gid) - d.SetId(gid) - - return identityGroupMemberEntityIdsRead(d, meta) + return identityGroupMemberEntityIdsRead(ctx, d, meta) } -func identityGroupMemberEntityIdsRead(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberEntityIdsRead(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client, e := provider.GetClient(d, meta) if e != nil { - return e - } - - id := d.Id() - - log.Printf("[DEBUG] Read IdentityGroupMemberEntityIds %s", id) - resp, err := readIdentityGroup(client, id, d.IsNewResource()) - if err != nil { - if isIdentityNotFoundError(err) { - log.Printf("[WARN] IdentityGroupMemberEntityIds %q not found, removing from state", id) - d.SetId("") - return nil - } - return err - } - - if err := d.Set(consts.FieldGroupID, id); err != nil { - return err - } - if err := d.Set(consts.FieldGroupName, resp.Data["name"]); err != nil { - return err - } - - if err := group.SetGroupMember(d, resp, consts.FieldMemberEntityIDs); err != nil { - return err + return diag.FromErr(e) } - return nil + return group.ReadGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs, true) } -func identityGroupMemberEntityIdsDelete(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberEntityIdsDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { id := d.Get(consts.FieldGroupID).(string) - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) client, e := provider.GetClient(d, meta) if e != nil { - return e - } - - log.Printf("[DEBUG] Deleting IdentityGroupMemberEntityIds %q", id) - - resp, err := readIdentityGroup(client, id, false) - if err != nil { - if isIdentityNotFoundError(err) { - return nil - } - return err - } - - data, err := group.DeleteGroupMember(d, resp, consts.FieldMemberEntityIDs) - if err != nil { - return err - } - - _, err = client.Logical().Write(path, data) - if err != nil { - return fmt.Errorf("error updating IdentityGroupMemberEntityIds %q: %s", id, err) + return diag.FromErr(e) } - log.Printf("[DEBUG] Updated IdentityGroupMemberEntityIds %q", id) - return nil + return group.DeleteGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs) } diff --git a/vault/resource_identity_group_member_entity_ids_test.go b/vault/resource_identity_group_member_entity_ids_test.go index 50fe5774b..7aa5b8180 100644 --- a/vault/resource_identity_group_member_entity_ids_test.go +++ b/vault/resource_identity_group_member_entity_ids_test.go @@ -309,8 +309,8 @@ func testAccCheckidentityGroupMemberEntityIdsDestroy(s *terraform.State) error { return e } - if _, err := readIdentityGroup(client, rs.Primary.ID, false); err != nil { - if isIdentityNotFoundError(err) { + if _, err := group.ReadIdentityGroup(client, rs.Primary.ID, false); err != nil { + if group.IsIdentityNotFoundError(err) { continue } return err @@ -362,7 +362,7 @@ func testAccIdentityGroupMemberEntityIdsCheckAttrs(resourceName string) resource } id := rs.Primary.ID - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) tAttrs := []*testutil.VaultStateTest{ { ResourceName: resourceName, diff --git a/vault/resource_identity_group_member_group_ids.go b/vault/resource_identity_group_member_group_ids.go index 39b68767d..72d4f16a7 100644 --- a/vault/resource_identity_group_member_group_ids.go +++ b/vault/resource_identity_group_member_group_ids.go @@ -1,9 +1,9 @@ package vault import ( - "fmt" - "log" + "context" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-vault/internal/consts" @@ -13,10 +13,10 @@ import ( func identityGroupMemberGroupIdsResource() *schema.Resource { return &schema.Resource{ - Create: identityGroupMemberGroupIdsUpdate, - Update: identityGroupMemberGroupIdsUpdate, - Read: ReadWrapper(identityGroupMemberGroupIdsRead), - Delete: identityGroupMemberGroupIdsDelete, + CreateContext: identityGroupMemberGroupIdsUpdate, + UpdateContext: identityGroupMemberGroupIdsUpdate, + ReadContext: ReadContextWrapper(identityGroupMemberGroupIdsRead), + DeleteContext: identityGroupMemberGroupIdsDelete, Schema: map[string]*schema.Schema{ consts.FieldMemberGroupIDs: { @@ -31,8 +31,8 @@ func identityGroupMemberGroupIdsResource() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: true, - Description: `Should the resource manage member group ids -exclusively? Beware of race conditions when disabling exclusive management`, + Description: `If set to true, allows the resource to manage member group ids +exclusively. Beware of race conditions when disabling exclusive management`, }, consts.FieldGroupID: { Type: schema.TypeString, @@ -44,106 +44,43 @@ exclusively? Beware of race conditions when disabling exclusive management`, } } -func identityGroupMemberGroupIdsUpdate(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberGroupIdsUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { gid := d.Get(consts.FieldGroupID).(string) - path := identityGroupIDPath(gid) + path := group.IdentityGroupIDPath(gid) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) client, e := provider.GetClient(d, meta) if e != nil { - return e + return diag.FromErr(e) } - log.Printf("[DEBUG] Updating IdentityGroupMemberGroupIds %q", gid) - - if d.HasChange(consts.FieldGroupID) { - o, n := d.GetChange(consts.FieldGroupID) - log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) - } - - resp, err := readIdentityGroup(client, gid, d.IsNewResource()) - if err != nil { - return err + if diag := group.UpdateGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs); diag != nil { + return diag } - data, err := group.GetGroupMember(d, resp, consts.FieldMemberGroupIDs) - if err != nil { - return err - } - - _, err = client.Logical().Write(path, data) - if err != nil { - return fmt.Errorf("error updating IdentityGroupMemberGroupIds %q: %s", gid, err) - } - log.Printf("[DEBUG] Updated IdentityGroupMemberGroupIds %q", gid) - - d.SetId(gid) - - return identityGroupMemberGroupIdsRead(d, meta) + return identityGroupMemberGroupIdsRead(ctx, d, meta) } -func identityGroupMemberGroupIdsRead(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberGroupIdsRead(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { client, e := provider.GetClient(d, meta) if e != nil { - return e - } - - id := d.Id() - - log.Printf("[DEBUG] Read IdentityGroupMemberGroupIds %s", id) - resp, err := readIdentityGroup(client, id, d.IsNewResource()) - if err != nil { - if isIdentityNotFoundError(err) { - log.Printf("[WARN] IdentityGroupMemberGroupIds %q not found, removing from state", id) - d.SetId("") - return nil - } - return err + return diag.FromErr(e) } - if err := d.Set(consts.FieldGroupID, id); err != nil { - return err - } - - if err := group.SetGroupMember(d, resp, consts.FieldMemberGroupIDs); err != nil { - return err - } - - return nil + return group.ReadGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs, false) } -func identityGroupMemberGroupIdsDelete(d *schema.ResourceData, meta interface{}) error { +func identityGroupMemberGroupIdsDelete(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { id := d.Get(consts.FieldGroupID).(string) - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) client, e := provider.GetClient(d, meta) if e != nil { - return e - } - - log.Printf("[DEBUG] Deleting IdentityGroupMemberGroupIds %q", id) - - resp, err := readIdentityGroup(client, id, false) - if err != nil { - if isIdentityNotFoundError(err) { - return nil - } - return err - } - - data, err := group.DeleteGroupMember(d, resp, consts.FieldMemberGroupIDs) - if err != nil { - return err - } - - _, err = client.Logical().Write(path, data) - if err != nil { - return fmt.Errorf("error updating IdentityGroupMemberGroupIds %q: %s", id, err) + return diag.FromErr(e) } - log.Printf("[DEBUG] Updated IdentityGroupMemberGroupIds %q", id) - return nil + return group.DeleteGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs) } diff --git a/vault/resource_identity_group_policies.go b/vault/resource_identity_group_policies.go index b847087af..8edc15c47 100644 --- a/vault/resource_identity_group_policies.go +++ b/vault/resource_identity_group_policies.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/util" ) @@ -58,7 +59,7 @@ func identityGroupPoliciesUpdate(d *schema.ResourceData, meta interface{}) error id := d.Get("group_id").(string) log.Printf("[DEBUG] Updating IdentityGroupPolicies %q", id) - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) @@ -106,9 +107,9 @@ func identityGroupPoliciesRead(d *schema.ResourceData, meta interface{}) error { id := d.Id() log.Printf("[DEBUG] Read IdentityGroupPolicies %s", id) - resp, err := readIdentityGroup(client, id, d.IsNewResource()) + resp, err := group.ReadIdentityGroup(client, id, d.IsNewResource()) if err != nil { - if isIdentityNotFoundError(err) { + if group.IsIdentityNotFoundError(err) { log.Printf("[WARN] IdentityGroupPolicies %q not found, removing from state", id) d.SetId("") return nil @@ -159,7 +160,7 @@ func identityGroupPoliciesDelete(d *schema.ResourceData, meta interface{}) error id := d.Get("group_id").(string) log.Printf("[DEBUG] Deleting IdentityGroupPolicies %q", id) - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) vaultMutexKV.Lock(path) defer vaultMutexKV.Unlock(path) diff --git a/vault/resource_identity_group_policies_test.go b/vault/resource_identity_group_policies_test.go index 23e017ac7..58a1cc5c3 100644 --- a/vault/resource_identity_group_policies_test.go +++ b/vault/resource_identity_group_policies_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/testutil" "github.com/hashicorp/terraform-provider-vault/util" @@ -85,8 +86,8 @@ func testAccCheckidentityGroupPoliciesDestroy(s *terraform.State) error { return e } - if _, err := readIdentityGroup(client, rs.Primary.ID, false); err != nil { - if isIdentityNotFoundError(err) { + if _, err := group.ReadIdentityGroup(client, rs.Primary.ID, false); err != nil { + if group.IsIdentityNotFoundError(err) { continue } return err @@ -134,7 +135,7 @@ func testAccIdentityGroupPoliciesCheckAttrs(resourceName string) resource.TestCh return err } - path := identityGroupIDPath(rs.Primary.ID) + path := group.IdentityGroupIDPath(rs.Primary.ID) attrs := map[string]string{ "group_id": "id", diff --git a/vault/resource_identity_group_test.go b/vault/resource_identity_group_test.go index bce3e528a..aa51b8706 100644 --- a/vault/resource_identity_group_test.go +++ b/vault/resource_identity_group_test.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/hashicorp/terraform-provider-vault/internal/identity/group" "github.com/hashicorp/terraform-provider-vault/internal/provider" "github.com/hashicorp/terraform-provider-vault/testutil" ) @@ -161,7 +162,7 @@ func testAccCheckIdentityGroupDestroy(s *terraform.State) error { return e } - secret, err := client.Logical().Read(identityGroupIDPath(rs.Primary.ID)) + secret, err := client.Logical().Read(group.IdentityGroupIDPath(rs.Primary.ID)) if err != nil { return fmt.Errorf("error checking for identity group %q: %s", rs.Primary.ID, err) } @@ -190,7 +191,7 @@ func testAccIdentityGroupCheckAttrs(resourceName string) resource.TestCheckFunc } id := rs.Primary.ID - path := identityGroupIDPath(id) + path := group.IdentityGroupIDPath(id) attrs := map[string]string{ "name": "name", From 6ca3b7c60ffd7425c3fea6b422b409e8d9e634d1 Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Tue, 25 Oct 2022 10:19:45 -0700 Subject: [PATCH 4/7] add documentation for exported functions and registry --- internal/identity/group/group.go | 15 ++- .../r/identity_group_member_group_ids.html.md | 95 +++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 website/docs/r/identity_group_member_group_ids.html.md diff --git a/internal/identity/group/group.go b/internal/identity/group/group.go index e2b676b46..3a9efacef 100644 --- a/internal/identity/group/group.go +++ b/internal/identity/group/group.go @@ -25,7 +25,7 @@ func IdentityGroupIDPath(id string) string { return fmt.Sprintf("%s/id/%s", IdentityGroupPath, id) } -// This function may return `nil` for the IdentityGroup if it does not exist +// ReadIdentityGroup may return `nil` for the IdentityGroup if it does not exist func ReadIdentityGroup(client *api.Client, groupID string, retry bool) (*api.Secret, error) { path := IdentityGroupIDPath(groupID) log.Printf("[DEBUG] Reading IdentityGroup %s from %q", groupID, path) @@ -37,6 +37,8 @@ func IsIdentityNotFoundError(err error) bool { return err != nil && errors.Is(err, entity.ErrEntityNotFound) } +// UpdateGroupMemberContextFunc is a common context function for all +// update operations to be performed on Identity Group Members func UpdateGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { gid := d.Get(consts.FieldGroupID).(string) path := IdentityGroupIDPath(gid) @@ -69,6 +71,8 @@ func UpdateGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, me return nil } +// ReadGroupMemberContextFunc is a common context function for all +// read operations to be performed on Identity Group Members func ReadGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string, setGroupName bool) diag.Diagnostics { id := d.Id() @@ -100,6 +104,8 @@ func ReadGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memb return nil } +// DeleteGroupMemberContextFunc is a common context function for all +// delete operations to be performed on Identity Group Members func DeleteGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { id := d.Get(consts.FieldGroupID).(string) path := IdentityGroupIDPath(id) @@ -128,6 +134,9 @@ func DeleteGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, me return nil } +// GetGroupMember returns group member data based on an input +// 'memberField'. It manages the lifecycle of internal group +// members appropriately by performing any necessary deduplication func GetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) (map[string]interface{}, error) { data := map[string]interface{}{} @@ -177,6 +186,8 @@ func GetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string return data, nil } +// SetGroupMember sets group member data to the TF state based +// on a 'memberField' func SetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) error { curIDS := resp.Data[memberField] if d.Get(consts.FieldExclusive).(bool) { @@ -208,6 +219,8 @@ func SetGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string return nil } +// DeleteGroupMember deletes group member data from Vault and the TF +// state based on a 'memberField' func DeleteGroupMember(d *schema.ResourceData, resp *api.Secret, memberField string) (map[string]interface{}, error) { data := map[string]interface{}{} diff --git a/website/docs/r/identity_group_member_group_ids.html.md b/website/docs/r/identity_group_member_group_ids.html.md new file mode 100644 index 000000000..1ab1f73c9 --- /dev/null +++ b/website/docs/r/identity_group_member_group_ids.html.md @@ -0,0 +1,95 @@ +--- +layout: "vault" +page_title: "Vault: vault_identity_group_member_group_ids resource" +sidebar_current: "docs-vault-resource-identity-group-member-group-ids" +description: |- +Manages member groups for an Identity Group for Vault. +--- + +# vault\_identity\_group\_member\_group\_ids + +Manages member groups for an Identity Group for Vault. The [Identity secrets engine](https://www.vaultproject.io/docs/secrets/identity/index.html) is the identity management solution for Vault. + +## Example Usage + +### Exclusive Member Groups + +```hcl +resource "vault_identity_group" "internal" { + name = "internal" + type = "internal" + external_member_group_ids = true + + metadata = { + version = "2" + } +} + +resource "vault_identity_group" "users" { + name = "users" + metadata = { + version = "2" + } +} + +resource "vault_identity_group_member_group_ids" "members" { + + exclusive = true + member_group_ids = [vault_identity_group.users.id] + group_id = vault_identity_group.internal.id +} +``` + +### Non-Exclusive Member Groups + +```hcl +resource "vault_identity_group" "internal" { + name = "internal" + type = "internal" + external_member_group_ids = true + + metadata = { + version = "2" + } +} + +resource "vault_identity_group" "users" { + name = "users" + metadata = { + version = "2" + } +} + +resource "vault_identity_group_member_group_ids" "members" { + + exclusive = false + member_group_ids = [vault_identity_group.users.id] + group_id = vault_identity_group.internal.id +} +``` + +## Argument Reference + +The following arguments are supported: + +* `namespace` - (Optional) The namespace to provision the resource in. + The value should not contain leading or trailing forward slashes. + The `namespace` is always relative to the provider's configured [namespace](/docs/providers/vault#namespace). + *Available only for Vault Enterprise*. + +* `member_group_ids` - (Required) List of member groups that belong to the group + +* `group_id` - (Required) Group ID to assign member entities to. + +* `exclusive` - (Optional) Defaults to `true`. + + If `true`, this resource will take exclusive control of the member groups that belong to the group and will set + it equal to what is specified in the resource. + + If set to `false`, this resource will simply ensure that the member groups specified in the resource are present + in the group. When destroying the resource, the resource will ensure that the member groups specified in the resource + are removed. + +## Attributes Reference + +No additional attributes are exported by this resource. \ No newline at end of file From 6a3d1943691b9cc830f7d96c23748646759dde9c Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Tue, 25 Oct 2022 15:26:21 -0700 Subject: [PATCH 5/7] move vault mutex and refactor common CRUD funcs --- internal/identity/group/group.go | 268 ++++++++---------- internal/identity/group/testhelpers.go | 90 ++++++ internal/provider/locker.go | 9 + vault/provider.go | 7 - vault/resource_identity_entity.go | 8 +- vault/resource_identity_entity_alias.go | 4 +- vault/resource_identity_entity_policies.go | 8 +- vault/resource_identity_group.go | 8 +- ...source_identity_group_member_entity_ids.go | 53 +--- ...esource_identity_group_member_group_ids.go | 53 +--- vault/resource_identity_group_policies.go | 8 +- vault/resource_identity_oidc_key.go | 12 +- ...rce_identity_oidc_key_allowed_client_id.go | 8 +- website/vault.erb | 4 + 14 files changed, 256 insertions(+), 284 deletions(-) create mode 100644 internal/identity/group/testhelpers.go create mode 100644 internal/provider/locker.go diff --git a/internal/identity/group/group.go b/internal/identity/group/group.go index 3a9efacef..d3c1990ef 100644 --- a/internal/identity/group/group.go +++ b/internal/identity/group/group.go @@ -1,24 +1,26 @@ package group import ( + "context" "errors" "fmt" "log" - "strconv" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/hashicorp/vault/api" "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/entity" + "github.com/hashicorp/terraform-provider-vault/internal/provider" ) const ( LookupPath = "identity/lookup/group" IdentityGroupPath = "/identity/group" + + GroupResourceType = iota + EntityResourceType ) func IdentityGroupIDPath(id string) string { @@ -37,101 +39,144 @@ func IsIdentityNotFoundError(err error) bool { return err != nil && errors.Is(err, entity.ErrEntityNotFound) } -// UpdateGroupMemberContextFunc is a common context function for all -// update operations to be performed on Identity Group Members -func UpdateGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { - gid := d.Get(consts.FieldGroupID).(string) - path := IdentityGroupIDPath(gid) +func getFieldFromResourceType(resourceType int) string { + var ret string + switch resourceType { + case GroupResourceType: + ret = consts.FieldMemberGroupIDs + case EntityResourceType: + ret = consts.FieldMemberEntityIDs + } - log.Printf("[DEBUG] Updating field %q on Identity Group %q", memberField, gid) + return ret +} - if d.HasChange(consts.FieldGroupID) { - o, n := d.GetChange(consts.FieldGroupID) - log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) - } +// GetGroupMemberUpdateContextFunc is a common context function for all +// Update operations to be performed on Identity Group Members +func GetGroupMemberUpdateContextFunc(resourceType int) func(context.Context, *schema.ResourceData, interface{}) diag.Diagnostics { + return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + gid := d.Get(consts.FieldGroupID).(string) + path := IdentityGroupIDPath(gid) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) + + client, e := provider.GetClient(d, meta) + if e != nil { + return diag.FromErr(e) + } - resp, err := ReadIdentityGroup(client, gid, d.IsNewResource()) - if err != nil { - return diag.FromErr(err) - } + memberField := getFieldFromResourceType(resourceType) - data, err := GetGroupMember(d, resp, memberField) - if err != nil { - return diag.FromErr(err) - } + log.Printf("[DEBUG] Updating field %q on Identity Group %q", memberField, gid) - _, err = client.Logical().Write(path, data) - if err != nil { - return diag.Errorf("error updating field %q on Identity Group %s: err=%s", memberField, gid, err) - } - log.Printf("[DEBUG] Updated field %q on Identity Group %s", memberField, gid) + if d.HasChange(consts.FieldGroupID) { + o, n := d.GetChange(consts.FieldGroupID) + log.Printf("[DEBUG] Group ID has changed old=%q, new=%q", o, n) + } + + resp, err := ReadIdentityGroup(client, gid, d.IsNewResource()) + if err != nil { + return diag.FromErr(err) + } - d.SetId(gid) + data, err := GetGroupMember(d, resp, memberField) + if err != nil { + return diag.FromErr(err) + } - return nil + _, err = client.Logical().Write(path, data) + if err != nil { + return diag.Errorf("error updating field %q on Identity Group %s: err=%s", memberField, gid, err) + } + log.Printf("[DEBUG] Updated field %q on Identity Group %s", memberField, gid) + + d.SetId(gid) + + return nil + } } -// ReadGroupMemberContextFunc is a common context function for all +// GetGroupMemberReadContextFunc is a common context function for all // read operations to be performed on Identity Group Members -func ReadGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string, setGroupName bool) diag.Diagnostics { - id := d.Id() - - log.Printf("[DEBUG] Reading Identity Group %s with field %q", id, memberField) - resp, err := ReadIdentityGroup(client, id, d.IsNewResource()) - if err != nil { - if IsIdentityNotFoundError(err) { - log.Printf("[WARN] Identity Group %s not found, removing from state", id) - d.SetId("") - return nil +func GetGroupMemberReadContextFunc(resourceType int, setGroupName bool) schema.ReadContextFunc { + return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + client, e := provider.GetClient(d, meta) + if e != nil { + return diag.FromErr(e) } - return diag.FromErr(err) - } - if err := d.Set(consts.FieldGroupID, id); err != nil { - return diag.FromErr(err) - } + id := d.Id() + + memberField := getFieldFromResourceType(resourceType) - if setGroupName { - if err := d.Set(consts.FieldGroupName, resp.Data[consts.FieldName]); err != nil { + log.Printf("[DEBUG] Reading Identity Group %s with field %q", id, memberField) + resp, err := ReadIdentityGroup(client, id, d.IsNewResource()) + if err != nil { + if IsIdentityNotFoundError(err) { + log.Printf("[WARN] Identity Group %s not found, removing from state", id) + d.SetId("") + return nil + } return diag.FromErr(err) } - } - if err := SetGroupMember(d, resp, memberField); err != nil { - return diag.FromErr(err) - } + if err := d.Set(consts.FieldGroupID, id); err != nil { + return diag.FromErr(err) + } - return nil + if setGroupName { + if err := d.Set(consts.FieldGroupName, resp.Data[consts.FieldName]); err != nil { + return diag.FromErr(err) + } + } + + if err := SetGroupMember(d, resp, memberField); err != nil { + return diag.FromErr(err) + } + + return nil + } } -// DeleteGroupMemberContextFunc is a common context function for all +// GetGroupMemberDeleteContextFunc is a common context function for all // delete operations to be performed on Identity Group Members -func DeleteGroupMemberContextFunc(d *schema.ResourceData, client *api.Client, memberField string) diag.Diagnostics { - id := d.Get(consts.FieldGroupID).(string) - path := IdentityGroupIDPath(id) +func GetGroupMemberDeleteContextFunc(resourceType int) schema.DeleteContextFunc { + return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + id := d.Get(consts.FieldGroupID).(string) + path := IdentityGroupIDPath(id) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) + + client, e := provider.GetClient(d, meta) + if e != nil { + return diag.FromErr(e) + } - log.Printf("[DEBUG] Deleting Identity Group %q with field %q", memberField, id) + memberField := getFieldFromResourceType(resourceType) - resp, err := ReadIdentityGroup(client, id, false) - if err != nil { - if IsIdentityNotFoundError(err) { - return nil + log.Printf("[DEBUG] Deleting Identity Group %q with field %q", memberField, id) + + resp, err := ReadIdentityGroup(client, id, false) + if err != nil { + if IsIdentityNotFoundError(err) { + return nil + } + return diag.FromErr(err) } - return diag.FromErr(err) - } - data, err := DeleteGroupMember(d, resp, memberField) - if err != nil { - return diag.FromErr(err) - } + data, err := DeleteGroupMember(d, resp, memberField) + if err != nil { + return diag.FromErr(err) + } - _, err = client.Logical().Write(path, data) - if err != nil { - return diag.Errorf("error deleting Identity Group %q with field %q; err=%s", id, memberField, err) - } - log.Printf("[DEBUG] Deleted Identity Group %q with field %q", memberField, id) + _, err = client.Logical().Write(path, data) + if err != nil { + return diag.Errorf("error deleting Identity Group %q with field %q; err=%s", id, memberField, err) + } + log.Printf("[DEBUG] Deleted Identity Group %q with field %q", memberField, id) - return nil + return nil + } } // GetGroupMember returns group member data based on an input @@ -260,82 +305,3 @@ func DeleteGroupMember(d *schema.ResourceData, resp *api.Secret, memberField str return data, nil } - -// needed for testing -type GroupMemberTester struct { - EntityIDS []string - GroupIDS []string -} - -func (r *GroupMemberTester) SetMemberEntities(resource string) resource.TestCheckFunc { - return func(s *terraform.State) error { - result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberEntityIDs) - if err != nil { - return err - } - r.EntityIDS = result - return nil - } -} - -func (r *GroupMemberTester) SetMemberGroups(resource string) resource.TestCheckFunc { - return func(s *terraform.State) error { - result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberGroupIDs) - if err != nil { - return err - } - r.GroupIDS = result - return nil - } -} - -func (r *GroupMemberTester) getGroupMemberResourceData(s *terraform.State, resource, memberField string) ([]string, error) { - var result []string - resourceState := s.Modules[0].Resources[resource] - if resourceState == nil { - return result, fmt.Errorf("resource not found in state") - } - - instanceState := resourceState.Primary - if instanceState == nil { - return result, fmt.Errorf("resource not found in state") - } - - count, err := strconv.Atoi(instanceState.Attributes[fmt.Sprintf("%s.#", memberField)]) - if err != nil { - return nil, err - } - - for i := 0; i < count; i++ { - k := fmt.Sprintf("%s.%d", memberField, i) - result = append(result, instanceState.Attributes[k]) - } - - return result, nil -} - -func (r *GroupMemberTester) CheckMemberEntities(resourceName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - for i, v := range r.EntityIDS { - k := fmt.Sprintf("member_entity_ids.%d", i) - f := resource.TestCheckResourceAttr(resourceName, k, v) - if err := f(s); err != nil { - return err - } - } - return nil - } -} - -func (r *GroupMemberTester) CheckMemberGroups(resourceName string) resource.TestCheckFunc { - return func(s *terraform.State) error { - for i, v := range r.GroupIDS { - k := fmt.Sprintf("member_group_ids.%d", i) - f := resource.TestCheckResourceAttr(resourceName, k, v) - if err := f(s); err != nil { - return err - } - } - return nil - } -} diff --git a/internal/identity/group/testhelpers.go b/internal/identity/group/testhelpers.go new file mode 100644 index 000000000..250a57afe --- /dev/null +++ b/internal/identity/group/testhelpers.go @@ -0,0 +1,90 @@ +package group + +import ( + "fmt" + "strconv" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + + "github.com/hashicorp/terraform-provider-vault/internal/consts" +) + +// needed for testing +type GroupMemberTester struct { + EntityIDS []string + GroupIDS []string +} + +func (r *GroupMemberTester) SetMemberEntities(resource string) resource.TestCheckFunc { + return func(s *terraform.State) error { + result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberEntityIDs) + if err != nil { + return err + } + r.EntityIDS = result + return nil + } +} + +func (r *GroupMemberTester) SetMemberGroups(resource string) resource.TestCheckFunc { + return func(s *terraform.State) error { + result, err := r.getGroupMemberResourceData(s, resource, consts.FieldMemberGroupIDs) + if err != nil { + return err + } + r.GroupIDS = result + return nil + } +} + +func (r *GroupMemberTester) getGroupMemberResourceData(s *terraform.State, resource, memberField string) ([]string, error) { + var result []string + resourceState := s.Modules[0].Resources[resource] + if resourceState == nil { + return result, fmt.Errorf("resource not found in state") + } + + instanceState := resourceState.Primary + if instanceState == nil { + return result, fmt.Errorf("resource not found in state") + } + + count, err := strconv.Atoi(instanceState.Attributes[fmt.Sprintf("%s.#", memberField)]) + if err != nil { + return nil, err + } + + for i := 0; i < count; i++ { + k := fmt.Sprintf("%s.%d", memberField, i) + result = append(result, instanceState.Attributes[k]) + } + + return result, nil +} + +func (r *GroupMemberTester) CheckMemberEntities(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + for i, v := range r.EntityIDS { + k := fmt.Sprintf("member_entity_ids.%d", i) + f := resource.TestCheckResourceAttr(resourceName, k, v) + if err := f(s); err != nil { + return err + } + } + return nil + } +} + +func (r *GroupMemberTester) CheckMemberGroups(resourceName string) resource.TestCheckFunc { + return func(s *terraform.State) error { + for i, v := range r.GroupIDS { + k := fmt.Sprintf("member_group_ids.%d", i) + f := resource.TestCheckResourceAttr(resourceName, k, v) + if err := f(s); err != nil { + return err + } + } + return nil + } +} diff --git a/internal/provider/locker.go b/internal/provider/locker.go new file mode 100644 index 000000000..d972ba6e1 --- /dev/null +++ b/internal/provider/locker.go @@ -0,0 +1,9 @@ +package provider + +import "github.com/hashicorp/terraform-provider-vault/helper" + +// This is a global MutexKV for use within this provider. +// Use this when you need to have multiple resources or even multiple instances +// of the same resource write to the same path in Vault. +// The key of the mutex should be the path in Vault. +var VaultMutexKV = helper.NewMutexKV() diff --git a/vault/provider.go b/vault/provider.go index c6f9daff2..e8c35a2c7 100644 --- a/vault/provider.go +++ b/vault/provider.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/vault/api" - "github.com/hashicorp/terraform-provider-vault/helper" "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/mfa" "github.com/hashicorp/terraform-provider-vault/internal/provider" @@ -36,12 +35,6 @@ const ( DefaultMaxHTTPRetriesCCC = 10 ) -// This is a global MutexKV for use within this provider. -// Use this when you need to have multiple resources or even multiple instances -// of the same resource write to the same path in Vault. -// The key of the mutex should be the path in Vault. -var vaultMutexKV = helper.NewMutexKV() - func Provider() *schema.Provider { dataSourcesMap, err := parse(DataSourceRegistry) if err != nil { diff --git a/vault/resource_identity_entity.go b/vault/resource_identity_entity.go index 79f34724e..c495c06e2 100644 --- a/vault/resource_identity_entity.go +++ b/vault/resource_identity_entity.go @@ -156,8 +156,8 @@ func identityEntityUpdate(d *schema.ResourceData, meta interface{}) error { log.Printf("[DEBUG] Updating IdentityEntity %q", id) path := entity.JoinEntityID(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := map[string]interface{}{} @@ -214,8 +214,8 @@ func identityEntityDelete(d *schema.ResourceData, meta interface{}) error { path := entity.JoinEntityID(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) log.Printf("[DEBUG] Deleting IdentityEntitty %q", id) _, err := client.Logical().Delete(path) diff --git a/vault/resource_identity_entity_alias.go b/vault/resource_identity_entity_alias.go index cea70a322..eab8de55b 100644 --- a/vault/resource_identity_entity_alias.go +++ b/vault/resource_identity_entity_alias.go @@ -251,11 +251,11 @@ func getEntityLockFuncs(d *schema.ResourceData, root string) (func(), func()) { mountAccessor := d.Get(consts.FieldMountAccessor).(string) lockKey := strings.Join([]string{root, mountAccessor}, "/") lock := func() { - vaultMutexKV.Lock(lockKey) + provider.VaultMutexKV.Lock(lockKey) } unlock := func() { - vaultMutexKV.Unlock(lockKey) + provider.VaultMutexKV.Unlock(lockKey) } return lock, unlock } diff --git a/vault/resource_identity_entity_policies.go b/vault/resource_identity_entity_policies.go index 08a304909..580573941 100644 --- a/vault/resource_identity_entity_policies.go +++ b/vault/resource_identity_entity_policies.go @@ -62,8 +62,8 @@ func identityEntityPoliciesUpdate(d *schema.ResourceData, meta interface{}) erro log.Printf("[DEBUG] Updating IdentityEntityPolicies %q", id) path := entity.JoinEntityID(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := make(map[string]interface{}) policies := d.Get("policies").(*schema.Set).List() @@ -153,8 +153,8 @@ func identityEntityPoliciesDelete(d *schema.ResourceData, meta interface{}) erro log.Printf("[DEBUG] Deleting IdentityEntityPolicies %q", id) path := entity.JoinEntityID(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := make(map[string]interface{}) diff --git a/vault/resource_identity_group.go b/vault/resource_identity_group.go index bdac474e3..29426a74a 100644 --- a/vault/resource_identity_group.go +++ b/vault/resource_identity_group.go @@ -219,8 +219,8 @@ func identityGroupUpdate(d *schema.ResourceData, meta interface{}) error { log.Printf("[DEBUG] Updating IdentityGroup %q", id) path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := map[string]interface{}{} @@ -281,8 +281,8 @@ func identityGroupDelete(d *schema.ResourceData, meta interface{}) error { path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) log.Printf("[DEBUG] Deleting IdentityGroup %q", id) _, err := client.Logical().Delete(path) diff --git a/vault/resource_identity_group_member_entity_ids.go b/vault/resource_identity_group_member_entity_ids.go index 24a4cf65e..ea22d466c 100644 --- a/vault/resource_identity_group_member_entity_ids.go +++ b/vault/resource_identity_group_member_entity_ids.go @@ -1,22 +1,18 @@ package vault import ( - "context" - - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/group" - "github.com/hashicorp/terraform-provider-vault/internal/provider" ) func identityGroupMemberEntityIdsResource() *schema.Resource { return &schema.Resource{ - CreateContext: identityGroupMemberEntityIdsUpdate, - UpdateContext: identityGroupMemberEntityIdsUpdate, - ReadContext: ReadContextWrapper(identityGroupMemberEntityIdsRead), - DeleteContext: identityGroupMemberEntityIdsDelete, + CreateContext: group.GetGroupMemberUpdateContextFunc(group.EntityResourceType), + UpdateContext: group.GetGroupMemberUpdateContextFunc(group.EntityResourceType), + ReadContext: ReadContextWrapper(group.GetGroupMemberReadContextFunc(group.EntityResourceType, true)), + DeleteContext: group.GetGroupMemberDeleteContextFunc(group.EntityResourceType), Schema: map[string]*schema.Schema{ consts.FieldMemberEntityIDs: { @@ -50,44 +46,3 @@ use "data.vault_identity_group.*.group_name", "vault_identity_group.*.group_name }, } } - -func identityGroupMemberEntityIdsUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - gid := d.Get(consts.FieldGroupID).(string) - path := group.IdentityGroupIDPath(gid) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) - - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - if diag := group.UpdateGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs); diag != nil { - return diag - } - - return identityGroupMemberEntityIdsRead(ctx, d, meta) -} - -func identityGroupMemberEntityIdsRead(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - return group.ReadGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs, true) -} - -func identityGroupMemberEntityIdsDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - id := d.Get(consts.FieldGroupID).(string) - path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) - - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - return group.DeleteGroupMemberContextFunc(d, client, consts.FieldMemberEntityIDs) -} diff --git a/vault/resource_identity_group_member_group_ids.go b/vault/resource_identity_group_member_group_ids.go index 72d4f16a7..3d21bdb61 100644 --- a/vault/resource_identity_group_member_group_ids.go +++ b/vault/resource_identity_group_member_group_ids.go @@ -1,22 +1,18 @@ package vault import ( - "context" - - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-provider-vault/internal/consts" "github.com/hashicorp/terraform-provider-vault/internal/identity/group" - "github.com/hashicorp/terraform-provider-vault/internal/provider" ) func identityGroupMemberGroupIdsResource() *schema.Resource { return &schema.Resource{ - CreateContext: identityGroupMemberGroupIdsUpdate, - UpdateContext: identityGroupMemberGroupIdsUpdate, - ReadContext: ReadContextWrapper(identityGroupMemberGroupIdsRead), - DeleteContext: identityGroupMemberGroupIdsDelete, + CreateContext: group.GetGroupMemberUpdateContextFunc(group.GroupResourceType), + UpdateContext: group.GetGroupMemberUpdateContextFunc(group.GroupResourceType), + ReadContext: ReadContextWrapper(group.GetGroupMemberReadContextFunc(group.GroupResourceType, false)), + DeleteContext: group.GetGroupMemberDeleteContextFunc(group.GroupResourceType), Schema: map[string]*schema.Schema{ consts.FieldMemberGroupIDs: { @@ -43,44 +39,3 @@ exclusively. Beware of race conditions when disabling exclusive management`, }, } } - -func identityGroupMemberGroupIdsUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - gid := d.Get(consts.FieldGroupID).(string) - path := group.IdentityGroupIDPath(gid) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) - - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - if diag := group.UpdateGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs); diag != nil { - return diag - } - - return identityGroupMemberGroupIdsRead(ctx, d, meta) -} - -func identityGroupMemberGroupIdsRead(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - return group.ReadGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs, false) -} - -func identityGroupMemberGroupIdsDelete(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - id := d.Get(consts.FieldGroupID).(string) - path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) - - client, e := provider.GetClient(d, meta) - if e != nil { - return diag.FromErr(e) - } - - return group.DeleteGroupMemberContextFunc(d, client, consts.FieldMemberGroupIDs) -} diff --git a/vault/resource_identity_group_policies.go b/vault/resource_identity_group_policies.go index 8edc15c47..9d84aff3d 100644 --- a/vault/resource_identity_group_policies.go +++ b/vault/resource_identity_group_policies.go @@ -61,8 +61,8 @@ func identityGroupPoliciesUpdate(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] Updating IdentityGroupPolicies %q", id) path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := make(map[string]interface{}) policies := d.Get("policies").(*schema.Set).List() @@ -162,8 +162,8 @@ func identityGroupPoliciesDelete(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] Deleting IdentityGroupPolicies %q", id) path := group.IdentityGroupIDPath(id) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := make(map[string]interface{}) diff --git a/vault/resource_identity_oidc_key.go b/vault/resource_identity_oidc_key.go index 5b58be3fd..7723bb059 100644 --- a/vault/resource_identity_oidc_key.go +++ b/vault/resource_identity_oidc_key.go @@ -90,8 +90,8 @@ func identityOidcKeyCreate(d *schema.ResourceData, meta interface{}) error { name := d.Get("name").(string) path := identityOidcKeyPath(name) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data := make(map[string]interface{}) @@ -114,8 +114,8 @@ func identityOidcKeyUpdate(d *schema.ResourceData, meta interface{}) error { name := d.Id() path := identityOidcKeyPath(name) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) log.Printf("[DEBUG] Updating IdentityOidcKey %s at %s", name, path) @@ -166,8 +166,8 @@ func identityOidcKeyDelete(d *schema.ResourceData, meta interface{}) error { name := d.Id() path := identityOidcKeyPath(name) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) log.Printf("[DEBUG] Deleting IdentityOidcKey %q", name) _, err := client.Logical().Delete(path) diff --git a/vault/resource_identity_oidc_key_allowed_client_id.go b/vault/resource_identity_oidc_key_allowed_client_id.go index 70d6fc963..df6f2630c 100644 --- a/vault/resource_identity_oidc_key_allowed_client_id.go +++ b/vault/resource_identity_oidc_key_allowed_client_id.go @@ -44,8 +44,8 @@ func identityOidcKeyAllowedClientIdWrite(d *schema.ResourceData, meta interface{ path := identityOidcKeyPath(name) clientID := d.Get("allowed_client_id").(string) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data, err := identityOidcKeyApiRead(name, client) if err != nil { @@ -109,8 +109,8 @@ func identityOidcKeyAllowedClientIdDelete(d *schema.ResourceData, meta interface path := identityOidcKeyPath(name) clientID := d.Get("allowed_client_id").(string) - vaultMutexKV.Lock(path) - defer vaultMutexKV.Unlock(path) + provider.VaultMutexKV.Lock(path) + defer provider.VaultMutexKV.Unlock(path) data, err := identityOidcKeyApiRead(name, client) if err != nil { diff --git a/website/vault.erb b/website/vault.erb index a479f6116..7e78d3d94 100644 --- a/website/vault.erb +++ b/website/vault.erb @@ -289,6 +289,10 @@ vault_identity_group_member_entity_ids + > + vault_identity_group_member_group_ids + + > vault_identity_group_policies From 1daca233b277be5a1edbd5176736a2bf043db16a Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Tue, 25 Oct 2022 15:34:33 -0700 Subject: [PATCH 6/7] wrap line exceeding line length --- website/docs/r/identity_group_member_group_ids.html.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/website/docs/r/identity_group_member_group_ids.html.md b/website/docs/r/identity_group_member_group_ids.html.md index 1ab1f73c9..eb30e3a95 100644 --- a/website/docs/r/identity_group_member_group_ids.html.md +++ b/website/docs/r/identity_group_member_group_ids.html.md @@ -8,7 +8,9 @@ Manages member groups for an Identity Group for Vault. # vault\_identity\_group\_member\_group\_ids -Manages member groups for an Identity Group for Vault. The [Identity secrets engine](https://www.vaultproject.io/docs/secrets/identity/index.html) is the identity management solution for Vault. +Manages member groups for an Identity Group for Vault. The +[Identity secrets engine](https://www.vaultproject.io/docs/secrets/identity/index.html) +is the identity management solution for Vault. ## Example Usage From 21f55a6d4542217f36e713c02aa9e3a8414db301 Mon Sep 17 00:00:00 2001 From: Vinay Gopalan Date: Wed, 26 Oct 2022 13:06:48 -0700 Subject: [PATCH 7/7] handle type assert panics and add missing docs --- internal/identity/group/group.go | 35 +++++++++++++++++++++------ website/docs/r/identity_group.html.md | 16 +++++++++--- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/internal/identity/group/group.go b/internal/identity/group/group.go index d3c1990ef..7e67a0089 100644 --- a/internal/identity/group/group.go +++ b/internal/identity/group/group.go @@ -39,23 +39,30 @@ func IsIdentityNotFoundError(err error) bool { return err != nil && errors.Is(err, entity.ErrEntityNotFound) } -func getFieldFromResourceType(resourceType int) string { +func getFieldFromResourceType(resourceType int) (string, error) { var ret string switch resourceType { case GroupResourceType: ret = consts.FieldMemberGroupIDs case EntityResourceType: ret = consts.FieldMemberEntityIDs + default: + return "", fmt.Errorf("unkown resource type") } - return ret + return ret, nil } // GetGroupMemberUpdateContextFunc is a common context function for all // Update operations to be performed on Identity Group Members func GetGroupMemberUpdateContextFunc(resourceType int) func(context.Context, *schema.ResourceData, interface{}) diag.Diagnostics { return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - gid := d.Get(consts.FieldGroupID).(string) + gidRaw := d.Get(consts.FieldGroupID) + gid, ok := gidRaw.(string) + if !ok { + return diag.Errorf("invalid Group ID type %T", gidRaw) + } + path := IdentityGroupIDPath(gid) provider.VaultMutexKV.Lock(path) defer provider.VaultMutexKV.Unlock(path) @@ -65,7 +72,10 @@ func GetGroupMemberUpdateContextFunc(resourceType int) func(context.Context, *sc return diag.FromErr(e) } - memberField := getFieldFromResourceType(resourceType) + memberField, err := getFieldFromResourceType(resourceType) + if err != nil { + return diag.FromErr(err) + } log.Printf("[DEBUG] Updating field %q on Identity Group %q", memberField, gid) @@ -107,7 +117,10 @@ func GetGroupMemberReadContextFunc(resourceType int, setGroupName bool) schema.R id := d.Id() - memberField := getFieldFromResourceType(resourceType) + memberField, err := getFieldFromResourceType(resourceType) + if err != nil { + return diag.FromErr(err) + } log.Printf("[DEBUG] Reading Identity Group %s with field %q", id, memberField) resp, err := ReadIdentityGroup(client, id, d.IsNewResource()) @@ -142,7 +155,12 @@ func GetGroupMemberReadContextFunc(resourceType int, setGroupName bool) schema.R // delete operations to be performed on Identity Group Members func GetGroupMemberDeleteContextFunc(resourceType int) schema.DeleteContextFunc { return func(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - id := d.Get(consts.FieldGroupID).(string) + idRaw := d.Get(consts.FieldGroupID) + id, ok := idRaw.(string) + if !ok { + return diag.Errorf("invalid Group ID type %T", idRaw) + } + path := IdentityGroupIDPath(id) provider.VaultMutexKV.Lock(path) defer provider.VaultMutexKV.Unlock(path) @@ -152,7 +170,10 @@ func GetGroupMemberDeleteContextFunc(resourceType int) schema.DeleteContextFunc return diag.FromErr(e) } - memberField := getFieldFromResourceType(resourceType) + memberField, err := getFieldFromResourceType(resourceType) + if err != nil { + return diag.FromErr(err) + } log.Printf("[DEBUG] Deleting Identity Group %q with field %q", memberField, id) diff --git a/website/docs/r/identity_group.html.md b/website/docs/r/identity_group.html.md index 5392c0cef..276b184f2 100644 --- a/website/docs/r/identity_group.html.md +++ b/website/docs/r/identity_group.html.md @@ -94,9 +94,19 @@ The following arguments are supported: * `member_entity_ids` - (Optional) A list of Entity IDs to be assigned as group members. Not allowed on `external` groups. -* `external_policies` - (Optional) `false` by default. If set to `true`, this resource will ignore any policies returned from Vault or specified in the resource. You can use [`vault_identity_group_policies`](identity_group_policies.html) to manage policies for this group in a decoupled manner. - -* `external_member_entity_ids` - (Optional) `false` by default. If set to `true`, this resource will ignore any Entity IDs returned from Vault or specified in the resource. You can use [`vault_identity_group_member_entity_ids`](identity_group_member_entity_ids.html) to manage Entity IDs for this group in a decoupled manner. +* `external_policies` - (Optional) `false` by default. If set to `true`, this resource will ignore any policies returned from + Vault or specified in the resource. You can use [`vault_identity_group_policies`](identity_group_policies.html) to manage + policies for this group in a decoupled manner. + +* `external_member_entity_ids` - (Optional) `false` by default. If set to `true`, this resource will ignore any Entity IDs + returned from Vault or specified in the resource. You can use + [`vault_identity_group_member_entity_ids`](identity_group_member_entity_ids.html) to manage Entity IDs for this group in a + decoupled manner. + +* `external_member_group_ids` - (Optional) `false` by default. If set to `true`, this resource will ignore any Group IDs + returned from Vault or specified in the resource. You can use + [`vault_identity_group_member_group_ids`](identity_group_member_group_ids.html) to manage Group IDs for this group in a + decoupled manner. ## Attributes Reference