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

Fix TTL on PKI certificate create to use string type and not int. #314

Merged
merged 2 commits into from
Mar 29, 2019

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Feb 21, 2019

The certificate creation call can uses a string such as 720h for
setting the TTL and shouldn't use an int.

Testing this locally the certificate creates successfully with the
following validity:

Not Before: Feb 21 15:25:01 2019 GMT
Not After : Mar 23 15:25:31 2019 GMT

Closes #313

The certificate creation call can uses a string such as 720h for
setting the TTL and shouldn't use an int.

Testing this locally the certificate creates successfully with the
following validity:
```
Not Before: Feb 21 15:25:01 2019 GMT
Not After : Mar 23 15:25:31 2019 GMT
```
@ghost ghost added the size/XS label Feb 21, 2019
@tyrannosaurus-becks tyrannosaurus-becks self-assigned this Feb 21, 2019
@tyrannosaurus-becks
Copy link
Contributor

Hey @jrasell! Thanks for submitting this! Would you be willing to add test coverage to those fields? I recently added info on how to do that by adding a CONTRIBUTING.md file.

@cvbarros
Copy link
Contributor

Hello @jrasell , like this one, there has been several bugs regarding the handling of TTL inside the provider. Due to lack of availability, I haven't taken a stab across equalizing all TTL treaments, but that's something I've been planning to do.

If you see #255 changes where this was introduced, there is proper validation for a duration string. This treatment was derived from Vault API code itself, so it matches what upstream expects.

Also, I'd like to call out that a change in schema.TypeInt -> schema.TypeString . would be a breaking change, thus state migration is advised.

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.

@jrasell fantastic! Thank you!

@tyrannosaurus-becks tyrannosaurus-becks merged commit e3fe61c into hashicorp:master Mar 29, 2019
@jrasell jrasell deleted the gh-313 branch March 30, 2019 08:23
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
Fix TTL on PKI certificate create to use string type and not int.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants