Skip to content

Commit 6ddd148

Browse files
authored
Make StorageCredentialCache safe for mutli-realm usage (#2021)
Injecting the request-scoped `RealmContext` into the application-scoped `StorageCredentialCache` makes things unnecessarily complicated. Similarly `StorageCredentialCacheKey` having a `@Nullable callContext` makes it more difficult to reason about. Instead we can determine all realm-specific values at the time of insertion (from the `PolarisCallContext` param of `getOrGenerateSubScopeCreds`).
1 parent 493bc8e commit 6ddd148

File tree

11 files changed

+83
-87
lines changed

11 files changed

+83
-87
lines changed

persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,7 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache(
224224
RealmContext realmContext) {
225225
if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) {
226226
storageCredentialCacheMap.put(
227-
realmContext.getRealmIdentifier(),
228-
new StorageCredentialCache(realmContext, configurationStore));
227+
realmContext.getRealmIdentifier(), new StorageCredentialCache(configurationStore));
229228
}
230229

231230
return storageCredentialCacheMap.get(realmContext.getRealmIdentifier());

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,7 @@ public synchronized StorageCredentialCache getOrCreateStorageCredentialCache(
182182
RealmContext realmContext) {
183183
if (!storageCredentialCacheMap.containsKey(realmContext.getRealmIdentifier())) {
184184
storageCredentialCacheMap.put(
185-
realmContext.getRealmIdentifier(),
186-
new StorageCredentialCache(realmContext, configurationStore));
185+
realmContext.getRealmIdentifier(), new StorageCredentialCache(configurationStore));
187186
}
188187

189188
return storageCredentialCacheMap.get(realmContext.getRealmIdentifier());

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

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,10 @@ public class StorageCredentialCache {
5050
private static final long CACHE_MAX_NUMBER_OF_ENTRIES = 10_000L;
5151

5252
private final LoadingCache<StorageCredentialCacheKey, StorageCredentialCacheEntry> cache;
53-
private final RealmContext realmContext;
5453
private final PolarisConfigurationStore configurationStore;
5554

5655
/** Initialize the creds cache */
57-
public StorageCredentialCache(
58-
RealmContext realmContext, PolarisConfigurationStore configurationStore) {
59-
this.realmContext = realmContext;
56+
public StorageCredentialCache(PolarisConfigurationStore configurationStore) {
6057
this.configurationStore = configurationStore;
6158
cache =
6259
Caffeine.newBuilder()
@@ -69,7 +66,7 @@ public StorageCredentialCache(
6966
0,
7067
Math.min(
7168
(entry.getExpirationTime() - System.currentTimeMillis()) / 2,
72-
this.maxCacheDurationMs()));
69+
entry.getMaxCacheDurationMs()));
7370
return Duration.ofMillis(expireAfterMillis);
7471
}))
7572
.build(
@@ -80,7 +77,7 @@ public StorageCredentialCache(
8077
}
8178

8279
/** How long credentials should remain in the cache. */
83-
private long maxCacheDurationMs() {
80+
private long maxCacheDurationMs(RealmContext realmContext) {
8481
var cacheDurationSeconds =
8582
configurationStore.getConfiguration(
8683
realmContext, FeatureConfiguration.STORAGE_CREDENTIAL_CACHE_DURATION_SECONDS);
@@ -123,26 +120,27 @@ public AccessConfig getOrGenerateSubScopeCreds(
123120
}
124121
StorageCredentialCacheKey key =
125122
new StorageCredentialCacheKey(
123+
callCtx.getRealmContext().getRealmIdentifier(),
126124
polarisEntity,
127125
allowListOperation,
128126
allowedReadLocations,
129-
allowedWriteLocations,
130-
callCtx);
127+
allowedWriteLocations);
131128
LOGGER.atDebug().addKeyValue("key", key).log("subscopedCredsCache");
132129
Function<StorageCredentialCacheKey, StorageCredentialCacheEntry> loader =
133130
k -> {
134131
LOGGER.atDebug().log("StorageCredentialCache::load");
135132
ScopedCredentialsResult scopedCredentialsResult =
136133
credentialVendor.getSubscopedCredsForEntity(
137-
k.getCallContext(),
134+
callCtx,
138135
k.getCatalogId(),
139136
k.getEntityId(),
140137
polarisEntity.getType(),
141138
k.isAllowedListAction(),
142139
k.getAllowedReadLocations(),
143140
k.getAllowedWriteLocations());
144141
if (scopedCredentialsResult.isSuccess()) {
145-
return new StorageCredentialCacheEntry(scopedCredentialsResult);
142+
long maxCacheDurationMs = maxCacheDurationMs(callCtx.getRealmContext());
143+
return new StorageCredentialCacheEntry(scopedCredentialsResult, maxCacheDurationMs);
146144
}
147145
LOGGER
148146
.atDebug()

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,19 @@ public class StorageCredentialCacheEntry {
3232
public final EnumMap<StorageAccessProperty, String> credsMap;
3333

3434
private final ScopedCredentialsResult scopedCredentialsResult;
35+
private final long maxCacheDurationMs;
3536

36-
public StorageCredentialCacheEntry(ScopedCredentialsResult scopedCredentialsResult) {
37+
public StorageCredentialCacheEntry(
38+
ScopedCredentialsResult scopedCredentialsResult, long maxCacheDurationMs) {
3739
this.scopedCredentialsResult = scopedCredentialsResult;
40+
this.maxCacheDurationMs = maxCacheDurationMs;
3841
this.credsMap = scopedCredentialsResult.getCredentials();
3942
}
4043

44+
public long getMaxCacheDurationMs() {
45+
return maxCacheDurationMs;
46+
}
47+
4148
/** Get the expiration time in millisecond for the cached entry */
4249
public long getExpirationTime() {
4350
if (credsMap.containsKey(StorageAccessProperty.GCS_ACCESS_TOKEN_EXPIRES_AT)) {

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

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@
1818
*/
1919
package org.apache.polaris.core.storage.cache;
2020

21-
import jakarta.annotation.Nullable;
2221
import java.util.Objects;
2322
import java.util.Set;
24-
import org.apache.polaris.core.PolarisCallContext;
25-
import org.apache.polaris.core.context.CallContext;
2623
import org.apache.polaris.core.entity.PolarisEntity;
2724
import org.apache.polaris.core.entity.PolarisEntityConstants;
2825

2926
public class StorageCredentialCacheKey {
3027

28+
private final String realmId;
3129
private final long catalogId;
3230

3331
/** The serialized string of the storage config. */
@@ -44,18 +42,13 @@ public class StorageCredentialCacheKey {
4442

4543
private final Set<String> allowedWriteLocations;
4644

47-
/**
48-
* The callContext is passed to be used to fetch subscoped creds, but is not used to hash/equals
49-
* as part of the cache key.
50-
*/
51-
private @Nullable PolarisCallContext callContext;
52-
5345
public StorageCredentialCacheKey(
46+
String realmId,
5447
PolarisEntity entity,
5548
boolean allowedListAction,
5649
Set<String> allowedReadLocations,
57-
Set<String> allowedWriteLocations,
58-
@Nullable PolarisCallContext callContext) {
50+
Set<String> allowedWriteLocations) {
51+
this.realmId = realmId;
5952
this.catalogId = entity.getCatalogId();
6053
this.storageConfigSerializedStr =
6154
entity
@@ -65,10 +58,10 @@ public StorageCredentialCacheKey(
6558
this.allowedListAction = allowedListAction;
6659
this.allowedReadLocations = allowedReadLocations;
6760
this.allowedWriteLocations = allowedWriteLocations;
68-
this.callContext = callContext;
69-
if (this.callContext == null) {
70-
this.callContext = CallContext.getCurrentContext().getPolarisCallContext();
71-
}
61+
}
62+
63+
public String getRealmId() {
64+
return realmId;
7265
}
7366

7467
public long getCatalogId() {
@@ -95,16 +88,13 @@ public Set<String> getAllowedWriteLocations() {
9588
return allowedWriteLocations;
9689
}
9790

98-
public @Nullable PolarisCallContext getCallContext() {
99-
return callContext;
100-
}
101-
10291
@Override
10392
public boolean equals(Object o) {
10493
if (this == o) return true;
10594
if (o == null || getClass() != o.getClass()) return false;
10695
StorageCredentialCacheKey cacheKey = (StorageCredentialCacheKey) o;
107-
return catalogId == cacheKey.getCatalogId()
96+
return Objects.equals(realmId, cacheKey.getRealmId())
97+
&& catalogId == cacheKey.getCatalogId()
10898
&& Objects.equals(storageConfigSerializedStr, cacheKey.getStorageConfigSerializedStr())
10999
&& allowedListAction == cacheKey.allowedListAction
110100
&& Objects.equals(allowedReadLocations, cacheKey.allowedReadLocations)
@@ -114,6 +104,7 @@ public boolean equals(Object o) {
114104
@Override
115105
public int hashCode() {
116106
return Objects.hash(
107+
realmId,
117108
catalogId,
118109
storageConfigSerializedStr,
119110
allowedListAction,
@@ -124,7 +115,9 @@ public int hashCode() {
124115
@Override
125116
public String toString() {
126117
return "StorageCredentialCacheKey{"
127-
+ "catalogId="
118+
+ "realmId="
119+
+ realmId
120+
+ ", catalogId="
128121
+ catalogId
129122
+ ", storageConfigSerializedStr='"
130123
+ storageConfigSerializedStr

0 commit comments

Comments
 (0)