Skip to content
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

fix: use registry.npmjs.com to fix shinkwrap resolves #16607

Merged
merged 2 commits into from Oct 13, 2021

Conversation

everett1992
Copy link
Contributor

@everett1992 everett1992 commented Sep 22, 2021

npm treats registry.npmjs.org as a special value that means 'the current configured package' in package-lock and npm-shrinkwrap. npm will request aws-cdk's dependencies from yarnpkg instead of from the installers configured registry because aws-cdk's shrinkwrap uses yarnpkg. This behavior seems new to npm v7. It causes issues for us because we run our builds with a isolated network and a private registry.

This commit changes the registry from yarnpkg to npmjs. I updated yarn.lock with sed.

sed 's|https://registry.yarnpkg.com|https://registry.npmjs.org|' yarn.lock -i

Alternatively we could modify the yarn-cling tool to replace the registry. registry.yarnpkg.com is a cname for registry.npmjs.org so changing to registry.npmjs.org shouldn't affect available packages or performance.

dig registry.yarnpkg.com | rg CNAME
registry.yarnpkg.com.	300	IN	CNAME	yarn.npmjs.org

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Sep 22, 2021

RomainMuller
RomainMuller previously approved these changes Sep 29, 2021
RomainMuller
RomainMuller previously approved these changes Sep 29, 2021
@RomainMuller RomainMuller added the pr-linter/exempt-test The PR linter will not require test changes label Sep 29, 2021
@everett1992
Copy link
Contributor Author

The CodeBuild step fails but I have to log in to view the details. Can you check what's wrong or explain how to test locally?

yarn install # works

@everett1992
Copy link
Contributor Author

everett1992 commented Sep 29, 2021

The commands from buildspec fail the same way on my branch and upstream.

./build.sh --extract && ./scripts/transform.sh --extract
@aws-cdk/aws-s3: WARNING: Retrying (Retry(total=4, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7f95f3cb4ed0>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/boto3/
@aws-cdk/aws-s3: WARNING: Retrying (Retry(total=3, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7f95f3cbbc10>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/boto3/
@aws-cdk/aws-s3: WARNING: Retrying (Retry(total=2, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7f95f3cbb6d0>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/boto3/
@aws-cdk/aws-s3: WARNING: Retrying (Retry(total=1, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7f95f3cbb610>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/boto3/
@aws-cdk/aws-s3: WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.VerifiedHTTPSConnection object at 0x7f95f3cbb910>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/boto3/
@aws-cdk/aws-s3: ERROR: Could not find a version that satisfies the requirement boto3==1.17.42 (from versions: none)
@aws-cdk/aws-s3: ERROR: No matching distribution found for boto3==1.17.42
@aws-cdk/aws-s3: The command '/bin/sh -c pip3 install boto3==1.17.42' returned a non-zero code: 1
@aws-cdk/aws-s3: FAIL test/notifications-resource.lambda.test.js (291.379 s)
@aws-cdk/aws-s3:   ● notifications handler
@aws-cdk/aws-s3:     expect(received).toBe(expected) // Object.is equality
@aws-cdk/aws-s3:     Expected: 0
@aws-cdk/aws-s3:     Received: 1
@aws-cdk/aws-s3:       5 |   const testScript = path.join(__dirname, 'notifications-resource-handler', 'test.sh');
@aws-cdk/aws-s3:       6 |   const result = spawnSync(testScript, { stdio: 'inherit' });
@aws-cdk/aws-s3:     > 7 |   expect(result.status).toBe(0);
@aws-cdk/aws-s3:         |                         ^
@aws-cdk/aws-s3:       8 | });
@aws-cdk/aws-s3:       9 |
@aws-cdk/aws-s3:       at Object.<anonymous> (test/notifications-resource.lambda.test.ts:7:25)

Test failed even after installing boto3

Use the npm registry instead of yarns mirror.
npm treats registry.npmjs.org as a special value that means 'the current
configured package' in package-lock and npm-shrinkwrap.  if we use
registry.yarnpkg.com in our shrinkwrap then users with a custom registry
will be forced to registry.yarnpkg.com.
npm/cli#3783

I updated yarn.lock with sed.

```
sed 's|https://registry.yarnpkg.com|https://registry.npmjs.org|' yarn.lock -i
```
@mergify
Copy link
Contributor

mergify bot commented Oct 13, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 2b6e3e9
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 8f91531 into aws:master Oct 13, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 13, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
npm treats registry.npmjs.org as a special value that means 'the current configured package' in package-lock and npm-shrinkwrap. npm will request aws-cdk's dependencies from yarnpkg instead of from the installers configured registry because aws-cdk's shrinkwrap uses yarnpkg. This behavior seems new to [npm v7]. It causes issues for us because we run our builds with a isolated network and a private registry.

[npm v7]: npm/cli#3783

This commit changes the registry from yarnpkg to npmjs. I updated yarn.lock with sed.

```
sed 's|https://registry.yarnpkg.com|https://registry.npmjs.org|' yarn.lock -i
```
Alternatively we could modify the yarn-cling tool to replace the registry. [registry.yarnpkg.com is a cname for registry.npmjs.org](https://yarnpkg.com/getting-started/qa#why-registryyarnpkgcom-does-facebook-track-us) so changing to registry.npmjs.org shouldn't affect available packages or performance.

```
dig registry.yarnpkg.com | rg CNAME
registry.yarnpkg.com.	300	IN	CNAME	yarn.npmjs.org
```

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants