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

Correctly handle 404 response in approle secret id request #1242

Merged
merged 1 commit into from
Dec 14, 2021
Merged

Correctly handle 404 response in approle secret id request #1242

merged 1 commit into from
Dec 14, 2021

Conversation

rnurgaliyev
Copy link
Contributor

@rnurgaliyev rnurgaliyev commented Nov 30, 2021

After hashicorp/vault#12788 was merged and released in 1.9.0, the role/:name/secret-id-accessor/lookup vault server endpoint now returns a 404 status code when the secret_id_accessor cannot be found. This breaks the vault_approle_auth_backend_role_secret_id resource, which will always return an error if 404 is returned.

This PR fixes this issue.

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

Closes #1241

Release note for CHANGELOG:

Fix crashing approle_auth_backend_role_secret_id resource when vault server returns 404.

Output from acceptance testing:

$ make dev testacc TESTARGS='-v -test.run TestAccAppRoleAuthBackendRole'
==> Checking that code complies with gofmt requirements...
go build -o terraform-provider-vault
mv terraform-provider-vault ~/.terraform.d/plugins/
TF_ACC=1 go test $(go list ./...) -v -v -test.run TestAccAppRoleAuthBackendRole -timeout 20m

[...]

=== RUN   TestAccAppRoleAuthBackendRoleID_basic
--- PASS: TestAccAppRoleAuthBackendRoleID_basic (3.39s)
=== RUN   TestAccAppRoleAuthBackendRoleID_customID
--- PASS: TestAccAppRoleAuthBackendRoleID_customID (3.15s)
=== RUN   TestAccAppRoleAuthBackendRoleSecretID_basic
--- PASS: TestAccAppRoleAuthBackendRoleSecretID_basic (1.87s)
=== RUN   TestAccAppRoleAuthBackendRoleSecretID_wrapped
--- PASS: TestAccAppRoleAuthBackendRoleSecretID_wrapped (1.90s)
=== RUN   TestAccAppRoleAuthBackendRoleSecretID_wrapped_namespace
--- PASS: TestAccAppRoleAuthBackendRoleSecretID_wrapped_namespace (3.20s)
=== RUN   TestAccAppRoleAuthBackendRoleSecretID_full
--- PASS: TestAccAppRoleAuthBackendRoleSecretID_full (1.92s)
=== RUN   TestAccAppRoleAuthBackendRole_import
--- PASS: TestAccAppRoleAuthBackendRole_import (2.34s)
=== RUN   TestAccAppRoleAuthBackendRole_basic
--- PASS: TestAccAppRoleAuthBackendRole_basic (1.82s)
=== RUN   TestAccAppRoleAuthBackendRole_update
--- PASS: TestAccAppRoleAuthBackendRole_update (3.12s)
=== RUN   TestAccAppRoleAuthBackendRole_full
--- PASS: TestAccAppRoleAuthBackendRole_full (1.79s)
=== RUN   TestAccAppRoleAuthBackendRole_fullUpdate
--- PASS: TestAccAppRoleAuthBackendRole_fullUpdate (4.42s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     29.282s

@hashicorp-cla
Copy link

hashicorp-cla commented Nov 30, 2021

CLA assistant check
All committers have signed the CLA.

@stefreak
Copy link

stefreak commented Dec 14, 2021

@benashz this is a huge blocker for us, cant you make a small bugfix release with this patch instead of planning it for a future major release?

Our Terraform CI pipeline does not run anymore without errors if we do not perform a manual intervention removing the approle secret id from the terraform state 😭

It would be super highly appreciated. Thanks for all the hard work even if you can't make a bugfix release 👍

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 9e1deea into hashicorp:main Dec 14, 2021
@benashz
Copy link
Contributor

benashz commented Dec 14, 2021

@benashz this is a huge blocker for us, cant you make a small bugfix release with this patch instead of planning it for a future major release?

Understood.

Our Terraform CI pipeline does not run anymore without errors if we do not perform a manual intervention removing the approle secret id from the terraform state 😭

It would be super highly appreciated. Thanks for all the hard work even if you can't make a bugfix release 👍

Thanks for that. Unfortunately we already have quite a few changes teed up for 3.1.0, so I don't think a patch release is in the cards at this point. I'll see if we can reduce a bit of 3.1.0's scope so we can get it out sooner!

@stefreak
Copy link

Thank you a lot

@rnurgaliyev rnurgaliyev deleted the fix-approle-http-404 branch April 1, 2022 08:52
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.

vault_approle_auth_backend_role_secret_id does not correctly handle 404 response
4 participants