Import role with byop #175
Conversation
Adding support for importing roles, ensured that include_default_policies can only be true for byop roles, and fixed some existing bugs with tests. Also added max_duration field to trustRole resource type, which hopefully wont affect anything.
assume_role_policy.go
Outdated
| } | ||
|
|
||
| //Broken into seperate function to allow for returning of errors. | ||
| func UnmarshalAndMarshal(oldPolicy *string, newPolicy *string) error { |
There was a problem hiding this comment.
A couple things here:
First, to make this function more reusable, why don't we instead refactor this to just act on a single policy string and then call it twice: once for each policy
Second, I noticed this function does the old-school modify-my-inputs-in-place tactic which can be weird to debug down the line. Sometimes when the inputs are really large people still do this for performance reasons but for our case I'd be fine sacrificing that tiny bit of performance to save us a few headaches down the road. Consider returning the new string containing JSON rather than modifying the input
| "max_session_duration_in_seconds": { | ||
| Type: schema.TypeInt, | ||
| Default: 3600, | ||
| Optional: true, | ||
| }, |
There was a problem hiding this comment.
If we're adding this field to trust roles, we should make sure that it works. Right now the resourceAlksIamTrustRoleCreate function doesn't read this field and pass it along in the API call to ALKS Go so it's effectively a broken field. The reason you probably had to add this was because alks_iamtrustrole shares the same read, update, and delete functions as the alks_iamrole resource, but it still has its own create function so that function needs to be updated
Description
Adds support for importing roles. Enforces that include_default_policies can only be true if using role_type. Fixes some random test failures.
Rally #US879130: ALKS TFP - Allow users to import ALKS IAM Roles
DE276723: ALKS TFP: Remove whitespace and check for valid JSON in Resource Schema
Type of change
How has this been tested?
Steps: