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.

Sign up for GitHub

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

Jump to bottom

Fix Non-Exclusive Behaviour of identity_group_policies #495

Merged
merged 2 commits into from
Aug 9, 2019
Merged

Fix Non-Exclusive Behaviour of identity_group_policies #495

merged 2 commits into from
Aug 9, 2019

Conversation

lawliet89
Copy link
Contributor

This PR uses the same utility functions added in #488 and should not result in any merge conflicts.

@ghost ghost added the size/L label Aug 8, 2019
@lawliet89 lawliet89 force-pushed the fix-non-exclusive branch 2 times, most recently from ca7a6a6 to 80c148d Compare August 8, 2019 10:35
@ghost ghost added size/XL and removed size/L labels Aug 8, 2019
@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Aug 8, 2019
@tyrannosaurus-becks
Copy link
Contributor

Hi @lawliet89 ! Weirdly, GH is still showing those utility functions in the diff even though I merged them to master. Would you be willing to pull master and push it up to fix the diff?

@ghost ghost added size/L and removed size/XL labels Aug 8, 2019
@lawliet89
Copy link
Contributor Author

@tyrannosaurus-becks I have done a rebase with master.

tyrannosaurus-becks
tyrannosaurus-becks reviewed Aug 8, 2019
View reviewed changes
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.

This looks great! My only substantive question is the one regarding the IsExpiredTokenErr.

vault/resource_identity_group.go
if err != nil {
return nil, err
}
if resp == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch here that the response can be nil.

vault/resource_identity_group.go Show resolved Hide resolved
vault/resource_identity_group_policies.go Show resolved Hide resolved
tyrannosaurus-becks
tyrannosaurus-becks approved these changes Aug 9, 2019
View reviewed changes
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.

🌠 Thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit 48d4153 into hashicorp:master Aug 9, 2019
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
@tyrannosaurus-becks
Merge pull request hashicorp#495 from lawliet89/fix-non-exclusive
077c4f7 
Fix Non-Exclusive Behaviour of `identity_group_policies`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyrannosaurus-becks tyrannosaurus-becks tyrannosaurus-becks approved these changes

@kalafut kalafut Awaiting requested review from kalafut

Assignees

@tyrannosaurus-becks tyrannosaurus-becks

Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lawliet89 @tyrannosaurus-becks