puppetlabs / puppetlabs-apt

Puppet module to help manage Apt
https://forge.puppetlabs.com/puppetlabs/apt
Apache License 2.0
215 stars 462 forks source link

apt::keyring::dir's default as class variable #1160

Open pschichtel opened 9 months ago

pschichtel commented 9 months ago

Use Case

In order to reuse the same keyring across several apt::sources I have to separately create an apt::keyring resource and then use apt::source's keyring parameter. However the latter expects an absolute path to a keyring file. apt::keyring's dir parameter is optional and I'd argue usually there is no need to explicitly pass a value to it. The parameters value, currently /etc/apt/keyrings, is currently hardcoded, which requires me to duplicate it when generating the absolute path of the keyring file for apt::source::keyring.

Describe the Solution You Would Like

My suggestion would be to move the default value to apt::params and reference it from there. that way users can construct the full path without duplicating the current value, which might change in the future. The path is currently also duplicated within apt::source, so the might make sense for other reasons, too.

Describe Alternatives You've Considered

A non-exclusive alternative would be to have a way to reference an existing apt::keyring, but I'm not sure how that would look like.

pschichtel commented 9 months ago

I can open a PR if my suggestion is acceptable

kenyon commented 9 months ago

The params.pp design pattern is obsolete so we shouldn't be adding anything to that. Also, apt::keyring doesn't include apt, so you wouldn't be able to use anything from apt::params.

I don't use apt::source::keyring. What I do is just use different name values with apt::source::keys, even if the key source is the same. That way you don't need to repeat the path to /etc/apt/keyrings. Maybe using apt::source::key would work better for you too.

pschichtel commented 9 months ago

Also, apt::keyring doesn't include apt, so you wouldn't be able to use anything from apt::params.

is there a reason why that can't be changed? Is apt::keyring supposed to be usable without managing apt in general?

What I do is just use different name values with apt::source::keys, even if the key source is the same.

I considered that, but I don't really like it

kenyon commented 9 months ago

I recently updated my puppet code to use keyrings: https://github.com/kenyon/puppet/commit/d7c64aef0be8ea53e2e6a6f863c84f51eec1f1dc?diff=split&w=1

Can you show your code so we can understand how your suggestion would be useful?

pschichtel commented 9 months ago

this is my simple class to manage gitlab's apt repos:

class gitlab::repo (
    String $release            = $facts['os']['distro']['codename'],
    Boolean $community_edition = true,
    Boolean $runner            = false,
) {
    $source_name = 'gitlab'

    $keyring_name = 'gitlab.gpg'
    apt::keyring { $keyring_name:
        source => 'puppet:///modules/gitlab/gitlab.gpg',
    }
    $keyring_path = "/etc/apt/keyrings/${keyring_name}"

    if $community_edition {
        $type = 'gitlab-ce'
    } else {
        $type = 'gitlab-ee'
    }

    apt::source { $source_name:
        location => "https://packages.gitlab.com/gitlab/${type}/debian/",
        release  => $release,
        repos    => 'main',
        keyring  => $keyring_path,
        include  => {
            deb => true,
            src => false,
        },
        require  => Apt::Keyring[$keyring_name],
    }

    if $runner {
        $runner_source_name = "${source_name}-runner"

        apt::source { $runner_source_name:
            location => 'https://packages.gitlab.com/runner/gitlab-runner/debian/',
            release  => $release,
            repos    => 'main',
            keyring  => $keyring_path,
            include  => {
                deb => true,
                src => false,
            },
            require  => [Apt::Source[$source_name], Apt::Keyring[$keyring_name]],
        }
    }
}