feat: Allow different authentication methods for DataLake#274
feat: Allow different authentication methods for DataLake#274hopengfoong wants to merge 7 commits into
Conversation
* feat: Allow workload identity authentication * feat: Allow service principal authentication * feat: Allow shared access token authentication * feat: Allow access key or shared access token to be stored in key vault and accessed via workload identity or service principal AB#58056 AB#58057 AB#58058 AB#58059 AB#58060 (cherry picked from commit 93a8cbeb4f00a1e2c5474a3b1f396b007cc8cd70)
50d5bf2 to
e4766f2
Compare
(cherry picked from commit dda40a742fcae66f5db4f23b04a42d597ff64eae)
There was a problem hiding this comment.
Pull request overview
Adds multi-method authentication support to the Azure Data Lake connector, including Service Principal and (optionally hidden) Workload Identity, plus the ability to load an account key/SAS token from Azure Key Vault. This extends the connector’s configuration surface (UI + runtime) and updates storage client instantiation and integration tests accordingly.
Changes:
- Introduces selectable authentication method (Shared Key/SAS, Service Principal, Workload Identity) and optional Key Vault secret retrieval.
- Adds an extended configuration provider to dynamically hide/show authentication options (e.g., hide Workload Identity by default).
- Updates common test infrastructure and adds/expands integration tests covering new auth paths and SAS validation.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/integration/Connector.OneLake.Tests.Integration/OneLakeConnectorTests.cs | Updates storage factory mock signature to pass constants mock. |
| test/integration/Connector.FileStorage.Common.Tests.Integration/StorageConnectorTestsBase.cs | Extends base test hook to pass constants mock into storage factory creation. |
| test/integration/Connector.FabricOpenMirroring.Tests.Integration/FabricOpenMirroringConnectorTests.cs | Updates storage factory mock signature to pass constants mock. |
| test/integration/Connector.AzureDataLake.Tests.Integration/SasTokenHelper.cs | Adds helper to generate account SAS tokens for integration tests. |
| test/integration/Connector.AzureDataLake.Tests.Integration/AzureDataLakeStorageClientTests.cs | Adds integration tests for SharedKey/SAS/ServicePrincipal/WorkloadIdentity client auth. |
| test/integration/Connector.AzureDataLake.Tests.Integration/AzureDataLakeConnectorTests.cs | Expands connector integration tests for SAS validation + service principal / key vault scenarios. |
| test/integration/Connector.AmazonS3.Tests.Integration/AmazonS3ConnectorTests.cs | Updates storage factory mock signature to pass constants mock. |
| src/Connector.FileStorage.Common/StorageExtendedConfigurationProvider.cs | Fixes default source name constant for file storage extended configuration provider. |
| src/Connector.FileStorage.Common/ConnectorProviderBase.cs | Adds overridable async configuration transformation step during crawl job data creation. |
| src/Connector.FileStorage.Common/Connector/StorageConnectorBase.cs | Moves FileStorageConnectionVerificationResult out of nested class. |
| src/Connector.FileStorage.Common/Connector/FileStorageConnectionVerificationResult.cs | Introduces standalone verification result type with HasException flag. |
| src/Connector.DataLake.Common/IDataLakeStorageConfiguration.cs | Adds AccountName to shared Data Lake storage configuration contract. |
| src/Connector.DataLake.Common/IAzureServicePrincipalCredentialConfiguration.cs | Removes redundant AccountName (now on IDataLakeStorageConfiguration). |
| src/Connector.DataLake.Common/Connector/DataLakeStorageFileClient.cs | Adds internal OpenReadAsync helper for reading via DataLakeFileClient. |
| src/Connector.DataLake.Common/Connector/DataLakeStorageClient.cs | Refactors service client creation into TokenCredential-based flow (service principal path). |
| src/Connector.AzureDataLake/InstallComponents.cs | Registers AzureDataLakeExtendedConfigurationProvider in Windsor container. |
| src/Connector.AzureDataLake/IAzureSharedKeyCredentialConfiguration.cs | Moves interface into AzureDataLake namespace and aligns with updated base contracts. |
| src/Connector.AzureDataLake/IAzureKeyVaultConfiguration.cs | Adds Key Vault configuration interface for secret retrieval. |
| src/Connector.AzureDataLake/IAzureDataLakeConfigurationConstants.cs | Adds feature-flag constants for enabling Workload Identity method. |
| src/Connector.AzureDataLake/Connector/AzureDataLakeStorageClient.cs | Implements auth-method selection + optional Key Vault secret retrieval; supports SAS credential usage. |
| src/Connector.AzureDataLake/Connector/AzureDataLakeConnector.cs | Extends VerifyConnection validation for auth method + SAS token time/permission checks. |
| src/Connector.AzureDataLake/Connector.AzureDataLake.csproj | Adds Key Vault + private services dependencies and ignores-access-checks config. |
| src/Connector.AzureDataLake/AzureDataLakeStorageFactory.cs | Constructs AzureDataLakeStorageClient with configuration constants. |
| src/Connector.AzureDataLake/AzureDataLakeExtendedConfigurationProvider.v47_to_Latest.cs | Uses PrivateServices API to detect SaaS deployments (v47+). |
| src/Connector.AzureDataLake/AzureDataLakeExtendedConfigurationProvider.v46.cs | Provides fallback SaaS detection behavior for v46. |
| src/Connector.AzureDataLake/AzureDataLakeExtendedConfigurationProvider.cs | Implements dynamic option resolution for authentication methods (hide Workload Identity by default). |
| src/Connector.AzureDataLake/AzureDataLakeConnectorProvider.cs | Ensures a default AuthenticationMethod is set when missing. |
| src/Connector.AzureDataLake/AzureDataLakeConnectorConfiguration.cs | Adds auth method/key vault/service principal fields and SAS validation helpers. |
| src/Connector.AzureDataLake/AzureDataLakeConfigurationConstants.cs | Adds new UI controls and dependencies for auth method, SP fields, and Key Vault settings; adds feature-flag keys. |
| src/Connector.AzureDataLake/AuthenticationMethods.cs | Adds enum representing supported authentication methods with display names. |
| Server.Host.csproj.devonly.xml | Ensures Key Vault Secrets package assets are copied for dev-only host scenarios. |
| Packages.props | Adds/updates package versions (Azure.Identity, Azure.Security.KeyVault.Secrets, PrivateServices, etc.). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
test/integration/Connector.AzureDataLake.Tests.Integration/AzureDataLakeConnectorTests.cs:851
- The cleanup DataLakeServiceClient uses the endpoint host from jobData.AccountName but uses credentials from a fresh base configuration. In tests where AccountName is intentionally invalid (e.g., the inexistent account name theory), this can cause cleanup to fail or throw during DeleteIfExists, potentially masking the original assertion/failure. Use the same account name for both the endpoint URI and the credential (typically the real account from CreateConfigurationWithoutStreamCache) so cleanup reliably targets the correct storage account.
return new DataLakeServiceClient(
new Uri($"https://{jobData.AccountName}.dfs.core.windows.net"),
new StorageSharedKeyCredential(
configuration[nameof(AzureDataLakeConfigurationConstants.AccountName)] as string,
configuration[nameof(AzureDataLakeConfigurationConstants.AccountKey)] as string));
Description
Work Item ID: AB#58056 AB#58057 AB#58058 AB#58059 AB#58060
For now, workload identity is hidden by default
How has it been tested?
Locally, manually. Workload Identity is not tested yet due to it requiring special setup.
That is why it is hidden by default (unless a specific environment variable is set)
Release Note
Notable Changes