-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
provider: add assume_role.0.duration
and deprecate assume_role.0.duration_seconds
#23077
Conversation
69500e9
to
f241a1b
Compare
internal/provider/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
Description: "The duration of the role session, e.g. 1h. Valid time units are ns, us (or µs), ms, s, h, or m.", | ||
ValidateFunc: verify.ValidTimeDuration, |
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.
Q for reviewer: This validation does not allow for empty strings...or should it?
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.
Looking at the validation functions provided by the Plugin SDK, and especially validation.IsRFC3339Time()
, we're probably fine not checking for it.
52ba408
to
4246ecc
Compare
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.
Looks good. I have a few comments
internal/provider/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
Description: "The duration of the role session, e.g. 1h. Valid time units are ns, us (or µs), ms, s, h, or m.", | ||
ValidateFunc: verify.ValidTimeDuration, |
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.
Looking at the validation functions provided by the Plugin SDK, and especially validation.IsRFC3339Time()
, we're probably fine not checking for it.
internal/provider/provider.go
Outdated
Type: schema.TypeString, | ||
Optional: true, | ||
Description: "The duration of the role session, e.g. 1h. Valid time units are ns, us (or µs), ms, s, h, or m.", | ||
ValidateFunc: verify.ValidTimeDuration, |
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.
We could also check for the minimum value of 15 minutes and the absolute maximum of 12 hours
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.
Test output after updates:
--- PASS: TestValidAssumeRoleDuration (0.00s)
Co-authored-by: Graham Davison <gdavison@hashicorp.com>
Co-authored-by: Graham Davison <gdavison@hashicorp.com>
Co-authored-by: Graham Davison <gdavison@hashicorp.com>
…b.com/hashicorp/terraform-provider-aws into f-provider-assume-role-duration-changes
d9eff35
to
1f82a5c
Compare
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.
Looks good! 🚀
This functionality has been released in v4.0.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Community Note
Closes #23028
Output from acceptance testing: