Azure: add support to specify token credential provider by munendrasn · Pull Request #14136 · apache/iceberg

munendrasn · 2025-09-21T06:07:39Z

Similar to AWS, adls.token-credential-provider can be used to specify the TokenCredential provider.
This is useful where user can configure custom token credential or in-built TokenCredential impl like ClientSecretCredential - where specifying clientId,secret not possible via env variables like federation.

In built-one support auto-refresh of access-token on the expiry.

public class ClientSecretCredentialProvider implements AzureTokenCredentialProvider {

  private static final String AZURE_CLIENT_ID = "azure.client-id";
  private static final String AZURE_TENANT_ID = "azure.tenant-id";
  private static final String AZURE_CLIENT_SECRET = "azure.client-secret";

  private String clientId;
  private String clientSecret;
  private String tenantId;


  @Override
  public TokenCredential credential() {
    return new ClientSecretCredentialBuilder()
        .tenantId(tenantId)
        .clientId(clientId)
        .clientSecret(clientSecret)
        .build();
  }

  @Override
  public void initialize(Map<String, String> properties) {
    this.clientId = properties.get(AZURE_CLIENT_ID);
    this.clientSecret = properties.get(AZURE_CLIENT_SECRET);
    this.tenantId = properties.get(AZURE_TENANT_ID);
  }
}

munendrasn · 2025-09-21T09:28:43Z

@kevinjqliu
Post creatio of the PR, I found a PR from you for similar functinality #13829. Please let me know if this aligns with your original idea.

As of now, I have avoided including the ClientSecretCredential Provider in the PR.
This provides user to have custom providers based on certificate, or workload and so on without the need for each impl to be part of Iceberg

nastra · 2025-09-29T11:02:10Z

+  TokenCredential credential();
+
+  /**
+   * Initialize Azure credential factory from catalog properties.


instead of "factory" this should probably just say "Initialize the credential provider..."

nastra · 2025-09-29T11:03:17Z

+   *
+   * @param properties catalog properties
+   */
+  void initialize(Map<String, String> properties);


similar to AWS we should probably only pass down properties that have a specific prefix, such as adls.token-credential-provider., so only adls.token-credential-provider.client_id would be passed to the credential provider but not any other catalog properties

nastra · 2025-09-29T11:05:00Z

+import com.azure.core.credential.TokenCredential;
+import java.util.Map;
+
+public interface AzureTokenCredentialProvider {


Suggested change

public interface AzureTokenCredentialProvider {

public interface AdlsTokenCredentialProvider {

Reasoning is same as below - applicable for any azure services

nastra · 2025-09-29T11:05:22Z

+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+import org.apache.iceberg.util.PropertyUtil;
+
+public class AzureTokenCredentialProviders {


Suggested change

public class AzureTokenCredentialProviders {

public class AdlsTokenCredentialProviders {

TokenCredential is applicable for all Azure services, so should we keep the Azure has prefix or Adls - considering we only support Adlsv2
Also, if renamed should we move the classes under adlsv2 package?

no need to move this to a different package but we should still rename the class to Adls... because the property also says adls....

renamed the classes

nastra · 2025-09-29T11:07:53Z

+  public void testDefaultFactory() {
+    AzureTokenCredentialProvider provider = AzureTokenCredentialProviders.defaultFactory();
+    assertThat(provider).isNotNull();
+    assertThat(provider)


you can combine both assertions and have assertThat(provider).isNotNull().isInstanceOf(..)

nastra · 2025-09-29T11:08:21Z

+public class TestAzureTokenCredentialProviders {
+
+  @Test
+  public void testDefaultFactory() {


we don't really need the test prefix in all of those new test methods. Instead just use meaningful method names, such as useDefaultFactory() or emptyPropertiesWithNoProvider() and so on

nastra · 2025-09-29T11:12:10Z

+
+    assertThat(provider).isNotNull();
+    assertThat(provider).isInstanceOf(DummyTokenCredentialProvider.class);
+    assertThat(provider.credential()).isInstanceOf(DummyTokenCredential.class);


I think we should also update TestAzureProperties to make sure the credential actually comes back correctly and has the right value

…tial provider

munendrasn · 2025-09-29T14:24:36Z

@nastra I have incorporated the most of the suggestions.
returning the same instance of DefaultTokenCredential - I have left couple of questions

nastra · 2025-09-29T15:23:47Z

+   * <p>Example: adls.token-credential-provider=com.example.MyCustomTokenCredentialProvider
+   *
+   * <p>When set, the {@link AdlsTokenCredentialProviders#from(Map)} method will use this provider
+   * to get Azure credentials instead of using the default.


Suggested change

* to get Azure credentials instead of using the default.

* to get ADLS credentials instead of using the default.

nastra · 2025-09-29T15:23:55Z

  public static final String ADLS_TOKEN = "adls.token";

+  /**
+   * Configure the Azure token credential provider used to get {@link TokenCredential}. A fully


Suggested change

* Configure the Azure token credential provider used to get {@link TokenCredential}. A fully

* Configure the ADLS token credential provider used to get {@link TokenCredential}. A fully

~~I'd prefer Azure here since the token credentials are from Azure Identity. Plus we're adding support for OneLake as well as ADLS.~~

nastra · 2025-09-29T15:26:03Z

+  @Test
+  public void emptyPropertiesWithNoProvider() {
+    Map<String, String> properties = ImmutableMap.of();
+    AdlsTokenCredentialProvider provider = AdlsTokenCredentialProviders.from(properties);


nit: the provider can be inlined in all of the tests: assertThat(AdlsTokenCredentialProviders.from(properties))...

nastra · 2025-09-29T15:27:46Z

+    }
+  }
+
+  // Dummy TokenCredential for testing


comment can be removed as I think it's obvious from the naming of the class

nastra · 2025-09-29T15:27:51Z

+    assertThat(provider.credential()).isInstanceOf(DummyTokenCredential.class);
+  }
+
+  // Dummy implementation for testing


can be removed

nastra · 2025-09-29T15:30:53Z

+  }
+
+  static class DummyTokenCredentialProvider implements AdlsTokenCredentialProvider {
+    static Map<String, String> lastInitializedProperties;


why not just call this properties?

nastra · 2025-09-29T15:31:03Z

+    verify(clientBuilder, never()).credential(any(StorageSharedKeyCredential.class));
+  }
+
+  // Helper classes for custom provider testing


can be removed

nastra

left a few more comments but overall LGTM

kevinjqliu

Thank you for the contribution! This is a lot cleaner than implementing individual auth method (like in #13829). Perhaps we can also refactor the static token auth (#13825) as a follow up.

~~Regarding the naming convention, ADLS vs Azure, I would prefer to use Azure since these are Azure identity auth methods.~~
~~We're also adding support for this on the Microsoft OneLake side. "Azure" would be less confusing than ADLS since OneLake is ADLS-compatible~~

Since everything else mentions ADLS (except AzureProperties), i think it would be less confusing to users if we just default everything to ADLS for now.
We can look to rename ADLS -> Azure in the future

kevinjqliu · 2025-09-29T15:49:57Z

  public static final String ADLS_TOKEN = "adls.token";

+  /**
+   * Configure the Azure token credential provider used to get {@link TokenCredential}. A fully


~~I'd prefer Azure here since the token credentials are from Azure Identity. Plus we're adding support for OneLake as well as ADLS.~~

munendrasn · 2025-09-29T17:03:24Z

Thanks @kevinjqliu @nastra for the review. I have addressed the last set of review suggestions. Please have a look

Azure: add support to specify token credential provider

74926b6

github-actions Bot added the AZURE label Sep 21, 2025

fix test failures

4dff504

nastra reviewed Sep 29, 2025

View reviewed changes

munendrasn added 2 commits September 29, 2025 19:06

address initial review feedback

003e806

pass on only adls.token-credential-provider. prefixed props to creden…

f4b18d4

…tial provider

munendrasn force-pushed the azure-credential-provider branch from 56e46e5 to f4b18d4 Compare September 29, 2025 14:22

munendrasn added 2 commits September 29, 2025 19:56

update javadoc for azure credential provider

8a0b259

rename the token credential

69b8b85

nastra reviewed Sep 29, 2025

View reviewed changes

nastra approved these changes Sep 29, 2025

View reviewed changes

kevinjqliu approved these changes Sep 29, 2025

View reviewed changes

kevinjqliu mentioned this pull request Sep 29, 2025

Feature Request: Support Explicit tenantId, clientId, and clientSecret Configuration in ADLSFileIO #13819

Closed

3 tasks

address review suggestion

3d29129

nastra merged commit 8ae7559 into apache:main Sep 30, 2025
42 checks passed

munendrasn deleted the azure-credential-provider branch September 30, 2025 07:40

gabeiglio pushed a commit to gabeiglio/iceberg that referenced this pull request Oct 1, 2025

Azure: Add support to specify token credential provider (apache#14136)

3f79fe3

adawrapub pushed a commit to adawrapub/iceberg that referenced this pull request Oct 16, 2025

Azure: Add support to specify token credential provider (apache#14136)

384c80d

aswirszcz mentioned this pull request Nov 28, 2025

Iceberg 1.11 release #14706

Closed

thomaschow pushed a commit to thomaschow/iceberg that referenced this pull request Jan 19, 2026

Azure: Add support to specify token credential provider (apache#14136)

d4f0bd1

I-am-aman mentioned this pull request Aug 1, 2026

GCP: add pluggable GCS token credential provider #17468

Closed

sachinnn99 mentioned this pull request Sep 8, 2026

Azure: Add scheduled refresh for storage credentials held by ADLSFileIO #15869

Open

	public interface AzureTokenCredentialProvider {
	public interface AdlsTokenCredentialProvider {

	public class AzureTokenCredentialProviders {
	public class AdlsTokenCredentialProviders {

	* to get Azure credentials instead of using the default.
	* to get ADLS credentials instead of using the default.

	* Configure the Azure token credential provider used to get {@link TokenCredential}. A fully
	* Configure the ADLS token credential provider used to get {@link TokenCredential}. A fully

Conversation

munendrasn commented Sep 21, 2025

Uh oh!

munendrasn commented Sep 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

munendrasn commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nastra left a comment

Choose a reason for hiding this comment

Uh oh!

kevinjqliu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinjqliu Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

munendrasn commented Sep 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

munendrasn commented Sep 29, 2025 •

edited

Loading

kevinjqliu Sep 29, 2025 •

edited

Loading

kevinjqliu left a comment •

edited

Loading

kevinjqliu Sep 29, 2025 •

edited

Loading