Implement V2 signing and add simpledb. #316

jjeffery · 2015-07-16T11:01:38Z

This pull request adds an implementation of V2 signing and includes the simpledb service.

Resolves #70.

jjeffery · 2015-07-20T03:52:44Z

When I authored this PR i thought it made sense to add a new package for the version 2 signer (internal/signer/v2) alongside the existing v4 package. I also modified the API template generator (internal/model/api/api.go) to use the v2 signer for services where the JSON API metadata specified a version 2 signature.

I know that SimpleDB is not a big priority for AWS, and although to my knowledge it is not officially deprecated, Amazon are not encouraging people to use SimpleDB. For this reason, would it be better to:

Revert the changes made to the API generation code
Move the v2 signer into the services/simpledb package as a private
Customize the SimpleDB service code to use v2 signing inside the initService() function.

The advantages of this approach would be that:

The legacy version 2 signer would not be visible to any other package other than the simpledb package.
Including SimpleDB in the SDK would involve a single, one-line change to internal/model/cli/gen-api/main.go, and all other implementation would be confined to the service/simpledb package itself.
There is no additional complication or complexity to the SDK by including support for SimpleDB.

I'm only too happy to make these changes there is a view that this would be a better implementation.

gaul · 2015-09-07T02:38:38Z

@jjeffery aws-sdk-go should include an optional v2 signer. Some third-party S3 implementations like Ceph and S3Proxy only support v2 signing.

Conflicts: internal/model/api/api.go internal/model/cli/gen-api/main.go

jjeffery · 2015-10-06T08:34:40Z

@andrewgaul: So does this mean you think I should leave the code as is, and not move the v2 signer into the simpledb package as per my previous comment?

gaul · 2015-10-06T12:58:21Z

@jjeffery I think you should leave the code as-is. Once upstream merges this I will submit a PR for configurable S3 v2 signing.

jasdel · 2015-10-06T17:17:25Z

I hanks for the update! I'll get a chance to finish review the PR tonight. Overall it looks good.

I agree for now keeping the v2 signer in the internal/signer package is best. In the future a nice improvement would be to refactor some of the internal packages out of internal to be exposed, signers would be refactored out also so that they could be used standalone and swapping signers.

When temporary credentials are used, the V2 signer should include a "SecurityToken" parameter with the session token.

jasdel · 2015-10-07T13:24:21Z

internal/signer/v2/v2.go

+	if req.HTTPRequest.Method != "POST" {
+		// The V2 signer is legacy and only used for SimpleDB.
+		// Given that SimpleDB only uses POST, this signer is the same.
+		req.Error = errInvalidMethod


We can probably remove this if check since it's not apart of the v2 signing.

While true we only plan to use the signer internally for SimpleDB support. If in the future someone wrote their code to connect to an s3 compatible service with the v2 signer they would only be able to make POST requests.

Fixed -- good point. I have implemented GET signatures. My reading of the V2 signature spec is that only GET and POST methods are supported, so I've changed the test to check for those methods. I have also modified the signature code to support GET, which was pretty straightforward.

jasdel · 2015-10-07T13:55:34Z

Hi @jjeffery thanks for taking the time, and working with us to get this PR built and submitted. It looks pretty solid, and the way the code generation is done works great. Overall it looks good. I only had a few minor comments mentioned above.

I'll also be able to add smoke integration test for SimpleDB once the PR is merged in.

As per PR revue by Jason Del Ponte.

As per PR review by Jason Del Ponte.

In particular, test for additional query parameter (SecurityToken) for temporary credentials with a session.

jjeffery · 2015-10-08T00:45:55Z

Hi @jasdel thanks for reviewing the PR. All good comments. I have actioned your comments and responded to them above. I have also put a little bit more work into testing the V2 signer.

I wouldn't mind putting a bit more work into the V2 tests. I'd like to find a test suite for version 2 similar to the V4 test suite (http://docs.aws.amazon.com/general/latest/gr/signature-v4-test-suite.html), but I have not been successful so far. If I do get my hands on some good test suite data I can upgrade the tests and include in a separate PR.

jasdel · 2015-10-08T16:42:17Z

@jjeffery Thanks a lot for your hard work getting writing the v2 signer so that we can adds the SimpleDB service to the Go SDK.

Implement V2 signing and add simpledb.

DamonYellow · 2018-10-22T02:14:43Z

Such a good news!
And could pls show us a code example to implement the request using V2 signing? @jjeffery

Fixes incorrectly thrown EOF error by s3manager, when upload is performed. Fixes aws#316

Services --- * Synced the V2 SDK with latest AWS service API definitions. SDK Enhancements --- * `aws/endpoints`: Expose DNSSuffix for partitions ([aws#369](aws/aws-sdk-go-v2#369)) * Exposes the underlying partition metadata's DNSSuffix value via the `DNSSuffix` method on the endpoint's `Partition` type. This allows access to the partition's DNS suffix, e.g. "amazon.com". * Fixes [aws#347](aws/aws-sdk-go-v2#347) * `private/protocol`: Add support for parsing fractional timestamp ([aws#367](aws/aws-sdk-go-v2#367)) * Fixes the SDK's ability to parse fractional unix timestamp values and adds tests. * Fixes [aws#365](aws/aws-sdk-go-v2#365) * `aws/ec2metadata`: Add marketplaceProductCodes to EC2 Instance Identity Document ([aws#374](aws/aws-sdk-go-v2#374)) * Adds `MarketplaceProductCodes` to the EC2 Instance Metadata's Identity Document. The ec2metadata client will now retrieve these values if they are available. * Related to: [aws#2781](aws#2781) * `aws`: Adds configurations to the default retryer ([aws#375](aws/aws-sdk-go-v2#375)) * Provides more customization options for retryer by adding a constructor for default Retryer which accepts functional options. Adds NoOpRetryer to support no retry behavior. Exposes members of default retryer. * Updates the underlying logic used by the default retryer to calculate jittered delay for retry. Handles int overflow for retry delay. * Fixes [aws#370](aws/aws-sdk-go-v2#370) * `aws` : Refactors request retry behavior path logic ([aws#384](aws/aws-sdk-go-v2#384)) * Retry utilities now follow a consistent code path. aws.IsErrorRetryable is the primary entry point to determine if a request is retryable. * Corrects sdk's behavior by not retrying errors with status code 501. Adds support for retrying the Kinesis API error, LimitExceededException. * Fixes [aws#372](aws/aws-sdk-go-v2#372), [aws#145](aws/aws-sdk-go-v2#145) SDK Bugs --- * `aws`: Fixes bug in calculating throttled retry delay ([aws#373](aws/aws-sdk-go-v2#373)) * The `Retry-After` duration specified in the request is now added to the Retry delay for throttled exception. Adds test for retry delays for throttled exceptions. Fixes bug where the throttled retry's math was off. * Fixes [aws#45](aws/aws-sdk-go-v2#45) * `aws` : Adds missing sdk error checking when seeking readers ([aws#379](aws/aws-sdk-go-v2#379)) * Adds support for nonseekable io.Reader. Adds support for streamed payloads for unsigned body request. * Fixes [aws#371](aws/aws-sdk-go-v2#371) * `service/s3` : Fixes unexpected EOF error by s3manager ([aws#386](aws/aws-sdk-go-v2#386)) * Fixes bug which threw unexpected EOF error when s3 upload is performed for a file of maximum allowed size * Fixes [aws#316](aws/aws-sdk-go-v2#316) * `private/model` : Fixes generated API Reference docs links being invalid ([387](aws/aws-sdk-go-v2#387)) * Fixes [aws#327](aws/aws-sdk-go-v2#327)

jjeffery added 3 commits July 16, 2015 20:55

Issue aws#70: Implement V2 signing and add simpledb.

724b2cd

golint suggestions

4fed43b

Add support for simpledb's custom error unmarshalling logic

8e1bfa6

jjeffery changed the title ~~Issue #70: Implement V2 signing and add simpledb.~~ Implement V2 signing and add simpledb. Jul 20, 2015

jjeffery added 2 commits October 6, 2015 18:04

Merge branch 'master' of github.com:aws/aws-sdk-go

40a4c53

Conflicts: internal/model/api/api.go internal/model/cli/gen-api/main.go

Fix sampled implementation after breaking changes in upstream.

3cfc969

Include "SecurityToken" parameter in V2 signer.

cbf6ae4

When temporary credentials are used, the V2 signer should include a "SecurityToken" parameter with the session token.

jasdel reviewed Oct 7, 2015
View reviewed changes

jjeffery added 3 commits October 8, 2015 09:09

Remove restriction on POST method for v2 signer.

100f3a1

As per PR revue by Jason Del Ponte.

Use "AKID", "SECRET", "SESSION" for credentials in tests.

7d88949

As per PR review by Jason Del Ponte.

Additional testing for v2 signer.

3bfa471

In particular, test for additional query parameter (SecurityToken) for temporary credentials with a session.

jasdel added a commit that referenced this pull request Oct 8, 2015

Merge pull request #316 from jjeffery/master

8deecfe

Implement V2 signing and add simpledb.

jasdel merged commit 8deecfe into aws:master Oct 8, 2015

gaul mentioned this pull request Oct 11, 2015

aws/signer: Add S3 V2 request signature #400

Closed

skotambkar added a commit to skotambkar/aws-sdk-go that referenced this pull request May 20, 2021

service/s3: Fixes incorrect EOF error by s3manager (aws#386)

2010860

Fixes incorrectly thrown EOF error by s3manager, when upload is performed. Fixes aws#316

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement V2 signing and add simpledb. #316

Implement V2 signing and add simpledb. #316

jjeffery commented Jul 16, 2015

jjeffery commented Jul 20, 2015

gaul commented Sep 7, 2015

jjeffery commented Oct 6, 2015

gaul commented Oct 6, 2015

jasdel commented Oct 6, 2015

jasdel Oct 7, 2015

jjeffery Oct 7, 2015

jasdel commented Oct 7, 2015

jjeffery commented Oct 8, 2015

jasdel commented Oct 8, 2015

DamonYellow commented Oct 22, 2018 •

edited

Loading

Implement V2 signing and add simpledb. #316

Implement V2 signing and add simpledb. #316

Conversation

jjeffery commented Jul 16, 2015

jjeffery commented Jul 20, 2015

gaul commented Sep 7, 2015

jjeffery commented Oct 6, 2015

gaul commented Oct 6, 2015

jasdel commented Oct 6, 2015

jasdel Oct 7, 2015

Choose a reason for hiding this comment

jjeffery Oct 7, 2015

Choose a reason for hiding this comment

jasdel commented Oct 7, 2015

jjeffery commented Oct 8, 2015

jasdel commented Oct 8, 2015

DamonYellow commented Oct 22, 2018 • edited Loading

DamonYellow commented Oct 22, 2018 •

edited

Loading