MonkeyCanCode opened a new pull request, #3501: URL: https://github.com/apache/polaris/pull/3501
<!-- ๐ 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). As discussed in https://github.com/apache/polaris/pull/3496, instead of fixing the check for isAwsS3, we will proceed with a new catalog config instead. Sample outputs: ``` # setup ./polaris --profile dev catalogs create --storage-type s3 --role-arn "arn:aws:iam::123456789012:role/polaris-warehouse-access" --default-base-location "s3://analytics-bucket/warehouse/" quickstart_catalog ./polaris --profile dev catalogs create --storage-type s3 --role-arn "arn:aws:iam::123456789012:role/polaris-warehouse-access" --default-base-location "s3://analytics-bucket-no-kms/warehouse/" --no-kms quickstart_catalog_no_kms # outputs ./polaris --profile dev catalogs get quickstart_catalog | jq { "type": "INTERNAL", "name": "quickstart_catalog", "properties": { "default-base-location": "s3://analytics-bucket/warehouse/" }, "createTimestamp": 1769052561792, "lastUpdateTimestamp": 1769052561792, "entityVersion": 1, "storageConfigInfo": { "storageType": "S3", "allowedLocations": [ "s3://analytics-bucket/warehouse/" ], "roleArn": "arn:aws:iam::123456789012:role/polaris-warehouse-access", "allowedKmsKeys": [], "stsUnavailable": false, "pathStyleAccess": false, "kmsUnavailable": false } } ./polaris --profile dev catalogs get quickstart_catalog_no_kms | jq { "type": "INTERNAL", "name": "quickstart_catalog_no_kms", "properties": { "default-base-location": "s3://analytics-bucket-no-kms/warehouse/" }, "createTimestamp": 1769052562086, "lastUpdateTimestamp": 1769052562086, "entityVersion": 1, "storageConfigInfo": { "storageType": "S3", "allowedLocations": [ "s3://analytics-bucket-no-kms/warehouse/" ], "roleArn": "arn:aws:iam::123456789012:role/polaris-warehouse-access", "allowedKmsKeys": [], "stsUnavailable": false, "pathStyleAccess": false, "kmsUnavailable": true } } ``` ## 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]
