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

vault_ssh_secret_backend_role: add algorithm_signer argument #809

Merged
merged 3 commits into from
Oct 12, 2020
Merged

vault_ssh_secret_backend_role: add algorithm_signer argument #809

merged 3 commits into from
Oct 12, 2020

Conversation

andreaso
Copy link
Contributor

@andreaso andreaso commented Jul 5, 2020

The algorithm_signer argument was introduced in Vault 1.4.3. See also hashicorp/vault#9096.

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

Release note for CHANGELOG:

resource/vault_ssh_secret_backend_role: Support new `algorithm_signer` argument

Output from acceptance testing:

$ TESTARGS="--run SSHSecretBackendRole" make testacc
...
=== RUN   TestAccSSHSecretBackendRole_basic
--- PASS: TestAccSSHSecretBackendRole_basic (0.27s)
=== RUN   TestAccSSHSecretBackendRoleOTP_basic
--- PASS: TestAccSSHSecretBackendRoleOTP_basic (0.16s)
=== RUN   TestAccSSHSecretBackendRole_import
--- PASS: TestAccSSHSecretBackendRole_import (0.18s)
...

@andreaso
vault_ssh_secret_backend_role: add algorithm_signer argument
646f528 
The `algorithm_signer` argument was introduced in Vault 1.4.3. See
also hashicorp/vault#9096.
@andreaso andreaso mentioned this pull request Jul 5, 2020
Closed
@andreaso
Copy link
Contributor Author

Hello @catsby. It was suggested in #vault-tool that I might want to ping you about this PR.

@catsby catsby added this to the vNext milestone Sep 15, 2020
@trigrab trigrab mentioned this pull request Oct 9, 2020
Merged
catsby
catsby requested changes Oct 9, 2020
View reviewed changes
Copy link
Member

@catsby catsby left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in review! I think we need to remove the Default at this time. Perhaps when this was originally opened the default was true, but I believe with hashicorp/vault#9824 that's not longer the case (and the docs on vaultproject.io need to be updated, I think)

vault/resource_ssh_secret_backend_role.go Outdated
"algorithm_signer": {
Type: schema.TypeString,
Optional: true,
Default: "ssh-rsa",
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to leave algorithm_signer as Optional: true and Computed: true and remove the default. The documentation on vaultproject.io says ssh-rsa is the default, but I believe that's not actually the case anymore. The API is returning "" now and not returning any defaults

@catsby
Copy link
Member

catsby commented Oct 12, 2020

Thank you for contributing! I made minor updates to account for the ssh-rsa no longer being a default, and I'm going to pull this in now. Thanks!

@catsby catsby merged commit 5a1d754 into hashicorp:master Oct 12, 2020
@andreaso
Copy link
Contributor Author

Thanks!

Any best guest on when there will be a new release of the provider?

dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
@andreaso @catsby
vault_ssh_secret_backend_role: add algorithm_signer argument (hashico…
96a2076 
…rp#809)

* vault_ssh_secret_backend_role: add algorithm_signer argument

The `algorithm_signer` argument was introduced in Vault 1.4.3. See
also hashicorp/vault#9096.

* make algorithm_signer optional, computed

* fix test expectation

Co-authored-by: catsby <clint@ctshryock.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@catsby catsby catsby requested changes

Assignees
No one assigned
Labels
documentation size/XS
Projects
None yet
Milestone
vNext
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@andreaso @catsby