Skip to content

[Feature] [Config] Add Azure Key Vault ConfigShade support#10962

Open
aakamara23 wants to merge 4 commits into
apache:devfrom
aakamara23:dev
Open

[Feature] [Config] Add Azure Key Vault ConfigShade support#10962
aakamara23 wants to merge 4 commits into
apache:devfrom
aakamara23:dev

Conversation

@aakamara23
Copy link
Copy Markdown

                                   Purpose of this pull request

This PR is for issue #10309.

I added initial Azure Key Vault support to SeaTunnel using the existing ConfigShade framework.

                    For this implementation, I:
  • Added an azure-kv ConfigShade provider
  • Added Azure Key Vault and Azure Identity dependencies
  • Configured secret resolution using vault.url
  • Registered the provider in the ConfigShade service file

The goal is to allow users to store sensitive values in Azure Key Vault instead of directly in configuration files.

Does this PR introduce any user-facing change?
Yes.

Users can now configure shade.identifier = "azure-kv" and reference secrets stored in Azure Key Vault.

How was this patch tested?

                         I tested the changes locally by running:

mvn -pl seatunnel-core/seatunnel-core-starter test -DskipITs -DskipShade -Dspotless.check.skip=true -U

                         The build completed successfully.

@github-actions github-actions Bot added the core SeaTunnel core module label May 27, 2026
@davidzollo davidzollo added the First-time contributor First-time contributor label May 27, 2026
Copy link
Copy Markdown
Contributor

@DanielLeens DanielLeens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a real external secret-manager direction here. I reviewed the latest head from the actual starter/config loading path, and I see two blockers before this should merge.

Runtime chain I checked:

Job startup / config loading
  -> ConfigShadeUtils.decryptConfig(config)
      -> processConfig(identifier, config, true, props)
      -> CONFIG_SHADES.getOrDefault("azure-kv", ...)
      -> configShade.open(props)

Azure Key Vault implementation
  -> AzureKeyVaultConfigShade.open(props)
      -> read vault.url
      -> build SecretClient with DefaultAzureCredential
  -> decrypt("${keyvault:azure:...}")
      -> trim to last path segment
      -> secretClient.getSecret(secretName).getValue()

Key files:

  • seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/AzureKeyVaultConfigShade.java:1-62
  • seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/ConfigShadeUtils.java:61-69,166-170
  • seatunnel-core/seatunnel-core-starter/pom.xml:70-80

Issue 1: the new Java file is missing the ASF license header

  • Location: seatunnel-core/seatunnel-core-starter/src/main/java/org/apache/seatunnel/core/starter/utils/AzureKeyVaultConfigShade.java:1
  • Why this is blocking:
    this is a brand new source file, and it currently starts directly with package .... The fork CI is already red on Run / License header, and the failing file reported there is exactly this one.
  • Severity: High

Issue 2: the Azure provider is wired directly into seatunnel-core-starter, so every starter distribution now pulls Azure SDKs whether the user needs them or not

  • Location: seatunnel-core/seatunnel-core-starter/pom.xml:70
  • Why this is a blocker:
    ConfigShade is SPI-based already, so this is naturally extensible. But the current change puts azure-security-keyvault-secrets and azure-identity straight into the core starter, which expands the default runtime footprint and dependency surface for every user, not only Azure users.
  • Better direction:
    move the Azure Key Vault implementation into a dedicated optional module and let the existing ServiceLoader mechanism discover it when present.
  • Severity: High

Issue 3: docs and regression coverage are not updated for a new built-in azure-kv capability

  • Location: docs/en/introduction/configuration/config-encryption-decryption.md:9
  • Why it matters:
    the current docs still describe only base64 and custom user implementations. There is no built-in azure-kv usage contract, no vault.url explanation, and no test coverage for the actual new provider path.
  • Severity: Medium

Compatibility:

  • Existing base64/default behavior remains intact.
  • The main compatibility concern is packaging: the latest head changes the starter dependency footprint for everyone.

Tests / CI:

  • I did not run this locally.
  • The current Build failure is directly related to this PR: Run / License header fails and reports AzureKeyVaultConfigShade.java as the only invalid file.
  • This is not unrelated CI noise; it is a real branch-local blocker.

Merge conclusion: can merge after fixes

  1. Blocking items
  • Issue 1 must be fixed first because the branch fails the ASF license header check.
  • Issue 2 should be resolved before merge because the current packaging choice is too heavy for seatunnel-core-starter.
  1. Suggested follow-up
  • Add the missing docs and at least a minimal regression test for the new azure-kv path from Issue 3.

Overall, the feature idea is useful. The current concern is not Azure itself, but how it is being shipped: we should keep the core starter lean and keep the new provider properly documented and testable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core SeaTunnel core module First-time contributor First-time contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants