Update signer v1 example for AOSS #252
Conversation
There was a problem hiding this comment.
Thanks for taking a look @dblock . I'll address you comments asap.
|
@dblock addressed your concerns. |
There was a problem hiding this comment.
Thank you @harrisonhjones for adding this!
It would be good to add this to the user guide as well: https://github.com/opensearch-project/opensearch-go/blob/main/USER_GUIDE.md#aws-sdk-v1
signer/aws/aws.go
Outdated
| @@ -22,54 +24,85 @@ import ( | |||
| v4 "github.com/aws/aws-sdk-go/aws/signer/v4" | |||
| ) | |||
|
|
|||
| // OpenSearchService AWS OpenSearchService Name | |||
| // OpenSearchService Amazon OpenSearch Service Name | |||
There was a problem hiding this comment.
You could keep it as "Amazon OpenSearch Service" and for L30 as "Amazon OpenSearch Serverless".
There was a problem hiding this comment.
I am likely misreading your comment but is this not what I'm doing? If not, can you clarify what you are looking for here?
There was a problem hiding this comment.
Also:
- Do we want to export these constants? They aren't exported in the v2 signer.
- If we do perhaps they should be
OpenSearchServiceNameandOpenSearchServerlessNameto clarify they areNames.
There was a problem hiding this comment.
Could even go further and create a custom Name type and have NewSignerWithService accept a service Name as a param.
Just a few ideas. I'm flexible.
signer/aws/aws.go
Outdated
| // OpenSearchServerless Amazon OpenSearch Serverless Name | ||
| const OpenSearchServerless = "aoss" | ||
|
|
||
| const emptyBodySHA256 = "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824" |
There was a problem hiding this comment.
You can skip the empty body hash since the signer assigns the empty body hash if the request body is null, looking at https://github.com/aws/aws-sdk-go/blob/main/aws/signer/v4/v4.go#L708
There was a problem hiding this comment.
As far as I can tell that's only if includeSHA256Header is true which isn't for aoss, only some s3 services.
There was a problem hiding this comment.
I'm good with this! @VachaShah has a good point that the signing code already signs an empty body, so let's avoid doing it twice?
+1 on users guide and CHANGELOG please?
|
Re-created but using a proper feature branch: #259 |
Description
Like #216 but for the v1 signer.
Issues Resolved
I did not open one but could if needed. Originally tried solving it via aws/aws-sdk-go#4761 but this way is more "right".
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.