puppetlabs / puppetlabs-docker

The Puppet Docker repository
Apache License 2.0
91 stars 311 forks source link

Use modern APT keyrings on Debian family #965

Open kenyon opened 10 months ago

kenyon commented 10 months ago

This makes use of https://github.com/puppetlabs/puppetlabs-apt/pull/1128 to store the public key in /etc/apt/keyrings and add a signed-by option to the sources.list.d entry.

This replaces #885 by using puppetlabs-apt rather than implementing keyring handling here in the docker module.

Fixes #884.

kenyon commented 10 months ago

Acceptance tests are failing in setup of the machines under test, not related to this PR.

saz commented 8 months ago

@kenyon What about adding the key to the module, just as it has been suggested in https://github.com/puppetlabs/puppetlabs-postgresql/pull/1563#pullrequestreview-1827072156 for the same change?

kenyon commented 8 months ago

@saz yes, that could be done. It means potentially more maintenance work for this module when the key needs to be updated. I'll leave it up to this module's @puppetlabs maintainers whether to do this.

saz commented 8 months ago

Looking at this module again, it's possible to set a custom URL for the key source. As puppet:///... will be a valid URL, it's easy to use a custom key.

bastelfreak commented 6 months ago

I think we should do a minor release before we merge this: https://github.com/puppetlabs/puppetlabs-docker/pull/978/files (and there are some other non-breaking changes that should be merged first)

psaintemarie commented 4 months ago

Is there any update on when that PR will be merged?

gdlx commented 1 month ago

@kenyon I just tried your fork and it works fine but I get the following warnings that I didn't have before:

Warning: Private key for 'my_server.fqdn.com' does not exist
Warning: Client certificate for 'my_server.fqdn.com' does not exist

Is it expected ? Theres's not much info even in debug so I don't know what kind of key/certificate I should set and where.

I've seen the apt mod requirement has changed but I was already using 9.4.0 so it shouldn't be related.

kenyon commented 1 month ago

@kenyon I just tried your fork and it works fine but I get the following warnings that I didn't have before:

Warning: Private key for 'my_server.fqdn.com' does not exist
Warning: Client certificate for 'my_server.fqdn.com' does not exist

Is it expected ? Theres's not much info even in debug so I don't know what kind of key/certificate I should set and where.

I've seen the apt mod requirement has changed but I was already using 9.4.0 so it shouldn't be related.

Those warnings sound unrelated to the changes in this pull request. I would try running puppet agent with --debug to see if that gives more context on where those warnings are coming from.

gdlx commented 1 month ago

Those warnings sound unrelated to the changes in this pull request. I would try running puppet agent with --debug to see if that gives more context on where those warnings are coming from.

@kenyon As I said, I already ran in debug mode but it didn't give much information except helping me to find it was in the middle of docker related stuff.

I've just disabled all my puppet code except this:

  class { 'docker':
  }

And I still get this in debug mode:

Debug: /Stage[main]/Docker::Repos/before: before to Class[Docker::Install]
Debug: /Stage[main]/Docker::Repos/Apt::Source[docker]/before: before to Package[docker]
Debug: /Stage[main]/Apt::Update/Exec[apt_update]/before: before to Package[cgroupfs-mount]
Debug: /Stage[main]/Apt/File[sources.list]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Apt/File[sources.list.d]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Apt/File[preferences]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Apt/File[preferences.d]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Apt/File[apt.conf.d]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Apt/File[/etc/apt/auth.conf]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Docker::Install/before: before to Class[Docker::Config]
Debug: /Stage[main]/Docker::Config/before: before to Class[Docker::Service]
Debug: /Stage[main]/Docker::Service/File[/etc/systemd/system/docker.service.d/service-overrides.conf]/before: before to Service[docker]
Debug: /Stage[main]/Docker::Service/File[/etc/systemd/system/docker.service.d/service-overrides.conf]/notify: notify to Exec[docker-systemd-reload-before-service]
Debug: /Stage[main]/Docker::Service/Exec[docker-systemd-reload-before-service]/notify: notify to Service[docker]
Debug: /Stage[main]/Docker::Service/File[/etc/default/docker-storage]/notify: notify to Service[docker]
Debug: /Stage[main]/Docker::Service/File[/etc/default/docker]/notify: notify to Service[docker]
Debug: /Stage[main]/Docker::Repos/Apt::Source[docker]/Apt::Keyring[docker.asc]/before: before to Apt::Setting[list-docker]
Debug: /Stage[main]/Apt/Apt::Setting[conf-update-stamp]/File[/etc/apt/apt.conf.d/15update-stamp]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Docker::Repos/Apt::Source[docker]/Apt::Setting[list-docker]/File[/etc/apt/sources.list.d/docker.list]/notify: notify to Class[Apt::Update]
Debug: /Stage[main]/Docker::Service/File[/etc/systemd/system/docker.service.d/service-overrides.conf]: Adding autorequire relationship with File[/etc/systemd/system/docker.service.d]
Debug: /Stage[main]/Apt/Apt::Setting[conf-update-stamp]/File[/etc/apt/apt.conf.d/15update-stamp]: Adding autorequire relationship with File[apt.conf.d]
Debug: /Stage[main]/Docker::Repos/Apt::Source[docker]/Apt::Keyring[docker.asc]/File[/etc/apt/keyrings/docker.asc]: Adding autorequire relationship with File[/etc/apt/keyrings]
Debug: /Stage[main]/Docker::Repos/Apt::Source[docker]/Apt::Setting[list-docker]/File[/etc/apt/sources.list.d/docker.list]: Adding autorequire relationship with File[sources.list.d]
Debug: /Stage[main]/Docker::Repos/Apt::Pin[docker]/Apt::Setting[pref-docker]/File[/etc/apt/preferences.d/docker.pref]: Adding autorequire relationship with File[preferences.d]
Debug: /Stage[main]/Docker::Systemd_reload/Exec[docker-systemd-reload]: 'systemctl daemon-reload' won't be executed because of failed check 'refreshonly'
Debug: /Stage[main]/Apt/File[/etc/apt/auth.conf]: Nothing to manage: no ensure and the resource doesn't exist
Debug: Prefetching apt resources for package
Debug: Executing '/usr/bin/dpkg-query -W --showformat '${Status} ${Package} ${Version}\n''
Debug: Executing: '/usr/bin/apt-mark showmanual'
Warning: Private key for 'my_server.fqdn.com' does not exist
Warning: Client certificate for 'my_server.fqdn.com' does not exist
Debug: Creating new connection for https://download.docker.com:443
Debug: Starting connection for https://download.docker.com:443
Debug: Using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256
Debug: HTTP HEAD https://download.docker.com/linux/debian/gpg returned 200 OK
Debug: Caching connection for https://download.docker.com:443
Debug: /Stage[main]/Apt::Update/Exec[apt_update]: '/usr/bin/apt-get update' won't be executed because of failed check 'refreshonly'
Debug: /Stage[main]/Docker::Service/File[/etc/systemd/system/docker.service.d/service-overrides.conf]/seltype: SELinux bindings not found. Ignoring parameter.
Debug: /Stage[main]/Docker::Service/Exec[docker-systemd-reload-before-service]: 'systemctl daemon-reload > /dev/null' won't be executed because of failed check 'refreshonly'
Debug: Executing: '/usr/bin/systemctl is-active -- docker'
Debug: Executing: '/usr/bin/systemctl is-enabled -- docker'
Debug: Finishing transaction 17800
Debug: Storing state
Debug: Pruned old state cache entries in 0.00 seconds
Debug: Stored state in 0.01 seconds
Notice: Applied catalog in 0.41 seconds
Debug: Applying settings catalog for sections reporting, metrics
Debug: Finishing transaction 43380
Debug: Received report to process from my_server.fqdn.com
Debug: Closing connection for https://download.docker.com:443
kenyon commented 1 month ago

@gdlx strange. I'm using this PR in my puppet code, and those warnings don't appear. I'm pretty sure they have to be coming from some other puppet module.

gdlx commented 1 month ago

@gdlx strange. I'm using this PR in my puppet code, and those warnings don't appear. I'm pretty sure they have to be coming from some other puppet module.

Maybe it depends on the OS ? I'm running Debian 12.

tdb commented 1 month ago

They're coming out of Puppet's code, rather than this module:

https://github.com/puppetlabs/puppet/blob/c8684e60453cbea539459e4721364957fa4dfc3c/lib/puppet/ssl/ssl_provider.rb#L100

gdlx commented 1 month ago

They're coming out of Puppet's code, rather than this module:

https://github.com/puppetlabs/puppet/blob/c8684e60453cbea539459e4721364957fa4dfc3c/lib/puppet/ssl/ssl_provider.rb#L100

Sure, yet reverting from the fork back to puppetlabs-docker 10.0.1 release removes the warnings (but restores the one in apt update).

It may be relevant to specify I'm using Puppet 8.9.0 in puppet-apply mode.

gdlx commented 1 month ago

I fixed the issue by generating requested key and certificate:

openssl genrsa -out /etc/puppetlabs/puppet/ssl/private_keys/myserver.domain.com.pem 4096
openssl req -new -x509 -key /etc/puppetlabs/puppet/ssl/private_keys/myserver.domain.com.pem -out /etc/puppetlabs/puppet/ssl/certs/myserver.domain.com.pem -days 3650 -subj "/CN=myserver.domain.com"
cp /etc/puppetlabs/puppet/ssl/certs/myserver.domain.com.pem /usr/local/share/ca-certificates/myserver.domain.com.crt
update-ca-certificates

It's probably related to the way puppetlabs-apt downloads the GPG key when using a name instead of an ID.

I haven't found a way to avoid creating this cert I don't actually need.

kenyon commented 1 month ago

@gdlx I think most puppet users are using puppet agent, so that key and certificate must already exist, which means we won't get those warnings. I don't know why this particular change would trigger those warnings though. Looks like it has to do with making a TLS connection to download the GPG key. Maybe you could come up with the minimal puppet code needed to cause the warnings, and then raise an issue in https://github.com/puppetlabs/puppet.

gdlx commented 1 month ago

The doc says the cert should be provided by the puppet-agent package: https://github.com/puppetlabs/puppet/blob/6923b0edccc61ebc23cdd2c4c430cfe6d2b6d8dc/lib/puppet/ssl/ssl_provider.rb#L41-L42

I don't know why it's not my case. I'll investigate about that, but this change in the docker module obviously only reveals this problem.

Thanks !

kenyon commented 1 month ago

@gdlx I suspect the cert is missing because it only gets generated if you run puppet in agent mode. Since you use apply mode, it never gets created.