Skip to content

Commit 8efb81e

Browse files
dimas-bdimas-b
committed
Refactor storage access configuration handling
This is a step towards supporting non-AWS S3 storage, but this refactoring is relevant to all storage backends. There is no change to existing behaviours. * Rename PolarisCredentialProperty to IcebergStorageAccessProperty and introduce non-credential properties (as an example for now) * IcebergStorageAccessProperty values are ultimately meant to be produced by PolarisStorageIntegration implementations * Some previous entries in IcebergStorageAccessProperty are not really credential properties, but their treatment is not changed in this PR to maintain exactly the same bahaviour as before. * Add AccessConfig to represent both credential and non-credential properties related to storage access.
1 parent de47f22 commit 8efb81e

23 files changed

+322
-204
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
import org.apache.polaris.core.policy.PolicyEntity;
6767
import org.apache.polaris.core.policy.PolicyMappingUtil;
6868
import org.apache.polaris.core.policy.PolicyType;
69-
import org.apache.polaris.core.storage.PolarisCredentialProperty;
69+
import org.apache.polaris.core.storage.IcebergStorageAccessProperty;
7070
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
7171
import org.apache.polaris.core.storage.PolarisStorageIntegration;
7272
import org.slf4j.Logger;
@@ -1605,7 +1605,7 @@ private void revokeGrantRecord(
16051605
PolarisStorageConfigurationInfo storageConfigurationInfo =
16061606
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
16071607
try {
1608-
EnumMap<PolarisCredentialProperty, String> creds =
1608+
EnumMap<IcebergStorageAccessProperty, String> creds =
16091609
storageIntegration.getSubscopedCreds(
16101610
callCtx.getDiagServices(),
16111611
storageConfigurationInfo,

‎polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/ScopedCredentialsResult.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@
2424
import jakarta.annotation.Nullable;
2525
import java.util.EnumMap;
2626
import java.util.Map;
27-
import org.apache.polaris.core.storage.PolarisCredentialProperty;
27+
import org.apache.polaris.core.storage.IcebergStorageAccessProperty;
2828

2929
/** Result of a getSubscopedCredsForEntity() call */
3030
public class ScopedCredentialsResult extends BaseResult {
3131

3232
// null if not success. Else, set of name/value pairs for the credentials
33-
private final EnumMap<PolarisCredentialProperty, String> credentials;
33+
private final EnumMap<IcebergStorageAccessProperty, String> credentials;
3434

3535
/**
3636
* Constructor for an error
@@ -49,7 +49,8 @@ public ScopedCredentialsResult(
4949
*
5050
* @param credentials credentials
5151
*/
52-
public ScopedCredentialsResult(@Nonnull EnumMap<PolarisCredentialProperty, String> credentials) {
52+
public ScopedCredentialsResult(
53+
@Nonnull EnumMap<IcebergStorageAccessProperty, String> credentials) {
5354
super(ReturnStatus.SUCCESS);
5455
this.credentials = credentials;
5556
}
@@ -60,13 +61,14 @@ private ScopedCredentialsResult(
6061
@JsonProperty("extraInformation") String extraInformation,
6162
@JsonProperty("credentials") Map<String, String> credentials) {
6263
super(returnStatus, extraInformation);
63-
this.credentials = new EnumMap<>(PolarisCredentialProperty.class);
64+
this.credentials = new EnumMap<>(IcebergStorageAccessProperty.class);
6465
if (credentials != null) {
65-
credentials.forEach((k, v) -> this.credentials.put(PolarisCredentialProperty.valueOf(k), v));
66+
credentials.forEach(
67+
(k, v) -> this.credentials.put(IcebergStorageAccessProperty.valueOf(k), v));
6668
}
6769
}
6870

69-
public EnumMap<PolarisCredentialProperty, String> getCredentials() {
71+
public EnumMap<IcebergStorageAccessProperty, String> getCredentials() {
7072
return credentials;
7173
}
7274
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
import org.apache.polaris.core.policy.PolicyEntity;
6868
import org.apache.polaris.core.policy.PolicyMappingUtil;
6969
import org.apache.polaris.core.policy.PolicyType;
70-
import org.apache.polaris.core.storage.PolarisCredentialProperty;
70+
import org.apache.polaris.core.storage.IcebergStorageAccessProperty;
7171
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
7272
import org.apache.polaris.core.storage.PolarisStorageIntegration;
7373
import org.slf4j.Logger;
@@ -2035,7 +2035,7 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
20352035
PolarisStorageConfigurationInfo storageConfigurationInfo =
20362036
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
20372037
try {
2038-
EnumMap<PolarisCredentialProperty, String> creds =
2038+
EnumMap<IcebergStorageAccessProperty, String> creds =
20392039
storageIntegration.getSubscopedCreds(
20402040
callCtx.getDiagServices(),
20412041
storageConfigurationInfo,

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.polaris.core.storage;
20+
21+
import java.util.Map;
22+
import org.apache.polaris.immutables.PolarisImmutable;
23+
24+
@PolarisImmutable
25+
public interface AccessConfig {
26+
Map<String, String> credentials();
27+
28+
Map<String, String> extraProperties();
29+
30+
static ImmutableAccessConfig.Builder builder() {
31+
return ImmutableAccessConfig.builder();
32+
}
33+
}

‎polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialProperty.java‎ renamed to ‎polaris-core/src/main/java/org/apache/polaris/core/storage/IcebergStorageAccessProperty.java‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,23 @@
1818
*/
1919
package org.apache.polaris.core.storage;
2020

21-
/** Enum of polaris supported credential properties */
22-
public enum PolarisCredentialProperty {
21+
/**
22+
* A subset of Iceberg catalog properties recognized by Polaris.
23+
*
24+
* <p>Most of these properties are meant to configure Iceberg FileIO objects for accessing data in
25+
* storage.
26+
*/
27+
public enum IcebergStorageAccessProperty {
2328
AWS_KEY_ID(String.class, "s3.access-key-id", "the aws access key id"),
2429
AWS_SECRET_KEY(String.class, "s3.secret-access-key", "the aws access key secret"),
2530
AWS_TOKEN(String.class, "s3.session-token", "the aws scoped access token"),
2631
AWS_SESSION_TOKEN_EXPIRES_AT_MS(
2732
String.class,
2833
"s3.session-token-expires-at-ms",
2934
"the time the aws session token expires, in milliseconds"),
35+
AWS_ENDPOINT(String.class, "s3.endpoint", "the S3 endpoint to use for requests", false),
36+
AWS_PATH_STYLE_ACCESS(
37+
Boolean.class, "s3.path-style-access", "whether to use S3 path style access", false),
3038
CLIENT_REGION(
3139
String.class, "client.region", "region to configure client for making requests to AWS"),
3240

@@ -50,19 +58,30 @@ public enum PolarisCredentialProperty {
5058
private final Class valueType;
5159
private final String propertyName;
5260
private final String description;
61+
private final boolean isCredential;
5362

5463
/*
5564
s3.access-key-id`: id for for credentials that provide access to the data in S3
5665
- `s3.secret-access-key`: secret for credentials that provide access to data in S3
5766
- `s3.session-token
5867
*/
59-
PolarisCredentialProperty(Class valueType, String propertyName, String description) {
68+
IcebergStorageAccessProperty(Class valueType, String propertyName, String description) {
69+
this(valueType, propertyName, description, true);
70+
}
71+
72+
IcebergStorageAccessProperty(
73+
Class valueType, String propertyName, String description, boolean isCredential) {
6074
this.valueType = valueType;
6175
this.propertyName = propertyName;
6276
this.description = description;
77+
this.isCredential = isCredential;
6378
}
6479

6580
public String getPropertyName() {
6681
return propertyName;
6782
}
83+
84+
public boolean isCredential() {
85+
return isCredential;
86+
}
6887
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public String getStorageIdentifierOrId() {
5454
* @param allowedWriteLocations a set of allowed to write locations
5555
* @return An enum map including the scoped credentials
5656
*/
57-
public abstract EnumMap<PolarisCredentialProperty, String> getSubscopedCreds(
57+
public abstract EnumMap<IcebergStorageAccessProperty, String> getSubscopedCreds(
5858
@Nonnull PolarisDiagnostics diagnostics,
5959
@Nonnull T storageConfig,
6060
boolean allowListOperation,

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

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030
import java.util.Set;
3131
import java.util.stream.Stream;
3232
import org.apache.polaris.core.PolarisDiagnostics;
33+
import org.apache.polaris.core.storage.IcebergStorageAccessProperty;
3334
import org.apache.polaris.core.storage.InMemoryStorageIntegration;
34-
import org.apache.polaris.core.storage.PolarisCredentialProperty;
3535
import org.apache.polaris.core.storage.StorageUtil;
3636
import software.amazon.awssdk.policybuilder.iam.IamConditionOperator;
3737
import software.amazon.awssdk.policybuilder.iam.IamEffect;
@@ -54,7 +54,7 @@ public AwsCredentialsStorageIntegration(StsClient stsClient) {
5454

5555
/** {@inheritDoc} */
5656
@Override
57-
public EnumMap<PolarisCredentialProperty, String> getSubscopedCreds(
57+
public EnumMap<IcebergStorageAccessProperty, String> getSubscopedCreds(
5858
@Nonnull PolarisDiagnostics diagnostics,
5959
@Nonnull AwsStorageConfigurationInfo storageConfig,
6060
boolean allowListOperation,
@@ -75,28 +75,30 @@ public EnumMap<PolarisCredentialProperty, String> getSubscopedCreds(
7575
.toJson())
7676
.durationSeconds(loadConfig(STORAGE_CREDENTIAL_DURATION_SECONDS))
7777
.build());
78-
EnumMap<PolarisCredentialProperty, String> credentialMap =
79-
new EnumMap<>(PolarisCredentialProperty.class);
80-
credentialMap.put(PolarisCredentialProperty.AWS_KEY_ID, response.credentials().accessKeyId());
78+
EnumMap<IcebergStorageAccessProperty, String> credentialMap =
79+
new EnumMap<>(IcebergStorageAccessProperty.class);
8180
credentialMap.put(
82-
PolarisCredentialProperty.AWS_SECRET_KEY, response.credentials().secretAccessKey());
83-
credentialMap.put(PolarisCredentialProperty.AWS_TOKEN, response.credentials().sessionToken());
81+
IcebergStorageAccessProperty.AWS_KEY_ID, response.credentials().accessKeyId());
82+
credentialMap.put(
83+
IcebergStorageAccessProperty.AWS_SECRET_KEY, response.credentials().secretAccessKey());
84+
credentialMap.put(
85+
IcebergStorageAccessProperty.AWS_TOKEN, response.credentials().sessionToken());
8486
Optional.ofNullable(response.credentials().expiration())
8587
.ifPresent(
8688
i -> {
8789
credentialMap.put(
88-
PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(i.toEpochMilli()));
90+
IcebergStorageAccessProperty.EXPIRATION_TIME, String.valueOf(i.toEpochMilli()));
8991
credentialMap.put(
90-
PolarisCredentialProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
92+
IcebergStorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
9193
String.valueOf(i.toEpochMilli()));
9294
});
9395

9496
if (storageConfig.getRegion() != null) {
95-
credentialMap.put(PolarisCredentialProperty.CLIENT_REGION, storageConfig.getRegion());
97+
credentialMap.put(IcebergStorageAccessProperty.CLIENT_REGION, storageConfig.getRegion());
9698
}
9799

98100
if (storageConfig.getAwsPartition().equals("aws-us-gov")
99-
&& credentialMap.get(PolarisCredentialProperty.CLIENT_REGION) == null) {
101+
&& credentialMap.get(IcebergStorageAccessProperty.CLIENT_REGION) == null) {
100102
throw new IllegalArgumentException(
101103
String.format(
102104
"AWS region must be set when using partition %s", storageConfig.getAwsPartition()));

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@
4747
import java.util.Set;
4848
import org.apache.polaris.core.PolarisDiagnostics;
4949
import org.apache.polaris.core.config.FeatureConfiguration;
50+
import org.apache.polaris.core.storage.IcebergStorageAccessProperty;
5051
import org.apache.polaris.core.storage.InMemoryStorageIntegration;
51-
import org.apache.polaris.core.storage.PolarisCredentialProperty;
5252
import org.slf4j.Logger;
5353
import org.slf4j.LoggerFactory;
5454
import reactor.core.publisher.Mono;
@@ -70,14 +70,14 @@ public AzureCredentialsStorageIntegration() {
7070
}
7171

7272
@Override
73-
public EnumMap<PolarisCredentialProperty, String> getSubscopedCreds(
73+
public EnumMap<IcebergStorageAccessProperty, String> getSubscopedCreds(
7474
@Nonnull PolarisDiagnostics diagnostics,
7575
@Nonnull AzureStorageConfigurationInfo storageConfig,
7676
boolean allowListOperation,
7777
@Nonnull Set<String> allowedReadLocations,
7878
@Nonnull Set<String> allowedWriteLocations) {
79-
EnumMap<PolarisCredentialProperty, String> credentialMap =
80-
new EnumMap<>(PolarisCredentialProperty.class);
79+
EnumMap<IcebergStorageAccessProperty, String> credentialMap =
80+
new EnumMap<>(IcebergStorageAccessProperty.class);
8181
String loc =
8282
!allowedWriteLocations.isEmpty()
8383
? allowedWriteLocations.stream().findAny().orElse(null)
@@ -170,10 +170,10 @@ public EnumMap<PolarisCredentialProperty, String> getSubscopedCreds(
170170
throw new RuntimeException(
171171
String.format("Endpoint %s not supported", location.getEndpoint()));
172172
}
173-
credentialMap.put(PolarisCredentialProperty.AZURE_SAS_TOKEN, sasToken);
174-
credentialMap.put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, storageDnsName);
173+
credentialMap.put(IcebergStorageAccessProperty.AZURE_SAS_TOKEN, sasToken);
174+
credentialMap.put(IcebergStorageAccessProperty.AZURE_ACCOUNT_HOST, storageDnsName);
175175
credentialMap.put(
176-
PolarisCredentialProperty.EXPIRATION_TIME,
176+
IcebergStorageAccessProperty.EXPIRATION_TIME,
177177
String.valueOf(sanitizedEndTime.toInstant().toEpochMilli()));
178178
return credentialMap;
179179
}

‎polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCache.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.github.benmanes.caffeine.cache.LoadingCache;
2424
import com.google.common.annotations.VisibleForTesting;
2525
import jakarta.annotation.Nonnull;
26+
import jakarta.annotation.Nullable;
2627
import java.time.Duration;
2728
import java.util.Map;
2829
import java.util.Optional;
@@ -35,6 +36,7 @@
3536
import org.apache.polaris.core.entity.PolarisEntity;
3637
import org.apache.polaris.core.entity.PolarisEntityType;
3738
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
39+
import org.apache.polaris.core.storage.AccessConfig;
3840
import org.apache.polaris.core.storage.PolarisCredentialVendor;
3941
import org.slf4j.Logger;
4042
import org.slf4j.LoggerFactory;
@@ -99,7 +101,7 @@ private static long maxCacheDurationMs() {
99101
* @param allowedWriteLocations a set of allowed to write locations.
100102
* @return the a map of string containing the scoped creds information
101103
*/
102-
public Map<String, String> getOrGenerateSubScopeCreds(
104+
public AccessConfig getOrGenerateSubScopeCreds(
103105
@Nonnull PolarisCredentialVendor credentialVendor,
104106
@Nonnull PolarisCallContext callCtx,
105107
@Nonnull PolarisEntity polarisEntity,
@@ -142,13 +144,19 @@ public Map<String, String> getOrGenerateSubScopeCreds(
142144
"Failed to get subscoped credentials: %s",
143145
scopedCredentialsResult.getExtraInformation());
144146
};
145-
return cache.get(key, loader).convertToMapOfString();
147+
return cache.get(key, loader).toAccessConfig();
146148
}
147149

148-
public Map<String, String> getIfPresent(StorageCredentialCacheKey key) {
150+
@VisibleForTesting
151+
@Nullable
152+
Map<String, String> getIfPresent(StorageCredentialCacheKey key) {
153+
return getAccessConfig(key).map(AccessConfig::credentials).orElse(null);
154+
}
155+
156+
@VisibleForTesting
157+
Optional<AccessConfig> getAccessConfig(StorageCredentialCacheKey key) {
149158
return Optional.ofNullable(cache.getIfPresent(key))
150-
.map(StorageCredentialCacheEntry::convertToMapOfString)
151-
.orElse(null);
159+
.map(StorageCredentialCacheEntry::toAccessConfig);
152160
}
153161

154162
private boolean isTypeSupported(PolarisEntityType type) {

0 commit comments

Comments
 (0)