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

Fix apt-key deprecated message #1610

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

miluxhd
Copy link

@miluxhd miluxhd commented Aug 23, 2024

Pull Request (PR) description

In the current implementation, apt::source stores GPG keys in the trusted.gpg file, which is no longer recommended. Additionally, with the deprecation of apt-key, running the apt update command triggers warnings on Debian-based distributions.
This PR modifies the handling of GPG keys so that they are stored in the keyrings directory, aligning with best practices and avoiding the deprecation warnings associated with apt-key

@miluxhd miluxhd force-pushed the fix/depricated-apt-key branch 9 times, most recently from c326f08 to e1fa81e Compare August 23, 2024 13:18
@kenyon kenyon changed the title Fix apt-key depricated message Fix apt-key deprecated message Aug 23, 2024
Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

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

Can you also update the README to say that at least puppetlabs-apt v9.2.0 is needed? That's the version which added support for "modern keyrings". Here in the readme:

puppet-nginx/README.md

Lines 19 to 21 in 12d6b62

* apt is now a soft dependency. If your system uses apt, you'll need to
configure an appropriate version of the apt module. Version 4.4.0 or higher is
recommended because of the proper handling of `apt-transport-https`.

https://github.com/puppetlabs/puppetlabs-apt/blob/main/CHANGELOG.md#v920---2023-12-04

@miluxhd miluxhd force-pushed the fix/depricated-apt-key branch 2 times, most recently from 11b0886 to 846f12a Compare August 26, 2024 11:28
@miluxhd miluxhd requested a review from kenyon August 26, 2024 12:31
@TheMeier
Copy link
Contributor

given the "new" version for the dependency, should this be marked backward-incompatible?

@kenyon
Copy link
Member

kenyon commented Aug 26, 2024

given the "new" version for the dependency, should this be marked backward-incompatible?

I think yes, because users of this module who use it with puppetlabs-apt would need to update their puppetlabs-apt module. Same as how we've done for this kind of change in other modules.

manifests/package/debian.pp Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@miluxhd miluxhd force-pushed the fix/depricated-apt-key branch 3 times, most recently from c3b21b7 to 7037314 Compare August 27, 2024 07:08
@miluxhd miluxhd requested a review from kenyon August 27, 2024 07:28
Copy link
Member

@kenyon kenyon left a comment

Choose a reason for hiding this comment

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

Oops, missed one other thing. The filenames need to end in .asc.

I think this also indicates that we are lacking acceptance test coverage.

manifests/package/debian.pp Outdated Show resolved Hide resolved
manifests/package/debian.pp Outdated Show resolved Hide resolved
manifests/package/debian.pp Outdated Show resolved Hide resolved
spec/classes/nginx_spec.rb Outdated Show resolved Hide resolved
spec/classes/nginx_spec.rb Outdated Show resolved Hide resolved
@kenyon kenyon reopened this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants