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

provider: add assume_role.0.duration and deprecate assume_role.0.duration_seconds #23077

Merged
merged 10 commits into from
Feb 10, 2022
22 changes: 18 additions & 4 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1936,11 +1936,20 @@ func assumeRoleSchema() *schema.Schema {
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"duration": {
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,
Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor Author

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)

ConflictsWith: []string{"assume_role.0.duration_seconds"},
},
"duration_seconds": {
Type: schema.TypeInt,
Optional: true,
Description: "The duration, in seconds, of the role session.",
ValidateFunc: validation.IntBetween(900, 43200),
Type: schema.TypeInt,
Optional: true,
Deprecated: "Use assume_role.0.duration instead",
Description: "The duration, in seconds, of the role session.",
ValidateFunc: validation.IntBetween(900, 43200),
ConflictsWith: []string{"assume_role.0.duration"},
},
"external_id": {
Type: schema.TypeString,
Expand Down Expand Up @@ -2022,6 +2031,11 @@ func endpointsSchema() *schema.Schema {
func expandAssumeRole(m map[string]interface{}) *awsbase.AssumeRole {
assumeRole := awsbase.AssumeRole{}

if v, ok := m["duration"].(string); ok && v != "" {
duration, _ := time.ParseDuration(v)
assumeRole.Duration = duration
}

if v, ok := m["duration_seconds"].(int); ok && v != 0 {
assumeRole.Duration = time.Duration(v) * time.Second
}
Expand Down
11 changes: 11 additions & 0 deletions internal/verify/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,17 @@ func ValidStringIsJSONOrYAML(v interface{}, k string) (ws []string, errors []err
return
}

// ValidTimeDuration validates a string can be parsed as a valid time.Duration
func ValidTimeDuration(v interface{}, k string) (ws []string, errors []error) {
_, err := time.ParseDuration(v.(string))

if err != nil {
errors = append(errors, fmt.Errorf("%q cannot be parsed as a valid time.Duration: %w", k, err))
anGie44 marked this conversation as resolved.
Show resolved Hide resolved
}

return
}

// ValidTypeStringNullableBoolean provides custom error messaging for TypeString booleans
// Some arguments require three values: true, false, and "" (unspecified).
// This ValidateFunc returns a custom message since the message with
Expand Down
52 changes: 52 additions & 0 deletions internal/verify/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,58 @@ import (
"testing"
)

func TestValidTimeDuration(t *testing.T) {
testCases := []struct {
val interface{}
expectedErr *regexp.Regexp
}{
{
val: "",
expectedErr: regexp.MustCompile(`cannot be parsed as a valid time.Duration`),
},
{
val: "1",
expectedErr: regexp.MustCompile(`cannot be parsed as a valid time.Duration`),
},
{
val: "-10h",
},
{
val: "10h",
},
{
val: "1h10m10s",
},
}

matchErr := func(errs []error, r *regexp.Regexp) bool {
// err must match one provided
for _, err := range errs {
if r.MatchString(err.Error()) {
return true
}
}

return false
}

for i, tc := range testCases {
_, errs := ValidTimeDuration(tc.val, "test_property")

if len(errs) == 0 && tc.expectedErr == nil {
continue
}

if len(errs) != 0 && tc.expectedErr == nil {
t.Fatalf("expected test case %d to produce no errors, got %v", i, errs)
}

if !matchErr(errs, tc.expectedErr) {
t.Fatalf("expected test case %d to produce error matching \"%s\", got %v", i, tc.expectedErr, errs)
}
}
}

func TestValidTypeStringNullableBoolean(t *testing.T) {
testCases := []struct {
val interface{}
Expand Down
3 changes: 2 additions & 1 deletion website/docs/index.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,8 @@ In addition to [generic `provider` arguments](https://www.terraform.io/docs/conf

The `assume_role` configuration block supports the following optional arguments:

* `duration_seconds` - (Optional) Number of seconds to restrict the assume role session duration. You can provide a value from 900 seconds (15 minutes) up to the maximum session duration setting for the role.
* `duration` - (Optional, Conflicts with `duration_seconds`) The assume role duration represented as a string such as `300ms`, `-1.5h` or `2h45m`. Valid time units are `ns`, `us` (or `µs`), `ms`, `s`, `m`, `h`.
anGie44 marked this conversation as resolved.
Show resolved Hide resolved
* `duration_seconds` - (Optional, **Deprecated** use `duration` instead) Number of seconds to restrict the assume role session duration. You can provide a value from 900 seconds (15 minutes) up to the maximum session duration setting for the role.
* `external_id` - (Optional) External identifier to use when assuming the role.
* `policy` - (Optional) IAM Policy JSON describing further restricting permissions for the IAM Role being assumed.
* `policy_arns` - (Optional) Set of Amazon Resource Names (ARNs) of IAM Policies describing further restricting permissions for the IAM Role being assumed.
Expand Down