Catalog/S3,GCS: Adopt IAM policies to new object-storage layout (Iceb…

…erg 1.7.0) (#9897)

catalog/files/impl/src/main/java/org/projectnessie/catalog/files/gcs/GcsStorageSupplier.java

@@ -52,7 +52,9 @@
 
 public final class GcsStorageSupplier {
   private static final Logger LOGGER = LoggerFactory.getLogger(GcsStorageSupplier.class);
-  static final String RANDOMIZED_PART = "([A-Za-z0-9=]+/)?";
+
+  // Suitable for both old object-storage layout (before Iceberg 1.7.0) and new (since 1.7.0)
+  static final String RANDOMIZED_PART = "([A-Za-z0-9=]+/|[01]{4}/[01]{4}/[01]{4}/[01]{8}/)?";
 
   private final HttpTransportFactory httpTransportFactory;
   private final GcsConfig gcsConfig;

catalog/files/impl/src/main/java/org/projectnessie/catalog/files/s3/S3IamPolicies.java

@@ -74,9 +74,12 @@ static String locationDependentPolicy(S3ClientIam clientIam, StorageLocations lo
       ArrayNode s3Prefix = stringLike.withArray("s3:prefix");
       s3Prefix.add(path);
       s3Prefix.add(path + "/*");
-      // For write.object-storage.enabled=true
+      // For write.object-storage.enabled=true / before Iceberg 1.7.0
       s3Prefix.add("*/" + path);
       s3Prefix.add("*/" + path + "/*");
+      // For write.object-storage.enabled=true / after Iceberg 1.7.0
+      s3Prefix.add("*/*/*/*/" + path);
+      s3Prefix.add("*/*/*/*/" + path + "/*");
     }
 
     // "Allow Write" for all remaining S3 actions on the bucket+path.
@@ -94,8 +97,10 @@ static String locationDependentPolicy(S3ClientIam clientIam, StorageLocations lo
         String bucket = iamEscapeString(location.requiredAuthority());
         String path = iamEscapeString(location.pathWithoutLeadingTrailingSlash());
         resources.add(format("arn:aws:s3:::%s/%s/*", bucket, path));
-        // For write.object-storage.enabled=true
+        // For write.object-storage.enabled=true / before Iceberg 1.7.0
         resources.add(format("arn:aws:s3:::%s/*/%s/*", bucket, path));
+        // For write.object-storage.enabled=true / since Iceberg 1.7.0
+        resources.add(format("arn:aws:s3:::%s/*/*/*/*/%s/*", bucket, path));
       }
     }
 
@@ -112,8 +117,10 @@ static String locationDependentPolicy(S3ClientIam clientIam, StorageLocations lo
         String bucket = iamEscapeString(location.requiredAuthority());
         String path = iamEscapeString(location.pathWithoutLeadingTrailingSlash());
         resources.add(format("arn:aws:s3:::%s%s/*", bucket, path));
-        // For write.object-storage.enabled=true
+        // For write.object-storage.enabled=true / before Iceberg 1.7.0
         resources.add(format("arn:aws:s3:::%s/*/%s/*", bucket, path));
+        // For write.object-storage.enabled=true / since Iceberg 1.7.0
+        resources.add(format("arn:aws:s3:::%s/*/*/*/*/%s/*", bucket, path));
       }
     }
 

catalog/files/impl/src/test/java/org/projectnessie/catalog/files/s3/TestS3IamPolicies.java

@@ -78,7 +78,7 @@ void multipleStorageLocations() throws Exception {
                 + "    \"Resource\" : \"arn:aws:s3:::bucket1\",\n"
                 + "    \"Condition\" : {\n"
                 + "      \"StringLike\" : {\n"
-                + "        \"s3:prefix\" : [ \"my/path/bar\", \"my/path/bar/*\", \"*/my/path/bar\", \"*/my/path/bar/*\" ]\n"
+                + "        \"s3:prefix\" : [ \"my/path/bar\", \"my/path/bar/*\", \"*/my/path/bar\", \"*/my/path/bar/*\", \"*/*/*/*/my/path/bar\", \"*/*/*/*/my/path/bar/*\" ]\n"
                 + "      }\n"
                 + "    }\n"
                 + "  }, {\n"
@@ -87,7 +87,7 @@ void multipleStorageLocations() throws Exception {
                 + "    \"Resource\" : \"arn:aws:s3:::bucket2\",\n"
                 + "    \"Condition\" : {\n"
                 + "      \"StringLike\" : {\n"
-                + "        \"s3:prefix\" : [ \"my/other/bar\", \"my/other/bar/*\", \"*/my/other/bar\", \"*/my/other/bar/*\" ]\n"
+                + "        \"s3:prefix\" : [ \"my/other/bar\", \"my/other/bar/*\", \"*/my/other/bar\", \"*/my/other/bar/*\", \"*/*/*/*/my/other/bar\", \"*/*/*/*/my/other/bar/*\" ]\n"
                 + "      }\n"
                 + "    }\n"
                 + "  }, {\n"
@@ -96,7 +96,7 @@ void multipleStorageLocations() throws Exception {
                 + "    \"Resource\" : \"arn:aws:s3:::bucket3\",\n"
                 + "    \"Condition\" : {\n"
                 + "      \"StringLike\" : {\n"
-                + "        \"s3:prefix\" : [ \"read/path/bar\", \"read/path/bar/*\", \"*/read/path/bar\", \"*/read/path/bar/*\" ]\n"
+                + "        \"s3:prefix\" : [ \"read/path/bar\", \"read/path/bar/*\", \"*/read/path/bar\", \"*/read/path/bar/*\", \"*/*/*/*/read/path/bar\", \"*/*/*/*/read/path/bar/*\" ]\n"
                 + "      }\n"
                 + "    }\n"
                 + "  }, {\n"
@@ -105,17 +105,17 @@ void multipleStorageLocations() throws Exception {
                 + "    \"Resource\" : \"arn:aws:s3:::bucket4\",\n"
                 + "    \"Condition\" : {\n"
                 + "      \"StringLike\" : {\n"
-                + "        \"s3:prefix\" : [ \"read/other/bar\", \"read/other/bar/*\", \"*/read/other/bar\", \"*/read/other/bar/*\" ]\n"
+                + "        \"s3:prefix\" : [ \"read/other/bar\", \"read/other/bar/*\", \"*/read/other/bar\", \"*/read/other/bar/*\", \"*/*/*/*/read/other/bar\", \"*/*/*/*/read/other/bar/*\" ]\n"
                 + "      }\n"
                 + "    }\n"
                 + "  }, {\n"
                 + "    \"Effect\" : \"Allow\",\n"
                 + "    \"Action\" : [ \"s3:GetObject\", \"s3:GetObjectVersion\", \"s3:PutObject\", \"s3:DeleteObject\" ],\n"
-                + "    \"Resource\" : [ \"arn:aws:s3:::bucket1/my/path/bar/*\", \"arn:aws:s3:::bucket1/*/my/path/bar/*\", \"arn:aws:s3:::bucket2/my/other/bar/*\", \"arn:aws:s3:::bucket2/*/my/other/bar/*\" ]\n"
+                + "    \"Resource\" : [ \"arn:aws:s3:::bucket1/my/path/bar/*\", \"arn:aws:s3:::bucket1/*/my/path/bar/*\", \"arn:aws:s3:::bucket1/*/*/*/*/my/path/bar/*\", \"arn:aws:s3:::bucket2/my/other/bar/*\", \"arn:aws:s3:::bucket2/*/my/other/bar/*\", \"arn:aws:s3:::bucket2/*/*/*/*/my/other/bar/*\" ]\n"
                 + "  }, {\n"
                 + "    \"Effect\" : \"Allow\",\n"
                 + "    \"Action\" : [ \"s3:GetObject\", \"s3:GetObjectVersion\" ],\n"
-                + "    \"Resource\" : [ \"arn:aws:s3:::bucket3read/path/bar/*\", \"arn:aws:s3:::bucket3/*/read/path/bar/*\", \"arn:aws:s3:::bucket4read/other/bar/*\", \"arn:aws:s3:::bucket4/*/read/other/bar/*\" ]\n"
+                + "    \"Resource\" : [ \"arn:aws:s3:::bucket3read/path/bar/*\", \"arn:aws:s3:::bucket3/*/read/path/bar/*\", \"arn:aws:s3:::bucket3/*/*/*/*/read/path/bar/*\", \"arn:aws:s3:::bucket4read/other/bar/*\", \"arn:aws:s3:::bucket4/*/read/other/bar/*\", \"arn:aws:s3:::bucket4/*/*/*/*/read/other/bar/*\" ]\n"
                 + "  }, {\n"
                 + "    \"Effect\" : \"Deny\",\n"
                 + "    \"Action\" : \"s3:*\",\n"
@@ -156,7 +156,11 @@ static Stream<Arguments> locationDependentPolicy() {
     return Stream.of(
         arguments(
             ImmutableS3ClientIam.builder().enabled(true).build(),
-            List.of("arn:aws:s3:::foo", "arn:aws:s3:::foo/b\"ar/*", "arn:aws:s3:::foo/*/b\"ar/*")),
+            List.of(
+                "arn:aws:s3:::foo",
+                "arn:aws:s3:::foo/b\"ar/*",
+                "arn:aws:s3:::foo/*/b\"ar/*",
+                "arn:aws:s3:::foo/*/*/*/*/b\"ar/*")),
         arguments(
             ImmutableS3ClientIam.builder()
                 .enabled(true)
@@ -168,6 +172,7 @@ static Stream<Arguments> locationDependentPolicy() {
                 "arn:aws:s3:::foo",
                 "arn:aws:s3:::foo/b\"ar/*",
                 "arn:aws:s3:::foo/*/b\"ar/*",
+                "arn:aws:s3:::foo/*/*/*/*/b\"ar/*",
                 "arn:aws:s3:::*/blocked\"Namespace/*")));
   }