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

Expand PR899 to also cover Entity Groups #1010

Merged
merged 1 commit into from
Jul 23, 2021
Merged

Expand PR899 to also cover Entity Groups #1010

merged 1 commit into from
Jul 23, 2021

Conversation

yermulnik
Copy link
Contributor

@yermulnik yermulnik commented Mar 23, 2021 •
edited
Loading

  • [resource_identity_entity.go] Log Wrote IdentityEntity only when resp != nil
  • [resource_identity_group.go] Catch empty response in identityGroupCreate() and prompt to import existing resource
  • [resource_identity_group.go] Add identityGrpupNamePath() function to align with resource_identity_entity.go
  • [identity_group.html.md] Add import details

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: #734, #899

Release note for CHANGELOG:

BUG FIXES:
* `resource/vault_identity_group`: Fix nil pointer exception when attempting to create Identity Group which already exists ([#1010](https://github.com/hashicorp/terraform-provider-vault/pull/1010))

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccIdentityGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccIdentityGroup -timeout 120m
?   	github.com/hashicorp/terraform-provider-vault	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/coverage	[no test files]
?   	github.com/hashicorp/terraform-provider-vault/cmd/generate	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/codegen	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/generated	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/decode	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/datasources/transform/encode	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/alphabet	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/role	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/template	(cached) [no tests to run]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/generated/resources/transform/transformation	(cached) [no tests to run]
?   	github.com/hashicorp/terraform-provider-vault/schema	[no test files]
testing: warning: no tests to run
PASS
ok  	github.com/hashicorp/terraform-provider-vault/util	(cached) [no tests to run]
=== RUN   TestAccIdentityGroupAlias
--- PASS: TestAccIdentityGroupAlias (11.17s)
=== RUN   TestAccIdentityGroupAliasUpdate
--- PASS: TestAccIdentityGroupAliasUpdate (19.20s)
=== RUN   TestAccIdentityGroupMemberEntityIdsExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsExclusiveEmpty (29.25s)
=== RUN   TestAccIdentityGroupMemberEntityIdsExclusive
    resource_identity_group_member_entity_ids_test.go:51: &{{{{0 0} 0 0 0 0} [] {0xc0022ffe00} false false false false map[] map[] []  [] 0xc00048d5c0 false false 0 0 testing.tRunner 0xc000103200 1 [17984120 17963375 17970430 17965803 31229432 17024246 17250081] TestAccIdentityGroupMemberEntityIdsExclusive {13839184608674777960 59620865687 0x29b4520} 0 0xc000279b00 0xc0003c8a80 [] {0 0}  <nil> 0} false 0xc000148ff0}
--- SKIP: TestAccIdentityGroupMemberEntityIdsExclusive (0.00s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty (30.76s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusive
    resource_identity_group_member_entity_ids_test.go:121: &{{{{0 0} 0 0 0 0} [] {0xc004708900} false false false false map[] map[] []  [] 0xc00048d5c0 false false 0 0 testing.tRunner 0xc000103200 1 [17984120 17963375 17970430 17965803 31229432 17024246 17250081] TestAccIdentityGroupMemberEntityIdsNonExclusive {13839184641725862504 90385257859 0x29b4520} 0 0xc004172ba0 0xc004172c00 [] {0 0}  <nil> 0} false 0xc000148ff0}
--- SKIP: TestAccIdentityGroupMemberEntityIdsNonExclusive (0.00s)
=== RUN   TestAccIdentityGroupPoliciesExclusive
--- PASS: TestAccIdentityGroupPoliciesExclusive (18.38s)
=== RUN   TestAccIdentityGroupPoliciesNonExclusive
--- PASS: TestAccIdentityGroupPoliciesNonExclusive (19.97s)
=== RUN   TestAccIdentityGroup
--- PASS: TestAccIdentityGroup (9.79s)
=== RUN   TestAccIdentityGroupUpdate
--- PASS: TestAccIdentityGroupUpdate (41.19s)
=== RUN   TestAccIdentityGroupExternal
--- PASS: TestAccIdentityGroupExternal (9.10s)
PASS
ok  	github.com/hashicorp/terraform-provider-vault/vault	190.247s

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 23, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@yermulnik
Copy link
Contributor Author

Build error looks unrelated:

=== RUN   TestTerraformCloudSecretBackend
--- FAIL: TestTerraformCloudSecretBackend (0.09s)
    testing.go:669: Step 0 error: errors during apply:
        
        Error: Error mounting to "tf-test-terraform-cloud-3576202624928866464": Error making API request.
        
        URL: POST http://127.0.0.1:8200/v1/sys/mounts/tf-test-terraform-cloud-3576202624928866464
        Code: 400. Errors:
        
        * plugin not found in the catalog: terraform
        
          on /tmp/tf-test244151559/main.tf line 2:
          (source code not available)
        
        
=== RUN   TestAccResourceTerraformCloudSecretCredsOrganizationBasic
--- SKIP: TestAccResourceTerraformCloudSecretCredsOrganizationBasic (0.00s)
    resource_terraform_cloud_secret_creds_test.go:27: TF_TOKEN and TF_ORGANIZATION must be set. Are currently  and  respectively
=== RUN   TestAccResourceTerraformCloudSecretCredsTeamBasic
--- SKIP: TestAccResourceTerraformCloudSecretCredsTeamBasic (0.00s)
    resource_terraform_cloud_secret_creds_test.go:56: TF_TOKEN, TF_ORGANIZATION, and TF_TEAM_ID must be set. Are currently ,  and  respectively
=== RUN   TestAccResourceTerraformCloudSecretCredsUserBasic
--- SKIP: TestAccResourceTerraformCloudSecretCredsUserBasic (0.00s)
    resource_terraform_cloud_secret_creds_test.go:85: TF_TOKEN and TF_USER_ID must be set. Are currently  and  respectively
=== RUN   TestTerraformCloudSecretRole
--- FAIL: TestTerraformCloudSecretRole (0.12s)
    testing.go:669: Step 0 error: errors during apply:
        
        Error: Error mounting to "tf-test-backend-4025106427670301869": Error making API request.
        
        URL: POST http://127.0.0.1:8200/v1/sys/mounts/tf-test-backend-4025106427670301869
        Code: 400. Errors:
        
        * plugin not found in the catalog: terraform
        
          on /tmp/tf-test081643217/main.tf line 2:
          (source code not available)

@yermulnik yermulnik mentioned this pull request Mar 23, 2021
Merged
@jasonodonnell
Copy link
Contributor

Hi @yermulnik, thanks for the contribution. Yes that CI is unrelated.

We're unable to merge this PR without the CLA being signed. Are you able to do so?

Thanks!

@yermulnik
Copy link
Contributor Author

@jasonodonnell Oops, I somehow missed that comment from the bot. Just went through signing step. Could you please verify whether it completed with success?

@benashz benashz self-requested a review July 23, 2021 14:23
@benashz
Copy link
Contributor

benashz commented Jul 23, 2021

Hi @yermulnik , this PR is looking good. Would you mind updating the changelog message a bit.

Suggestion:

fix: handle preexisting identity_group

* detect preexisting identity group prior to creation, and
  suggest importing the resource in this case.

@yermulnik
Copy link
Contributor Author

yermulnik commented Jul 23, 2021 •
edited
Loading

Hi @benashz,
Yep, no problem. Do you mean to edit my initial message up above (Release note for CHANGELOG:)?
Or you mean the CHANGELOG.md per se? (which I'd be a bit confused with since I don't know what Release version this PR will be a part of)

@benashz
Copy link
Contributor

benashz commented Jul 23, 2021

Yep, no problem. Do you mean to edit my initial message up above (Release note for CHANGELOG:)?

No, I was referring to your commit message:

git commit --amend

Or you mean the CHANGELOG.md per se? (which I'd be a bit confused with since I don't know what Release version this PR will be a part of)

No need to modify CHANGELOG.md, we will take care of that when this PR is merged.

@github-actions github-actions bot added size/S and removed size/XS labels Jul 23, 2021
@yermulnik
Copy link
Contributor Author

@benashz Done

@yermulnik
fix: handle preexisting identity_group
9b38f43 
* detect preexisting identity group prior to creation, and
  suggest importing the resource in this case.
benashz
benashz approved these changes Jul 23, 2021
View reviewed changes
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution to HashiCorp!

@benashz benashz merged commit 7d09bf0 into hashicorp:master Jul 23, 2021
@yermulnik yermulnik deleted the expand_pr899_to_cover_entity_groups branch July 23, 2021 16:32
davidmontoyago pushed a commit to davidmontoyago/terraform-provider-vault that referenced this pull request Aug 17, 2021
@yermulnik @davidmontoyago
fix: handle preexisting identity_group (hashicorp#1010)
654d5ac 
* detect preexisting identity group prior to creation, and
  suggest importing the resource in this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benashz benashz benashz approved these changes

Assignees
No one assigned
Labels
documentation size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yermulnik @hashicorp-cla @jasonodonnell @benashz