Skip to content

Updated support for s3 urls. #232

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Updated support for s3 urls. #232

wants to merge 5 commits into from

Conversation

@DonFreed
Copy link

@DonFreed DonFreed commented Jun 26, 2015

Added parsing for additional file types for hisremote.

Added code to incorporate an 'AWS_SESSION_TOKEN' environmental variable. Added this token as a CononicalizedAmzHeader to the signature message and a header incorporating the token. The token is used for granting temporary access.

Changed the request style from virtual hosted to path. The bucket I was accessing had a name that was not DNS compliant.

jmarshall and others added 2 commits May 29, 2015 13:29
@jmarshall
Add support for S3 pseudo-URLs
a5dd1ed 
Rewrite S3 pseudo-URLs to http/https URLs, adding Date and Authorization
headers for Amazon S3.

At present, access keys may be specified in the URL (in the usual URL
authority "[id:secret@]bucket" way) or via the usual AWS_ACCESS_KEY_ID
and AWS_SECRET_ACCESS_KEY environment variables.  It remains to add code
to read them from config files -- probably just ~/.aws/credentials and
~/.awssecret.
@DonFreed
Added parsing for additional file types for hisremote.
87226f5 
Added code to incorporate an 'AWS_SESSION_TOKEN' environmental variable. Added this token as a CononicalizedAmzHeader to the signature message and a header incorporating the token.

Changed the request style from virtual hosted to path.
jmarshall added a commit that referenced this pull request Jul 1, 2015
@jmarshall
Fall back to path-style S3 bucket access if necessary
9344717 
Check whether bucket names are DNS-compliant according to the rules at
http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
Fixes part 3 of #232, hat tip @DonFreed.
@jmarshall
Copy link
Member

jmarshall commented Jul 1, 2015

Thanks for testing this out. There's three parts to this pull request:

  1. hisremote() additions — this is taken care of by the imminent plugin reorganisation
  2. AWS_SESSION_TOKEN and temporary security credentials. Okay, we should probably support this. Is there any documentation around the interaction of the usual AWS_ACCESS_KEY_ID credentials and the temporary ones beyond what's at this page?
  3. Using path-style access. I'd rather use the virtual hosted-style as it appears to be the new modern and to have better redirection possibilities for non-US regions, but we can fall back to path-style where needed. Does 1fec5f3 let you access your old-style-named bucket (and does its is_dns_compliant() look right to you?)?

@jmarshall
Fall back to path-style S3 bucket access if necessary
1fec5f3 
Check whether bucket names are DNS-compliant according to the rules at
http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
Fixes part 3 of samtools#232, hat tip @DonFreed.
@jmarshall jmarshall added this to the 1.3-maybe milestone Jul 6, 2015
@DonFreed
Copy link
Author

DonFreed commented Jul 9, 2015

  1. Great, I am glad to hear this will be taken care of.
  2. Yes, there is a bit of documentation. These temporary credentials are created by a call to GetSessionToken or GetFederationToken using the AWS Security Token Service. For cloud access to the National Database for Autism Research (NDAR), for example, researchers can use their username and password to generate these credentials for access to the database in S3.
  3. Virtual hosted-style access is probably preferred and 1fec5f3 worked in my test case. is_dns_compliant() looked great except for the handling of buckets starting or ending with hyphens.

jmarshall added a commit that referenced this pull request Jul 23, 2015
@jmarshall
Fall back to path-style S3 bucket access if necessary
1574156 
Check whether bucket names are DNS-compliant according to the rules at
http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
Fixes part 3 of #232, hat tip @DonFreed.
@jmarshall
Copy link
Member

jmarshall commented Jul 23, 2015

is_dns_compliant() looked great except for the handling of buckets starting or ending with hyphens.

D'oh! Thanks for spotting this.

jmarshall added a commit that referenced this pull request Oct 23, 2015
@jmarshall
Fall back to path-style S3 bucket access if necessary
b32afba 
Check whether bucket names are DNS-compliant according to the rules at
http://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html
Fixes part 3 of #232, hat tip @DonFreed.
@jmarshall
Copy link
Member

jmarshall commented Oct 29, 2015

The libcurl branch has (finally!) been merged to develop (PR #292).

1 and 3 have been taken care of. 2, AWS_SESSION_TOKEN and temporary security credentials, is still on the list to get added post-merge.

@jmarshall jmarshall mentioned this pull request Oct 29, 2015
Closed
@jmarshall
Copy link
Member

jmarshall commented Nov 24, 2015

I've now extended the AWS_SESSION_TOKEN stuff into PR #303, which looks for credentials (including temporary credentials) in the conventional AWS configuration files. Apologies for the delay.

Please test PR #303 if you've got a few moments. Thanks!

@jmarshall jmarshall closed this Nov 24, 2015
@jmarshall jmarshall removed this from the 1.3-maybe milestone Nov 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DonFreed @jmarshall