tokoko commented on code in PR #3699:
URL: https://github.com/apache/polaris/pull/3699#discussion_r3337858639
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java:
##########
@@ -68,41 +72,96 @@
/** Azure credential vendor that supports generating SAS token */
public class AzureCredentialsStorageIntegration
- extends InMemoryStorageIntegration<AzureStorageConfigurationInfo> {
+ extends CachingStorageIntegration<AzureStorageConfigurationInfo> {
private static final Logger LOGGER =
LoggerFactory.getLogger(AzureCredentialsStorageIntegration.class);
final DefaultAzureCredential defaultAzureCredential;
- public AzureCredentialsStorageIntegration(AzureStorageConfigurationInfo
config) {
- super(config, AzureCredentialsStorageIntegration.class.getName());
+ public AzureCredentialsStorageIntegration(
+ AzureStorageConfigurationInfo storageConfig, RealmConfig realmConfig) {
+ this(null, storageConfig, realmConfig);
+ }
+
+ public AzureCredentialsStorageIntegration(
+ org.apache.polaris.core.storage.cache.StorageCredentialCache cache,
+ AzureStorageConfigurationInfo storageConfig,
+ RealmConfig realmConfig) {
+ super(cache, realmConfig, storageConfig);
// The DefaultAzureCredential will by default load the environment
variables for client id,
// client secret, tenant id
defaultAzureCredential = new DefaultAzureCredentialBuilder().build();
}
@Override
- public StorageAccessConfig getSubscopedCreds(
- @NonNull RealmConfig realmConfig,
- boolean allowListOperation,
- @NonNull Set<String> allowedReadLocations,
- @NonNull Set<String> allowedWriteLocations,
- @NonNull PolarisPrincipal polarisPrincipal,
- Optional<String> refreshCredentialsEndpoint,
- @NonNull CredentialVendingContext credentialVendingContext) {
- // Note: Azure SAS tokens do not support session tags like AWS STS.
- // The credentialVendingContext is accepted for interface compatibility
but not used.
+ protected StorageCredentialCacheKey buildCacheKey(
+ @NonNull List<LocationGrant> grants,
+ @NonNull Optional<String> refreshEndpoint,
+ @NonNull CredentialVendingContext context) {
+ return buildCacheKey(
+ allowList(grants), readLocations(grants), writeLocations(grants),
refreshEndpoint, context);
+ }
+
+ private static boolean allowList(List<LocationGrant> grants) {
+ return grants.stream()
+ .flatMap(g -> g.actions().stream())
+ .anyMatch(a -> a == PolarisStorageActions.LIST || a ==
PolarisStorageActions.ALL);
+ }
+
+ private static Set<String> readLocations(List<LocationGrant> grants) {
Review Comment:
That's azure limitation and technically present even in current design.
Azure can't put locations with different permissions in a single token.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
