morningman commented on PR #67076:
URL: https://github.com/apache/doris/pull/67076#issuecomment-5480927840
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67076
commit: 3cf2a0873268a4d43bc14e1d23cb86a2b33cbab3
base: ad179b5afac6a4420729da98a74860c90bf917a7
reviewed_at: 2026-08-31T23:58+08:00
reviewer: morningman
model: claude-fable-5
effort: max
findings: {blocker: 0, major: 0, minor: 3, nit: 7}
rounds: 1
converged: true
```
**Notes for maintainers**
- `common/cpp/custom_aws_credentials_provider_chain.cpp:51-56` —
pre-existing, not introduced here: the IMDS provider is appended
unconditionally after the container provider and `AWS_EC2_METADATA_DISABLED` is
read but ignored; on EKS Pod Identity a start-up race against the agent
DaemonSet (the `S3Client` ctor walks the chain three times inside
`CloudStorageEngine::open()`) can pin an S3 client to the node role until BE
restart, with no log line at the default `aws_log_level`. A follow-up mirroring
the SDK chain (container XOR IMDS) is suggested in the review documents.
- `common/cpp/aws_common.cpp:125-134` — "Created container credentials
provider ..." is logged at INFO even when the SDK refused the endpoint
(`IsValid()==false`, e.g. a non-loopback `http://` full URI); the SDK's refusal
reason is a WARN line suppressed at the default `aws_log_level=2`, and raising
it needs a restart. Checking `IsValid()` and logging the cause (while still
returning the provider) would close the gap.
- Not verified locally: no build or unit-test run. CI for this commit had
COMPILE, Cloud UT, FE UT green and BE UT turned green as the review closed; the
regression suites were still running. The ctor-argument order `(token,
token_path)` was verified against the aws-sdk-cpp 1.11.221 definition and
aws/aws-sdk-cpp#3162 (header fixed, definition unchanged).
- Backport: issue #66554 reports 4.1.x is affected too; this fix needs
aws-sdk-cpp 1.11.221 (`GeneralHTTPCredentialsProvider.h`), so a branch-4.1 pick
must carry the #66851 thirdparty bump first.
- The PR body ticks "Behavior changed: Yes" but has no `### Release note`
section; the `CONTAINER`/`ECS` provider modes and the default chain now honour
`AWS_CONTAINER_CREDENTIALS_FULL_URI` and re-read
`AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE` on every refresh.
<sub>Reviewed locally with the `doris-repo-review` pipeline (a local port of
`.github/workflows/code-review-runner.yml`). This is not a CI status
check.</sub>
<!-- doris-repo-review:v1:end -->
--
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]
