Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some issues with vault_identity_group #301

Merged
merged 6 commits into from
Feb 20, 2019

Conversation

lawliet89
Copy link
Contributor

@lawliet89 lawliet89 commented Feb 13, 2019

  • policies were not updated properly from reading the resource from Vault
  • Update fields policies, member_group_ids, and member_entity_ids to be of schema.TypeSet instead. These lists should contain unique items and the order should not matter when calculating the diff
  • Handle member_entity_ids for external groups (otherwise we get errors like error updating IdentityGroup "a6c92cfa-d0b0-8d61-7122-a59cf1123a45": cannot set 'member_entity_ids' on external groups)

Other fixes:

  • Fix typo in vault_identity_group_alias

Can't really write tests for the diff suppression without "logging in" with a real auth backend. It works in my local tests, though.

Before:

  ~ vault_identity_group.admins
      member_entity_ids.#: "1" => "0"
      member_entity_ids.0: "2b83bd73-954c-7b4d-b102-bec997008baf" => ""

  ~ vault_identity_group.users
      member_entity_ids.#: "1" => "0"
      member_entity_ids.0: "2b83bd73-954c-7b4d-b102-bec997008baf" => ""

After:

No changes. Infrastructure is up-to-date.

@ghost ghost added the size/XS label Feb 13, 2019
@lawliet89 lawliet89 changed the title Fix vault_identity_group not updating policies while reading the resource Fix some issues with vault_identity_group Feb 13, 2019
@ghost ghost added size/S and removed size/XS labels Feb 13, 2019
@ghost ghost added size/M and removed size/S labels Feb 13, 2019
Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lawliet89 thank you for working on this! Looks great!

One question - should there be any doc updates accompanying this?

@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Feb 19, 2019
@lawliet89
Copy link
Contributor Author

The entire resource is undocumented. Is it ok for that to be done in a separate PR?

@lawliet89
Copy link
Contributor Author

That's done in #311 .

Copy link
Contributor

@tyrannosaurus-becks tyrannosaurus-becks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lawliet89 I saw and approved the docs PR. Thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit 3298d0d into hashicorp:master Feb 20, 2019
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Fix some issues with `vault_identity_group`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants