Skip to content

Commit

Permalink
Fully populates aws.Config before calling resolveCredentials (#1682)
Browse files Browse the repository at this point in the history
Fixes a bug in LoadDefaultConfig to correctly assign ConfigSources so all config resolvers have access to the config sources. This fixes the feature/ec2/imds client not having configuration applied via config.LoadOptions such as EC2IMDSClientEnableState
  • Loading branch information
gdavison authored May 6, 2022
1 parent 6074cb3 commit 8afaa75
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 9 deletions.
8 changes: 8 additions & 0 deletions .changelog/4cb8965feb3b48b6afcfa099607e4bb0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "4cb8965f-eb3b-48b6-afcf-a099607e4bb0",
"type": "bugfix",
"description": "Fixes a bug in LoadDefaultConfig to correctly assign ConfigSources so all config resolvers have access to the config sources. This fixes the feature/ec2/imds client not having configuration applied via config.LoadOptions such as EC2IMDSClientEnableState. PR [#1682](https://github.com/aws/aws-sdk-go-v2/pull/1682)",
"modules": [
"config"
]
}
7 changes: 0 additions & 7 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,10 @@ func (cs configs) ResolveAWSConfig(ctx context.Context, resolvers []awsConfigRes

for _, fn := range resolvers {
if err := fn(ctx, &cfg, cs); err != nil {
// TODO provide better error?
return aws.Config{}, err
}
}

var sources []interface{}
for _, s := range cs {
sources = append(sources, s)
}
cfg.ConfigSources = sources

return cfg, nil
}

Expand Down
10 changes: 8 additions & 2 deletions config/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,15 @@ import (
// This should be used as the first resolver in the slice of resolvers when
// resolving external configuration.
func resolveDefaultAWSConfig(ctx context.Context, cfg *aws.Config, cfgs configs) error {
var sources []interface{}
for _, s := range cfgs {
sources = append(sources, s)
}

*cfg = aws.Config{
Credentials: aws.AnonymousCredentials{},
Logger: logging.NewStandardLogger(os.Stderr),
Credentials: aws.AnonymousCredentials{},
Logger: logging.NewStandardLogger(os.Stderr),
ConfigSources: sources,
}
return nil
}
Expand Down
122 changes: 122 additions & 0 deletions config/resolve_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"context"
"errors"
"fmt"
"io/ioutil"
"net/http"
Expand All @@ -15,9 +16,11 @@ import (
"time"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
"github.com/aws/aws-sdk-go-v2/internal/awstesting"
"github.com/aws/aws-sdk-go-v2/service/sso"
"github.com/aws/aws-sdk-go-v2/service/sts"
"github.com/aws/smithy-go"
"github.com/aws/smithy-go/middleware"
smithytime "github.com/aws/smithy-go/time"
)
Expand Down Expand Up @@ -471,3 +474,122 @@ func TestResolveCredentialsCacheOptions(t *testing.T) {
t.Errorf("expect options to be called")
}
}

func TestResolveCredentialsIMDSClient(t *testing.T) {
expectEnabled := func(t *testing.T, err error) {
if err == nil {
t.Fatalf("expect error got none")
}
if e, a := "expected HTTP client error", err.Error(); !strings.Contains(a, e) {
t.Fatalf("expected %v error in %v", e, a)
}
}

expectDisabled := func(t *testing.T, err error) {
var oe *smithy.OperationError
if !errors.As(err, &oe) {
t.Fatalf("unexpected error: %v", err)
} else {
e := errors.Unwrap(oe)
if e == nil {
t.Fatalf("unexpected empty operation error: %v", oe)
} else {
if !strings.HasPrefix(e.Error(), "access disabled to EC2 IMDS") {
t.Fatalf("unexpected operation error: %v", oe)
}
}
}
}

testcases := map[string]struct {
enabledState imds.ClientEnableState
envvar string
expectedState imds.ClientEnableState
expectedError func(*testing.T, error)
}{
"default no options": {
expectedState: imds.ClientDefaultEnableState,
expectedError: expectEnabled,
},

"state enabled": {
enabledState: imds.ClientEnabled,
expectedState: imds.ClientEnabled,
expectedError: expectEnabled,
},
"state disabled": {
enabledState: imds.ClientDisabled,
expectedState: imds.ClientDisabled,
expectedError: expectDisabled,
},

"env var DISABLED true": {
envvar: "true",
expectedState: imds.ClientDisabled,
expectedError: expectDisabled,
},
"env var DISABLED false": {
envvar: "false",
expectedState: imds.ClientEnabled,
expectedError: expectEnabled,
},

"option state enabled overrides env var DISABLED true": {
enabledState: imds.ClientEnabled,
envvar: "true",
expectedState: imds.ClientEnabled,
expectedError: expectEnabled,
},
"option state disabled overrides env var DISABLED false": {
enabledState: imds.ClientDisabled,
envvar: "false",
expectedState: imds.ClientDisabled,
expectedError: expectDisabled,
},
}

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
restoreEnv := awstesting.StashEnv()
defer awstesting.PopEnv(restoreEnv)

var httpClient HTTPClient
if tc.expectedState == imds.ClientDisabled {
httpClient = stubErrorClient{err: fmt.Errorf("expect HTTP client not to be called")}
} else {
httpClient = stubErrorClient{err: fmt.Errorf("expected HTTP client error")}
}

opts := []func(*LoadOptions) error{
WithRetryer(func() aws.Retryer { return aws.NopRetryer{} }),
WithHTTPClient(httpClient),
}

if tc.enabledState != imds.ClientDefaultEnableState {
opts = append(opts,
WithEC2IMDSClientEnableState(tc.enabledState),
)
}

if tc.envvar != "" {
os.Setenv("AWS_EC2_METADATA_DISABLED", tc.envvar)
}

c, err := LoadDefaultConfig(context.TODO(), opts...)
if err != nil {
t.Fatalf("could not load config: %s", err)
}

creds := c.Credentials

_, err = creds.Retrieve(context.TODO())
tc.expectedError(t, err)
})
}
}

type stubErrorClient struct {
err error
}

func (c stubErrorClient) Do(*http.Request) (*http.Response, error) { return nil, c.err }

0 comments on commit 8afaa75

Please sign in to comment.