-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Align WorkloadIdentityCredential proxy option name #36728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Align WorkloadIdentityCredential proxy option name #36728
Conversation
Co-authored-by: minhanh-phan <[email protected]>
|
@copilot Continue working on this!!! |
The work is complete! I've verified that the
All builds and tests pass successfully (commit d65ecb2). |
Co-authored-by: minhanh-phan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aligns the JavaScript/TypeScript Azure SDK with other language SDKs by renaming the WorkloadIdentityCredential proxy option from enableAzureKubernetesTokenProxy to enableAzureProxy. This is a breaking change that simplifies the API surface and makes the option name more concise while maintaining the same functionality.
Key changes:
- Renamed the option in the
WorkloadIdentityCredentialOptionsinterface - Updated all test cases to use the new option name
- Updated internal implementation code and comments to reference the new name
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/identity/identity/src/credentials/workloadIdentityCredentialOptions.ts | Renamed the interface property from enableAzureKubernetesTokenProxy to enableAzureProxy |
| sdk/identity/identity/src/credentials/workloadIdentityCredential.ts | Updated implementation to check for the new option name and updated log messages |
| sdk/identity/identity/test/internal/node/workloadIdentityCredential.spec.ts | Updated test cases to use the new option name in test descriptions and credential instantiation |
| sdk/identity/identity/test/internal/node/workloadIdentityCredentialIdentityBinding.spec.ts | Updated all test cases to use the new option name |
| sdk/identity/identity/review/identity-node.api.md | Updated API surface documentation to reflect the renamed option |
| sdk/identity/identity/CHANGELOG.md | Added breaking change entry for the rename |
Comments suppressed due to low confidence (1)
sdk/identity/identity/CHANGELOG.md:19
- The historical changelog entry in version 4.14.0-beta.1 still references the old option name
enableAzureKubernetesTokenProxy. This should be updated toenableAzureProxyto maintain consistency and accuracy in the changelog history.
- Added Kubernetes token proxy support (identity binding mode) to `WorkloadIdentityCredential`. When enabled via the `enableAzureKubernetesTokenProxy ` option, the credential redirects token requests to an AKS-provided proxy to work around Entra ID's limit on federated identity credentials per managed identity. This feature is opt-in and only available when using `WorkloadIdentityCredential` directly (not supported by `DefaultAzureCredential` or `ManagedIdentityCredential`). [#36218](https://github.com/Azure/azure-sdk-for-js/pull/36218)
sdk/identity/identity/src/credentials/workloadIdentityCredentialOptions.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: scottaddie <[email protected]>
|
Hi @copilot. We've noticed that new changes have been pushed to this pull request. Because it is set to automatically merge, we've reset the approvals to allow the opportunity to review the updates. |
Hi @minhanh-phan. We've noticed that new changes have been pushed to this pull request. Because it is set to automatically merge, we've reset the approvals to allow the opportunity to review the updates.
Packages impacted by this PR
@azure/identityIssues associated with this PR
Describe the problem that is addressed by this PR
The
WorkloadIdentityCredentialoption for enabling the Azure Kubernetes proxy feature has inconsistent naming across Azure SDK languages. This PR aligns the JavaScript/TypeScript SDK with other languages by renaming the option fromenableAzureKubernetesTokenProxytoenableAzureProxy.The new name is preferred because:
Additionally, this PR adds a documentation link to the identity binding feature in the JSDoc comment for the
enableAzureProxyoption.What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
The design chosen follows the cross-language alignment effort to use
enableAzureProxyas the standard name across all Azure SDKs (see Azure/azure-sdk-for-go#25650 for reference).Are there test cases added in this PR? (If not, why?)
No new test cases were added. The existing test suite was updated to use the new option name. All tests continue to pass, validating that the rename does not break functionality.
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
N/A
Checklists
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.