fix(@angular/cli): handle NPM_CONFIG environment variables during ng update and ng add

[mikejancar]

Some organizations are moving away from storing tokens/secrets in an npm config file in favor of environment variables that only exist for the span of a terminal session. This commit will read those variables in when no options have been configured via a file.

From a testing perspective, I was not able to get the full unit test/e2e suites to run successfully. The unit test suite fails immediately with a Bazel build failure. If you have any insight into what might be causing this, I'm more than happy to keep trying things. I wasn't very successful at finding similar issues in the normal places (i.e. Stack Overflow).

The full e2e suite failed at different steps in each of my attempts. The latest one is below as an example. If there's some common gotchas with the e2e tests, I'm willing to try again.

I was able to successfully run the following tests which looked to be the most relevant to the logic I was updating.

• tests\legacy-cli\e2e\tests\update\*.ts
• tests\legacy-cli\e2e\tests\commands\add\*.ts

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[mikejancar]

@googlebot I signed it!

[alan-agius4]

Thanks for your contribution.

I did take a quick look at this and it appears that some wrong assumptions were made.

• You can use NPMRC togather environment variables. The latter will override the former.

engine-strict = true
registry = http://foo.com

NPM_CONFIG_REGISTRY=http://bar.com npm i

Should result in the following

{
   registry: 'http://bar.com',
   'engine-strict': true,
}

• Environment name variables need to be normalized NPM_CONFIG_HTTPS_PROXY -> https-proxy. The rules are trim the first 11 characters, lowercase and replace _ with -.

Once the above are addressed, can you also add an E2E test, something similar to https://github.com/angular/angular-cli/blob/master/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts but instead of using an NPMRC to set the authentication, use an environment variable?. When you are at this stage, I can help you with this.

[mikejancar]

Thanks for reviewing so quickly, Alan. I'll get started on the changes and let you know if I have any issues.

[mikejancar]

Just to clarify, @alan-agius4, the use case that I'm trying to address is when there is no configuration file present. When that occurs, we have found that the environment variables are never looked at because the substitution is only done in the existing code if a file is found. This leads to ng update failing since it isn't using our private registry with the proper authentication.

The code I added will address this scenario. Are you saying that the code shouldn't be needed at all or are you asking for a change to the logic I've added?

I'll work on adding an E2E test in the meantime to illustrate this use case.

[alan-agius4]

Hi, I am asking for changes in logic to make this code behave as NPM would. Because certain assumptions made during this change are not correct.

1. It is assumed that when using enviorment variables no config file is used. This is not correct as one can use both at the same time. As described here fix(@angular/cli): handle NPM_CONFIG environment variables during ng update and ng add #21297 (review)
2. Environment variables names needs to be normalised. Ex: NPM_CONFIG_HTTPS_PROXY -> https-proxy, See the full constraint here fix(@angular/cli): handle NPM_CONFIG environment variables during ng update and ng add #21297 (review)

The commit message should also be updated to something like the below;

fix(@angular/cli): handle NPM_CONFIG enviorment variables during ng update and ng add

[mikejancar]

Thanks for the clarification. Changes coming.

[mikejancar]

@alan-agius4 I believe I have everything up and running now. Took me a while to pin down some of the inadvertent changes to the environment variables in the tests but I think I found them all. Please review and let me know what you think.

[alan-agius4]

Thanks for this, a couple of NITs.

[mikejancar]

Looks like we're back to a passing test suite. Let me know if there's anything else you'd like me to do.

[alan-agius4]

Thanks for your contribution.

[mikejancar]

Thanks for your patience and guidance, @alan-agius4.

[alan-agius4]

@mikejancar, welcome and thanks again for your contribution. FYI: I did follow up to support Yarn enviorment vars #21378

[mikejancar]

I did see that. Nice follow up.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}