nodejs / unofficial-builds

Unofficial binaries for Node.js
https://unofficial-builds.nodejs.org
221 stars 46 forks source link

Don't fetch gpg keys in fetch-source Dockerfile #109

Closed alex closed 5 months ago

alex commented 5 months ago

fetch-source's run.sh already fetches the gpg keys, and doing it in both places can lead to hangs due to permissions

rvagg commented 5 months ago

Have you checked this with the latest version on main? I merged a change yesterday that moved USER node above this line, so in fetch-source it now writes them as ~node so it should be pre-caching them for each run. The more time we can save doing normal builds the better and this is one thing that doesn't need to be done repeatedly.

alex commented 5 months ago

Yes, this is atop the latest main.

I assume it's in run.sh in addition to Dockerfile so that if it's gone out of date between build and run, it'll get fixed?

richardlau commented 5 months ago

fetch-source's run.sh already fetches the gpg keys, and doing it in both places can lead to hangs due to permissions

I'm not sure it is permissions -- I can reproduce the hang and it looks like there's a lock file:

$ git status
On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean
$ git rev-parse main
ee6ebd9ef3df343cc84984223b294d1a084f968c
$ docker build recipes/fetch-source -t unofficial-build-recipe-fetch-source --build-arg UID=$(id -u) --build-arg GID=$(id -g) --no-cache
[+] Building 8.7s (10/10) FINISHED                                                                                                                                                                docker:default
 => [internal] load .dockerignore                                                                                                                                                                           0.0s
 => => transferring context: 2B                                                                                                                                                                             0.0s
 => [internal] load build definition from Dockerfile                                                                                                                                                        0.0s
 => => transferring dockerfile: 644B                                                                                                                                                                        0.0s
 => [internal] load metadata for docker.io/library/alpine:latest                                                                                                                                            0.1s
 => CACHED [1/5] FROM docker.io/library/alpine:latest@sha256:51b67269f354137895d43f3b3d810bfacd3945438e94dc5ac55fdac340352f48                                                                               0.0s
 => [internal] load build context                                                                                                                                                                           0.0s
 => => transferring context: 87B                                                                                                                                                                            0.0s
 => [2/5] RUN addgroup -g 1004 node     && adduser -u 1004 -G node -s /bin/sh -D node                                                                                                                       0.2s
 => [3/5] RUN apk add --no-cache bash gnupg curl                                                                                                                                                            0.8s
 => [4/5] COPY --chown=node:node run.sh /home/node/run.sh                                                                                                                                                   0.0s
 => [5/5] RUN for key in $(curl -sL https://raw.githubusercontent.com/nodejs/docker-node/HEAD/keys/node.keys)   ; do       gpg --batch --keyserver hkps://keys.openpgp.org --recv-keys "$key" ||       gpg  7.3s
 => exporting to image                                                                                                                                                                                      0.2s
 => => exporting layers                                                                                                                                                                                     0.1s
 => => writing image sha256:103cc497719851138e8e6ce840fc27e0a868d24ab7ceea328401d13b170ea215                                                                                                                0.0s
 => => naming to docker.io/library/unofficial-build-recipe-fetch-source                                                                                                                                     0.0s
$ docker run --rm -it --entrypoint "" unofficial-build-recipe-fetch-source ls -al /home/node/.gnupg/public-keys.d/
total 124
drwxr-x---    2 node     node            96 Jan 22 13:45 .
-rw-r--r--    2 node     node            27 Jan 22 13:44 .#lk0x00007fa76375bf90.buildkitsandbox.13
drwx--S---    5 node     node          4096 Jan 22 13:44 ..
-rw-r--r--    1 node     node        114688 Jan 22 13:45 pubring.db
-rw-r--r--    2 node     node            27 Jan 22 13:44 pubring.db.lock
$

Removing the lock file allows /home/node/run.sh to run. It's not clear to me why the lock file is being left behind after the docker build.

alex commented 5 months ago

Huh. I wonder why gpg is leaving the lockfile lying around...

alex commented 5 months ago

Would it be preferrable for me to change this PR to remove teh lockfile?

rvagg commented 5 months ago

Two possible ways forward, gleaned from https://unix.stackexchange.com/questions/101400/gpg-uses-temporary-files-instead-of-pipe

  1. Run with --lock-never
  2. Try mkdir ~/.gnupg before using gpg

I think either would be fine if it fixes the problem.

richardlau commented 5 months ago
  1. Try mkdir ~/.gnupg before using gpg

This might be the way to go. It looks like more recent versions of gpg will change behaviour depending on whether or not that directory already exists:

https://github.com/gpg/gnupg/blob/dfa60c09f5cd992515df5fdb275dbee7f8f23b71/README#L130C1-L141

Since version 2.3.0 it is possible to store the keys in an SQLite database instead of the keyring.kbx file. This is in particular useful for large keyrings or if many instances of gpg and gpgsm may run concurrently. This is implemented using another daemon process, the "keyboxd". To enable the use of the keyboxd put the option "use-keyboxd" into the configuration file ~/.gnupg/common.conf or the global /etc/gnupg/common.conf. See also doc/examples/common.conf. Only public keys and X.509 certificates are managed by the keyboxd; private keys are still stored as separate files.

Since version 2.4.1 the keyboxd will be used by default for a fresh install; i.e. if a ~/.gnupg directory did not yet exist.

In our current fetch-source Dockerfile, creating ~/.gnupg first results in ~/.gnupg/public-keys.d/ not being created, whereas without creating ~/.gnupg first it is (with a corresponding lock file -- perhaps because the keyboxd daemon process is still running when the docker build completes?).