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

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

Conversation

Madrigal
Copy link
Contributor

Context

EKS (Elastic Kubernetes Service) is a managed service for running Kubernetes clusters. Pods are the smallest deployable units in Kubernetes, consisting of one or more containers, and are managed within a cluster.EKS Pod identity is a new feature of AWS, which allows individual pods to be mapped to a specific IAM roles

For the SDKs, this means that EKS Pod Identity sets AWS_CONTAINER_CREDENTIALS_FULL_URI and AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, which are used to retrieve credentials.

This IAM role must exist within your own AWS account.

use case

Customers want to be able to easily be able to assume the role of another AWS account, since it's a common pattern to have one AWS account managing the EKS cluster and others that host things like DynamoDB tables or S3 buckets. Right now, customers can setup IAM role chaining via code, but they want to be able to do it via profiles, like

[profile other-account]
role_arn = arn:aws:iam::1234567890:role/other-account-role
credential_source = EcsContainer

Problem

Go SDK currently didn't read from AWS_CONTAINER_CREDENTIALS_FULL_URI when resolving credentials from a shared AWS config file, it only read AWS_CONTAINER_CREDENTIALS_RELATIVE_URI. Since this is not what EKS Pod identity sets, the request failed.

Additionally, while we have a SEP that defines the priority between these variables, this wasn't implemented.

…ading a profile with `credential_source`.

Also ensure `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` is always read before it
@Madrigal Madrigal requested a review from a team as a code owner September 13, 2024 23:07
}
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.

@Madrigal Madrigal merged commit 1282c53 into main Sep 16, 2024
12 checks passed
@Madrigal Madrigal deleted the fix-aws-credentials-full-uri-order branch September 16, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants