parthchandra commented on PR #6025:
URL:
https://github.com/apache/datafusion-comet/pull/6025#issuecomment-5769353025
Thanks for the careful reviews @sunchao. Rather than keep patching the
hand-built `ProviderConfig`
one setting at a time, I reworked how the provider is constructed so the
whole class of
"a hand-assembled config drops an SDK-resolved setting" goes away.
**What changed:** `build_provider` now loads the AWS SDK's fully-resolved
`SdkConfig` and
builds the STS client from it, then calls `AssumeRoleWithWebIdentity`
directly:
```rust
let sdk = aws_config::defaults(BehaviorVersion::latest())
.retry_config(RetryConfig::standard().with_max_attempts(cfg.max_attempts))
.load().await;
Arc::new(web_identity_provider_from(cfg, aws_sdk_sts::Client::new(&sdk)))
```
Because the STS client comes from the resolved `SdkConfig` instead of a
config I assemble by
hand, it honors region, `use_fips`, `use_dual_stack`, the profile, and any
profile-configured
or custom STS `endpoint_url` exactly as the default chain would — there is
nothing left for me
to copy across and get wrong. That resolves both remaining items:
- **Profile-configured STS endpoint (inline on L306):** the STS client is
built from the
`SdkConfig`, which carries the parsed profile, so a profile `[services]`
endpoint is honored.
- **Config-only default profile:** `configured_profile()` now also treats a
present
`AWS_CONFIG_FILE` / `~/.aws/config` as a configured profile and stands
aside, so `[default]`
credentials living only in the config file keep precedence.
**Still intact and unchanged:**
- Web-identity only — it only ever calls `AssumeRoleWithWebIdentity`, no
chain, no
IMDS/instance-role fallback, so a throttle that outlasts the retries
errors rather than
downgrading.
- The credential-precedence stand-asides (`explicit_env_credentials` +
`configured_profile`)
remain — that's the deliberate policy that env/profile credentials outrank
web-identity.
- The shared per-process cache, single-flight, jitter, and failed-refresh
cooldown (the actual
throttle fix) are unchanged.
**Tests:** all 13 web-identity unit tests pass. The
throttle/retry/coalescing tests now inject
the in-memory HTTP stub through `aws_config::defaults().http_client(...)`;
the FIPS test passes
structurally (the STS client picks up `AWS_USE_FIPS_ENDPOINT` from the
resolved config); and
new tests cover the config-file profile and env-credential precedence.
**Dependency:** added `aws-sdk-sts` (already in the lock transitively via
`aws-config`) with
`default-features = false`, so no new crates are pulled.
Validation caveat unchanged: these are component tests with an in-memory STS
stub, not a live
STS/S3 integration test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
