Skip to content

Conversation

@dvdksn
Copy link
Contributor

@dvdksn dvdksn commented Jul 4, 2023

Signed-off-by: David Karlsson [email protected]

Proposed changes

  • Changes the GPG keyrings location from /etc/apt/keyrings to /usr/share/keyrings
  • Removes the install command since /usr/share/keyrings already exists by default

Related issues (optional)

Closes #17471

@netlify
Copy link

netlify bot commented Jul 4, 2023

Deploy Preview for docsdocker ready!

Name Link
🔨 Latest commit 8a72f98
🔍 Latest deploy log https://app.netlify.com/sites/docsdocker/deploys/64a41d7e7f1fe00008dc692c
😎 Deploy Preview https://deploy-preview-17686--docsdocker.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dvdksn dvdksn requested a review from thaJeztah July 4, 2023 13:25
@thaJeztah
Copy link
Member

/cc @tianon PTAL

The location was updated to /etc/apt/keyrings in #14813, which mentions that that location was based on recommendations of the apt maintainers;

See https://tracker.debian.org/news/1305679/accepted-apt-240-source-into-unstable/:

* Install an empty /etc/apt/keyrings directory.
  This directory is intended to provide an alternative to
  /usr/share/keyrings for placing keys used with signed-by.

See also https://wiki.debian.org/DebianRepository/UseThirdParty?action=diff&rev2=47&rev1=46 (which was edited following a discussion with the APT maintainers about the expected usage):

If future updates to the key will be managed by an apt/dpkg package as recommended below, then it SHOULD be downloaded into /usr/share/keyrings using the same filename that will be provided by the package. If it will be managed locally , it SHOULD be downloaded into /etc/apt/keyrings instead.

FWIW; ISTR changing these instructions may potentially cause issues (package was signed by ...)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaving a "request changes" review, as I'd like to have more eyes on this change before we merge.

@dvdksn dvdksn requested a review from tianon July 4, 2023 13:43
@tianon
Copy link
Contributor

tianon commented Jul 5, 2023

Yeah, the current documented path (/etc/apt/keyrings) is much more appropriate for user-managed keyrings like this one than the one suggested in the linked issue -- in a healthy deb-based system, files in /usr/share should typically only be put there by installed packages, which is why we should only use that path if we end up creating something like a docker-inc-keyring package that installs a keyring file there (and then we should use the exact same filename so that installing the package overwrites the manually created file).

For distribution versions which contain APT version 2.3.10 or newer, we could switch to the newer "deb822" format for configuring these sources, in which we can embed the key directly (see https://lists.debian.org/debian-devel/2021/11/msg00026.html), but from https://packages.debian.org/apt and https://packages.ubuntu.com/apt, that limits us to Debian bookworm+ (the latest stable just recently released) and Ubuntu jammy+ (which is 22.04, so slightly less new, but still pretty recent).

See also the following from the APT 2.4.0 changelog (https://tracker.debian.org/news/1305679/accepted-apt-240-source-into-unstable/):

  * Install an empty /etc/apt/keyrings directory.
    This directory is intended to provide an alternative to
    /usr/share/keyrings for placing keys used with signed-by.

@dvdksn
Copy link
Contributor Author

dvdksn commented Jul 6, 2023

Thank you so much for the detailed explanation @tianon. I'll close this PR and related issue.

Didn't know about the new APT features, let's see when we can incorporate those into our docs (probably when jammy/bookworm is our oldest supported version?)

@dvdksn dvdksn closed this Jul 6, 2023
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.

Change path for Ubuntu docker GPG key

3 participants