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.

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

Read AWS_CONTAINER_CREDENTIALS_FULL_URI env variable if set when reading a profile with credential_source. #2790

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Read AWS_CONTAINER_CREDENTIALS_FULL_URI env variable if set when reading a profile with credential_source. #2790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1884bea
Read `AWS_CONTAINER_CREDENTIALS_FULL_URI` env variable if set when re…
Madrigal Sep 13, 2024
db10e85
Add changelog
Madrigal Sep 13, 2024
90c33b5
Fix formatting on test file
Madrigal Sep 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changelog/3230f94ad7814d24b10beaed0739d43c.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"id": "3230f94a-d781-4d24-b10b-eaed0739d43c",
"type": "bugfix",
"description": "Read `AWS_CONTAINER_CREDENTIALS_FULL_URI` env variable if set when reading a profile with `credential_source`. Also ensure `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` is always read before it",
"modules": [
"config"
]
}
15 changes: 9 additions & 6 deletions config/resolve_credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ func resolveCredsFromProfile(ctx context.Context, cfg *aws.Config, envConfig *En
// Get credentials from CredentialProcess
err = processCredentials(ctx, cfg, sharedConfig, configs)

case len(envConfig.ContainerCredentialsEndpoint) != 0:
err = resolveLocalHTTPCredProvider(ctx, cfg, envConfig.ContainerCredentialsEndpoint, envConfig.ContainerAuthorizationToken, configs)

case len(envConfig.ContainerCredentialsRelativePath) != 0:
err = resolveHTTPCredProvider(ctx, cfg, ecsContainerURI(envConfig.ContainerCredentialsRelativePath), envConfig.ContainerAuthorizationToken, configs)

case len(envConfig.ContainerCredentialsEndpoint) != 0:
err = resolveLocalHTTPCredProvider(ctx, cfg, envConfig.ContainerCredentialsEndpoint, envConfig.ContainerAuthorizationToken, configs)

default:
err = resolveEC2RoleCredentials(ctx, cfg, configs)
}
Expand Down Expand Up @@ -355,10 +355,13 @@ func resolveCredsFromSource(ctx context.Context, cfg *aws.Config, envConfig *Env
cfg.Credentials = credentials.StaticCredentialsProvider{Value: envConfig.Credentials}

case credSourceECSContainer:
if len(envConfig.ContainerCredentialsRelativePath) == 0 {
return fmt.Errorf("EcsContainer was specified as the credential_source, but 'AWS_CONTAINER_CREDENTIALS_RELATIVE_URI' was not set")
if len(envConfig.ContainerCredentialsRelativePath) != 0 {
return resolveHTTPCredProvider(ctx, cfg, ecsContainerURI(envConfig.ContainerCredentialsRelativePath), envConfig.ContainerAuthorizationToken, configs)
}
if len(envConfig.ContainerCredentialsEndpoint) != 0 {
return resolveLocalHTTPCredProvider(ctx, cfg, envConfig.ContainerCredentialsEndpoint, envConfig.ContainerAuthorizationToken, configs)
}
return resolveHTTPCredProvider(ctx, cfg, ecsContainerURI(envConfig.ContainerCredentialsRelativePath), envConfig.ContainerAuthorizationToken, configs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't really a fallback but rather a function call after checking the args are valid

if (!isValid(a)) {
    return error
}
func(a)

This is however building an ECS path, and you are correct that we could pass an empty path to ecsContainerURI to resolve to the base path directly. However, the SEP doesn't like this

If none of the above sources (the two env variables) yield a value, the provider MUST fail to resolve indicating as such.

return fmt.Errorf("EcsContainer was specified as the credential_source, but neither 'AWS_CONTAINER_CREDENTIALS_RELATIVE_URI' or AWS_CONTAINER_CREDENTIALS_FULL_URI' was set")

default:
return fmt.Errorf("credential_source values must be EcsContainer, Ec2InstanceMetadata, or Environment")
Expand Down
150 changes: 148 additions & 2 deletions config/resolve_credentials_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,51 @@ func swapECSContainerURI(path string) func() {
}
}

func setupCredentialsEndpoints(t *testing.T) (aws.EndpointResolverWithOptions, func()) {
const ecsFullPathResponse = `{
"Code": "Success",
"Type": "AWS-HMAC",
"AccessKeyId": "ecs-full-path-access-key",
"SecretAccessKey": "ecs-full-path-ecs-secret-key",
"Token": "ecs-full-path-token",
"Expiration": "2100-01-01T00:00:00Z",
"LastUpdated": "2009-11-23T00:00:00Z"
}`

const assumeRoleRespEcsFullPathMsg = `
<AssumeRoleResponse xmlns="https://sts.amazonaws.com/doc/2011-06-15/">
<AssumeRoleResult>
<AssumedRoleUser>
<Arn>arn:aws:sts::account_id:assumed-role/role/session_name</Arn>
<AssumedRoleId>AKID:session_name</AssumedRoleId>
</AssumedRoleUser>
<Credentials>
<AccessKeyId>AKID-Full-Path</AccessKeyId>
<SecretAccessKey>SECRET-Full-Path</SecretAccessKey>
<SessionToken>SESSION_TOKEN-Full-Path</SessionToken>
<Expiration>%s</Expiration>
</Credentials>
</AssumeRoleResult>
<ResponseMetadata>
<RequestId>request-id</RequestId>
</ResponseMetadata>
</AssumeRoleResponse>
`

var ecsMetadataServerURL string

func setupCredentialsEndpoints() (aws.EndpointResolverWithOptions, func()) {
ecsMetadataServer := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/ECS" {
w.Write([]byte(ecsResponse))
// Used when we specify a full path instead of relative path
} else if r.URL.Path == "/ECSFullPath" {
w.Write([]byte(ecsFullPathResponse))
} else {
w.Write([]byte(""))
}
}))
ecsMetadataServerURL = ecsMetadataServer.URL
resetECSEndpoint := swapECSContainerURI(ecsMetadataServer.URL)

ec2MetadataServer := httptest.NewServer(http.HandlerFunc(
Expand Down Expand Up @@ -74,6 +110,15 @@ func setupCredentialsEndpoints(t *testing.T) (aws.EndpointResolverWithOptions, f

switch form.Get("Action") {
case "AssumeRole":
if val, ok := r.Header["X-Amz-Security-Token"]; ok {
if val[0] == "ecs-full-path-token" {
w.Write([]byte(fmt.Sprintf(
assumeRoleRespEcsFullPathMsg,
smithytime.FormatDateTime(time.Now().
Add(15*time.Minute)))))
return
}
}
w.Write([]byte(fmt.Sprintf(
assumeRoleRespMsg,
smithytime.FormatDateTime(time.Now().
Expand Down Expand Up @@ -394,7 +439,7 @@ func TestSharedConfigCredentialSource(t *testing.T) {
os.Setenv("AWS_PROFILE", c.envProfile)
}

endpointResolver, cleanupFn := setupCredentialsEndpoints(t)
endpointResolver, cleanupFn := setupCredentialsEndpoints()
defer cleanupFn()

var cleanup func()
Expand Down Expand Up @@ -604,6 +649,107 @@ func TestResolveCredentialsIMDSClient(t *testing.T) {
}
}

func TestResolveCredentialsEcsContainer(t *testing.T) {
testCases := map[string]struct {
expectedAccessKey string
expectedSecretKey string
envVar map[string]string
configFile string
}{
"only relative ECS URI set": {
expectedAccessKey: "ecs-access-key",
expectedSecretKey: "ecs-secret-key",
envVar: map[string]string{
"AWS_CONTAINER_CREDENTIALS_RELATIVE_URI": "/ECS",
},
},
"only full ECS URI set": {
expectedAccessKey: "ecs-full-path-access-key",
expectedSecretKey: "ecs-full-path-ecs-secret-key",
envVar: map[string]string{
"AWS_CONTAINER_CREDENTIALS_FULL_URI": "placeholder-replaced-at-runtime",
},
},
"relative ECS URI has precedence over full": {
expectedAccessKey: "ecs-access-key",
expectedSecretKey: "ecs-secret-key",
envVar: map[string]string{
"AWS_CONTAINER_CREDENTIALS_RELATIVE_URI": "/ECS",
"AWS_CONTAINER_CREDENTIALS_FULL_URI": "placeholder-replaced-at-runtime",
},
},
"credential source only relative ECS URI set": {
expectedAccessKey: "AKID",
expectedSecretKey: "SECRET",
envVar: map[string]string{
"AWS_PROFILE": "ecscontainer",
"AWS_CONTAINER_CREDENTIALS_RELATIVE_URI": "/ECS",
},
configFile: filepath.Join("testdata", "config_source_shared"),
},
"credential source only full ECS URI set": {
expectedAccessKey: "AKID-Full-Path",
expectedSecretKey: "SECRET-Full-Path",
envVar: map[string]string{
"AWS_CONTAINER_CREDENTIALS_FULL_URI": "placeholder-replaced-at-runtime",
"AWS_PROFILE": "ecscontainer",
},
configFile: filepath.Join("testdata", "config_source_shared"),
},
"credential source relative ECS URI has precedence over full": {
expectedAccessKey: "AKID",
expectedSecretKey: "SECRET",
envVar: map[string]string{
"AWS_CONTAINER_CREDENTIALS_RELATIVE_URI": "/ECS",
"AWS_CONTAINER_CREDENTIALS_FULL_URI": "placeholder-replaced-at-runtime",
"AWS_PROFILE": "ecscontainer",
},
configFile: filepath.Join("testdata", "config_source_shared"),
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
endpointResolver, cleanupFn := setupCredentialsEndpoints()
defer cleanupFn()
restoreEnv := awstesting.StashEnv()
defer awstesting.PopEnv(restoreEnv)
var sharedConfigFiles []string
if tc.configFile != "" {
sharedConfigFiles = append(sharedConfigFiles, tc.configFile)
}
opts := []func(*LoadOptions) error{
WithEndpointResolverWithOptions(endpointResolver),
WithRetryer(func() aws.Retryer { return aws.NopRetryer{} }),
WithSharedConfigFiles(sharedConfigFiles),
WithSharedCredentialsFiles([]string{}),
}
for k, v := range tc.envVar {
// since we don't know the value of this until the server starts
if k == "AWS_CONTAINER_CREDENTIALS_FULL_URI" {
v = ecsMetadataServerURL + "/ECSFullPath"
}
os.Setenv(k, v)
}
cfg, err := LoadDefaultConfig(context.TODO(), opts...)
if err != nil {
t.Fatalf("could not load config: %s", err)
}
actual, err := cfg.Credentials.Retrieve(context.TODO())
if err != nil {
t.Fatalf("could not retrieve credentials: %s", err)
}
if actual.AccessKeyID != tc.expectedAccessKey {
t.Errorf("expected access key to be %s, got %s", tc.expectedAccessKey, actual.AccessKeyID)
}
if actual.SecretAccessKey != tc.expectedSecretKey {
t.Errorf("expected secret key to be %s, got %s", tc.expectedSecretKey, actual.SecretAccessKey)
}
})
}

}

type stubErrorClient struct {
err error
}
Expand Down
Loading