feat: assume role support for cross-account#455
Conversation
Signed-off-by: Greg Nyahay <greg.nyahay@anchore.com>
Signed-off-by: Greg Nyahay <greg.nyahay@anchore.com>
| // credentials. The credentials cache resolves them lazily and refreshes them automatically | ||
| // as they expire, which suits the long-running daemon. The role may live in the same or a | ||
| // different AWS account. | ||
| if assumeRoleARN != "" { |
There was a problem hiding this comment.
No test coverage for the new assume-role path.
Consider a small unit test asserting the provider is configured with the ARN + ExternalID when set, and skipped when assumeRoleARN == "".
| } | ||
|
|
||
| // If an assume-role ARN is configured, swap the config's credentials for STS assume-role | ||
| // credentials. The credentials cache resolves them lazily and refreshes them automatically |
There was a problem hiding this comment.
The cross-cycle refresh benefit described here is not actually realized. PeriodicallyGetInventoryReport (pkg/lib.go:24-32) calls GetInventoryReportsForRegion on every ticker tick, and this function rebuilds the entire aws.Config + NewAssumeRoleProvider + NewCredentialsCache from scratch each call. So the cache is discarded every poll cycle and a fresh AssumeRole is issued roughly every polling-interval-seconds (default 300s) regardless of the ~1h STS credential TTL — the automatic refresh across the daemon lifetime never happens.
Functionally harmless (one redundant STS call per cycle at default intervals), but the comment overstates the benefit. If you want the refresh behavior it describes, the config/cache would need to be built once and reused across poll cycles.
bradleyjones
left a comment
bradleyjones
left a comment
There was a problem hiding this comment.
I think the test coverage is worth improving, otherwise it looks good to me would like to give this a live test before we merge but might need help in setting up the environment to do so
2 participants
add support for assuming a role to allow centralizing agents