Skip to content

Commit 7b04abe

Browse files
dimas-bdimas-b
authored
Push AccessConfig creation to PolarisStorageIntegration (#2171)
This refactoring does not change Polaris behaviour. * Move storage-specific access properties processing logic from core code to storage integration implementations. * Add `isExpirationTimestamp` flag to `StorageAccessProperty` to allow them to be processed uniformly. * Prepare for supporting access config properties that may have different values in Polaris Servers and Clients. This enables future enhancements to support different S3 endpoint DNS names in servers and clients for #1530
1 parent a39bce4 commit 7b04abe

21 files changed

+413
-361
lines changed

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import jakarta.annotation.Nonnull;
2222
import jakarta.annotation.Nullable;
2323
import java.util.ArrayList;
24-
import java.util.EnumMap;
2524
import java.util.HashMap;
2625
import java.util.HashSet;
2726
import java.util.List;
@@ -71,9 +70,9 @@
7170
import org.apache.polaris.core.policy.PolicyEntity;
7271
import org.apache.polaris.core.policy.PolicyMappingUtil;
7372
import org.apache.polaris.core.policy.PolicyType;
73+
import org.apache.polaris.core.storage.AccessConfig;
7474
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
7575
import org.apache.polaris.core.storage.PolarisStorageIntegration;
76-
import org.apache.polaris.core.storage.StorageAccessProperty;
7776
import org.slf4j.Logger;
7877
import org.slf4j.LoggerFactory;
7978

@@ -1614,14 +1613,14 @@ private void revokeGrantRecord(
16141613
PolarisStorageConfigurationInfo storageConfigurationInfo =
16151614
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
16161615
try {
1617-
EnumMap<StorageAccessProperty, String> creds =
1616+
AccessConfig accessConfig =
16181617
storageIntegration.getSubscopedCreds(
16191618
callCtx,
16201619
storageConfigurationInfo,
16211620
allowListOperation,
16221621
allowedReadLocations,
16231622
allowedWriteLocations);
1624-
return new ScopedCredentialsResult(creds);
1623+
return new ScopedCredentialsResult(accessConfig);
16251624
} catch (Exception ex) {
16261625
return new ScopedCredentialsResult(
16271626
BaseResult.ReturnStatus.SUBSCOPE_CREDS_ERROR, ex.getMessage());

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

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,15 @@
1818
*/
1919
package org.apache.polaris.core.persistence.dao.entity;
2020

21-
import com.fasterxml.jackson.annotation.JsonCreator;
22-
import com.fasterxml.jackson.annotation.JsonProperty;
2321
import jakarta.annotation.Nonnull;
2422
import jakarta.annotation.Nullable;
25-
import java.util.EnumMap;
26-
import java.util.Map;
27-
import org.apache.polaris.core.storage.StorageAccessProperty;
23+
import org.apache.polaris.core.storage.AccessConfig;
2824

2925
/** Result of a getSubscopedCredsForEntity() call */
3026
public class ScopedCredentialsResult extends BaseResult {
3127

3228
// null if not success. Else, set of name/value pairs for the credentials
33-
private final EnumMap<StorageAccessProperty, String> credentials;
29+
private final AccessConfig accessConfig;
3430

3531
/**
3632
* Constructor for an error
@@ -41,32 +37,20 @@ public class ScopedCredentialsResult extends BaseResult {
4137
public ScopedCredentialsResult(
4238
@Nonnull ReturnStatus errorCode, @Nullable String extraInformation) {
4339
super(errorCode, extraInformation);
44-
this.credentials = null;
40+
this.accessConfig = null;
4541
}
4642

4743
/**
4844
* Constructor for success
4945
*
50-
* @param credentials credentials
46+
* @param accessConfig credentials
5147
*/
52-
public ScopedCredentialsResult(@Nonnull EnumMap<StorageAccessProperty, String> credentials) {
48+
public ScopedCredentialsResult(AccessConfig accessConfig) {
5349
super(ReturnStatus.SUCCESS);
54-
this.credentials = credentials;
50+
this.accessConfig = accessConfig;
5551
}
5652

57-
@JsonCreator
58-
private ScopedCredentialsResult(
59-
@JsonProperty("returnStatus") @Nonnull ReturnStatus returnStatus,
60-
@JsonProperty("extraInformation") String extraInformation,
61-
@JsonProperty("credentials") Map<String, String> credentials) {
62-
super(returnStatus, extraInformation);
63-
this.credentials = new EnumMap<>(StorageAccessProperty.class);
64-
if (credentials != null) {
65-
credentials.forEach((k, v) -> this.credentials.put(StorageAccessProperty.valueOf(k), v));
66-
}
67-
}
68-
69-
public EnumMap<StorageAccessProperty, String> getCredentials() {
70-
return credentials;
53+
public AccessConfig getAccessConfig() {
54+
return accessConfig;
7155
}
7256
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import jakarta.annotation.Nonnull;
2323
import jakarta.annotation.Nullable;
2424
import java.util.ArrayList;
25-
import java.util.EnumMap;
2625
import java.util.HashMap;
2726
import java.util.HashSet;
2827
import java.util.List;
@@ -76,9 +75,9 @@
7675
import org.apache.polaris.core.policy.PolicyEntity;
7776
import org.apache.polaris.core.policy.PolicyMappingUtil;
7877
import org.apache.polaris.core.policy.PolicyType;
78+
import org.apache.polaris.core.storage.AccessConfig;
7979
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
8080
import org.apache.polaris.core.storage.PolarisStorageIntegration;
81-
import org.apache.polaris.core.storage.StorageAccessProperty;
8281
import org.slf4j.Logger;
8382
import org.slf4j.LoggerFactory;
8483

@@ -2062,14 +2061,14 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
20622061
PolarisStorageConfigurationInfo storageConfigurationInfo =
20632062
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
20642063
try {
2065-
EnumMap<StorageAccessProperty, String> creds =
2064+
AccessConfig accessConfig =
20662065
storageIntegration.getSubscopedCreds(
20672066
callCtx,
20682067
storageConfigurationInfo,
20692068
allowListOperation,
20702069
allowedReadLocations,
20712070
allowedWriteLocations);
2072-
return new ScopedCredentialsResult(creds);
2071+
return new ScopedCredentialsResult(accessConfig);
20732072
} catch (Exception ex) {
20742073
return new ScopedCredentialsResult(
20752074
BaseResult.ReturnStatus.SUBSCOPE_CREDS_ERROR, ex.getMessage());

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

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
*/
1919
package org.apache.polaris.core.storage;
2020

21+
import com.google.errorprone.annotations.CanIgnoreReturnValue;
22+
import java.time.Instant;
2123
import java.util.Map;
24+
import java.util.Optional;
2225
import org.apache.polaris.immutables.PolarisImmutable;
2326

2427
@PolarisImmutable
@@ -27,7 +30,52 @@ public interface AccessConfig {
2730

2831
Map<String, String> extraProperties();
2932

30-
static ImmutableAccessConfig.Builder builder() {
33+
/**
34+
* Configuration properties that are relevant only to the Polaris Server, but not to clients.
35+
* These properties override corresponding entries from {@link #extraProperties()}.
36+
*/
37+
Map<String, String> internalProperties();
38+
39+
Optional<Instant> expiresAt();
40+
41+
default String get(StorageAccessProperty key) {
42+
if (key.isCredential()) {
43+
return credentials().get(key.getPropertyName());
44+
} else {
45+
String value = internalProperties().get(key.getPropertyName());
46+
return value != null ? value : extraProperties().get(key.getPropertyName());
47+
}
48+
}
49+
50+
static AccessConfig.Builder builder() {
3151
return ImmutableAccessConfig.builder();
3252
}
53+
54+
interface Builder {
55+
@CanIgnoreReturnValue
56+
Builder putCredential(String key, String value);
57+
58+
@CanIgnoreReturnValue
59+
Builder putExtraProperty(String key, String value);
60+
61+
@CanIgnoreReturnValue
62+
Builder putInternalProperty(String key, String value);
63+
64+
@CanIgnoreReturnValue
65+
Builder expiresAt(Instant expiresAt);
66+
67+
default Builder put(StorageAccessProperty key, String value) {
68+
if (key.isExpirationTimestamp()) {
69+
expiresAt(Instant.ofEpochMilli(Long.parseLong(value)));
70+
}
71+
72+
if (key.isCredential()) {
73+
return putCredential(key.getPropertyName(), value);
74+
} else {
75+
return putExtraProperty(key.getPropertyName(), value);
76+
}
77+
}
78+
79+
AccessConfig build();
80+
}
3381
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.polaris.core.storage;
2020

2121
import jakarta.annotation.Nonnull;
22-
import java.util.EnumMap;
2322
import java.util.Map;
2423
import java.util.Set;
2524
import org.apache.polaris.core.context.CallContext;
@@ -53,7 +52,7 @@ public String getStorageIdentifierOrId() {
5352
* @param allowedWriteLocations a set of allowed to write locations
5453
* @return An enum map including the scoped credentials
5554
*/
56-
public abstract EnumMap<StorageAccessProperty, String> getSubscopedCreds(
55+
public abstract AccessConfig getSubscopedCreds(
5756
@Nonnull CallContext callContext,
5857
@Nonnull T storageConfig,
5958
boolean allowListOperation,

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ public enum StorageAccessProperty {
3131
AWS_SESSION_TOKEN_EXPIRES_AT_MS(
3232
String.class,
3333
"s3.session-token-expires-at-ms",
34-
"the time the aws session token expires, in milliseconds"),
34+
"the time the aws session token expires, in milliseconds",
35+
true,
36+
true),
3537
AWS_ENDPOINT(String.class, "s3.endpoint", "the S3 endpoint to use for requests", false),
3638
AWS_PATH_STYLE_ACCESS(
3739
Boolean.class, "s3.path-style-access", "whether to use S3 path style access", false),
@@ -42,23 +44,26 @@ public enum StorageAccessProperty {
4244
GCS_ACCESS_TOKEN_EXPIRES_AT(
4345
String.class,
4446
"gcs.oauth2.token-expires-at",
45-
"the time the gcs access token expires, in milliseconds"),
47+
"the time the gcs access token expires, in milliseconds",
48+
true,
49+
true),
4650

4751
// Currently not using ACCESS TOKEN as the ResolvingFileIO is using ADLSFileIO for azure case and
4852
// it expects for SAS
4953
AZURE_ACCESS_TOKEN(String.class, "", "the azure scoped access token"),
5054
AZURE_SAS_TOKEN(String.class, "adls.sas-token.", "an azure shared access signature token"),
51-
AZURE_ACCOUNT_HOST(
52-
String.class,
53-
"the azure storage account host",
54-
"the azure account name + endpoint that will append to the ADLS_SAS_TOKEN_PREFIX"),
5555
EXPIRATION_TIME(
56-
Long.class, "expiration-time", "the expiration time for the access token, in milliseconds");
56+
Long.class,
57+
"expiration-time",
58+
"the expiration time for the access token, in milliseconds",
59+
true,
60+
true);
5761

5862
private final Class valueType;
5963
private final String propertyName;
6064
private final String description;
6165
private final boolean isCredential;
66+
private final boolean isExpirationTimestamp;
6267

6368
/*
6469
s3.access-key-id`: id for for credentials that provide access to the data in S3
@@ -71,10 +76,20 @@ public enum StorageAccessProperty {
7176

7277
StorageAccessProperty(
7378
Class valueType, String propertyName, String description, boolean isCredential) {
79+
this(valueType, propertyName, description, isCredential, false);
80+
}
81+
82+
StorageAccessProperty(
83+
Class valueType,
84+
String propertyName,
85+
String description,
86+
boolean isCredential,
87+
boolean isExpirationTimestamp) {
7488
this.valueType = valueType;
7589
this.propertyName = propertyName;
7690
this.description = description;
7791
this.isCredential = isCredential;
92+
this.isExpirationTimestamp = isExpirationTimestamp;
7893
}
7994

8095
public String getPropertyName() {
@@ -84,4 +99,8 @@ public String getPropertyName() {
8499
public boolean isCredential() {
85100
return isCredential;
86101
}
102+
103+
public boolean isExpirationTimestamp() {
104+
return isExpirationTimestamp;
105+
}
87106
}

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

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@
2222

2323
import jakarta.annotation.Nonnull;
2424
import java.net.URI;
25-
import java.util.EnumMap;
2625
import java.util.HashMap;
2726
import java.util.Map;
2827
import java.util.Optional;
2928
import java.util.Set;
3029
import java.util.stream.Stream;
3130
import org.apache.polaris.core.context.CallContext;
31+
import org.apache.polaris.core.storage.AccessConfig;
3232
import org.apache.polaris.core.storage.InMemoryStorageIntegration;
3333
import org.apache.polaris.core.storage.StorageAccessProperty;
3434
import org.apache.polaris.core.storage.StorageUtil;
@@ -66,7 +66,7 @@ public AwsCredentialsStorageIntegration(
6666

6767
/** {@inheritDoc} */
6868
@Override
69-
public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
69+
public AccessConfig getSubscopedCreds(
7070
@Nonnull CallContext callContext,
7171
@Nonnull AwsStorageConfigurationInfo storageConfig,
7272
boolean allowListOperation,
@@ -90,50 +90,48 @@ public EnumMap<StorageAccessProperty, String> getSubscopedCreds(
9090
credentialsProvider.ifPresent(
9191
cp -> request.overrideConfiguration(b -> b.credentialsProvider(cp)));
9292

93+
String region = storageConfig.getRegion();
9394
@SuppressWarnings("resource")
9495
// Note: stsClientProvider returns "thin" clients that do not need closing
9596
StsClient stsClient =
96-
stsClientProvider.stsClient(
97-
StsDestination.of(storageConfig.getStsEndpointUri(), storageConfig.getRegion()));
97+
stsClientProvider.stsClient(StsDestination.of(storageConfig.getStsEndpointUri(), region));
9898

9999
AssumeRoleResponse response = stsClient.assumeRole(request.build());
100-
EnumMap<StorageAccessProperty, String> credentialMap =
101-
new EnumMap<>(StorageAccessProperty.class);
102-
credentialMap.put(StorageAccessProperty.AWS_KEY_ID, response.credentials().accessKeyId());
103-
credentialMap.put(
100+
AccessConfig.Builder accessConfig = AccessConfig.builder();
101+
accessConfig.put(StorageAccessProperty.AWS_KEY_ID, response.credentials().accessKeyId());
102+
accessConfig.put(
104103
StorageAccessProperty.AWS_SECRET_KEY, response.credentials().secretAccessKey());
105-
credentialMap.put(StorageAccessProperty.AWS_TOKEN, response.credentials().sessionToken());
104+
accessConfig.put(StorageAccessProperty.AWS_TOKEN, response.credentials().sessionToken());
106105
Optional.ofNullable(response.credentials().expiration())
107106
.ifPresent(
108107
i -> {
109-
credentialMap.put(
108+
accessConfig.put(
110109
StorageAccessProperty.EXPIRATION_TIME, String.valueOf(i.toEpochMilli()));
111-
credentialMap.put(
110+
accessConfig.put(
112111
StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
113112
String.valueOf(i.toEpochMilli()));
114113
});
115114

116-
if (storageConfig.getRegion() != null) {
117-
credentialMap.put(StorageAccessProperty.CLIENT_REGION, storageConfig.getRegion());
115+
if (region != null) {
116+
accessConfig.put(StorageAccessProperty.CLIENT_REGION, region);
118117
}
119118

120119
URI endpointUri = storageConfig.getEndpointUri();
121120
if (endpointUri != null) {
122-
credentialMap.put(StorageAccessProperty.AWS_ENDPOINT, endpointUri.toString());
121+
accessConfig.put(StorageAccessProperty.AWS_ENDPOINT, endpointUri.toString());
123122
}
124123

125124
if (Boolean.TRUE.equals(storageConfig.getPathStyleAccess())) {
126-
credentialMap.put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS, Boolean.TRUE.toString());
125+
accessConfig.put(StorageAccessProperty.AWS_PATH_STYLE_ACCESS, Boolean.TRUE.toString());
127126
}
128127

129-
if (storageConfig.getAwsPartition().equals("aws-us-gov")
130-
&& credentialMap.get(StorageAccessProperty.CLIENT_REGION) == null) {
128+
if (storageConfig.getAwsPartition().equals("aws-us-gov") && region == null) {
131129
throw new IllegalArgumentException(
132130
String.format(
133131
"AWS region must be set when using partition %s", storageConfig.getAwsPartition()));
134132
}
135133

136-
return credentialMap;
134+
return accessConfig.build();
137135
}
138136

139137
/**

0 commit comments

Comments
 (0)