Skip to content

Commit

Permalink
aws/ec2rolecreds: Fix security creds path to include trailing slash (#…
Browse files Browse the repository at this point in the history
…356)

Fixes the EC2 Instance Metadata Service client to no longer squash the trailing slash when requesting instance metadata. Also, fixes the iamSecurityCredsPath var to include a trailing slash preventing redirects when making requests to the EC2 Instance Metadata service.

Fix #351
Related to #351
  • Loading branch information
jasdel authored Aug 20, 2019
1 parent 30227e2 commit 8d7fdba
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 19 deletions.
6 changes: 3 additions & 3 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
### SDK Enhancements

### SDK Bugs
* `private/model`: Handles empty map vs unset map behavior in send request ([#337](https://github.com/aws/aws-sdk-go-v2/pull/337))
* Updates shape marshal model to handle the empty map vs nil map behavior. Also adds a test case to assert behavior when a user sends an empty map vs unset map.
* Fixes [#332](https://github.com/aws/aws-sdk-go-v2/issues/332)
* `aws/ec2rolecreds`: Fix security creds path to include trailing slash ([#356](https://github.com/aws/aws-sdk-go-v2/pull/356))
* Fixes the iamSecurityCredsPath var to include a trailing slash preventing redirects when making requests to the EC2 Instance Metadata service.
* Fixes [#351](https://github.com/aws/aws-sdk-go-v2/issues/351)
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func TestClientDisableIMDS(t *testing.T) {

cfg := unit.Config()
cfg.LogLevel = aws.LogDebugWithHTTPBody
cfg.Logger = t

svc := ec2metadata.New(cfg)
resp, err := svc.Region()
Expand Down
14 changes: 11 additions & 3 deletions aws/ec2metadata/api_ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c *Client) GetMetadata(p string) (string, error) {
op := &aws.Operation{
Name: "GetMetadata",
HTTPMethod: "GET",
HTTPPath: path.Join("/", "meta-data", p),
HTTPPath: suffixPath("/meta-data", p),
}

output := &metadataOutput{}
Expand All @@ -35,7 +35,7 @@ func (c *Client) GetUserData() (string, error) {
op := &aws.Operation{
Name: "GetUserData",
HTTPMethod: "GET",
HTTPPath: path.Join("/", "user-data"),
HTTPPath: "/user-data",
}

output := &metadataOutput{}
Expand All @@ -56,7 +56,7 @@ func (c *Client) GetDynamicData(p string) (string, error) {
op := &aws.Operation{
Name: "GetDynamicData",
HTTPMethod: "GET",
HTTPPath: path.Join("/", "dynamic", p),
HTTPPath: suffixPath("/dynamic", p),
}

output := &metadataOutput{}
Expand Down Expand Up @@ -160,3 +160,11 @@ type EC2InstanceIdentityDocument struct {
RamdiskID string `json:"ramdiskId"`
Architecture string `json:"architecture"`
}

func suffixPath(base, add string) string {
reqPath := path.Join(base, add)
if len(add) != 0 && add[len(add)-1] == '/' {
reqPath += "/"
}
return reqPath
}
39 changes: 28 additions & 11 deletions aws/ec2metadata/api_test.go → aws/ec2metadata/api_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io/ioutil"
"net/http"
"net/http/httptest"
"path"
"strings"
"testing"

Expand Down Expand Up @@ -68,7 +67,7 @@ func TestEndpoint(t *testing.T) {
op := &aws.Operation{
Name: "GetMetadata",
HTTPMethod: "GET",
HTTPPath: path.Join("/", "meta-data", "testpath"),
HTTPPath: "/meta-data/testpath",
}

req := c.NewRequest(op, nil, nil)
Expand All @@ -93,9 +92,29 @@ func TestGetMetadata(t *testing.T) {
c := ec2metadata.New(cfg)

resp, err := c.GetMetadata("some/path")
if err != nil {
t.Fatalf("expect no error, got %v", err)
}
if e, a := "success", resp; e != a {
t.Errorf("expect %v, got %v", e, a)
}
}

func TestGetMetadata_TrailingSlash(t *testing.T) {
server := initTestServer(
"/latest/meta-data/some/path/",
"success", // real response includes suffix
)
defer server.Close()

cfg := unit.Config()
cfg.EndpointResolver = aws.ResolveWithEndpointURL(server.URL + "/latest")

c := ec2metadata.New(cfg)

resp, err := c.GetMetadata("some/path/")
if err != nil {
t.Errorf("expect no error, got %v", err)
t.Fatalf("expect no error, got %v", err)
}
if e, a := "success", resp; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand All @@ -115,9 +134,8 @@ func TestGetUserData(t *testing.T) {
c := ec2metadata.New(cfg)

resp, err := c.GetUserData()

if err != nil {
t.Errorf("expect no error, got %v", err)
t.Fatalf("expect no error, got %v", err)
}
if e, a := "success", resp; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand Down Expand Up @@ -152,7 +170,7 @@ func TestGetUserData_Error(t *testing.T) {

resp, err := c.GetUserData()
if err == nil {
t.Errorf("expect error")
t.Fatalf("expect error")
}
if len(resp) != 0 {
t.Errorf("expect empty, got %v", resp)
Expand All @@ -177,9 +195,8 @@ func TestGetRegion(t *testing.T) {
c := ec2metadata.New(cfg)

region, err := c.Region()

if err != nil {
t.Errorf("expect no error, got %v", err)
t.Fatalf("expect no error, got %v", err)
}
if e, a := "us-west-2", region; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand Down Expand Up @@ -217,7 +234,7 @@ func TestMetadataIAMInfo_success(t *testing.T) {

iamInfo, err := c.IAMInfo()
if err != nil {
t.Errorf("expect no error, got %v", err)
t.Fatalf("expect no error, got %v", err)
}
if e, a := "Success", iamInfo.Code; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand All @@ -244,7 +261,7 @@ func TestMetadataIAMInfo_failure(t *testing.T) {

iamInfo, err := c.IAMInfo()
if err == nil {
t.Errorf("expect error")
t.Fatalf("expect error")
}
if e, a := "", iamInfo.Code; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand Down Expand Up @@ -310,7 +327,7 @@ func TestEC2RoleProviderInstanceIdentity(t *testing.T) {

doc, err := c.GetInstanceIdentityDocument()
if err != nil {
t.Errorf("expect no error, got %v", err)
t.Fatalf("expect no error, got %v", err)
}
if e, a := doc.AccountID, "123456789012"; e != a {
t.Errorf("expect %v, got %v", e, a)
Expand Down
2 changes: 1 addition & 1 deletion aws/ec2rolecreds/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ type ec2RoleCredRespBody struct {
Message string
}

const iamSecurityCredsPath = "/iam/security-credentials"
const iamSecurityCredsPath = "/iam/security-credentials/"

// requestCredList requests a list of credentials from the EC2 service.
// If there are no credentials, or there is an error making or receiving the request
Expand Down
2 changes: 1 addition & 1 deletion aws/ec2rolecreds/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const credsFailRespTmpl = `{

func initTestServer(expireOn string, failAssume bool) *httptest.Server {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/latest/meta-data/iam/security-credentials" {
if r.URL.Path == "/latest/meta-data/iam/security-credentials/" {
fmt.Fprintln(w, "RoleName")
} else if r.URL.Path == "/latest/meta-data/iam/security-credentials/RoleName" {
if failAssume {
Expand Down

0 comments on commit 8d7fdba

Please sign in to comment.