XJDKC commented on code in PR #1191:
URL: https://github.com/apache/polaris/pull/1191#discussion_r2006144562


##########
service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -664,6 +665,15 @@ private void validateUpdateCatalogDiffOrThrow(
             "Cannot modify ExternalId in storage config from %s to %s",
             currentStorageConfig, newStorageConfig);
       }
+
+      if ((currentAwsConfig.getUserARN() != null
+              && 
!currentAwsConfig.getUserARN().equals(newAwsConfig.getUserARN()))
+          || (newAwsConfig.getUserARN() != null
+              && 
!newAwsConfig.getUserARN().equals(currentAwsConfig.getUserARN()))) {
+        throw new BadRequestException(
+            "Cannot modify userARN in storage config from %s to %s",

Review Comment:
   `userARN` represents the polaris service, from my understanding, it should 
be configured by the polaris service provider rather than the user.
   
   The owner of the catalog is an user, user only needs to provide an IAM role, 
polaris service will use its AWS credential (the `userARN`) to assume the role 
and get the temp AWS credential. How could an user provide an AWS user ARN for 
polaris service? 
   
   Also, for the current implementation, we completely ignore the `userARN` 
during the catalog creation, see:
   1. 
https://github.com/apache/polaris/blob/a1e9d8a14d45519bf3cfbdcad01e9ec8167c5d3c/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java#L81-L97
   2. 
https://github.com/apache/polaris/blob/a1e9d8a14d45519bf3cfbdcad01e9ec8167c5d3c/polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java#L244-L255
   This also shows that `userARN` shouldn't be provided by users and can't be 
modify by users since it's a fixed value provided by polaris service itself.
   
   Here is the guidance from Open Catalog for creating a catalog: 
https://other-docs.snowflake.com/en/opencatalog/create-catalog
   The entire flow is:
   1. User/Catalog Owner creates an IAM role, this role needs to have access to 
the S3 bucket
   4. User/Catalog Owner creates a Polaris Catalog and provides this IAM role
   5. User/Catalog Owner can get the `userARN` info provided by Polaris Service 
by sending a getCatalog request
   6. User adds the userARN into the trust relationship of that IAM role so 
that Polaris service can assume the IAM role provided by the user.
   7. Polaris uses its AWS credential (`userARN`) to assume the IAM role and 
get temp AWS credential, this AWS credential can be used to access the storage 
later



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