MonkeyCanCode opened a new pull request, #3496:
URL: https://github.com/apache/polaris/pull/3496

   <!--
   ๐Ÿ“ Describe what changes you're proposing, especially breaking or user-facing 
changes. 
   ๐Ÿ“– See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
   -->
   This PR addressed one of the issues reported in 
https://github.com/apache/polaris/issues/3440 where when end-user is non-AWS 
S3-compatible backend and have region set on the catalog property. With current 
code, we are determining if a S3 backend if AWS or not based on checking if 
account id and region are both set which is a bit problematic IMO. The account 
id here is derived from IAM role and the region is be set implicitly. That 
being said, when an user is trying to use assume role to auth and interact with 
S3-compatible storage, it can cause good amount of confusion as our current 
code base will add wildcard KMS policy to it if the backend is "AWS" (in this 
case, if a region and account id are both present...and region itself is valid 
for MinIO AFAIK and it default to us-east-1).
   
   Now with this, if an user is trying to use assume role for auth via STS and 
have region set, polaris will implicitly set wildcard KMS policy which is not 
compatible with MinIO thus raised error reported above. I am purposing we 
should be checking if an endpoint is being implicitly set and not contains 
"amazonaws.com" to correctly check if the backend is AWS or not.
   
   If we think this is good to change, please wait for 
https://github.com/apache/polaris/pull/3493 and 
https://github.com/apache/polaris/pull/3494 to be merged first then I will 
refactor this one accordingly. 
   
   ## Checklist
   - [x] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact [email protected])
   - [x] ๐Ÿ”— Clearly explained why the changes are needed, or linked related 
issues: Fixes #
   - [x] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and 
explained how)
   - [x] ๐Ÿ’ก Added comments for complex logic
   - [x] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [x] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


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