Skip to content
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

aws/credentials/ec2rolecreds: Avoid unnecessary redirect #2037

Merged
merged 3 commits into from
Jul 12, 2018

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Jul 6, 2018

Updates #2002 to preserve the trailing slash on EC2 Metadata requests. This removes the unnecessary redirect for /latest/meta-data/iam/security-credentials/.


hasTrailing := strings.HasSuffix(elems[len(elems)-1], "/")
str := path.Join(elems...)
if hasTrailing && str != "/" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be str[len(str)-1] != '/'?

@jasdel jasdel merged commit 3cec29e into aws:master Jul 12, 2018
@jasdel jasdel deleted the pr2002 branch July 12, 2018 20:32
@awstools awstools mentioned this pull request Jul 12, 2018
@stefansundin
Copy link
Contributor

@jasdel @xibz Just a reminder that this bug also affects v2. I'll let you two fix it there.

@stefansundin
Copy link
Contributor

I can also confirm that v1.14.26 works as expected in my use case.

Before:

10.40.11.67 - - [12/Jul/2018:14:59:35 PDT] "GET /latest/meta-data/iam/security-credentials HTTP/1.1" 301 0
- -> /latest/meta-data/iam/security-credentials
10.40.11.67 - - [12/Jul/2018:14:59:35 PDT] "GET /latest/meta-data/iam/security-credentials/ HTTP/1.1" 200 4
http://169.254.169.254/latest/meta-data/iam/security-credentials -> /latest/meta-data/iam/security-credentials/
10.40.11.67 - - [12/Jul/2018:14:59:35 PDT] "GET /latest/meta-data/iam/security-credentials/role HTTP/1.1" 200 611
- -> /latest/meta-data/iam/security-credentials/role

v1.14.26:

10.40.11.67 - - [12/Jul/2018:14:59:47 PDT] "GET /latest/meta-data/iam/security-credentials/ HTTP/1.1" 200 4
- -> /latest/meta-data/iam/security-credentials/
10.40.11.67 - - [12/Jul/2018:14:59:47 PDT] "GET /latest/meta-data/iam/security-credentials/role HTTP/1.1" 200 611
- -> /latest/meta-data/iam/security-credentials/role

@jasdel
Copy link
Contributor Author

jasdel commented Jul 12, 2018

Thanks for the update @stefansundin I created aws-sdk-go-v2#198 to track this work being ported to the V2 SDK.

SoManyHs added a commit to SoManyHs/amazon-ecs-cli that referenced this pull request Aug 2, 2018
xibz pushed a commit to xibz/aws-sdk-go that referenced this pull request Sep 12, 2018
Updates aws#2002 to preserve the trailing slash on EC2 Metadata requests. This removes the unnecessary redirect for /latest/meta-data/iam/security-credentials/.
usrenmae pushed a commit to usrenmae/aws-sdk-go that referenced this pull request Sep 23, 2018
Updates aws#2002 to preserve the trailing slash on EC2 Metadata requests. This removes the unnecessary redirect for /latest/meta-data/iam/security-credentials/.
@diehlaws diehlaws added needs-review This issue or pull request needs review from a core team member. and removed review-needed labels Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review This issue or pull request needs review from a core team member.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants