Repository navigation
Support account-ID based endpoints for IMDS and container credentials - #7513
S-Saranya1 wants to merge 5 commits into
Conversation
| "type": "feature", | ||
| "category": "AWS SDK for Java v2", | ||
| "contributor": "", | ||
| "description": "Add support for account-ID based endpoints when using IMDS credential provider. The SDK now sends the `X-amzn-extended: 1` header on IMDS credential requests and parses the `AccountId` field from the response when available." |
There was a problem hiding this comment.
Should this describe more specifically what will change for users of container credentials (ECS/EKS)? Wouldn't the endpoint change for them as well? If they are allowlisting traffic, adding the account ID to the hostname may break that; the effect on container credential providers isn't called out here.
There was a problem hiding this comment.
Good point! ECS and EKS don't return AccountId today, so there's no change for container users right now. If they start returning it, the SDK will pick it up, so good to update the changelog to mention about it, and added tests for container credentials as well for this.
|
|
||
| /** | ||
| * Test that loadCredentials does not fail when response contains unexpected/unknown fields. | ||
| * This verifies the SEP requirement: "Parsing MUST NOT fail in the presence of unexpected fields." |
| "type": "feature", | ||
| "category": "AWS SDK for Java v2", | ||
| "contributor": "", | ||
| "description": "Add support for account-ID based endpoints when using IMDS credential provider. The SDK now sends the `X-amzn-extended: 1` header on IMDS credential requests and parses the `AccountId` field from the response when available." |
There was a problem hiding this comment.
Might also be worth reiterating how to opt out (if we did that before when adding support).
| .endpoint(URI.create(imdsHostname + SECURITY_CREDENTIALS_RESOURCE | ||
| + securityCredentials[0])) | ||
| .headers(getTokenHeaders(token)) | ||
| .headers(getCredentialHeaders(token)) |
There was a problem hiding this comment.
Not introduced by this PR, but related. createEndpointProvider() calls getSecurityCredentials() on every refresh, so the profile name is looked up each time. The spec expects the name to be stored after the first lookup. The shared test suite expects one profile GET followed by two credential GETs, so it would fail here. Happy for this to be a follow-up issue rather than block this PR.
|
|
||
| private MockSyncHttpClient mockHttpClient; | ||
|
|
||
| @BeforeEach |
There was a problem hiding this comment.
nit: the IMDS endpoint system property is set in @BeforeEach but only cleared in @AfterAll. Could we pair them (@BeforeEach/@AfterEach, or set it once in @BeforeAll)? Also, should this test pin accountIdEndpointMode, or clear AWS_ACCOUNT_ID_ENDPOINT_MODE, so a value from the environment can't change the endpoint assertions?
| import com.github.tomakehurst.wiremock.junit5.WireMockExtension; | ||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import software.amazon.awssdk.http.AbortableInputStream; |
There was a problem hiding this comment.
nit: import order. The software.amazon.awssdk.http.* imports come before the org.junit.* ones. Checkstyle's CustomImportOrder flags this.
|
One gap before approval: the spec ships a standardized JSON test suite ( |
Motivation and Context
EC2 introduced an opt-in header (
X-amzn-extended: 1) on the existing IMDS credentials path that returns the Account ID in the response. This enables account ID based endpoint resolution for services that support it.The container credentials provider shares the same response parser, so it will also pick up
AccountIdwhen the container credentials endpoint returns it. ECS and EKS don't return it today, so there's no change for container users yet.Modifications
X-amzn-extended: 1header on the credential GET request (not on token PUT or profile discovery GET). Returns immutable header map.AccountIdfrom the credentials response and attach it to the resolved credentials. Gracefully handles absence (null when not present).No changes to endpoint resolution, the existing
accountIdEndpointModepipeline already handles Account ID when available on credentials.Testing
Added these tests -
AccountIdis returned from IMDS and populated on the resolved credentials, and that credentials still resolve withoutAccountIdwhen it is not returned.Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License