Azure-sdk-for-js: [Identity] Create a separate package for optional native components of @azure/identity

Created on 17 Mar 2021  路  20Comments  路  Source: Azure/azure-sdk-for-js

In Identity, we would like to avoid adding any packages with native binaries as dependencies (not even peer or optional dependencies).

  • [ ] VisualStudioCodeCredential depends on keytar, which uses native binaries to commmunicate with the platform credential manager.
  • [ ] persistence through msal-node-extensions requires a native DPAPI component that is installed with the package. As Mike reported here: https://github.com/Azure/azure-sdk-for-js/pull/14343#issuecomment-801458068 some customers refuse or are unable to install native dependencies.

We would like to provide these facilities through a separate package that can inject or provide these native module dependencies outside of the mainline identity package. This issue tracks that work.

Azure.Identity Client blocking-release

Most helpful comment

@octogonz, @iclanton,

While we wait for v2, if it is indeed the case that rush currently uses only the device code login flow and you do not care about whether the underlying implementation for this uses the newer MSAL or not, then another option for you can be to update the identity dependency to use ~1.0.0 instead of say ^1.0.0. The keytar dependency came into play when we added the VisualStudioCodeCredential in 1.1.0. So, versions 1.0.* should be safe for you to use device code login flow from @azure/identity.

Once we have resolved this issue for native components, you can get right back to using the latest identity package

All 20 comments

Making the dependency optional doesn't help much, as explained in https://github.com/Azure/azure-sdk-for-js/issues/13950. Instead, I think the identity package should be split into one or more packages, one with native dependencies and one without.

@mikeharder that sounds about right. I've been thinking of two packages too!

I've updated the description and title of this issue to reflect the current proposed solution.

@witemple-msft How does Make msal-node-extensions an optional dependency solve the originally stated problem of we would like to avoid adding any packages with native binaries as dependencies (not even peer or optional dependencies)?

FWIW Rush really cannot wait much longer. Today someone reported that Docker fails to install the Rush tool because keytar's postbuild-install fails to write to the /root/.npm/ folder when it's trying to download the native binaries.

If a solution isn't forthcoming, then it seems that Rush should consider instead eliminating Azure SDK. We already recently eliminated Amazon's SDK due to similar issues with problematic NPM dependencies.

@octogonz The Identity package will release a v2-beta in a couple of weeks without Keytar. The solution @willmtemple is working on is on having an entirely separate package that will contain the binaries. The main @azure/identity v2 won't have any binary dependency. This v2 will go GA in a couple of months though. Would you be able to work with the v2-beta we'll release in a couple of weeks? We'll make sure to work with you on the transition to v2 (though it should be seamless for Rush, but in case anything surprising comes up).

The only thing Rush actually uses from @azure/identity is the device code login flow. That seems like it could be a pretty self-contained portion of the package. For us, it would be ideal if that functionality was in its own package. Would something like that be possible? Something like @azure/device-code-authentication.

This v2 will go GA in a couple of months though. Would you be able to work with the v2-beta we'll release in a couple of weeks?

@sadasant Yes -- given that the Azure build cache feature is still considered "experimental", Rush seems like a great candidate for testing a v2-beta.

@octogonz: You might be able to use a v2-alpha even earlier.

@sadasant: How soon do you think we might have a v2-alpha without any native deps?

@octogonz My title change apparently wasn't saved. It's updated now. As far as timeline, we are pushing towards a GA solution within the next month and a half or so, beta availability for a stripped down package within a couple of weeks. @azure/identity 2.0.0 will not have any native package dependencies, peer, optional, or otherwise, and native extensions to identity will be moved into their own package.

@iclanton The principle of @azure/identity is to be a fixpoint within the Azure SDK for JavaScript for AAD authentication. In the backend, we are using MSAL. If you aren't using Identity to work with the Azure SDK data-plane libraries, you could consider using @azure/msal-node and its support for device code grants directly, but it's a more involved API than Identity is. We are reluctant to separate the components of identity at all, but the native dependencies present enough of a problem to justify removing the VS Code Credential.

Local dev builds of 2.0.0-beta.3 are about 25MB on disk, as we are able to throw off a bit by not installing keytar and prebuild-install. It's up to you all if that's acceptable. @mikeharder I hope to have a basic version of this merged tomorrow or Monday so we should have a dev artifact by Tuesday.

CC @schaabs, @bterlson

@octogonz, @iclanton,

While we wait for v2, if it is indeed the case that rush currently uses only the device code login flow and you do not care about whether the underlying implementation for this uses the newer MSAL or not, then another option for you can be to update the identity dependency to use ~1.0.0 instead of say ^1.0.0. The keytar dependency came into play when we added the VisualStudioCodeCredential in 1.1.0. So, versions 1.0.* should be safe for you to use device code login flow from @azure/identity.

Once we have resolved this issue for native components, you can get right back to using the latest identity package

@ramya-rao-a I confirmed that your suggestion does eliminate the keytar dependency (PR https://github.com/microsoft/rushstack/pull/2647)

@ramya-rao-a Downgrading to ~1.0.0 did not work for us in PR https://github.com/microsoft/rushstack/pull/2647:

Error: src/logic/buildCache/AzureStorageBuildCacheProvider.ts:15:10 - (TS2305) Module '"../../../node_modules/@azure/identity/types/identity"' has no exported member 'AzureAuthorityHosts'.

It seems that maybe we do need to wait for 2.0.0-beta.x.

CC @elliot-nelson

Following up, we are still having trouble getting https://github.com/microsoft/rushstack/pull/2647 to build.

How close are we to having a 2.0.0-beta.x beta release?

_Update_: The rush team has been unblocked for now. Please see https://github.com/microsoft/rushstack/pull/2647 for details

Any guidance on work around until this is released?

@01 The workaround depends on which features of @azure/identity you depend on. Can you share which credential are you using? If using DefaultAzureCredential, which of the multiple credentials it uses internally is used by your code?

@01 The workaround depends on which features of @azure/identity you depend on. Can you share which credential are you using? If using DefaultAzureCredential, which of the multiple credentials it uses internally is used by your code?

Simply using DefaultAzureCredential with a system assigned managed identity for an Azure App Service

@01 In that case, please try the solution suggested at https://github.com/Azure/azure-sdk-for-js/issues/14346#issuecomment-825995352.

FYI since we downgraded to "@azure/identity": "~1.0.0", we are now seeing this warning during installation:

npm WARN deprecated @opentelemetry/[email protected]: Package renamed to @opentelemetry/api, see https://github.com/open-telemetry/opentelemetry-js

It's only a minor annoyance, but we're looking forward to whenever @azure/identity 2.0.0 is ready so we can eliminate that warning.

@octogonz @iclanton -- Our latest beta release @azure/[email protected] (tag: next) on NPM has removed the optional dependency on keytar as well. Our plan is to move forward with a base @azure/identity 2.0.0 package that is free of machine code dependencies, and the functionality that the machine code dependencies provided in Identity 1.x will be restored through a separate extension package.

We don't have the design of the extension package nailed down yet, but our mainline @azure/identity package will continue in this fashion without native deps.

Was this page helpful?
0 / 5 - 0 ratings