qzyu999 commented on PR #3424:
URL: https://github.com/apache/fluss/pull/3424#issuecomment-4996402127

   Hi @fresh-borzoni, thanks for the clarification, you're right. I re-traced 
the code path on `v0.9.1-incubating` and confirmed Method C 
(`fs.s3a.aws.credentials.provider`) does work on 0.9.1:
   
   **How it works on 0.9.1** (`S3FileSystemPlugin.java`):
   
   1. **Line 87-94** (`getHadoopConfiguration`): Iterates all Fluss config keys 
matching prefixes `"s3.", "s3a.", "fs.s3a."` and copies them to Hadoop config 
with `fs.s3a.` prefix. So `fs.s3a.aws.credentials.provider` set in 
`server.yaml` reaches Hadoop config.
   
   2. **Line 132-155** (`setCredentialProvider`): Checks `hasStaticKeys` (line 
133-135) and `hasRoleArn` (line 136). When neither is set, falls into the 
`else` branch (line 143) which calls 
`S3DelegationTokenReceiver.updateHadoopConfig()`.
   
   3. **`S3DelegationTokenReceiver.updateHadoopConfig()`**: Reads the existing 
`fs.s3a.aws.credentials.provider` value, and **prepends** 
`DynamicTemporaryAWSCredentialsProvider` to the chain (doesn't overwrite). 
Final chain becomes: 
`DynamicTemporaryAWSCredentialsProvider,ContainerCredentialsProvider,...`
   
   4. At runtime, `DynamicTemporaryAWSCredentialsProvider` tries to generate 
delegation tokens → fails with the non-fatal warning → Hadoop S3A falls through 
to `ContainerCredentialsProvider` → works.
   
   **Your bfaaffb8 fix**: Adds an early `return` when `hasCredentialProvider` 
is true, skipping the delegation token prepend entirely. Result is cleaner (no 
warning), but the end behavior is the same.
   
   **So the doc and Quick Start script are correct for 0.9.1.** The only 
user-visible difference is this non-fatal warning on 0.9.x:
   
   > `Session credentials from the configured AWS credentials provider are not 
supported for Fluss S3 client-token generation.`
   
   I've documented this in the Authentication section NOTE. Should I also add a 
note in the Authentication section that this warning is expected on 0.9.x and 
goes away in the next release? Also to what extent do you think I should 
elaborate on these details within the doc itself?
   
   **Compatibility matrix:**
   
   | Auth Method | Version | ECS Fargate | EKS (IRSA) | EC2 | Notes |
   |---|---|---|---|---|---|
   | A: Static keys (`s3.access.key`+`s3.secret.key`) | 0.9.x | ✅ | ✅ | ✅ | 
Keys expire if session tokens |
   | A: Static keys | next | ✅ | ✅ | ✅ | Same |
   | B: Assumed role (`s3.assumed.role.arn`) | 0.9.x | ❌ | ✅ | ✅ | Relies on 
Hadoop default chain (no ContainerCredentialsProvider) |
   | B: Assumed role | next | ❌ | ✅ | ✅ | Same as 0.9.x. Cannot combine with C 
(throws error). |
   | C: Provider chain (`fs.s3a.aws.credentials.provider`) | 0.9.x | ✅ ⚠️ | ✅ 
⚠️ | ✅ ⚠️ | Works; delegation token warning (non-fatal) |
   | C: Provider chain | next | ✅ | ✅ | ✅ | Clean — no warning |
   
   ⚠️ = Non-fatal warning logged but S3 operations succeed.
   
   Given this, the doc recommends C as the primary approach (works everywhere, 
all versions) and A as a testing-only fallback. B is not documented since it 
only works on a subset of environments and cannot be combined with C on the 
next release.
   
   
   _**Note: I was only able to test with ECS and am not able to test EKS/EC2. I 
relied on AI to do a line-by-line code walkthrough to estimate what would be 
the results for the interaction between Fluss versioning and AWS environment. 
I've also cleaned up all the other review comments, PTAL and LMK how we can 
best proceed.**_


-- 
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]

Reply via email to