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(lambda-nodejs): docker build is not working #10885

Merged
merged 6 commits into from
Oct 21, 2020

Conversation

ncaq
Copy link
Contributor

@ncaq ncaq commented Oct 15, 2020

fixes #10881

fix from sh to bash

lib/bundlers.ts actually uses not sh but bash,
bash is a good way to test.

lock yarn version

aws-lambda-nodejs have issue

From issue comment,

For future reference, you can (should) pin your version rather than use whatever the latest is on npm (by using yarn@1.22.6, etc) - it's a good practice anyway regardless of the conditions, as you never know which bug could slip by us. You can also use the yarn-path setting to ensure that upgrades go through the appropriate review processes (including CI).

yarnpkg/yarn#8358 (comment)

So we'll follow it.

And from issue comment,

Fwiw we don't plan to add any more features to Yarn 1, as all of our resources have shifted to Yarn 2. The past few commits have been aimed toward making the transition a bit easier, in particular thanks to the Corepack initiative which we hope will make it easier to use Yarn (both 1 & 2) by removing the need to manually install them.

yarnpkg/yarn#8358 (comment)

There's not much need to be concerned with the latest version of 1.

allow execute command for non root user

amazon/aws-sam-cli-build-image-nodejs12.x don't have user that index is 1000.
So create non root user.

/ in amazon/aws-sam-cli-build-image-nodejs12.x permission is 700.
change to allow execute command for non root user.
I really don't want to change around the permissions,
but I don't have a choice.


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

`lib/bundlers.ts` actually uses not `sh` but `bash`,
`bash` is a good way to test.
aws-lambda-nodejs have issue

* [npm install -g yarn · Issue #6 · nodejs/corepack](nodejs/corepack#6)
* [npm install yarn --global fails in docker container · Issue aws#8358 · yarnpkg/yarn](yarnpkg/yarn#8358)

From issue comment,

> For future reference, you can (should) pin your version rather than use whatever the latest is on npm (by using yarn@1.22.6, etc) - it's a good practice anyway regardless of the conditions, as you never know which bug could slip by us. You can also use the yarn-path setting to ensure that upgrades go through the appropriate review processes (including CI).
>
> <yarnpkg/yarn#8358 (comment)>

So we'll follow it.

And from issue comment,

> Fwiw we don't plan to add any more features to Yarn 1, as all of our resources have shifted to Yarn 2. The past few commits have been aimed toward making the transition a bit easier, in particular thanks to the Corepack initiative which we hope will make it easier to use Yarn (both 1 & 2) by removing the need to manually install them.
>
> <yarnpkg/yarn#8358 (comment)>

There's not much need to be concerned with the latest version of 1.
`amazon/aws-sam-cli-build-image-nodejs12.x` don't have user that index is 1000.
So create non root user.

`/` in `amazon/aws-sam-cli-build-image-nodejs12.x` permission is `700`.
change to allow execute command for non root user.
I really don't want to change around the permissions,
but I don't have a choice.
@gitpod-io
Copy link

gitpod-io bot commented Oct 15, 2020

@jogold
Copy link
Contributor

jogold commented Oct 15, 2020

bash is a good way to test.

@ncaq what do you mean exactly here?

@ncaq
Copy link
Contributor Author

ncaq commented Oct 16, 2020

bash is a good way to test.

@ncaq what do you mean exactly here?

The lib/bundlers.ts actually uses not sh but bash.

command: ['bash', '-c', command],

So test code use bash likewise.

@eladb
Copy link
Contributor

eladb commented Oct 21, 2020

@jogold awaiting your LGTM

@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2020

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).

@eladb eladb changed the title fix(aws-lambda-nodejs) docker build is not working fixes #10881 fix(lambda-nodejs): docker build is not working Oct 21, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 38fed27
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2020

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).

@mergify mergify bot merged commit 191d7b7 into aws:master Oct 21, 2020
@ncaq ncaq deleted the fix/aws-lambda-nodejs branch October 21, 2020 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[aws-lambda-nodejs] docker build is not working
4 participants