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 issue where group member entities were lost #1409

Merged
merged 2 commits into from
Apr 12, 2022

Conversation

benashz
Copy link
Contributor

@benashz benashz commented Apr 8, 2022

The identity_group resource was removing the member_entity_ids when
configured with external_member_entity_ids. In this case the
member_entity_ids are handled externally and should not be modified by
the identity_group resource. The issue only manifested on resource
update when any of the tracked fields had changes.

Fixes:

  • completely re-work the tests for
    identity_group_member_entity_ids, making them more reliable
  • add test cases that include changing a identity_group's group_name,
    which triggers the issue in the identity_group resource
  • make set type operations simpler
  • numerous other fixes

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Release note for CHANGELOG:


Output from acceptance testing:

$ make testacc TESTARGS='-v -test.run TestAccIdentityGroupMemberEntity*'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -v -v -test.run TestAccIdentityGroupMemberEntity* -timeout 30m ./...

[...]

=== RUN   TestAccIdentityGroupMemberEntityIdsExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsExclusiveEmpty (9.01s)
=== RUN   TestAccIdentityGroupMemberEntityIdsExclusive
--- PASS: TestAccIdentityGroupMemberEntityIdsExclusive (3.00s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty (4.30s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusive
--- PASS: TestAccIdentityGroupMemberEntityIdsNonExclusive (4.30s)
=== RUN   TestAccIdentityGroupMemberEntityIdsDynamic
--- PASS: TestAccIdentityGroupMemberEntityIdsDynamic (6.27s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     27.478s

The identity_group resource was removing the member_entity_ids when
configured with external_member_entity_ids. In this case the
member_entity_ids are handled externally and should not be modified by
the identity_group resource. The issue only manifested on resource
update when any of the tracked fields had changes.

Fixes:
- completely re-work the tests for
  identity_group_member_entity_ids, making them more reliable
- add test cases that include changing a identity_group's group_name,
  which triggers the issue in the identity_group resource
- make set type operations simpler
- numerous other fixes
return fmt.Errorf("resource not found in state")
}
// vaultStateTest
type vaultStateTest struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! 👍🏼

@benashz benashz merged commit 4330724 into main Apr 12, 2022
@benashz benashz deleted the VAULT-5266/fix-lifecycle-grp-memb-entity-ids branch April 12, 2022 14:24
@benashz benashz added this to the 3.5.0 milestone Apr 12, 2022
marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022
The identity_group resource was removing the member_entity_ids when
configured with external_member_entity_ids. In this case the
member_entity_ids are handled externally and should not be modified by
the identity_group resource. The issue only manifested on resource
update when any of the tracked fields had changes.

Fixes:
- completely re-work the tests for
  identity_group_member_entity_ids, making them more reliable
- add test cases that include changing a identity_group's group_name,
  which triggers the issue in the identity_group resource
- make set type operations simpler
- numerous other fixes
- add extra group name change to the dynamic test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants