(MODULES-9695) Debian: use modern APT keyring format

[kenyon]

This updates puppet_agent::osfamily::debian to use modern APT keyrings (https://github.com/puppetlabs/puppetlabs-apt/pull/1128) instead of the deprecated apt-key method used by apt::key and apt::source.key without name.

For this to work properly, it needs a release of puppetlabs-apt which contains https://github.com/puppetlabs/puppetlabs-apt/pull/1128, which should be the release after v9.1.0.

This also removes the legacy key, because keys not used for signing package sources aren't needed.

/etc/pki is not needed anymore (also this directory is a RedHatism) because keyrings are now stored in the default location of /etc/apt/keyrings. We don't clean it up though, in case people are using the files there for something else.

[kenyon]

The commit message check wants only a PA jira issue ID, but the issue for this is in the MODULES jira project. Caused by #642.

[mhashizume]

Thanks for this @kenyon , could you rebase to pick up the changes from https://github.com/puppetlabs/puppetlabs-puppet_agent/pull/682 ?

[kenyon]

@mhashizume already did that. That's what the last force push was. Commit e48620a6c61c5c4c753d817eb8004a753c4bb887 is on top of current main.

[kenyon]

FWIW I'm using this plus puppetlabs-apt at https://github.com/puppetlabs/puppetlabs-apt/commit/cb21d0b93f3b44130adef034b58ef3f2193640e3 on my infrastructure, and it does the right thing on Ubuntu 18.04, 20.04, and 22.04.

[kenyon]

Modern APT keyring format also needs to be used in puppet_enterprise::repo::config, which doesn't use puppetlabs-apt. Bug report: https://github.com/puppetlabs/puppet-enterprise_issues/issues/2

[kenyon]

I'm not seeing why the Debian tests would be running on Windows agents, and why my changes would cause these test failures.

[janit42]

as we have nightly puppet-agent packages for Debian 12 now it would be great to have this merged if possible

[joshcooper]

@kenyon can you resolve the test failures so we can get this merged?

[kenyon]

@kenyon can you resolve the test failures so we can get this merged?

I'd like to, but like I mentioned in https://github.com/puppetlabs/puppetlabs-puppet_agent/pull/681#issuecomment-1852750322, I investigated and couldn't figure out why these changes have anything to do with Windows tests. So I'll need a little assistance.

[kenyon]

My guess is that something more fundamental is wrong with the tests, like there are tests executing on platforms that they shouldn't be executing on.

[linuxdaemon]

My guess is that something more fundamental is wrong with the tests, like there are tests executing on platforms that they shouldn't be executing on.

Looking at the log, the current failures look to be dependency related. It is happening in main as well, so it seems to be an issue with the Gemfile.

[mhashizume]

Thanks for all of your work on this @kenyon , would it make sense to bump the puppetlabs-apt requirement in metadata.json since this is only supported in >= 9.2.0?

[kenyon]

Thanks for all of your work on this @kenyon , would it make sense to bump the puppetlabs-apt requirement in metadata.json since this is only supported in >= 9.2.0?

@mhashizume good catch (I started this PR before puppetlabs-apt 9.2.0 was released 😄). Updated metadata.json.

[mhashizume]

We'll also want to update puppetlab-apt in tests here: https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/3ce04a2b0504b00bbd8f753991f3820887fa426a/acceptance/helpers.rb#L182

Edit: And here https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/3ce04a2b0504b00bbd8f753991f3820887fa426a/spec/spec_helper_acceptance.rb#L119

puppetlabs-apt is also pinned to an older version in our various dockerfiles.

[kenyon]

@mhashizume I updated the versions in those helpers. The Dockerfiles don't specify versions (there are some branch specifications, but those lines are commented out).

[mhashizume]

@kenyon A few more things:

• After bumping puppetlabs-apt in metadata.json and helpers, we should also bump its dependency, stdlib (to >= 9.0.0). Because inifile depends on stdlib, we should also bump it to >= 6.0.0 in metadata.json and helpers.
• With your PR and the above changes, I ran into errors with our upgrade tests from Puppet 6 to Puppet 7. I believe the issue is that if there are multiple Puppet repos (like for different major version) and one of them does not include the signed-by option, apt errors:

E: Conflicting values set for option Signed-By regarding source http://nightlies.puppet.com/apt/ buster: /etc/apt/keyrings/GPG-KEY-puppet-20250406.asc != 

I'm not sure what the best solution is for the second issue, if we need to update anything on our end or if we just need to update documentation to let users know to update their repo configuration to point to the key.

Thoughts?

[kenyon]

@mhashizume I updated the metadata. Without looking at it myself, I'm not sure either what to do about that upgrade issue. I still have one PE 2019 instance that I need to upgrade, but the puppet_agent and apt modules would remain on Puppet 6-compatible versions until after the upgrade to Puppet 7. So I don't think it would be a problem. Which brings up this line:

https://github.com/puppetlabs/puppetlabs-puppet_agent/blob/3da119fde89d55ce1e35349cbcb1050fd4f5f932/metadata.json#L75-L78

IMO this should be bumped to require Puppet 7 since the apt, stdlib, and inifile dependencies require it. Should I make that change too?

[kenyon]

@mhashizume I added a commit that bumps the Puppet version requirement to 7. I think this is the only safe way to go if the dependent modules require Puppet 7. Otherwise there is no guarantee that the Ruby interpreter used in Puppet 5 or 6 would work with the Ruby code in these newer modules, for example.

[mhashizume]

The bump makes sense to me. I found a few more things that needed to be updated:

• inifile needs to be a minimum of 6.1.0 to work with newer stdlib
• a hacky change to the tests so there aren't conflicts that make apt sad

You can see what I did in addition to what you have here in this commit: https://github.com/puppetlabs/puppetlabs-puppet_agent/commit/627d06c966843c658f7b796ffd6a4d1b12d87ea1

I think after you incorporate those changes, this PR should be good to go.

[kenyon]

@mhashizume done.

Not sure where the proper fix would go for that acceptance helper, maybe around here: https://github.com/puppetlabs/beaker-puppet/blob/1146cb9783f74812421ebd52dc556f68710c57fe/lib/beaker-puppet/install_utils/foss_utils.rb#L1091

[mhashizume]

Thanks again @kenyon ! Really appreciate all your work.

I'm not sure what the upstream fix is for the issue we had to work around in the acceptance helper. The fundamental issue is that we had two sources.list with the same URL, but only one had the signed-by option enabled. The sources.list without the signed-by option is our release package installed by beaker-puppet. Our release packages don't use apt-key (AFAIK) and doesn't otherwise cause any warnings or errors with apt, so I don't think we need to change anything with beaker-puppet or our release packages.

Maybe we should document this circumstance somewhere, but I'm not sure where makes the most sense (and I'm not sure how often users will run into it).

[kenyon]

Well I've run into a problem with this. I set up the puppet-tools apt source, which uses the same keyring as the pc_repo apt source that puppetlabs-puppet_agent sets up, here: https://github.com/kenyon/puppet/blob/3196bea6a8e8f54acf814c5cb5a0fb65e01cb6db/data/os/family/Debian.yaml#L107-L116

apt complains about conflicting values set for signed-by if there are different values for the same location:

E: Conflicting values set for option Signed-By regarding source https://apt.puppet.com/ bullseye: /etc/apt/keyrings/GPG-KEY-puppet-20250406.asc != /etc/apt/keyrings/puppet.gpg
E: Couldn't read list of package sources

Because of the way apt::keyring is implemented, I don't think there is a way to use the same keyring for multiple sources without causing duplicate resource catalog compilation errors. I was able to work around this case by using apt.puppetlabs.com for puppet-tools instead of apt.puppet.com, but I think the real solution would be to add $name to the file resource title to allow for making the file resource unique, here: https://github.com/puppetlabs/puppetlabs-apt/blob/cd24e6dc8e654a38aef34b7138f40c3cd4a619ec/manifests/keyring.pp#L50

[kenyon]

Nevermind, that wouldn't work, of course you can't manage the same file path multiple times with puppet even if they have different resource titles. So I'm not sure what the best solution is for this kind of situation.

[kenyon]

Maybe puppetlabs-apt's apt::source type needs a way to specify the signed-by value without actually managing any file resources.

[kenyon]

Maybe puppetlabs-apt's apt::source type needs a way to specify the signed-by value without actually managing any file resources.

Sigh, nevermind, I forgot that this already exists, the keyring parameter of apt::source. Here it is in use: https://github.com/kenyon/puppet/blob/8787920f3480365f5972072262a24ef973c8290e/data/os/family/Debian.yaml#L107-L114