Try to download pre-built alpine binary before building #1026

LaurentGoderre · 2019-04-24T16:02:29Z

No description provided.

LaurentGoderre · 2019-04-24T16:03:46Z

SimenB

This is awesome! Would be so good to not have to build for alpine (albeit we'll probably be building node 10 for a while)

10/alpine/Dockerfile

LaurentGoderre · 2019-04-24T16:07:00Z

The only thing missing is the different architecture. The output in Alpine is different than on Debian.

LaurentGoderre · 2019-04-24T16:07:59Z

@tianon do you have those?

LaurentGoderre · 2019-04-24T16:28:36Z

Nevermind, I just won't error on the other archs

chorrell · 2019-04-24T18:47:38Z

10/alpine/Dockerfile

-        make \
-        python \
+RUN ARCH= && alpineArch="$(arch)" \
+  && case "${alpineArch##*-}" in \


Is this needed? x64 is the only option so this can be simplified be removing the alpine specific case statement

We need to detect the arch anywys because the other arch will still need to build from source

chorrell · 2019-04-24T18:48:12Z

Dockerfile-alpine.template

-        make \
-        python \
+RUN ARCH= && alpineArch="$(arch)" \
+  && case "${alpineArch##*-}" in \


Same as previous comment:

Is this needed? x64 is the only option so this can be simplified be removing the alpine specific case statement

nschonni · 2019-04-24T18:52:54Z

6/alpine/Dockerfile

@@ -2,20 +2,19 @@ FROM alpine:3.9



Since this hits EOL on the 30th, maybe better to skip instead of pushing a new version

We can wait after the 30th an rebase this after version 6 has been removed.

yosifkit · 2019-04-24T22:36:05Z

10/alpine/Dockerfile

@@ -34,6 +33,23 @@ RUN addgroup -g 1000 node \
    gpg --batch --keyserver hkp://ipv4.pool.sks-keyservers.net --recv-keys "$key" || \
    gpg --batch --keyserver hkp://pgp.mit.edu:80 --recv-keys "$key" ; \
  done \
+  && ( \
+    curl -fsSLO --compressed "https://unofficial-builds.nodejs.org/download/release/v$NODE_VERSION/node-v$NODE_VERSION-linux-$ARCH-musl.tar.xz" \
+    && curl -fsSLO --compressed "https://unofficial-builds.nodejs.org/download/release/v$NODE_VERSION/SHASUMS256.txt" \


Downloading the shasums file at the same time as the tar doesn't really increase the authenticity of the artifact. If someone is able to compromise the unofficial-builds server (or the connection to it), then they can easily change both files to match.

We usually recommend downloading the sha file separately (like during update.sh) so that it can be embedded in the Dockerfile. See https://github.com/docker-library/official-images/tree/3b512a89a9719efa190d5938ea3ca4ca5291d68c#image-build

There is also a bug in that a failed sha256sum (or tar, rm, or ln) could leave the tar, shasum, or extracted files intact in the image while still then building from source. For RUN lines that need more complex logic, you might want to move to using set -eu with semicolons rather than && (docker-library/mongo#313 (comment)).

Overall, these new builds will be nice to have!

So, it should really only try building from source if neither file exists, since a sha or other failure indicates a different problem and the image should appropriately fail to build.

I don't think these comments have been addressed yet. 😉

True, but this approach is in line with the other image so maybe it warrants a separate discussion.

I'm not following -- which "other image" are you referring to?

We also need to address when these lines fail:

There is also a bug in that a failed sha256sum (or tar, rm, or ln) could leave the tar, shasum, or extracted files intact in the image while still then building from source. For RUN lines that need more complex logic, you might want to move to using set -eu with semicolons rather than && (docker-library/mongo#313 (comment)).

If the tar downloads (and fails to shasum or something), it should consider that as the only possible route for the given version, rather than falling back to building, so that the build failure can be noticed and the unofficial artifacts fixed.

Done!!! Only thing left is to modify the update script to embed the shasum.

@yosifkit @tianon all done!

The current implementation still suffers here -- as written, it will always attempt to download an unofficial-builds tarball whether one is expected to exist or not, and even if we're on an architecture where we expect to have such a tarball, if it does not exist, we'll happily throw a harmless error in the logs nobody will check followed by downloading the source and building instead. Not a bad strategy from a "resilient builds" perspective, but it doesn't help catch issues with the unofficial-builds project.

Here's an alternative approach (created from 10/alpine/Dockerfile to illustrate so I'd have something concrete I could test/verify):

FROM alpine:3.9 ENV NODE_VERSION 10.16.1 RUN set -eux; \ addgroup -g 1000 node; \ adduser -u 1000 -G node -s /bin/sh -D node; \ apk add --no-cache \ libstdc++ \ ; \ apk add --no-cache --virtual .build-deps \ curl \ ; \ apkArch="$(apk --print-arch)"; \ url=; checksum=; \ case "$apkArch" in \ x86_64) \ url="https://unofficial-builds.nodejs.org/download/release/v$NODE_VERSION/node-v$NODE_VERSION-linux-x64-musl.tar.xz"; \ checksum='c5b5adc27dfe1c9bded56b30158f9a10c617c93dde1f74febc19866dc5ff487c'; \ ;; \ esac; \ if [ -n "$url" ] && [ -n "$checksum" ]; then \ curl -fsSL -o node.txz --compressed "$url"; \ echo "$checksum *node.txz" | sha256sum -c -; \ tar -xJf node.txz -C /usr/local --strip-components=1 --no-same-owner; \ rm node.txz; \ else \ apk add --no-cache --virtual .build-deps-full \ binutils-gold \ g++ \ gcc \ gnupg \ libgcc \ linux-headers \ make \ python \ ; \ # gpg keys listed at https://github.com/nodejs/node#release-keys for key in \ 94AE36675C464D64BAFA68DD7434390BDBE9B9C5 \ FD3A5288F042B6850C66B31F09FE44734EB7990E \ 71DCFD284A79C3B38668286BC97EC7A07EDE3FC1 \ DD8F2338BAE7501E3DD5AC78C273792F7D83545D \ C4F0DFFF4E8C1A8236409D08E73BC641CC11F4C8 \ B9AE9905FFD7803F25714661B63B535A4C206CA9 \ 77984A986EBC2AA786BC0F66B01FBB92821C587A \ 8FCCA13FEF1D0C2E91008E09770F7A9A5AE15600 \ 4ED778F539E3634C779C87C6D7062848A1AB005C \ A48C2BEE680E841632CD4E44F07496B3EB3C1762 \ B9E2F5981AA6E0CD28160D9FF13993A75599653C \ ; do \ gpg --batch --keyserver hkp://p80.pool.sks-keyservers.net:80 --recv-keys "$key" || \ gpg --batch --keyserver hkp://ipv4.pool.sks-keyservers.net --recv-keys "$key" || \ gpg --batch --keyserver hkp://pgp.mit.edu:80 --recv-keys "$key" ; \ done; \ curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/node-v$NODE_VERSION.tar.xz"; \ curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/SHASUMS256.txt.asc"; \ gpg --batch --decrypt --output SHASUMS256.txt SHASUMS256.txt.asc; \ grep " node-v$NODE_VERSION.tar.xz\$" SHASUMS256.txt | sha256sum -c -; \ tar -xf "node-v$NODE_VERSION.tar.xz"; \ cd "node-v$NODE_VERSION"; \ ./configure; \ make -j$(getconf _NPROCESSORS_ONLN) V=; \ make install; \ apk del --no-network .build-deps-full; \ cd ..; \ rm -Rf "node-v$NODE_VERSION"; \ rm "node-v$NODE_VERSION.tar.xz" SHASUMS256.txt.asc SHASUMS256.txt; \ fi; \ apk del --no-network .build-deps; \ # basic sanity/smoke test node --version ENV YARN_VERSION 1.17.3 RUN apk add --no-cache --virtual .build-deps-yarn curl gnupg tar \ && for key in \ 6A010C5166006599AA17F08146C2130DFD2497F5 \ ; do \ gpg --batch --keyserver hkp://p80.pool.sks-keyservers.net:80 --recv-keys "$key" || \ gpg --batch --keyserver hkp://ipv4.pool.sks-keyservers.net --recv-keys "$key" || \ gpg --batch --keyserver hkp://pgp.mit.edu:80 --recv-keys "$key" ; \ done \ && curl -fsSLO --compressed "https://yarnpkg.com/downloads/$YARN_VERSION/yarn-v$YARN_VERSION.tar.gz" \ && curl -fsSLO --compressed "https://yarnpkg.com/downloads/$YARN_VERSION/yarn-v$YARN_VERSION.tar.gz.asc" \ && gpg --batch --verify yarn-v$YARN_VERSION.tar.gz.asc yarn-v$YARN_VERSION.tar.gz \ && mkdir -p /opt \ && tar -xzf yarn-v$YARN_VERSION.tar.gz -C /opt/ \ && ln -s /opt/yarn-v$YARN_VERSION/bin/yarn /usr/local/bin/yarn \ && ln -s /opt/yarn-v$YARN_VERSION/bin/yarnpkg /usr/local/bin/yarnpkg \ && rm yarn-v$YARN_VERSION.tar.gz.asc yarn-v$YARN_VERSION.tar.gz \ && apk del .build-deps-yarn COPY docker-entrypoint.sh /usr/local/bin/ ENTRYPOINT ["docker-entrypoint.sh"] CMD [ "node" ]

And the diff, which should make the changes I've made more obvious:

$ GIT_PAGER=cat git diff --ignore-all-space diff --git a/10/alpine/Dockerfile b/10/alpine/Dockerfile index c165a6c..b653f4e 100644 --- a/10/alpine/Dockerfile +++ b/10/alpine/Dockerfile @@ -2,13 +2,31 @@ FROM alpine:3.9 ENV NODE_VERSION 10.16.1 -RUN addgroup -g 1000 node \ - && adduser -u 1000 -G node -s /bin/sh -D node \ - && apk add --no-cache \ +RUN set -eux; \ + addgroup -g 1000 node; \ + adduser -u 1000 -G node -s /bin/sh -D node; \ + apk add --no-cache \ libstdc++ \ - && apk add --no-cache --virtual .build-deps \ - binutils-gold \ + ; \ + apk add --no-cache --virtual .build-deps \ curl \ + ; \ + apkArch="$(apk --print-arch)"; \ + url=; checksum=; \ + case "$apkArch" in \ + x86_64) \ + url="https://unofficial-builds.nodejs.org/download/release/v$NODE_VERSION/node-v$NODE_VERSION-linux-x64-musl.tar.xz"; \ + checksum='c5b5adc27dfe1c9bded56b30158f9a10c617c93dde1f74febc19866dc5ff487c'; \ + ;; \ + esac; \ + if [ -n "$url" ] && [ -n "$checksum" ]; then \ + curl -fsSL -o node.txz --compressed "$url"; \ + echo "$checksum *node.txz" | sha256sum -c -; \ + tar -xJf node.txz -C /usr/local --strip-components=1 --no-same-owner; \ + rm node.txz; \ + else \ + apk add --no-cache --virtual .build-deps-full \ + binutils-gold \ g++ \ gcc \ gnupg \ @@ -16,8 +34,9 @@ RUN addgroup -g 1000 node \ linux-headers \ make \ python \ + ; \ # gpg keys listed at https://github.com/nodejs/node#release-keys - && for key in \ + for key in \ 94AE36675C464D64BAFA68DD7434390BDBE9B9C5 \ FD3A5288F042B6850C66B31F09FE44734EB7990E \ 71DCFD284A79C3B38668286BC97EC7A07EDE3FC1 \ @@ -33,20 +52,24 @@ RUN addgroup -g 1000 node \ gpg --batch --keyserver hkp://p80.pool.sks-keyservers.net:80 --recv-keys "$key" || \ gpg --batch --keyserver hkp://ipv4.pool.sks-keyservers.net --recv-keys "$key" || \ gpg --batch --keyserver hkp://pgp.mit.edu:80 --recv-keys "$key" ; \ - done \ - && curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/node-v$NODE_VERSION.tar.xz" \ - && curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/SHASUMS256.txt.asc" \ - && gpg --batch --decrypt --output SHASUMS256.txt SHASUMS256.txt.asc \ - && grep " node-v$NODE_VERSION.tar.xz\$" SHASUMS256.txt | sha256sum -c - \ - && tar -xf "node-v$NODE_VERSION.tar.xz" \ - && cd "node-v$NODE_VERSION" \ - && ./configure \ - && make -j$(getconf _NPROCESSORS_ONLN) V= \ - && make install \ - && apk del .build-deps \ - && cd .. \ - && rm -Rf "node-v$NODE_VERSION" \ - && rm "node-v$NODE_VERSION.tar.xz" SHASUMS256.txt.asc SHASUMS256.txt + done; \ + curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/node-v$NODE_VERSION.tar.xz"; \ + curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/SHASUMS256.txt.asc"; \ + gpg --batch --decrypt --output SHASUMS256.txt SHASUMS256.txt.asc; \ + grep " node-v$NODE_VERSION.tar.xz\$" SHASUMS256.txt | sha256sum -c -; \ + tar -xf "node-v$NODE_VERSION.tar.xz"; \ + cd "node-v$NODE_VERSION"; \ + ./configure; \ + make -j$(getconf _NPROCESSORS_ONLN) V=; \ + make install; \ + apk del --no-network .build-deps-full; \ + cd ..; \ + rm -Rf "node-v$NODE_VERSION"; \ + rm "node-v$NODE_VERSION.tar.xz" SHASUMS256.txt.asc SHASUMS256.txt; \ + fi; \ + apk del --no-network .build-deps; \ +# basic sanity/smoke test + node --version ENV YARN_VERSION 1.17.3

If you like this approach, I'd be happy to put together a proper PR.

rvagg · 2019-04-24T23:00:40Z

albeit we'll probably be building node 10 for a while

Well, the way unofficial-builds is currently set up is that it's just going to churn out builds for any new release of any release line because it's just watching for changes, which may not be a great idea because it's applying Node 12 build standards across the board and maybe there should be some differentiation. But the result is that you'll get unofficial-builds linux-x64-musl binaries for Node 8 and 10 too. They're using Alpine 3.9, so musl 1.1.20, if you're shipping older Alpine containers then it might be a problem.

rvagg · 2019-04-24T23:01:10Z

there's also nothing precluding doing anything different with unofficial-builds, it's essentially a blank slate looking for contributors to make it do what they need

SimenB · 2019-04-25T06:33:47Z

Well, the way unofficial-builds is currently set up is that it's just going to churn out builds for any new release of any release line because it's just watching for changes

Ah, didn't know that! Pretty sweet. Maybe we could try to ship both our current "build in container" in addition to those others and ask people to try them out?

They're using Alpine 3.9, so musl 1.1.20, if you're shipping older Alpine containers then it might be a problem.

We are just on 3.9 (we don't have the capacity to build for multiple), so that shouldn't be an issue.

rvagg · 2019-04-26T01:46:15Z

The only thing missing is the different architecture. The output in Alpine is different than on Debian.

What does this mean? Are you shipping both x86 and x64 versions of the Alpine container or some other architecture? Is there much of a call for x86 Docker containers? I've just added x64-musl to unofficial-builds because my assumption was that this is all people are using with Alpine. If that's not the case and there's popular use of other architectures then maybe we should expand the offering there? It's a fairly straightforward matter of adding a new recipe to the chain.

Also regarding availability timing on unofficial-builds, this all depends on how long it takes to compile everything. New releases are detected within 5 minutes and then start building. So if I look at the timestamp on index.json at https://nodejs.org/download/release/ which reflects the 12.0.0 release and compare it to the index.json at https://unofficial-builds.nodejs.org/download/release/ which reflects the availability of builds there; the former is 15:49 and the latter is 16:19. So it took exactly 30 minutes to compile the 3 binary types after release. That will extend if/when we add new build types to it, but it could also shorten if we switch to promoting builds as they are completed and prioritising certain ones, rather than waiting for all builds to complete. So this means that unless the hand-off from the Node releaser to docker-node and the merge of the PR to this repo happens quicker than 30 minutes, you should have binaries available to download.

This all assumes that the builds go without a hitch. Currently they are all dependent on each other so one failure will fail them all. That's not ideal and needs to change (contributions welcome!). We still have Alpine in the Node CI cluster but no x86 and no ARMv6 so it's more likely that one of the others will fail to build at some point in the future than musl.

LaurentGoderre · 2019-04-26T18:44:39Z

@rvagg we unfortunately can't get tag specific metrics from docker but we have had issues/questions about the alpine variant on other archs than x64. Here is the architecture matrix for version 11 we use: https://github.com/nodejs/docker-node/blob/master/11/architectures

rvagg · 2019-04-30T01:59:06Z

That's a lotta architectures. per-arch metrics would be fascinating. I bet there'd be no squeals if you trimmed that list a bit; it's a shame you can't know for sure.

LaurentGoderre · 2019-04-30T13:59:31Z

@rvagg I think it might be a reasonable compromise to use the unofficial build infrastructure to deliver the most common arch (x64 only or x64 and i386) and have the other builds occuring on the docker build infrastructure. It would allow us to deliver new release to the majority way faster while not affecting the other archs.

rvagg · 2019-05-01T11:23:58Z

I did some experimenting with trying to get musl x86 binaries on the 64-bit host but I can't see a way to do it without some serious hoop jumping. There's no combination of musl + gcc multilib support that I can find anywhere so it either requires a custom gcc compile or a 32-bit host. Without metrics that show that these are even used I might drop the idea for now (although others are welcome to take up the challenge!).

LaurentGoderre · 2019-05-01T12:45:45Z

@rvagg just having the x64 one is a huge help for us. We can always look at i386 later if need be.

chorrell · 2019-05-01T13:33:32Z

I agree. Just having x64 solves the immediate problem of the slow travis builds. That will speed up our part of the release process significantly.

LaurentGoderre · 2019-05-03T13:01:06Z

Is there any blockers left for this?

LaurentGoderre · 2019-10-24T14:07:01Z

Ok! removed the archive="" and rebased to add Node 13 and the new checksums.

LaurentGoderre · 2019-10-24T14:54:19Z

@PeterDaveHello I think we are good to go!!

10/alpine/Dockerfile

LaurentGoderre · 2019-10-25T15:12:49Z

@PeterDaveHello all done!

PeterDaveHello · 2019-10-25T15:31:51Z

the last minor issue: #1026 (comment)

LaurentGoderre · 2019-10-25T15:51:18Z

@PeterDaveHello in theory though, the whole if/else should be indented one level further because it doesn't align

PeterDaveHello

Yeah, that's right, but just want to keep the diff minimized in such huge change, never mind. I was going to merge this one but noticed it appears another commit and seems an accident that you want to remove it first?

LaurentGoderre · 2019-10-28T13:16:43Z

@PeterDaveHello done!

nodejs-github-bot · 2019-10-28T17:13:19Z

Created PR to the official-images repo (docker-library/official-images#6871)

rvagg · 2019-10-28T23:52:43Z

dang, you just ticked over the 6 month mark for this PR

congrats anyway!

misterpaul · 2019-10-30T15:04:55Z

This change appears to have rolled back the version of musl. Up until this change, we were pulling in musl-1.1.20-r5. As of two days ago node:10-alpine and node:12.13-alpine (those are all I've checked) are pulling musl-1.1.20-r4, which has the vulnerability CVE-2019-14697. (CVSS v2 score of 7.5 and CVSS v3 score of 9.8 - https://nvd.nist.gov/vuln/detail/CVE-2019-14697)

This is not too surprising, as the original pull request is from before musl-1.1.20-r5 was released.

But boy does it speed up the build! :) Unfortunately, it also breaks the build if you block builds that contain high or critical vulnerabilities.

Try to download pre-built alpine binary before building #1026

Try to download pre-built alpine binary before building #1026

Conversation

LaurentGoderre commented Apr 24, 2019

LaurentGoderre commented Apr 24, 2019

SimenB left a comment

Choose a reason for hiding this comment

LaurentGoderre commented Apr 24, 2019 • edited Loading

LaurentGoderre commented Apr 24, 2019

LaurentGoderre commented Apr 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvagg commented Apr 24, 2019

rvagg commented Apr 24, 2019

SimenB commented Apr 25, 2019

rvagg commented Apr 26, 2019

LaurentGoderre commented Apr 26, 2019

rvagg commented Apr 30, 2019

LaurentGoderre commented Apr 30, 2019

rvagg commented May 1, 2019

LaurentGoderre commented May 1, 2019

chorrell commented May 1, 2019

LaurentGoderre commented May 3, 2019

LaurentGoderre commented Oct 24, 2019

LaurentGoderre commented Oct 24, 2019

LaurentGoderre commented Oct 25, 2019

PeterDaveHello commented Oct 25, 2019

LaurentGoderre commented Oct 25, 2019

PeterDaveHello left a comment

Choose a reason for hiding this comment

LaurentGoderre commented Oct 28, 2019

nodejs-github-bot commented Oct 28, 2019

rvagg commented Oct 28, 2019

misterpaul commented Oct 30, 2019

LaurentGoderre commented Oct 30, 2019

PeterDaveHello commented Mar 29, 2024

LaurentGoderre commented Apr 24, 2019 •

edited

Loading