Skip to content

Commit 10b7187

Browse files
committed
Enforce that S3 credentials are vended when requested
This is a follow-up change to #2672 striving to improve user-facing error reporting for S3 storage systems without STS. * Add property to `AccessConfig` to indicate whether the backing storage integration can produce credentials. * Add a check to `IcebergCatalogHandler` (leading to 400) that storage credentials are vended when requested and the backend is capable of vending credentials in principle. * Update `PolarisStorageIntegrationProviderImpl` to indicate that FILE storage does not support credential vending (requesitng redential vending with FILE storage does not produce any credentials and does not flag an error, which matches current Polaris behaviour). * Only those S3 systems where STS is not available (or disabled / not permitted) are affected. * Other storage integrations are not affected by this PR.
1 parent baacb47 commit 10b7187

File tree

7 files changed

+98
-10
lines changed

7 files changed

+98
-10
lines changed

polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
20932093
PolarisStorageIntegration<PolarisStorageConfigurationInfo> storageIntegration =
20942094
ms.loadPolarisStorageIntegrationInCurrentTxn(callCtx, reloadedEntity.getEntity());
20952095

2096+
storageIntegration.config();
20962097
// cannot be null
20972098
getDiagnostics()
20982099
.checkNotNull(

polaris-core/src/main/java/org/apache/polaris/core/storage/AccessConfig.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.Map;
2424
import java.util.Optional;
2525
import org.apache.polaris.immutables.PolarisImmutable;
26+
import org.immutables.value.Value;
2627

2728
@PolarisImmutable
2829
public interface AccessConfig {
@@ -38,6 +39,15 @@ public interface AccessConfig {
3839

3940
Optional<Instant> expiresAt();
4041

42+
/**
43+
* Indicates whether the storage integration subsystem that produced this object is capable of
44+
* credential vending in principle.
45+
*/
46+
@Value.Default
47+
default boolean supportsCredentialVending() {
48+
return true;
49+
}
50+
4151
default String get(StorageAccessProperty key) {
4252
if (key.isCredential()) {
4353
return credentials().get(key.getPropertyName());
@@ -64,6 +74,9 @@ interface Builder {
6474
@CanIgnoreReturnValue
6575
Builder expiresAt(Instant expiresAt);
6676

77+
@CanIgnoreReturnValue
78+
Builder supportsCredentialVending(boolean supportsCredentialVending);
79+
6780
default Builder put(StorageAccessProperty key, String value) {
6881
if (key.isExpirationTimestamp()) {
6982
expiresAt(Instant.ofEpochMilli(Long.parseLong(value)));

polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageIntegration.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public PolarisStorageIntegration(T config, String identifierOrId) {
4040
this.integrationIdentifierOrId = identifierOrId;
4141
}
4242

43-
protected T config() {
43+
public T config() {
4444
return config;
4545
}
4646

@@ -108,6 +108,14 @@ public abstract AccessConfig getSubscopedCreds(
108108
@Nonnull Set<PolarisStorageActions> actions,
109109
@Nonnull Set<String> locations);
110110

111+
/**
112+
* @param credentialsRequired if {@code true} indicates that the caller requires the returned
113+
* {@link AccessConfig} to have storage credentials; if {@code false} the returned {@link
114+
* AccessConfig} may or may not contain credentials.
115+
*/
116+
public void validateCredentials(
117+
@Nonnull AccessConfig accessConfig, boolean credentialsRequired) {}
118+
111119
/**
112120
* Result of calling {@link #validateAccessToLocations(RealmConfig,
113121
* PolarisStorageConfigurationInfo, Set, Set)}

runtime/service/src/intTest/java/org/apache/polaris/service/it/RestCatalogMinIOSpecialIT.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static org.apache.polaris.service.catalog.AccessDelegationMode.VENDED_CREDENTIALS;
3232
import static org.apache.polaris.service.it.env.PolarisClient.polarisClient;
3333
import static org.assertj.core.api.Assertions.assertThat;
34+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3435

3536
import com.google.common.collect.ImmutableMap;
3637
import io.quarkus.test.junit.QuarkusIntegrationTest;
@@ -301,6 +302,62 @@ public void testInternalEndpoints() throws IOException {
301302
}
302303
}
303304

305+
@Test
306+
public void testCreateTableFailureWithCredentialVendingWithoutSts() throws IOException {
307+
try (RESTCatalog restCatalog =
308+
createCatalog(
309+
Optional.of(endpoint),
310+
Optional.of("http://sts.example.com"), // not called
311+
false,
312+
Optional.of(VENDED_CREDENTIALS),
313+
false)) {
314+
StorageConfigInfo storageConfig =
315+
managementApi.getCatalog(catalogName).getStorageConfigInfo();
316+
assertThat((AwsStorageConfigInfo) storageConfig)
317+
.extracting(
318+
AwsStorageConfigInfo::getEndpoint,
319+
AwsStorageConfigInfo::getStsEndpoint,
320+
AwsStorageConfigInfo::getEndpointInternal,
321+
AwsStorageConfigInfo::getPathStyleAccess,
322+
AwsStorageConfigInfo::getStsUnavailable)
323+
.containsExactly(endpoint, "http://sts.example.com", null, false, true);
324+
325+
catalogApi.createNamespace(catalogName, "test-ns");
326+
TableIdentifier id = TableIdentifier.of("test-ns", "t2");
327+
// Credential vending is not supported without STS
328+
assertThatThrownBy(() -> restCatalog.createTable(id, SCHEMA))
329+
.hasMessageContaining("but no credentials are available")
330+
.hasMessageContaining(id.toString());
331+
}
332+
}
333+
334+
@Test
335+
public void testLoadTableFailureWithCredentialVendingWithoutSts() throws IOException {
336+
try (RESTCatalog restCatalog =
337+
createCatalog(
338+
Optional.of(endpoint),
339+
Optional.of("http://sts.example.com"), // not called
340+
false,
341+
Optional.empty(),
342+
false)) {
343+
344+
catalogApi.createNamespace(catalogName, "test-ns");
345+
TableIdentifier id = TableIdentifier.of("test-ns", "t3");
346+
restCatalog.createTable(id, SCHEMA);
347+
348+
// Credential vending is not supported without STS
349+
assertThatThrownBy(
350+
() ->
351+
catalogApi.loadTable(
352+
catalogName,
353+
id,
354+
"ALL",
355+
Map.of("X-Iceberg-Access-Delegation", VENDED_CREDENTIALS.protocolValue())))
356+
.hasMessageContaining("but no credentials are available")
357+
.hasMessageContaining(id.toString());
358+
}
359+
}
360+
304361
public LoadTableResponse doTestCreateTable(
305362
RESTCatalog restCatalog, Optional<AccessDelegationMode> dm) {
306363
catalogApi.createNamespace(catalogName, "test-ns");

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,7 @@ public AccessConfig getAccessConfig(
844844
.atWarn()
845845
.addKeyValue("tableIdentifier", tableIdentifier)
846846
.log("Table entity has no storage configuration in its hierarchy");
847-
return AccessConfig.builder().build();
847+
return AccessConfig.builder().supportsCredentialVending(false).build();
848848
}
849849
return FileIOUtil.refreshAccessConfig(
850850
callContext,

runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -804,13 +804,22 @@ private LoadTableResponse.Builder buildLoadTableResponseWithDelegationCredential
804804
credentialDelegation.getAccessConfig(
805805
tableIdentifier, tableMetadata, actions, refreshCredentialsEndpoint);
806806
Map<String, String> credentialConfig = accessConfig.credentials();
807-
if (!credentialConfig.isEmpty() && delegationModes.contains(VENDED_CREDENTIALS)) {
808-
responseBuilder.addAllConfig(credentialConfig);
809-
responseBuilder.addCredential(
810-
ImmutableCredential.builder()
811-
.prefix(tableMetadata.location())
812-
.config(credentialConfig)
813-
.build());
807+
if (delegationModes.contains(VENDED_CREDENTIALS)) {
808+
if (!credentialConfig.isEmpty()) {
809+
responseBuilder.addAllConfig(credentialConfig);
810+
responseBuilder.addCredential(
811+
ImmutableCredential.builder()
812+
.prefix(tableMetadata.location())
813+
.config(credentialConfig)
814+
.build());
815+
} else {
816+
Boolean skipCredIndirection =
817+
realmConfig.getConfig(FeatureConfiguration.SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION);
818+
Preconditions.checkArgument(
819+
!accessConfig.supportsCredentialVending() || skipCredIndirection,
820+
"Credential vending was requested for table %s, but no credentials are available",
821+
tableIdentifier);
822+
}
814823
}
815824
responseBuilder.addAllConfig(accessConfig.extraProperties());
816825
}

runtime/service/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public AccessConfig getSubscopedCreds(
115115
@Nonnull Set<String> allowedReadLocations,
116116
@Nonnull Set<String> allowedWriteLocations,
117117
Optional<String> refreshCredentialsEndpoint) {
118-
return AccessConfig.builder().build();
118+
return AccessConfig.builder().supportsCredentialVending(false).build();
119119
}
120120

121121
@Override

0 commit comments

Comments
 (0)