Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "breaking",
"description": "Updated the credential chain precedence so assume role credentials are resolved before session and static profile keys."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "enhancement",
"description": "Updated profile session and static key providers to defer when the selected profile declares an assume-role configuration."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "enhancement",
"description": "Deprecated the built-in IMDS and container credentials resolvers in favor of the resolvers provided by the `aws-credentials-imds` and `aws-credentials-http` packages."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "enhancement",
"description": "Updated environment credentials provider to defer when `profile_name` is passed to `IdentityChain.create()`."
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,9 @@ async def create[ChainIdentity: Identity](
:param identity_type: The identity type to resolve.
:param config_file: Parsed config/credentials file. Loaded from disk
when not set.
:param profile_name: Profile name to use. If omitted, the shared config
provider uses ``AWS_PROFILE`` when set, otherwise ``default``.
:param profile_name: Explicit profile name to use. When set, top-level
environment credential resolution is suppressed. If omitted, the shared
config provider uses ``AWS_PROFILE`` when set, otherwise ``default``.
:param region_override: Region to use for providers whose resolvers
fetch credentials through a service call.
:param http_client: HTTP client to use for providers whose resolvers make
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ class StandardProvider(Enum):
ENVIRONMENT = "Environment", None
WEB_IDENTITY_TOKEN_ENV = "WebIdentityTokenEnv", "aws-credentials-sts"
SHARED_CONFIG = "SharedConfig", None
PROFILE_ASSUME_ROLE = "ProfileAssumeRole", "aws-credentials-sts"
PROFILE_SESSION_KEYS = "ProfileSessionKeys", None
PROFILE_STATIC_KEYS = "ProfileStaticKeys", None
PROFILE_ASSUME_ROLE = "ProfileAssumeRole", "aws-credentials-sts"
PROFILE_WEB_IDENTITY = "ProfileWebIdentity", "aws-credentials-sts"
PROFILE_SSO_SESSION = "ProfileSsoSession", "aws-credentials-sso"
PROFILE_LOGIN = "Login", "aws-credentials-login"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@


class EnvironmentCredentialsProvider:
"""Adds an environment resolver when credentials are configured in the environment."""
"""Adds an environment resolver unless an explicit profile is selected."""

@property
def name(self) -> str:
Expand All @@ -35,6 +35,9 @@ async def setup(
if identity_type is not AWSCredentialsIdentity:
return

if setup.profile_name is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR #771 passes config_context.profile_name into IdentityChain.create(), but it looks like SharedConfigContext.profile_name is never None? It falls back to AWS_PROFILE, then default.

def _resolve_profile_name(
self, explicit_profile: str | None
) -> tuple[str, str | None]:
"""Determine the active profile name and where it came from.
Priority: explicit argument > AWS_PROFILE env var > "default"
:returns: Tuple of (profile_name, origin), where origin describes the
source for error messages and is None when the name was defaulted.
"""
if explicit_profile is not None:
return explicit_profile, "the profile argument"
env_profile = os.environ.get(_PROFILE_ENV_VAR)
if env_profile is not None:
return env_profile, _PROFILE_ENV_VAR
return _DEFAULT_PROFILE, None

So once both PRs land, a user with just AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY set and
no profile configured still gets the environment provider skipped.

Since the context already tracks profile_origin, should #771 pass the profile only when it came from the explicit argument?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I'll update #771 to only set the profile if its origin is an explicit override.

return

if not os.getenv(_ACCESS_KEY_ID) or not os.getenv(_SECRET_ACCESS_KEY):
return

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
_SECRET_ACCESS_KEY = "aws_secret_access_key" # noqa: S105
_SESSION_TOKEN = "aws_session_token" # noqa: S105
_ACCOUNT_ID = "aws_account_id"
_ROLE_ARN = "role_arn"


class ProfileSessionCredentialsProvider:
Expand All @@ -36,6 +37,9 @@ async def setup(self, identity_type: type[Identity], setup: ChainSetup) -> None:
if config_file is None or profile_name is None:
return

if config_file.get(profile_name, _ROLE_ARN) is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[The comment applies to line 81 as well]

With this deferment, consider a user whose profile looks like:

[default]
aws_access_key_id = AKIA...
aws_secret_access_key = ...
role_arn = arn:aws:iam::123:role/Foo
source_profile = default

but who doesn't have aws-credentials-sts installed. The profile key providers now defer, the assume-role provider isn't discovered, and _find_unclaimed_sources can't flag it either - PROFILE_ASSUME_ROLE.is_detected() is always False. So the user gets an IdentityChainError with no "install aws-credentials-sts" suggestion, but the SEP says the point of deferring is that "the chain reports the missing STS module".

Is this a gap we should address?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am aware of this gap. However adding it to this PR expands its scope. This PR focuses on behavioral changes we want to get in before our next release. The module suggestions improves customer experience but doesn't change behavior.

I already have a follow up PR to add better module suggestions which will add profile based provider suggestions to ChainSetup at the SharedConfigProvider. We should try to squeeze it in but can definitely wait until a future release.

return

access_key_id = config_file.get(profile_name, _ACCESS_KEY_ID)
secret_access_key = config_file.get(profile_name, _SECRET_ACCESS_KEY)
session_token = config_file.get(profile_name, _SESSION_TOKEN)
Expand Down Expand Up @@ -74,6 +78,9 @@ async def setup(self, identity_type: type[Identity], setup: ChainSetup) -> None:
if config_file is None or profile_name is None:
return

if config_file.get(profile_name, _ROLE_ARN) is not None:
return

access_key_id = config_file.get(profile_name, _ACCESS_KEY_ID)
secret_access_key = config_file.get(profile_name, _SECRET_ACCESS_KEY)
if access_key_id is None or secret_access_key is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ipaddress
import json
import os
import warnings
from dataclasses import dataclass
from datetime import UTC, datetime
from urllib.parse import urlparse
Expand Down Expand Up @@ -106,7 +107,12 @@ def _is_allowed_container_metadata_host(self, hostname: str) -> bool:
class ContainerCredentialsResolver(
IdentityResolver[AWSCredentialsIdentity, AWSIdentityProperties]
):
"""Resolves AWS Credentials from container credential sources."""
"""Resolves AWS Credentials from container credential sources.

.. warning::
This resolver is deprecated. Use the resolver provided by the
``aws-credentials-http`` package instead.
"""

ENV_VAR = "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI"
ENV_VAR_FULL = "AWS_CONTAINER_CREDENTIALS_FULL_URI"
Expand All @@ -118,6 +124,12 @@ def __init__(
http_client: HTTPClient,
config: ContainerCredentialsConfig | None = None,
):
warnings.warn(
"`ContainerCredentialsResolver` is deprecated; install "
"`aws-credentials-http` and use its resolver instead.",
DeprecationWarning,
stacklevel=2,
)
self._http_client = http_client
self._config = config or ContainerCredentialsConfig()
self._client = ContainerMetadataClient(http_client, self._config)
Expand Down
14 changes: 13 additions & 1 deletion packages/smithy-aws-core/src/smithy_aws_core/identity/imds.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0
import asyncio
import json
import warnings
from dataclasses import dataclass
from datetime import UTC, datetime, timedelta
from types import MappingProxyType
Expand Down Expand Up @@ -182,11 +183,22 @@ async def get(self, *, path: str) -> str:
class IMDSCredentialsResolver(
IdentityResolver[AWSCredentialsIdentity, AWSIdentityProperties]
):
"""Resolves AWS Credentials from an EC2 Instance Metadata Service (IMDS) client."""
"""Resolves AWS Credentials from an EC2 Instance Metadata Service (IMDS) client.

.. warning::
This resolver is deprecated. Use the resolver provided by the
``aws-credentials-imds`` package instead.
"""

_METADATA_PATH_BASE = "/latest/meta-data/iam/security-credentials"

def __init__(self, http_client: HTTPClient, config: Config | None = None):
warnings.warn(
"`IMDSCredentialsResolver` is deprecated; install "
"`aws-credentials-imds` and use its resolver instead.",
DeprecationWarning,
stacklevel=2,
)
# TODO: Respect IMDS specific config values from aws shared config file and environment.
self._http_client = http_client
self._ec2_metadata_client = EC2Metadata(http_client=http_client, config=config)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,34 @@ async def test_registers_terminal_resolver(
assert len(setup.resolvers) == 1
assert setup.resolvers[0].provider_name == "Environment"
assert isinstance(setup.resolvers[0].resolver, EnvironmentCredentialsResolver)


async def test_explicit_profile_suppresses_environment_credentials(
setup_provider: Callable[..., Awaitable[ChainSetup]],
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "akid")
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "secret")

setup = await setup_provider(
EnvironmentCredentialsProvider(),
profile_name="work",
)

assert setup.resolvers == ()
assert not setup.terminal


async def test_aws_profile_env_var_does_not_suppress_environment_credentials(
setup_provider: Callable[..., Awaitable[ChainSetup]],
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("AWS_PROFILE", "work")
monkeypatch.setenv("AWS_ACCESS_KEY_ID", "akid")
monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "secret")

setup = await setup_provider(EnvironmentCredentialsProvider())

assert setup.terminal
assert len(setup.resolvers) == 1
assert isinstance(setup.resolvers[0].resolver, EnvironmentCredentialsResolver)
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,35 @@ async def test_registers_terminal_resolver_for_complete_profile(
assert identity == expected


@pytest.mark.parametrize(
"provider",
[ProfileSessionCredentialsProvider(), ProfileStaticCredentialsProvider()],
)
async def test_defers_when_selected_profile_declares_assume_role(
provider: Any,
setup_provider: Callable[..., Awaitable[ChainSetup]],
merged_config: Callable[..., MergedConfig],
) -> None:
setup = await setup_provider(
provider,
config_file=merged_config(
{
"default": {
"aws_access_key_id": "akid",
"aws_secret_access_key": "secret",
"aws_session_token": "token",
"role_arn": "arn:aws:iam::123456789012:role/test",
"source_profile": "default",
}
}
),
profile_name="default",
)

assert setup.resolvers == ()
assert not setup.terminal


@pytest.mark.parametrize(
"provider, properties",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,26 @@ def test_sort_orders_standards_by_slot_declaration() -> None:
static = _StubProvider(
"static", Standard(slot=StandardProvider.PROFILE_STATIC_KEYS)
)
assume_role = _StubProvider(
"assume-role", Standard(slot=StandardProvider.PROFILE_ASSUME_ROLE)
)
session = _StubProvider(
"session", Standard(slot=StandardProvider.PROFILE_SESSION_KEYS)
)
env = _StubProvider("env", Standard(slot=StandardProvider.ENVIRONMENT))
shared = _StubProvider("shared", Standard(slot=StandardProvider.SHARED_CONFIG))

ordered = chain_module._sort_by_ordering((static, env, shared))
ordered = chain_module._sort_by_ordering(
(static, session, env, assume_role, shared)
)

assert [p.name for p in ordered] == ["env", "shared", "static"]
assert [p.name for p in ordered] == [
"env",
"shared",
"assume-role",
"session",
"static",
]


def test_sort_places_before_and_after_around_slot() -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ def test_shared_config_detection(
"aws-credentials-sts",
),
(StandardProvider.SHARED_CONFIG, "SharedConfig", None),
(StandardProvider.PROFILE_SESSION_KEYS, "ProfileSessionKeys", None),
(StandardProvider.PROFILE_STATIC_KEYS, "ProfileStaticKeys", None),
(
StandardProvider.PROFILE_ASSUME_ROLE,
"ProfileAssumeRole",
"aws-credentials-sts",
),
(StandardProvider.PROFILE_SESSION_KEYS, "ProfileSessionKeys", None),
(StandardProvider.PROFILE_STATIC_KEYS, "ProfileStaticKeys", None),
(
StandardProvider.PROFILE_WEB_IDENTITY,
"ProfileWebIdentity",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@

ISO8601 = "%Y-%m-%dT%H:%M:%SZ"

pytestmark = pytest.mark.filterwarnings(
"ignore:`ContainerCredentialsResolver` is deprecated:DeprecationWarning"
)


def test_config_custom_values():
config = ContainerCredentialsConfig(timeout=10, retries=5)
Expand Down
4 changes: 4 additions & 0 deletions packages/smithy-aws-core/tests/unit/identity/test_imds.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
from smithy_core.aio.retries import SimpleRetryStrategy
from smithy_http.aio import HTTPRequest

pytestmark = pytest.mark.filterwarnings(
"ignore:`IMDSCredentialsResolver` is deprecated:DeprecationWarning"
)


def test_config_defaults():
config = Config()
Expand Down
Loading