Skip to content

Commit 987c554

Browse files
dimas-bdimas-b
authored
Add pathStyleAccess to AwsStorageConfigInfo (#2012)
* Add `pathStyleAccess` to AwsStorageConfigInfo This change allows configuring the "path-style" access mode in S3 clients (both in Polaris Servers and Iceberg REST Catalog API clients). This change is applicable both to AWS storage and to non-AWS S3-compatible storage (#1530).
1 parent d035344 commit 987c554

File tree

11 files changed

+173
-21
lines changed

11 files changed

+173
-21
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ at locations that better optimize for object storage.
4646

4747
### Changes
4848

49+
- Polaris Management API clients must be prepared to deal with new attributes in `AwsStorageConfigInfo` objects.
50+
4951
### Deprecations
5052

5153
* The property `polaris.active-roles-provider.type` is deprecated in favor of

‎api/management-model/src/test/java/org/apache/polaris/core/admin/model/CatalogSerializationTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ public void testJsonFormat() throws JsonProcessingException {
6868
+ "\"properties\":{\"default-base-location\":\"s3://test/\"},"
6969
+ "\"storageConfigInfo\":{"
7070
+ "\"roleArn\":\"arn:aws:iam::123456789012:role/test-role\","
71+
+ "\"pathStyleAccess\":false,"
7172
+ "\"storageType\":\"S3\","
7273
+ "\"allowedLocations\":[]"
7374
+ "}}");

‎integration-tests/src/main/java/org/apache/polaris/service/it/env/CatalogApi.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,27 @@ public void dropTable(String catalog, TableIdentifier id) {
186186
}
187187

188188
public LoadTableResponse loadTable(String catalog, TableIdentifier id, String snapshots) {
189+
return loadTable(catalog, id, snapshots, Map.of());
190+
}
191+
192+
public LoadTableResponse loadTableWithAccessDelegation(
193+
String catalog, TableIdentifier id, String snapshots) {
194+
return loadTable(
195+
catalog, id, snapshots, Map.of("X-Iceberg-Access-Delegation", "vended-credentials"));
196+
}
197+
198+
public LoadTableResponse loadTable(
199+
String catalog, TableIdentifier id, String snapshots, Map<String, String> headers) {
200+
HashMap<String, String> allHeaders = new HashMap<>(defaultHeaders());
201+
allHeaders.putAll(headers);
202+
189203
String ns = RESTUtil.encodeNamespace(id.namespace());
190204
try (Response res =
191205
request(
192206
"v1/{cat}/namespaces/" + ns + "/tables/{table}",
193207
Map.of("cat", catalog, "table", id.name()),
194-
snapshots == null ? Map.of() : Map.of("snapshots", snapshots))
208+
snapshots == null ? Map.of() : Map.of("snapshots", snapshots),
209+
allHeaders)
195210
.get()) {
196211
if (res.getStatus() == Response.Status.OK.getStatusCode()) {
197212
return res.readEntity(LoadTableResponse.class);

‎integration-tests/src/main/java/org/apache/polaris/service/it/env/RestApi.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,18 @@ public Invocation.Builder request(String path, Map<String, String> templateValue
4747
return request(path, templateValues, Map.of());
4848
}
4949

50-
public Invocation.Builder request(
51-
String path, Map<String, String> templateValues, Map<String, String> queryParams) {
50+
protected Map<String, String> defaultHeaders() {
5251
Map<String, String> headers = new HashMap<>();
5352
headers.put(endpoints.realmHeaderName(), endpoints.realmId());
5453
if (authToken != null) {
5554
headers.put("Authorization", "Bearer " + authToken);
5655
}
57-
return request(path, templateValues, queryParams, headers);
56+
return headers;
57+
}
58+
59+
public Invocation.Builder request(
60+
String path, Map<String, String> templateValues, Map<String, String> queryParams) {
61+
return request(path, templateValues, queryParams, defaultHeaders());
5862
}
5963

6064
public Invocation.Builder request(

‎polaris-core/src/main/java/org/apache/polaris/core/entity/CatalogEntity.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ private StorageConfigInfo getStorageInfo(Map<String, String> internalProperties)
140140
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
141141
.setAllowedLocations(awsConfig.getAllowedLocations())
142142
.setRegion(awsConfig.getRegion())
143+
.setEndpoint(awsConfig.getEndpoint())
144+
.setStsEndpoint(awsConfig.getStsEndpoint())
145+
.setPathStyleAccess(awsConfig.getPathStyleAccess())
143146
.build();
144147
}
145148
if (configInfo instanceof AzureStorageConfigurationInfo) {
@@ -275,7 +278,8 @@ public Builder setStorageConfigurationInfo(
275278
awsConfigModel.getExternalId(),
276279
awsConfigModel.getRegion(),
277280
awsConfigModel.getEndpoint(),
278-
awsConfigModel.getStsEndpoint());
281+
awsConfigModel.getStsEndpoint(),
282+
awsConfigModel.getPathStyleAccess());
279283
awsConfig.validateArn(awsConfigModel.getRoleArn());
280284
config = awsConfig;
281285
break;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,10 @@ public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
122122
credentialMap.put(StorageAccessProperty.AWS_ENDPOINT, endpointUri.toString());
123123
}
124124

125+
if (Boolean.TRUE.equals(storageConfig.getPathStyleAccess())) {
126+
credentialMap.put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS, Boolean.TRUE.toString());
127+
}
128+
125129
if (storageConfig.getAwsPartition().equals("aws-us-gov")
126130
&& credentialMap.get(StorageAccessProperty.CLIENT_REGION) == null) {
127131
throw new IllegalArgumentException(

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

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ public class AwsStorageConfigurationInfo extends PolarisStorageConfigurationInfo
6262
@JsonProperty(value = "stsEndpoint")
6363
private @Nullable String stsEndpoint;
6464

65+
/** A flag indicating whether path-style bucket access should be forced in S3 clients. */
66+
@JsonProperty(value = "pathStyleAccess")
67+
private Boolean pathStyleAccess;
68+
6569
@JsonCreator
6670
public AwsStorageConfigurationInfo(
6771
@JsonProperty(value = "storageType", required = true) @Nonnull StorageType storageType,
@@ -71,21 +75,23 @@ public AwsStorageConfigurationInfo(
7175
@JsonProperty(value = "externalId") @Nullable String externalId,
7276
@JsonProperty(value = "region", required = false) @Nullable String region,
7377
@JsonProperty(value = "endpoint") @Nullable String endpoint,
74-
@JsonProperty(value = "stsEndpoint") @Nullable String stsEndpoint) {
78+
@JsonProperty(value = "stsEndpoint") @Nullable String stsEndpoint,
79+
@JsonProperty(value = "pathStyleAccess") @Nullable Boolean pathStyleAccess) {
7580
super(storageType, allowedLocations);
7681
this.roleARN = roleARN;
7782
this.externalId = externalId;
7883
this.region = region;
7984
this.endpoint = endpoint;
8085
this.stsEndpoint = stsEndpoint;
86+
this.pathStyleAccess = pathStyleAccess;
8187
}
8288

8389
public AwsStorageConfigurationInfo(
8490
@Nonnull StorageType storageType,
8591
@Nonnull List<String> allowedLocations,
8692
@Nonnull String roleARN,
8793
@Nullable String region) {
88-
this(storageType, allowedLocations, roleARN, null, region, null, null);
94+
this(storageType, allowedLocations, roleARN, null, region, null, null, null);
8995
}
9096

9197
public AwsStorageConfigurationInfo(
@@ -94,7 +100,7 @@ public AwsStorageConfigurationInfo(
94100
@Nonnull String roleARN,
95101
@Nullable String externalId,
96102
@Nullable String region) {
97-
this(storageType, allowedLocations, roleARN, externalId, region, null, null);
103+
this(storageType, allowedLocations, roleARN, externalId, region, null, null, null);
98104
}
99105

100106
@Override
@@ -143,12 +149,27 @@ public void setRegion(@Nullable String region) {
143149
this.region = region;
144150
}
145151

152+
@Nullable
153+
public String getEndpoint() {
154+
return endpoint;
155+
}
156+
146157
@JsonIgnore
147158
@Nullable
148159
public URI getEndpointUri() {
149160
return endpoint == null ? null : URI.create(endpoint);
150161
}
151162

163+
/** Returns a flag indicating whether path-style bucket access should be forced in S3 clients. */
164+
public @Nullable Boolean getPathStyleAccess() {
165+
return pathStyleAccess;
166+
}
167+
168+
@Nullable
169+
public String getStsEndpoint() {
170+
return stsEndpoint;
171+
}
172+
152173
/** Returns the STS endpoint if set, defaulting to {@link #getEndpointUri()} otherwise. */
153174
@JsonIgnore
154175
@Nullable

‎polaris-core/src/test/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfoTest.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,13 @@
2929
public class AwsStorageConfigurationInfoTest {
3030

3131
private static AwsStorageConfigurationInfo config(String endpoint, String stsEndpoint) {
32+
return config(endpoint, stsEndpoint, false);
33+
}
34+
35+
private static AwsStorageConfigurationInfo config(
36+
String endpoint, String stsEndpoint, Boolean pathStyle) {
3237
return new AwsStorageConfigurationInfo(
33-
S3, List.of(), "role", null, null, endpoint, stsEndpoint);
38+
S3, List.of(), "role", null, null, endpoint, stsEndpoint, pathStyle);
3439
}
3540

3641
@Test
@@ -56,4 +61,11 @@ public void testStsEndpoint() {
5661
AwsStorageConfigurationInfo::getStsEndpointUri)
5762
.containsExactly(URI.create("http://s3.example.com"), URI.create("http://sts.example.com"));
5863
}
64+
65+
@Test
66+
public void testPathStyleAccess() {
67+
assertThat(config(null, null, null).getPathStyleAccess()).isNull();
68+
assertThat(config(null, null, false).getPathStyleAccess()).isFalse();
69+
assertThat(config(null, null, true).getPathStyleAccess()).isTrue();
70+
}
5971
}

‎runtime/service/src/intTest/java/org/apache/polaris/service/quarkus/it/QuarkusRestCatalogMinIoIT.java‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.apache.iceberg.io.PositionOutputStream;
4949
import org.apache.iceberg.rest.RESTCatalog;
5050
import org.apache.iceberg.rest.auth.OAuth2Properties;
51+
import org.apache.iceberg.rest.responses.LoadTableResponse;
5152
import org.apache.iceberg.types.Types;
5253
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
5354
import org.apache.polaris.core.admin.model.Catalog;
@@ -68,9 +69,10 @@
6869
import org.junit.jupiter.api.AfterEach;
6970
import org.junit.jupiter.api.BeforeAll;
7071
import org.junit.jupiter.api.BeforeEach;
71-
import org.junit.jupiter.api.Test;
7272
import org.junit.jupiter.api.TestInfo;
7373
import org.junit.jupiter.api.extension.ExtendWith;
74+
import org.junit.jupiter.params.ParameterizedTest;
75+
import org.junit.jupiter.params.provider.ValueSource;
7476
import software.amazon.awssdk.services.s3.S3Client;
7577
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
7678
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
@@ -145,13 +147,15 @@ public void before(TestInfo testInfo) {
145147
catalogName = client.newEntityName(testInfo.getTestMethod().orElseThrow().getName());
146148
}
147149

148-
private RESTCatalog createCatalog(Optional<String> endpoint, Optional<String> stsEndpoint) {
150+
private RESTCatalog createCatalog(
151+
Optional<String> endpoint, Optional<String> stsEndpoint, boolean pathStyleAccess) {
149152
AwsStorageConfigInfo.Builder storageConfig =
150153
AwsStorageConfigInfo.builder()
151154
.setRoleArn("arn:aws:iam::123456789012:role/polaris-test")
152155
.setExternalId("externalId123")
153156
.setUserArn("arn:aws:iam::123456789012:user/polaris-test")
154157
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
158+
.setPathStyleAccess(pathStyleAccess)
155159
.setAllowedLocations(List.of(storageBase.toString()));
156160

157161
endpoint.ifPresent(storageConfig::setEndpoint);
@@ -190,9 +194,11 @@ public void cleanUp() {
190194
client.cleanUp(adminCredentials);
191195
}
192196

193-
@Test
194-
public void testCreateTable() throws IOException {
195-
try (RESTCatalog restCatalog = createCatalog(Optional.of(endpoint), Optional.empty())) {
197+
@ParameterizedTest
198+
@ValueSource(booleans = {true, false})
199+
public void testCreateTable(boolean pathStyle) throws IOException {
200+
try (RESTCatalog restCatalog =
201+
createCatalog(Optional.of(endpoint), Optional.empty(), pathStyle)) {
196202
catalogApi.createNamespace(catalogName, "test-ns");
197203
TableIdentifier id = TableIdentifier.of("test-ns", "t1");
198204
Table table = restCatalog.createTable(id, SCHEMA);
@@ -212,14 +218,25 @@ public void testCreateTable() throws IOException {
212218
.response();
213219
assertThat(response.contentLength()).isGreaterThan(0);
214220

221+
LoadTableResponse loadTableResponse =
222+
catalogApi.loadTableWithAccessDelegation(catalogName, id, "ALL");
223+
assertThat(loadTableResponse.config()).containsKey("s3.endpoint");
224+
225+
if (pathStyle) {
226+
assertThat(loadTableResponse.config())
227+
.containsEntry("s3.path-style-access", Boolean.TRUE.toString());
228+
}
229+
215230
restCatalog.dropTable(id);
216231
assertThat(restCatalog.tableExists(id)).isFalse();
217232
}
218233
}
219234

220-
@Test
221-
public void testAppendFiles() throws IOException {
222-
try (RESTCatalog restCatalog = createCatalog(Optional.of(endpoint), Optional.of(endpoint))) {
235+
@ParameterizedTest
236+
@ValueSource(booleans = {true, false})
237+
public void testAppendFiles(boolean pathStyle) throws IOException {
238+
try (RESTCatalog restCatalog =
239+
createCatalog(Optional.of(endpoint), Optional.of(endpoint), pathStyle)) {
223240
catalogApi.createNamespace(catalogName, "test-ns");
224241
TableIdentifier id = TableIdentifier.of("test-ns", "t1");
225242
Table table = restCatalog.createTable(id, SCHEMA);
@@ -228,7 +245,11 @@ public void testAppendFiles() throws IOException {
228245
@SuppressWarnings("resource")
229246
FileIO io = table.io();
230247

231-
URI loc = URI.create(table.locationProvider().newDataLocation("test-file1.txt"));
248+
URI loc =
249+
URI.create(
250+
table
251+
.locationProvider()
252+
.newDataLocation(String.format("test-file-%s.txt", pathStyle)));
232253
OutputFile f1 = io.newOutputFile(loc.toString());
233254
try (PositionOutputStream os = f1.create()) {
234255
os.write("Hello World".getBytes(UTF_8));

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

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
*/
1919
package org.apache.polaris.service.quarkus.entity;
2020

21+
import static org.assertj.core.api.Assertions.assertThat;
22+
23+
import com.fasterxml.jackson.core.JsonProcessingException;
24+
import com.fasterxml.jackson.databind.ObjectMapper;
2125
import java.util.List;
26+
import java.util.stream.Stream;
2227
import org.apache.polaris.core.PolarisCallContext;
2328
import org.apache.polaris.core.PolarisDefaultDiagServiceImpl;
2429
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
@@ -37,9 +42,12 @@
3742
import org.junit.jupiter.api.BeforeEach;
3843
import org.junit.jupiter.api.Test;
3944
import org.junit.jupiter.params.ParameterizedTest;
45+
import org.junit.jupiter.params.provider.Arguments;
46+
import org.junit.jupiter.params.provider.MethodSource;
4047
import org.junit.jupiter.params.provider.ValueSource;
4148

4249
public class CatalogEntityTest {
50+
private static final ObjectMapper MAPPER = new ObjectMapper();
4351

4452
private CallContext callContext;
4553

@@ -286,7 +294,7 @@ public void testCatalogTypeDefaultsToInternal() {
286294
.build();
287295

288296
Catalog catalog = catalogEntity.asCatalog();
289-
Assertions.assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.INTERNAL);
297+
assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.INTERNAL);
290298
}
291299

292300
@Test
@@ -309,7 +317,7 @@ public void testCatalogTypeExternalPreserved() {
309317
.build();
310318

311319
Catalog catalog = catalogEntity.asCatalog();
312-
Assertions.assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.EXTERNAL);
320+
assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.EXTERNAL);
313321
}
314322

315323
@Test
@@ -332,6 +340,60 @@ public void testCatalogTypeInternalExplicitlySet() {
332340
.build();
333341

334342
Catalog catalog = catalogEntity.asCatalog();
335-
Assertions.assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.INTERNAL);
343+
assertThat(catalog.getType()).isEqualTo(Catalog.TypeEnum.INTERNAL);
344+
}
345+
346+
@Test
347+
public void testAwsConfigJsonPropertiesPresence() throws JsonProcessingException {
348+
AwsStorageConfigInfo.Builder b =
349+
AwsStorageConfigInfo.builder()
350+
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
351+
.setRoleArn("arn:aws:iam::012345678901:role/test-role");
352+
assertThat(MAPPER.writeValueAsString(b.build())).contains("roleArn");
353+
assertThat(MAPPER.writeValueAsString(b.build())).doesNotContain("endpoint");
354+
assertThat(MAPPER.writeValueAsString(b.build())).doesNotContain("stsEndpoint");
355+
356+
b.setEndpoint("http://s3.example.com");
357+
b.setStsEndpoint("http://sts.example.com");
358+
b.setPathStyleAccess(false);
359+
assertThat(MAPPER.writeValueAsString(b.build())).contains("roleArn");
360+
assertThat(MAPPER.writeValueAsString(b.build())).contains("endpoint");
361+
assertThat(MAPPER.writeValueAsString(b.build())).contains("stsEndpoint");
362+
assertThat(MAPPER.writeValueAsString(b.build())).contains("pathStyleAccess");
363+
}
364+
365+
@ParameterizedTest
366+
@MethodSource
367+
public void testAwsConfigRoundTrip(AwsStorageConfigInfo config) throws JsonProcessingException {
368+
String configStr = MAPPER.writeValueAsString(config);
369+
CatalogEntity catalogEntity =
370+
new CatalogEntity.Builder()
371+
.setName("testAwsConfigRoundTrip")
372+
.setDefaultBaseLocation(config.getAllowedLocations().getFirst())
373+
.setCatalogType(Catalog.TypeEnum.INTERNAL.name())
374+
.setStorageConfigurationInfo(
375+
callContext,
376+
MAPPER.readValue(configStr, StorageConfigInfo.class),
377+
config.getAllowedLocations().getFirst())
378+
.build();
379+
380+
Catalog catalog = catalogEntity.asCatalog();
381+
assertThat(catalog.getStorageConfigInfo()).isEqualTo(config);
382+
assertThat(MAPPER.writeValueAsString(catalog.getStorageConfigInfo())).isEqualTo(configStr);
383+
}
384+
385+
public static Stream<Arguments> testAwsConfigRoundTrip() {
386+
AwsStorageConfigInfo.Builder b =
387+
AwsStorageConfigInfo.builder()
388+
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
389+
.setAllowedLocations(List.of("s3://example.com"))
390+
.setRoleArn("arn:aws:iam::012345678901:role/test-role");
391+
return Stream.of(
392+
Arguments.of(b.build()),
393+
Arguments.of(b.setExternalId("ex1").build()),
394+
Arguments.of(b.setRegion("us-west-2").build()),
395+
Arguments.of(b.setEndpoint("http://s3.example.com:1234").build()),
396+
Arguments.of(b.setStsEndpoint("http://sts.example.com:1234").build()),
397+
Arguments.of(b.setPathStyleAccess(true).build()));
336398
}
337399
}

0 commit comments

Comments
 (0)