Skip to content

Commit 532ee51

Browse files
authored
Remove config parameter from PolarisStorageIntegration#getSubscopedCreds (#2235)
Instances of `PolarisStorageIntegration` are created for a particular `PolarisStorageConfigurationInfo`, the same value is then passed into `PSI.getSubscopedCreds()`. This change removes the config parameter, as it's already known when `PolarisStorageIntegration` instances are created.
1 parent 64d815a commit 532ee51

16 files changed

+104
-136
lines changed

polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,14 +1607,10 @@ private void revokeGrantRecord(
16071607
catalogId,
16081608
entityId);
16091609

1610-
PolarisStorageConfigurationInfo storageConfigurationInfo =
1611-
BaseMetaStoreManager.extractStorageConfiguration(
1612-
callCtx.getDiagServices(), reloadedEntity.getEntity());
16131610
try {
16141611
AccessConfig accessConfig =
16151612
storageIntegration.getSubscopedCreds(
16161613
callCtx.getRealmConfig(),
1617-
storageConfigurationInfo,
16181614
allowListOperation,
16191615
allowedReadLocations,
16201616
allowedWriteLocations);

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,14 +2054,10 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
20542054
catalogId,
20552055
entityId);
20562056

2057-
PolarisStorageConfigurationInfo storageConfigurationInfo =
2058-
BaseMetaStoreManager.extractStorageConfiguration(
2059-
callCtx.getDiagServices(), reloadedEntity.getEntity());
20602057
try {
20612058
AccessConfig accessConfig =
20622059
storageIntegration.getSubscopedCreds(
20632060
callCtx.getRealmConfig(),
2064-
storageConfigurationInfo,
20652061
allowListOperation,
20662062
allowedReadLocations,
20672063
allowedWriteLocations);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@
3939
public abstract class InMemoryStorageIntegration<T extends PolarisStorageConfigurationInfo>
4040
extends PolarisStorageIntegration<T> {
4141

42-
public InMemoryStorageIntegration(String identifierOrId) {
43-
super(identifierOrId);
42+
protected InMemoryStorageIntegration(T config, String identifierOrId) {
43+
super(config, identifierOrId);
4444
}
4545

4646
/**

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,17 @@
3232
public abstract class PolarisStorageIntegration<T extends PolarisStorageConfigurationInfo> {
3333

3434
private final String integrationIdentifierOrId;
35+
private final T config;
3536

36-
public PolarisStorageIntegration(String identifierOrId) {
37+
public PolarisStorageIntegration(T config, String identifierOrId) {
38+
this.config = config;
3739
this.integrationIdentifierOrId = identifierOrId;
3840
}
3941

42+
protected T config() {
43+
return config;
44+
}
45+
4046
public String getStorageIdentifierOrId() {
4147
return integrationIdentifierOrId;
4248
}
@@ -45,7 +51,6 @@ public String getStorageIdentifierOrId() {
4551
* Subscope the creds against the allowed read and write locations.
4652
*
4753
* @param realmConfig the call context
48-
* @param storageConfig storage configuration
4954
* @param allowListOperation whether to allow LIST on all the provided allowed read/write
5055
* locations
5156
* @param allowedReadLocations a set of allowed to read locations
@@ -54,7 +59,6 @@ public String getStorageIdentifierOrId() {
5459
*/
5560
public abstract AccessConfig getSubscopedCreds(
5661
@Nonnull RealmConfig realmConfig,
57-
@Nonnull T storageConfig,
5862
boolean allowListOperation,
5963
@Nonnull Set<String> allowedReadLocations,
6064
@Nonnull Set<String> allowedWriteLocations);

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
* PolarisStorageConfigurationInfo}.
2626
*/
2727
public interface PolarisStorageIntegrationProvider {
28-
@SuppressWarnings("unchecked")
2928
<T extends PolarisStorageConfigurationInfo>
3029
@Nullable PolarisStorageIntegration<T> getStorageIntegrationForConfig(
3130
PolarisStorageConfigurationInfo polarisStorageConfigurationInfo);

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,21 @@ public class AwsCredentialsStorageIntegration
4949
private final StsClientProvider stsClientProvider;
5050
private final Optional<AwsCredentialsProvider> credentialsProvider;
5151

52-
public AwsCredentialsStorageIntegration(StsClient fixedClient) {
53-
this((destination) -> fixedClient);
52+
public AwsCredentialsStorageIntegration(
53+
AwsStorageConfigurationInfo config, StsClient fixedClient) {
54+
this(config, (destination) -> fixedClient);
5455
}
5556

56-
public AwsCredentialsStorageIntegration(StsClientProvider stsClientProvider) {
57-
this(stsClientProvider, Optional.empty());
57+
public AwsCredentialsStorageIntegration(
58+
AwsStorageConfigurationInfo config, StsClientProvider stsClientProvider) {
59+
this(config, stsClientProvider, Optional.empty());
5860
}
5961

6062
public AwsCredentialsStorageIntegration(
61-
StsClientProvider stsClientProvider, Optional<AwsCredentialsProvider> credentialsProvider) {
62-
super(AwsCredentialsStorageIntegration.class.getName());
63+
AwsStorageConfigurationInfo config,
64+
StsClientProvider stsClientProvider,
65+
Optional<AwsCredentialsProvider> credentialsProvider) {
66+
super(config, AwsCredentialsStorageIntegration.class.getName());
6367
this.stsClientProvider = stsClientProvider;
6468
this.credentialsProvider = credentialsProvider;
6569
}
@@ -68,12 +72,12 @@ public AwsCredentialsStorageIntegration(
6872
@Override
6973
public AccessConfig getSubscopedCreds(
7074
@Nonnull RealmConfig realmConfig,
71-
@Nonnull AwsStorageConfigurationInfo storageConfig,
7275
boolean allowListOperation,
7376
@Nonnull Set<String> allowedReadLocations,
7477
@Nonnull Set<String> allowedWriteLocations) {
7578
int storageCredentialDurationSeconds =
7679
realmConfig.getConfig(STORAGE_CREDENTIAL_DURATION_SECONDS);
80+
AwsStorageConfigurationInfo storageConfig = config();
7781
AssumeRoleRequest.Builder request =
7882
AssumeRoleRequest.builder()
7983
.externalId(storageConfig.getExternalId())

polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ public class AzureCredentialsStorageIntegration
6464

6565
final DefaultAzureCredential defaultAzureCredential;
6666

67-
public AzureCredentialsStorageIntegration() {
68-
super(AzureCredentialsStorageIntegration.class.getName());
67+
public AzureCredentialsStorageIntegration(AzureStorageConfigurationInfo config) {
68+
super(config, AzureCredentialsStorageIntegration.class.getName());
6969
// The DefaultAzureCredential will by default load the environment variables for client id,
7070
// client secret, tenant id
7171
defaultAzureCredential = new DefaultAzureCredentialBuilder().build();
@@ -74,7 +74,6 @@ public AzureCredentialsStorageIntegration() {
7474
@Override
7575
public AccessConfig getSubscopedCreds(
7676
@Nonnull RealmConfig realmConfig,
77-
@Nonnull AzureStorageConfigurationInfo storageConfig,
7877
boolean allowListOperation,
7978
@Nonnull Set<String> allowedReadLocations,
8079
@Nonnull Set<String> allowedWriteLocations) {
@@ -119,7 +118,7 @@ public AccessConfig getSubscopedCreds(
119118
OffsetDateTime.ofInstant(
120119
start.plusSeconds(3600), ZoneOffset.UTC); // 1 hr to sync with AWS and GCP Access token
121120

122-
AccessToken accessToken = getAccessToken(storageConfig.getTenantId());
121+
AccessToken accessToken = getAccessToken(config().getTenantId());
123122
// Get user delegation key.
124123
// Set the new generated user delegation key expiry to 7 days and minute 1 min
125124
// Azure strictly requires the end time to be <= 7 days from the current time, -1 min to avoid

polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ public class GcpCredentialsStorageIntegration
5959
private final HttpTransportFactory transportFactory;
6060

6161
public GcpCredentialsStorageIntegration(
62-
GoogleCredentials sourceCredentials, HttpTransportFactory transportFactory) {
63-
super(GcpCredentialsStorageIntegration.class.getName());
62+
GcpStorageConfigurationInfo config,
63+
GoogleCredentials sourceCredentials,
64+
HttpTransportFactory transportFactory) {
65+
super(config, GcpCredentialsStorageIntegration.class.getName());
6466
// Needed for when environment variable GOOGLE_APPLICATION_CREDENTIALS points to google service
6567
// account key json
6668
this.sourceCredentials =
@@ -71,7 +73,6 @@ public GcpCredentialsStorageIntegration(
7173
@Override
7274
public AccessConfig getSubscopedCreds(
7375
@Nonnull RealmConfig realmConfig,
74-
@Nonnull GcpStorageConfigurationInfo storageConfig,
7576
boolean allowListOperation,
7677
@Nonnull Set<String> allowedReadLocations,
7778
@Nonnull Set<String> allowedWriteLocations) {

polaris-core/src/test/java/org/apache/polaris/core/storage/InMemoryStorageIntegrationTest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.junit.jupiter.params.ParameterizedTest;
3434
import org.junit.jupiter.params.provider.CsvSource;
3535
import org.junit.jupiter.params.provider.ValueSource;
36+
import org.mockito.Mockito;
3637

3738
class InMemoryStorageIntegrationTest {
3839

@@ -203,13 +204,14 @@ public void testValidateAccessToLocationsWithPrefixOfAllowedLocation() {
203204
private static final class MockInMemoryStorageIntegration
204205
extends InMemoryStorageIntegration<PolarisStorageConfigurationInfo> {
205206
public MockInMemoryStorageIntegration() {
206-
super(MockInMemoryStorageIntegration.class.getName());
207+
super(
208+
Mockito.mock(PolarisStorageConfigurationInfo.class),
209+
MockInMemoryStorageIntegration.class.getName());
207210
}
208211

209212
@Override
210213
public AccessConfig getSubscopedCreds(
211214
@Nonnull RealmConfig realmConfig,
212-
@Nonnull PolarisStorageConfigurationInfo storageConfig,
213215
boolean allowListOperation,
214216
@Nonnull Set<String> allowedReadLocations,
215217
@Nonnull Set<String> allowedWriteLocations) {

0 commit comments

Comments
 (0)