Skip to content

Commit 468bec1

Browse files
committed
Revert "Merge branch 'onprem-s3-dell-ecs' into handle-custom-sts-endpoint"
This reverts commit 69e402e, reversing changes made to a169a79.
1 parent 5dc1050 commit 468bec1

File tree

3 files changed

+10
-62
lines changed

3 files changed

+10
-62
lines changed

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -211,24 +211,14 @@ private IamPolicy policyString(
211211
Map<String, IamStatement.Builder> bucketGetLocationStatementBuilder = new HashMap<>();
212212

213213
String arnPrefix = arnPrefixForPartition(awsPartition);
214-
boolean isEcsPartition = "ecs".equals(awsPartition);
215214
Stream.concat(readLocations.stream(), writeLocations.stream())
216215
.distinct()
217216
.forEach(
218217
location -> {
219218
URI uri = URI.create(location);
220-
// Some on-prem S3/STSc implementations (for example ECS) do not accept object ARNs
221-
// that include the path portion (bucket/key/*). For those, scope object permissions
222-
// to
223-
// the whole bucket (bucket/*) and rely on s3:prefix conditions for finer granularity.
224-
if (isEcsPartition) {
225-
allowGetObjectStatementBuilder.addResource(
226-
IamResource.create(arnPrefix + StorageUtil.getBucket(uri) + "/*"));
227-
} else {
228-
allowGetObjectStatementBuilder.addResource(
229-
IamResource.create(
230-
arnPrefix + StorageUtil.concatFilePrefixes(parseS3Path(uri), "*", "/")));
231-
}
219+
allowGetObjectStatementBuilder.addResource(
220+
IamResource.create(
221+
arnPrefix + StorageUtil.concatFilePrefixes(parseS3Path(uri), "*", "/")));
232222
final var bucket = arnPrefix + StorageUtil.getBucket(uri);
233223
if (allowList) {
234224
bucketListStatementBuilder
@@ -262,14 +252,9 @@ private IamPolicy policyString(
262252
writeLocations.forEach(
263253
location -> {
264254
URI uri = URI.create(location);
265-
if (isEcsPartition) {
266-
allowPutObjectStatementBuilder.addResource(
267-
IamResource.create(arnPrefix + StorageUtil.getBucket(uri) + "/*"));
268-
} else {
269-
allowPutObjectStatementBuilder.addResource(
270-
IamResource.create(
271-
arnPrefix + StorageUtil.concatFilePrefixes(parseS3Path(uri), "*", "/")));
272-
}
255+
allowPutObjectStatementBuilder.addResource(
256+
IamResource.create(
257+
arnPrefix + StorageUtil.concatFilePrefixes(parseS3Path(uri), "*", "/")));
273258
});
274259
policyBuilder.addStatement(allowPutObjectStatementBuilder.build());
275260
}
@@ -290,13 +275,7 @@ private IamPolicy policyString(
290275
}
291276

292277
private static String arnPrefixForPartition(String awsPartition) {
293-
// Some on-prem S3 compatible systems (e.g. ECS) use a non-standard partition value
294-
// but expect S3 resource ARNs to use the 'aws' partition form (arn:aws:s3:::bucket).
295-
String partition = awsPartition != null ? awsPartition : "aws";
296-
if ("ecs".equals(partition)) {
297-
partition = "aws";
298-
}
299-
return String.format("arn:%s:s3:::", partition);
278+
return String.format("arn:%s:s3:::", awsPartition != null ? awsPartition : "aws");
300279
}
301280

302281
private static @Nonnull String parseS3Path(URI uri) {

polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,7 @@ public static ImmutableAwsStorageConfigurationInfo.Builder builder() {
4646

4747
// Technically, it should be ^arn:(aws|aws-cn|aws-us-gov):iam::(\d{12}):role/.+$,
4848
@JsonIgnore
49-
// Account id may be a 12-digit AWS account number or a vendor-specific namespace that must
50-
// not be purely numeric (must start with a letter, underscore or hyphen followed by allowed
51-
// chars).
52-
public static final String ROLE_ARN_PATTERN =
53-
"^(arn|urn):(aws|aws-us-gov|ecs):iam::((\\d{12})|([a-zA-Z_-][a-zA-Z0-9_-]*)):role/.+$";
49+
public static final String ROLE_ARN_PATTERN = "^arn:(aws|aws-us-gov):iam::(\\d{12}):role/.+$";
5450

5551
private static final Pattern ROLE_ARN_PATTERN_COMPILED = Pattern.compile(ROLE_ARN_PATTERN);
5652

@@ -129,8 +125,7 @@ public String getAwsAccountId() {
129125
if (arn != null) {
130126
Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
131127
checkState(matcher.matches());
132-
// group(3) is the account identifier (either 12-digit AWS account or vendor namespace)
133-
return matcher.group(3);
128+
return matcher.group(2);
134129
}
135130
return null;
136131
}
@@ -142,8 +137,7 @@ public String getAwsPartition() {
142137
if (arn != null) {
143138
Matcher matcher = ROLE_ARN_PATTERN_COMPILED.matcher(arn);
144139
checkState(matcher.matches());
145-
// group(2) captures the partition (e.g. aws, aws-us-gov, ecs)
146-
return matcher.group(2);
140+
return matcher.group(1);
147141
}
148142
return null;
149143
}

runtime/service/src/test/java/org/apache/polaris/service/entity/CatalogEntityTest.java

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -285,31 +285,6 @@ public void testInvalidArn(String roleArn) {
285285
.hasMessage(expectedMessage);
286286
}
287287

288-
@Test
289-
public void testUrnRoleArnAccepted() {
290-
String urnRole = "urn:ecs:iam::test-namespace:role/test-role";
291-
String baseLocation = "s3://externally-owned-bucket";
292-
AwsStorageConfigInfo awsStorageConfigModel =
293-
AwsStorageConfigInfo.builder()
294-
.setRoleArn(urnRole)
295-
.setExternalId("externalId")
296-
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
297-
.setAllowedLocations(List.of(baseLocation))
298-
.build();
299-
300-
CatalogProperties prop = new CatalogProperties(baseLocation);
301-
Catalog awsCatalog =
302-
PolarisCatalog.builder()
303-
.setType(Catalog.TypeEnum.INTERNAL)
304-
.setName("name")
305-
.setProperties(prop)
306-
.setStorageConfigInfo(awsStorageConfigModel)
307-
.build();
308-
309-
Assertions.assertThatCode(() -> CatalogEntity.fromCatalog(realmConfig, awsCatalog))
310-
.doesNotThrowAnyException();
311-
}
312-
313288
@Test
314289
public void testCatalogTypeDefaultsToInternal() {
315290
String baseLocation = "s3://test-bucket/path";

0 commit comments

Comments
 (0)