-
Notifications
You must be signed in to change notification settings - Fork 40
Conversation
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.
LGTM
Are there any tests we want to add for this? |
added one, but not feeling very creative. Would need e2e to make sure I suppose |
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.
Ill comment as I am not 100% sure, but this means we run on all partitions now?
Should we add a config option to define what partitions we want to execute on? i.e by default don't have fetch from gov partitions, as I assume, most users don't have those regions enabled.
Other then that LGTM.
We run on what describeregions return, or regions that are explicitly specified in the hcl. @roneli |
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.
LGTM
To add on to what @disq said: AWS credentials are scoped to a specific partition. So if a user uses a single provider block with multiple accounts in it they could still specify specific regions on a per account basis |
Should fix #806 (untested)