Make region optional #431
Make region optional #431
Conversation
| @@ -13,6 +13,7 @@ const MAX_TAG_VALUE_LENGTH = 256; | |||
| const SANITIZATION_CHARACTER = '_'; | |||
| const ROLE_SESSION_NAME = 'GitHubActions'; | |||
| const REGION_REGEX = /^[a-z0-9-]+$/g; | |||
| const AWS_CURRENT_REGION = process.env.AWS_REGION | |||
There was a problem hiding this comment.
or should this be AWS_DEFAULT_REGION ?
There was a problem hiding this comment.
I don't think so; you want to be able to override the default region, which is what AWS_REGION is for
There was a problem hiding this comment.
Thanks a bunch for the submission @nitrocode!
In addition to my comments, could we be sure to update the README? I'd like to document that you don't have to supply this parameter anymore in the README, and explaining the functionality of the environment variables AWS_REGION and AWS_DEFAULT_REGION
| @@ -13,6 +13,7 @@ const MAX_TAG_VALUE_LENGTH = 256; | |||
| const SANITIZATION_CHARACTER = '_'; | |||
| const ROLE_SESSION_NAME = 'GitHubActions'; | |||
| const REGION_REGEX = /^[a-z0-9-]+$/g; | |||
| const AWS_CURRENT_REGION = process.env.AWS_REGION | |||
There was a problem hiding this comment.
We shouldn't set this as a global variable - instead use process.env where the value is needed.
| @@ -264,7 +265,7 @@ async function run() { | |||
| // Get inputs | |||
| const accessKeyId = core.getInput('aws-access-key-id', { required: false }); | |||
| const secretAccessKey = core.getInput('aws-secret-access-key', { required: false }); | |||
| const region = core.getInput('aws-region', { required: true }); | |||
| const region = core.getInput('aws-region', { required: false }) || AWS_CURRENT_REGION; | |||
There was a problem hiding this comment.
Like I mentioned above, we should use process.env.XXX instead of a global variable (after all you're only using it on this line)
To add on to the discussion above regarding AWS_REGION vs AWS_DEFAULT_REGION... why not both? Action input > AWS_REGION > AWS_DEFAULT_REGION should work here no?
|
Closing this as stale. If you wish to reopen, please submit a new PR. |
Issue #, if available:
Description of changes:
This will continue retrieving the region using the
aws-regionbut if it is not provided, it will retrieve it from theAWS_REGIONenvironment variable.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.