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
feat(aws): allow service and region to be specified in aws options. #2741
base: master
Are you sure you want to change the base?
Conversation
apologies. I did run tests locally, but there appears to be a problem. I'm looking into it. |
There we go! |
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.
This looks good to me overall 😄
@@ -1364,6 +1364,8 @@ Request.prototype.aws = function (opts, now) { | |||
host: self.uri.host, | |||
path: self.uri.path, | |||
method: self.method, | |||
service: opts.service, |
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.
Is it expected that custom domains would only be available for version 4 signing?
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.
I'm not positive what service
and region
might be used for other than API Gateway. API Gateway only supports version 4 though.
👍 for merging this, working around it currently. |
When can we get this merged? we need 'region' to sign AWS API Gateway url with custom DNS |
When this will be merged? I need to sign4 request to API Gateway, and the service param is needed to create the signature. |
Please merge |
we need this feature. Please merge |
PR Checklist:
npm test
locally and all tests are passing.PR Description
This PR picks up where #2188 and #2339 left off to allow aws signing for API Gateway requests which require service and region as part of the signature. aws4 supports adding both
service
andregion
so this is a simple change.Many thanks to @NewGyu who originally wrote this code which I've simply applied to the latest master!