From 521aa801f4ed7b6dfc9352153ce7534e652854f6 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 13:28:35 -0700 Subject: [PATCH 01/10] update --- .../core/storage/cache/StorageCredentialCacheEntry.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index ae799457fd..f266549033 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -68,6 +68,10 @@ public Map convertToMapOfString() { key.getPropertyName() + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST), value); + resCredsMap.put( + key.getPropertyName() + + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST) + ".dfs.windows.net", + value); } else if (!key.equals(PolarisCredentialProperty.AZURE_ACCOUNT_HOST)) { resCredsMap.put(key.getPropertyName(), value); } From 1ae35b7cc0b76716e65ced1f24d4ef0fdfbc01fd Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 13:28:38 -0700 Subject: [PATCH 02/10] autolint --- .../core/storage/cache/StorageCredentialCacheEntry.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index f266549033..edbdd5f139 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -70,7 +70,8 @@ public Map convertToMapOfString() { value); resCredsMap.put( key.getPropertyName() - + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST) + ".dfs.windows.net", + + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST) + + ".dfs.windows.net", value); } else if (!key.equals(PolarisCredentialProperty.AZURE_ACCOUNT_HOST)) { resCredsMap.put(key.getPropertyName(), value); From 33c36abcc209efcb4bd0e9d031488e4f44614fc7 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 13:49:34 -0700 Subject: [PATCH 03/10] fix --- .../AzureCredentialsStorageIntegration.java | 2 ++ .../cache/StorageCredentialCacheEntry.java | 18 +++++++++--------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 62e4fc4dc1..02f1f77354 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -60,6 +60,8 @@ public class AzureCredentialsStorageIntegration private static final Logger LOGGER = LoggerFactory.getLogger(AzureCredentialsStorageIntegration.class); + public static String AZURE_ACCOUNT_HOST_SUFFIX = "dfs.windows.net"; + final DefaultAzureCredential defaultAzureCredential; public AzureCredentialsStorageIntegration() { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index edbdd5f139..94564fd485 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -23,6 +23,7 @@ import java.util.Map; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.storage.PolarisCredentialProperty; +import org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration; /** A storage credential cached entry. */ public class StorageCredentialCacheEntry { @@ -64,15 +65,14 @@ public Map convertToMapOfString() { // only Azure needs special handle, the target key is dynamically with storageaccount // endpoint appended if (key.equals(PolarisCredentialProperty.AZURE_SAS_TOKEN)) { - resCredsMap.put( - key.getPropertyName() - + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST), - value); - resCredsMap.put( - key.getPropertyName() - + credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST) - + ".dfs.windows.net", - value); + String host = credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST); + resCredsMap.put(key.getPropertyName() + host, value); + // Iceberg 1.7.x may expect this to _not_ be suffixed with .dfs.windows.net + if (host.endsWith(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX)) { + int suffixIndex = host.lastIndexOf(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX); + String withSuffixStripped = host.substring(0, suffixIndex); + resCredsMap.put(key.getPropertyName() + withSuffixStripped, value); + } } else if (!key.equals(PolarisCredentialProperty.AZURE_ACCOUNT_HOST)) { resCredsMap.put(key.getPropertyName(), value); } From 34ac2416aaf6f117748724d9806c9b58fdf33457 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 13:49:37 -0700 Subject: [PATCH 04/10] autolint --- .../core/storage/cache/StorageCredentialCacheEntry.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index 94564fd485..eb7c894ca0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -69,7 +69,8 @@ public Map convertToMapOfString() { resCredsMap.put(key.getPropertyName() + host, value); // Iceberg 1.7.x may expect this to _not_ be suffixed with .dfs.windows.net if (host.endsWith(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX)) { - int suffixIndex = host.lastIndexOf(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX); + int suffixIndex = + host.lastIndexOf(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX); String withSuffixStripped = host.substring(0, suffixIndex); resCredsMap.put(key.getPropertyName() + withSuffixStripped, value); } From bc519de92b660a18b0e107077f7fc4b1a120c92b Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 14:03:02 -0700 Subject: [PATCH 05/10] clean up --- .../AzureCredentialsStorageIntegration.java | 3 +- .../cache/StorageCredentialCacheEntry.java | 42 +++++++++++++------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 02f1f77354..9f7b4778e6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -43,6 +43,7 @@ import java.time.temporal.ChronoUnit; import java.util.EnumMap; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Set; import org.apache.polaris.core.PolarisDiagnostics; @@ -60,8 +61,6 @@ public class AzureCredentialsStorageIntegration private static final Logger LOGGER = LoggerFactory.getLogger(AzureCredentialsStorageIntegration.class); - public static String AZURE_ACCOUNT_HOST_SUFFIX = "dfs.windows.net"; - final DefaultAzureCredential defaultAzureCredential; public AzureCredentialsStorageIntegration() { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index eb7c894ca0..7802cb4be3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -24,6 +24,7 @@ import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.storage.PolarisCredentialProperty; import org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration; +import org.apache.polaris.core.storage.azure.AzureLocation; /** A storage credential cached entry. */ public class StorageCredentialCacheEntry { @@ -52,28 +53,45 @@ public long getExpirationTime() { return Long.MAX_VALUE; } + /** + * Azure needs special handling, the credential key is dynamically generated based on + * the storage account endpoint + */ + private void handleAzureCredential( + HashMap results, + PolarisCredentialProperty credentialProperty, + String value) { + if (credentialProperty.equals(PolarisCredentialProperty.AZURE_SAS_TOKEN)) { + String host = credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST); + results.put(credentialProperty.getPropertyName() + host, value); + + // Iceberg 1.7.x may expect the credenetial to _not_ be suffixed with endpoint + if (host.endsWith(AzureLocation.ADLS_ENDPOINT)) { + int suffixIndex = host.lastIndexOf(AzureLocation.ADLS_ENDPOINT); + String withSuffixStripped = host.substring(0, suffixIndex); + results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + } + + if (host.endsWith(AzureLocation.BLOB_ENDPOINT)) { + int suffixIndex = host.lastIndexOf(AzureLocation.BLOB_ENDPOINT); + String withSuffixStripped = host.substring(0, suffixIndex); + results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + } + } + } + /** * Get the map of string creds that is needed for the query engine. * * @return a map of string representing the subscoped creds info. */ public Map convertToMapOfString() { - Map resCredsMap = new HashMap<>(); + HashMap resCredsMap = new HashMap<>(); if (!credsMap.isEmpty()) { credsMap.forEach( (key, value) -> { - // only Azure needs special handle, the target key is dynamically with storageaccount - // endpoint appended if (key.equals(PolarisCredentialProperty.AZURE_SAS_TOKEN)) { - String host = credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST); - resCredsMap.put(key.getPropertyName() + host, value); - // Iceberg 1.7.x may expect this to _not_ be suffixed with .dfs.windows.net - if (host.endsWith(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX)) { - int suffixIndex = - host.lastIndexOf(AzureCredentialsStorageIntegration.AZURE_ACCOUNT_HOST_SUFFIX); - String withSuffixStripped = host.substring(0, suffixIndex); - resCredsMap.put(key.getPropertyName() + withSuffixStripped, value); - } + handleAzureCredential(resCredsMap, key, value); } else if (!key.equals(PolarisCredentialProperty.AZURE_ACCOUNT_HOST)) { resCredsMap.put(key.getPropertyName(), value); } From 631a7ca228a28dbf0b619ad9038d74a1e941badd Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 14:03:04 -0700 Subject: [PATCH 06/10] autolint --- .../azure/AzureCredentialsStorageIntegration.java | 1 - .../core/storage/cache/StorageCredentialCacheEntry.java | 9 +++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java index 9f7b4778e6..62e4fc4dc1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/azure/AzureCredentialsStorageIntegration.java @@ -43,7 +43,6 @@ import java.time.temporal.ChronoUnit; import java.util.EnumMap; import java.util.HashSet; -import java.util.List; import java.util.Objects; import java.util.Set; import org.apache.polaris.core.PolarisDiagnostics; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index 7802cb4be3..06e4a616a5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -23,7 +23,6 @@ import java.util.Map; import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult; import org.apache.polaris.core.storage.PolarisCredentialProperty; -import org.apache.polaris.core.storage.azure.AzureCredentialsStorageIntegration; import org.apache.polaris.core.storage.azure.AzureLocation; /** A storage credential cached entry. */ @@ -54,13 +53,11 @@ public long getExpirationTime() { } /** - * Azure needs special handling, the credential key is dynamically generated based on - * the storage account endpoint + * Azure needs special handling, the credential key is dynamically generated based on the storage + * account endpoint */ private void handleAzureCredential( - HashMap results, - PolarisCredentialProperty credentialProperty, - String value) { + HashMap results, PolarisCredentialProperty credentialProperty, String value) { if (credentialProperty.equals(PolarisCredentialProperty.AZURE_SAS_TOKEN)) { String host = credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST); results.put(credentialProperty.getPropertyName() + host, value); From 951a3b63b934c86d3499dda4642991318a4c4e0d Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 14:20:47 -0700 Subject: [PATCH 07/10] test --- .../cache/StorageCredentialCacheEntry.java | 4 +- .../cache/StorageCredentialCacheTest.java | 76 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index 06e4a616a5..be095fa652 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -64,13 +64,13 @@ private void handleAzureCredential( // Iceberg 1.7.x may expect the credenetial to _not_ be suffixed with endpoint if (host.endsWith(AzureLocation.ADLS_ENDPOINT)) { - int suffixIndex = host.lastIndexOf(AzureLocation.ADLS_ENDPOINT); + int suffixIndex = host.lastIndexOf(AzureLocation.ADLS_ENDPOINT) - 1; String withSuffixStripped = host.substring(0, suffixIndex); results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); } if (host.endsWith(AzureLocation.BLOB_ENDPOINT)) { - int suffixIndex = host.lastIndexOf(AzureLocation.BLOB_ENDPOINT); + int suffixIndex = host.lastIndexOf(AzureLocation.BLOB_ENDPOINT) - 1; String withSuffixStripped = host.substring(0, suffixIndex); results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java index f3b4fc2f62..05c7f99bcb 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -45,6 +45,7 @@ import org.apache.polaris.core.persistence.transactional.TreeMapMetaStore; import org.apache.polaris.core.persistence.transactional.TreeMapTransactionalPersistenceImpl; import org.apache.polaris.core.storage.PolarisCredentialProperty; +import org.apache.polaris.core.storage.azure.AzureLocation; import org.assertj.core.api.Assertions; import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; @@ -438,4 +439,79 @@ private static List getPolarisEntities() { return Arrays.asList(polarisEntity1, polarisEntity2, polarisEntity3); } + + + @Test + public void testAzureCredentialFormatting() { + storageCredentialCache = new StorageCredentialCache(); + List mockedScopedCreds = List.of( + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_1") + .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account") + .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) + .buildOrThrow())), + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_2") + .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account." + AzureLocation.ADLS_ENDPOINT) + .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) + .buildOrThrow())), + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_3") + .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account." + AzureLocation.BLOB_ENDPOINT) + .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) + .buildOrThrow())) + ); + + Mockito.when( + metaStoreManager.getSubscopedCredsForEntity( + Mockito.any(), + Mockito.anyLong(), + Mockito.anyLong(), + Mockito.any(), + Mockito.anyBoolean(), + Mockito.anySet(), + Mockito.anySet())) + .thenReturn(mockedScopedCreds.get(0)) + .thenReturn(mockedScopedCreds.get(1)) + .thenReturn(mockedScopedCreds.get(2)); + List entityList = getPolarisEntities(); + + Map noSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(0), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Assertions.assertThat(noSuffixResult.size()).isEqualTo(2); + Assertions.assertThat(noSuffixResult).containsKey("adls.sas-token.some_account"); + + Map adlsSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(1), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Assertions.assertThat(adlsSuffixResult.size()).isEqualTo(3); + Assertions.assertThat(adlsSuffixResult).containsKey("adls.sas-token.some_account"); + Assertions.assertThat(adlsSuffixResult).containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); + + Map blobSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(2), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Assertions.assertThat(blobSuffixResult.size()).isEqualTo(3); + Assertions.assertThat(blobSuffixResult).containsKey("adls.sas-token.some_account"); + Assertions.assertThat(blobSuffixResult).containsKey("adls.sas-token.some_account." + AzureLocation.BLOB_ENDPOINT); + } } From 13d8411fd4890c4bcc44a29e26a10c34c5c5519f Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 14:20:50 -0700 Subject: [PATCH 08/10] autolint --- .../cache/StorageCredentialCacheTest.java | 108 ++++++++++-------- 1 file changed, 61 insertions(+), 47 deletions(-) diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java index 05c7f99bcb..54c31cf101 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheTest.java @@ -440,33 +440,42 @@ private static List getPolarisEntities() { return Arrays.asList(polarisEntity1, polarisEntity2, polarisEntity3); } - @Test public void testAzureCredentialFormatting() { storageCredentialCache = new StorageCredentialCache(); - List mockedScopedCreds = List.of( - new ScopedCredentialsResult( - new EnumMap<>( - ImmutableMap.builder() - .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_1") - .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account") - .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) - .buildOrThrow())), - new ScopedCredentialsResult( - new EnumMap<>( - ImmutableMap.builder() - .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_2") - .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account." + AzureLocation.ADLS_ENDPOINT) - .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) - .buildOrThrow())), - new ScopedCredentialsResult( - new EnumMap<>( - ImmutableMap.builder() - .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_3") - .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account." + AzureLocation.BLOB_ENDPOINT) - .put(PolarisCredentialProperty.EXPIRATION_TIME, String.valueOf(Long.MAX_VALUE)) - .buildOrThrow())) - ); + List mockedScopedCreds = + List.of( + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_1") + .put(PolarisCredentialProperty.AZURE_ACCOUNT_HOST, "some_account") + .put( + PolarisCredentialProperty.EXPIRATION_TIME, + String.valueOf(Long.MAX_VALUE)) + .buildOrThrow())), + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_2") + .put( + PolarisCredentialProperty.AZURE_ACCOUNT_HOST, + "some_account." + AzureLocation.ADLS_ENDPOINT) + .put( + PolarisCredentialProperty.EXPIRATION_TIME, + String.valueOf(Long.MAX_VALUE)) + .buildOrThrow())), + new ScopedCredentialsResult( + new EnumMap<>( + ImmutableMap.builder() + .put(PolarisCredentialProperty.AZURE_SAS_TOKEN, "sas_token_azure_3") + .put( + PolarisCredentialProperty.AZURE_ACCOUNT_HOST, + "some_account." + AzureLocation.BLOB_ENDPOINT) + .put( + PolarisCredentialProperty.EXPIRATION_TIME, + String.valueOf(Long.MAX_VALUE)) + .buildOrThrow()))); Mockito.when( metaStoreManager.getSubscopedCredsForEntity( @@ -482,36 +491,41 @@ public void testAzureCredentialFormatting() { .thenReturn(mockedScopedCreds.get(2)); List entityList = getPolarisEntities(); - Map noSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( - metaStoreManager, - callCtx, - entityList.get(0), - true, - new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), - new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Map noSuffixResult = + storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(0), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); Assertions.assertThat(noSuffixResult.size()).isEqualTo(2); Assertions.assertThat(noSuffixResult).containsKey("adls.sas-token.some_account"); - Map adlsSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( - metaStoreManager, - callCtx, - entityList.get(1), - true, - new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), - new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Map adlsSuffixResult = + storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(1), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); Assertions.assertThat(adlsSuffixResult.size()).isEqualTo(3); Assertions.assertThat(adlsSuffixResult).containsKey("adls.sas-token.some_account"); - Assertions.assertThat(adlsSuffixResult).containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); + Assertions.assertThat(adlsSuffixResult) + .containsKey("adls.sas-token.some_account." + AzureLocation.ADLS_ENDPOINT); - Map blobSuffixResult = storageCredentialCache.getOrGenerateSubScopeCreds( - metaStoreManager, - callCtx, - entityList.get(2), - true, - new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), - new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); + Map blobSuffixResult = + storageCredentialCache.getOrGenerateSubScopeCreds( + metaStoreManager, + callCtx, + entityList.get(2), + true, + new HashSet<>(Arrays.asList("s3://bucket1/path", "s3://bucket2/path")), + new HashSet<>(Arrays.asList("s3://bucket3/path", "s3://bucket4/path"))); Assertions.assertThat(blobSuffixResult.size()).isEqualTo(3); Assertions.assertThat(blobSuffixResult).containsKey("adls.sas-token.some_account"); - Assertions.assertThat(blobSuffixResult).containsKey("adls.sas-token.some_account." + AzureLocation.BLOB_ENDPOINT); + Assertions.assertThat(blobSuffixResult) + .containsKey("adls.sas-token.some_account." + AzureLocation.BLOB_ENDPOINT); } } From 1a07638bd7e6982b1d39da7a9a1d99f227e26dce Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 14:22:02 -0700 Subject: [PATCH 09/10] paranoid check --- .../storage/cache/StorageCredentialCacheEntry.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index be095fa652..0ed5aaf174 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -65,14 +65,18 @@ private void handleAzureCredential( // Iceberg 1.7.x may expect the credenetial to _not_ be suffixed with endpoint if (host.endsWith(AzureLocation.ADLS_ENDPOINT)) { int suffixIndex = host.lastIndexOf(AzureLocation.ADLS_ENDPOINT) - 1; - String withSuffixStripped = host.substring(0, suffixIndex); - results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + if (suffixIndex > 0) { + String withSuffixStripped = host.substring(0, suffixIndex); + results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + } } if (host.endsWith(AzureLocation.BLOB_ENDPOINT)) { int suffixIndex = host.lastIndexOf(AzureLocation.BLOB_ENDPOINT) - 1; - String withSuffixStripped = host.substring(0, suffixIndex); - results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + if (suffixIndex > 0) { + String withSuffixStripped = host.substring(0, suffixIndex); + results.put(credentialProperty.getPropertyName() + withSuffixStripped, value); + } } } } From fdcd7e929fc1c6d08373a43c6da179a5a2538054 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 25 Mar 2025 15:01:15 -0700 Subject: [PATCH 10/10] typofix --- .../polaris/core/storage/cache/StorageCredentialCacheEntry.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java index 0ed5aaf174..2649ee99c3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/cache/StorageCredentialCacheEntry.java @@ -62,7 +62,7 @@ private void handleAzureCredential( String host = credsMap.get(PolarisCredentialProperty.AZURE_ACCOUNT_HOST); results.put(credentialProperty.getPropertyName() + host, value); - // Iceberg 1.7.x may expect the credenetial to _not_ be suffixed with endpoint + // Iceberg 1.7.x may expect the credential key to _not_ be suffixed with endpoint if (host.endsWith(AzureLocation.ADLS_ENDPOINT)) { int suffixIndex = host.lastIndexOf(AzureLocation.ADLS_ENDPOINT) - 1; if (suffixIndex > 0) {