Skip to content

Commit

Permalink
fix(lambda-nodejs): docker build is not working (#10885)
Browse files Browse the repository at this point in the history
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

* [npm install -g yarn · Issue #6 · nodejs/corepack](nodejs/corepack#6)
* [npm install yarn --global fails in docker container · Issue #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.

# 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*
  • Loading branch information
ncaq committed Oct 21, 2020
1 parent 6956aad commit 191d7b7
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-lambda-nodejs/parcel/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ARG IMAGE=amazon/aws-sam-cli-build-image-nodejs12.x
FROM $IMAGE

# Install yarn
RUN npm install --global yarn
RUN npm install --global yarn@1.22.5

# Install parcel 2 (fix the version since it's still in beta)
# install at "/" so that node_modules will be in the path for /asset-input
Expand All @@ -19,4 +19,7 @@ RUN mkdir /tmp/npm-cache && \
# Disable npm update notifications
RUN npm config --global set update-notifier false

# create non root user and change allow execute command for non root user
RUN /sbin/useradd -u 1000 user && chmod 711 /

CMD [ "parcel" ]
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-lambda-nodejs/test/docker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ beforeAll(() => {
test('parcel is available', async () => {
const proc = spawnSync('docker', [
'run', 'parcel',
'sh', '-c',
'bash', '-c',
'$(node -p "require.resolve(\'parcel\')") --version',
]);
expect(proc.status).toEqual(0);
Expand All @@ -20,7 +20,7 @@ test('parcel is installed without a package-lock.json file', async () => {
// See https://github.com/aws/aws-cdk/pull/10039#issuecomment-682738396
const proc = spawnSync('docker', [
'run', 'parcel',
'sh', '-c',
'bash', '-c',
'test ! -f /package-lock.json',
]);
expect(proc.status).toEqual(0);
Expand All @@ -30,7 +30,7 @@ test('can npm install with non root user', async () => {
const proc = spawnSync('docker', [
'run', '-u', '1000:1000',
'parcel',
'sh', '-c', [
'bash', '-c', [
'mkdir /tmp/test',
'cd /tmp/test',
'npm i constructs',
Expand All @@ -43,7 +43,7 @@ test('can yarn install with non root user', async () => {
const proc = spawnSync('docker', [
'run', '-u', '500:500',
'parcel',
'sh', '-c', [
'bash', '-c', [
'mkdir /tmp/test',
'cd /tmp/test',
'yarn add constructs',
Expand Down

0 comments on commit 191d7b7

Please sign in to comment.