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

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

Closed
wants to merge 1 commit into from

Conversation

alex
Copy link

@alex alex commented Jan 21, 2024

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

fetch-source's run.sh already fetches the gpg keys, and doing it in both places can lead to hangs due to permissions
@alex alex mentioned this pull request Jan 21, 2024
@rvagg
Copy link
Member

rvagg commented Jan 22, 2024

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
Copy link
Author

alex commented Jan 22, 2024

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
Copy link
Member

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
Copy link
Author

alex commented Jan 22, 2024

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

@alex
Copy link
Author

alex commented Jan 27, 2024

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

@rvagg
Copy link
Member

rvagg commented Jan 29, 2024

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
Copy link
Member

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

rvagg added a commit that referenced this pull request Jan 29, 2024
addresses a problem where gnupg creates a lockfile in the directory which
causes problems when trying to invoke it later on during a run of fetch-source

Ref: #109
Closes: #109
rvagg added a commit that referenced this pull request Jan 29, 2024
addresses a problem where gnupg creates a lockfile in the directory which
causes problems when trying to invoke it later on during a run of fetch-source

Ref: #109
Ref: #110
Closes: #109
@rvagg rvagg closed this in #111 Jan 29, 2024
rvagg added a commit that referenced this pull request Jan 29, 2024
addresses a problem where gnupg creates a lockfile in the directory which
causes problems when trying to invoke it later on during a run of fetch-source

Ref: #109
Ref: #110
Closes: #109
@alex alex deleted the patch-1 branch January 29, 2024 23:39
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.

3 participants