Adds accountId endpoint mode #4984
Conversation
and resolver to aws core
|
| package software.amazon.awssdk.codegen.model.internal; | ||
|
|
||
| /** | ||
| * Represents a generic parameter that can be code generated, but isn't tied to a specific model type |
There was a problem hiding this comment.
What does model type refer to here?
There was a problem hiding this comment.
A model shape that can be a parameter, such as a context parameter. Our existing codegen types are tightly coupled to the model, which didn't work for account id routing mode. Any suggestions for updating the description?
There was a problem hiding this comment.
I think "model shape" as you describe sounds better
| new LocalParameter("accountIdEndpointMode", | ||
| AccountIdEndpointMode.class, | ||
| "Sets the behavior when account ID based endpoints are created. " | ||
| + "See {@link AccountIdEndpointMode} for values")); |
There was a problem hiding this comment.
minor: when referencing classes in a javadoc, we should still use $T placeholder in case the class is not already imported by something else
There was a problem hiding this comment.
Thanks for the reminder, will fix!
| * The AWS account ID associated with the identity resolved for this request. | ||
| */ |
There was a problem hiding this comment.
Is this accurate? The attribute name is "mode" but this says account ID
There was a problem hiding this comment.
Ahh... that could be a typo. Will update
| builder.addStatement("$T identity = $T.joinLikeSync(selectedAuthScheme.identity())", TypeVariableName.get("T"), | ||
| CompletableFutureUtils.class); | ||
| builder.addStatement("$T accountId = null", String.class); | ||
| builder.beginControlFlow("if (identity instanceof $T)", AwsCredentialsIdentity.class); |
There was a problem hiding this comment.
Just curious, will AwsCredentialsIdentity be the only identity type that supports account ID?
There was a problem hiding this comment.
We'd have to look in the crystal ball for that one. Nothing in the pipelines as for now, but it would be good to have another round of discussions here. It's a dilemma to build at the right level - neither adding support that's super generic and never diversified nor being too specific and then getting a broader use case is great.
There was a problem hiding this comment.
If we have to add accountId in the future, we can support it through an interface.
| PREFERRED, | ||
| DISABLED, | ||
| REQUIRED; |
There was a problem hiding this comment.
Can we get javadocs for each value?
cenedhryn
merged commit 091fb43
into
feature/master/accountid-endpoint-routing
* Add accountId endpoint mode enum, system properties, profile property and resolver to aws core * Supports resolving accountId and accountId endpoint mode parameters * Testing accountId and accountId endpoint mode resolution logic * Adds latest rules to accountId custom branch * Sets up SRA auth for DynamoDb in custom branch (not yet active in public) * Adds a temporary functional test for accountId to DynamoDb module * fixing merge conflicts * Minor fixes
…5119) * Adds account ID as a built-in endpoint parameter (#4016) * Refactors credential identity with separate implementing classes (#4024) (#4028) * Adds accountId as a parameter to AWS credentials identity (#4029) * Adds accountId to STS credentials providers (#4040) * Adding accountId support to environment variable credential provider (#4327) * Adds accountId support to process credentials provider (#4332) * Adds account ID support for profile credentials provider sources (#4340) * Adds accountId endpoint mode (#4984) * Adding back existing endpoint files (#5120) * changelog (#5121)
…ws#5119) * Adds account ID as a built-in endpoint parameter (aws#4016) * Refactors credential identity with separate implementing classes (aws#4024) (aws#4028) * Adds accountId as a parameter to AWS credentials identity (aws#4029) * Adds accountId to STS credentials providers (aws#4040) * Adding accountId support to environment variable credential provider (aws#4327) * Adds accountId support to process credentials provider (aws#4332) * Adds account ID support for profile credentials provider sources (aws#4340) * Adds accountId endpoint mode (aws#4984) * Adding back existing endpoint files (aws#5120) * changelog (aws#5121)
Motivation and Context
Account Id endpoint routing has been unblocked by DDB being enabled for SRA identity and auth, thereby resolving credentials before endpoints. Some code changes are required to support this change, because the original code for account Id was written before SRA.
In addition, it adds AccountId Endpoint Mode as a built-in endpoint parameter. AccountIdMode should be configurable in profile file, system settings/env vars and at the client level
Modifications
LocalParameterEndpointParamsKnowledgeIndex, but for now accountIdMode is the only example.AwsEndpointProviderUtils, because this class is copied to all services. Instead, EndpointResolverInterceptorSpec.java generates logic for those parameters inside the provider itself (accountId, accountIdEndpointMode, s3GlobalEndpoint).