-
Notifications
You must be signed in to change notification settings - Fork 540
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
add not_before_duration to ssh_secret_backend_role #2019
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mju Thanks for the contribution! Could we please add a changelog entry and documentation for the new field? Here: https://github.com/hashicorp/terraform-provider-vault/blob/main/website/docs/r/ssh_secret_backend_role.html.md
Done. I have two questions.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
@mju Hello, looks like there is an error in the tests:
You can run tests with a command like:
|
vault's default value for this attribute is 30. I'll see how I can make it work. About the Acceptance test, I don't have a https-enabled local vault to use. With
So I think for now I need to rely on the CI tests. |
Since the default is 30, I think you will need to update this to 30: https://github.com/hashicorp/terraform-provider-vault/pull/2019/files#diff-de680eba573f1d8768d49ad251da1d8c74870de89e5e4a4cef27a999c12f226bR51 Maybe we could change this to something other than 30: https://github.com/hashicorp/terraform-provider-vault/pull/2019/files#diff-de680eba573f1d8768d49ad251da1d8c74870de89e5e4a4cef27a999c12f226bR311 to check that the update changes from 30 => NEW_VALUE? |
Weird. I initially used 30s or 30m to start with and a unit test (
Note that it's 30m here. The default is 30s. I'll make a change to make it clear though. I think I might have gotten 30m instead of 30s by luck. |
@fairclothjm I figured out how to run acceptance tests locally. It really should be fixed now. It seems CI tests are not triggered automatically? |
CI does not run for PRs from forks. Thanks for testing them locally!
We do a release roughly once a month. We just did a release last week so approximately a month from now. |
Thanks @mju ! |
add not_before_duration to ssh_secret_backend_role
Description
https://registry.terraform.io/providers/hashicorp/vault/latest/docs/resources/ssh_secret_backend_role currently don't have
https://developer.hashicorp.com/vault/api-docs/v1.12.x/secret/ssh#not_before_duration. I can make a good use of it
and thought it'd be a good idea to add it to this repo so many more people can benefit from it.
To comment on why we don't just use generic_endpoint everywhere,
we found that with generic_endpoint, the terraform plan returns way less information.
I ran the unit tests.
Checklist
Output from acceptance testing:
To run the acceptance tests, run
vault server -dev-tls
first to up a vault server withhttps/tls enabled. Then do the following on the other terminal.
The output looks like
Community Note