Add sts, imds, and http credential provider packages - #72
Conversation
| retries: int = _DEFAULT_RETRIES, | ||
| ): | ||
| self._http_client = http_client | ||
| self._timeout = timeout |
There was a problem hiding this comment.
self._timeout seems never used. Should it be applied to the request, or should the parameter be dropped? Or do you want to to add a TODO?
There was a problem hiding this comment.
I'm not sure why this wasn't applied to the request in the first place.
I confirmed we should apply the default timeout by looking at botocore. However, we need to apply the value to both the read timeout and connect timeout. Looks like HTTPRequestConfiguration only supports read_timeout for now which may be why it wasn't used.
I applied the value as the read_timeout in the request configuration and added a TODO to apply the value to connect_timeout once its supported.
| fields.set_field(Field(name="Authorization", values=[auth_token])) | ||
| elif self.ENV_VAR_AUTH_TOKEN in os.environ: | ||
| auth_token = os.environ[self.ENV_VAR_AUTH_TOKEN] | ||
| fields.set_field(Field(name="Authorization", values=[auth_token])) |
There was a problem hiding this comment.
The token goes into the Authorization header unvalidated, while botocore rejects \r and \n:
def _build_headers(self):
auth_token = None
if self.ENV_VAR_AUTH_TOKEN_FILE in self._environ:
auth_token_file_path = self._environ[self.ENV_VAR_AUTH_TOKEN_FILE]
with open(auth_token_file_path) as token_file:
auth_token = token_file.read()
elif self.ENV_VAR_AUTH_TOKEN in self._environ:
auth_token = self._environ[self.ENV_VAR_AUTH_TOKEN]
if auth_token is not None:
self._validate_auth_token(auth_token)
return {'Authorization': auth_token}
def _validate_auth_token(self, auth_token):
if "\r" in auth_token or "\n" in auth_token:
raise ValueError("Auth token value is not a legal header value")Should we add some checks as well?
alexgromero
left a comment
There was a problem hiding this comment.
One callout across all three new credential packages: they add async tests, but the package local pyproject.toml files don't declare the test setup needed to run them standalone.
This worked in smithy-python because it already had that wiring, but here uv sync && uv run pytest does not install an async pytest plugin. Even with pytest-asyncio, the async tests still need either @pytest.mark.asyncio or asyncio_mode = "auto".
Could we add a shared test dependency group and pytest async configuration to each package so their test suites can run on their own?
I'm actually going to follow up with a PR that introduces CI + testing infrastructure using uv workspaces. This is similar to Smithy Python's setup. We will need this for our internal infrastructure to test these packages as part of our release process. For now, I'd prefer we keep the pytest configurations and dependencies centralized, instead of per-package, to avoid drift. |
|
The new commit LGTM overall. Regarding #72 (comment), we should have a follow-up though: in smithy-http, |
| async def invalidate(self) -> None: | ||
| """Discard cached credentials so the next resolution re-queries the endpoint.""" | ||
| self._credentials = None |
There was a problem hiding this comment.
Should we make this a no-op for now? Clearing the cached credentials works against static stability. The refresh behavior we're moving toward expires them only when the rejected identity matches, rather than discarding them outright.
Since we can't implement the identity-match behavior yet (it needs the rejected identity passed in), a no-op seems safer than carrying over the clear. What do you think?
Description
This PR introduces three new packages that register network credential providers into the SDK's modular AWS credential chain:
aws-credentials-imds- EC2 Instance Metadata Service (IMDSv2) credential resolver andEc2InstanceMetadatachain provideraws-credentials-http- container HTTP credential resolver (ECS/EKS) andEcsContainerchain provideraws-credentials-sts- STS AssumeRole resolvers andProfileAssumeRolechain providerIMDS and HTTP are direct ports of the existing
smithy_aws_core.identity.imdsandcontainerresolvers, which will be deprecated. The credential resolution behavior is mostly identical. Changes are limited to:client.py/resolvers.py/providers.pysplit).ContainerMetadataClientbecomesHttpCredentialsClient,EC2MetadatabecomesIMDSClient, etc).ContainerCredentialsConfigis flattened into class constructor arguments to make user interface cleaner.*ConfigurationError(SmithyError)types instead of bareValueError, reservingSmithyIdentityErrorfor resolution-time failures.STS is new. It ships two resolvers that separate the AssumeRole call itself from the profile configuration that feeds it:
AssumeRoleCredentialsResolverperforms the STSAssumeRolecall. It takes an explicitrole_arnand asource_resolverthat provides the credentials used to make the call. This is the low-level resolver, with no knowledge of profiles, and can be used standalone outside the chain.ProfileAssumeRoleCredentialsResolveris the profile-driven resolver used by the chain. It reads a profile from the shared config file and resolves the credential source from that profile'ssource_profile(chaining to another profile, including nested role chains that terminate in static credentials) orcredential_source(delegating to theEnvironment,EcsContainer, orEc2InstanceMetadataprovider). It then hands that source to anAssumeRoleCredentialsResolverto perform the call.Splitting the two keeps the STS call logic isolated and reusable.
AssumeRoleCredentialsResolvercan be constructed directly with any source resolver, whileProfileAssumeRoleCredentialsResolverowns only the profile parsing and source resolution.Important
The underlying STS client used for Assume Role calls in imported from the
aws-sdk-stsclient. This means thataws-credentials-stshas a required dependency onaws-sdk-sts. This does not cause a dependency cycle as theaws-sdk-stsclient will never have a required dependency onaws-credentials-stsand instead is opt in. If artifact size foraws-credentials-stsbecomes a concern due to the full import of the STS client package, we can explore a slim client implementation. For now, this is the most maintainable solution.Usage
Installing any of these packages auto-registers its provider into the SDK's credential chain via entry points. Each resolver can also be used directly:
Testing
IdentityChainwith live service calls~/.aws/configprofiles and environment variables.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.