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

Fully populates aws.Config before calling resolveCredentials #1682

Merged
merged 5 commits into from May 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelog/4cb8965feb3b48b6afcfa099607e4bb0.json
@@ -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
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
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
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 }