search

geerlingguy / ansible-role-repo-epel

Ansible Role - EPEL Repository for RHEL/CentOS
https://galaxy.ansible.com/geerlingguy/repo-epel/
MIT License
186 stars 148 forks source link

Use URL to retrieve the repo gpgkey #45

Closed cognifloyd closed 4 years ago

cognifloyd commented 4 years ago

Fixes #43

Also, this uses the fingerprint parameter which was introduced in ansible 2.9. So, a version comparison is needed to exclude that parameter if running on an older ansible version. The version test was renamed to version in ansible 2.5, so bump minimum ansible version as well.

triwats commented 4 years ago

Looks good to me @cognifloyd - the only feedback I might have is to move the when to the end of the block to ensure that it's uniform with the rest of the code.

Out of interested, do we know how regularly these fingerprints change? And is there any release lifecycle documentation around it since we're including them hardcoded in the module?

cognifloyd commented 4 years ago

K. I adjusted the location of when and added the retry safety since downloading the key could have intermittent network errors.

Also, changing the gpg key is afaik an unheard of thing to do. Every package in a given repo is signed with that key, so if the key were changed every rpm would have to be resigned. So, it would take something extreme to require changing that key. iow: It should be safe to hard-code the fingerprints.

geerlingguy commented 4 years ago

@cognifloyd - As someone who has seen keys change with some frequency (at least once every 5-10 years) causing a fun amount of havoc, I'd rather not hardcode the fingerprint into my role—it also requires at least 2.9 for that feature, which is fine, but I'd rather not add the complexity and would rather just improve things using the 2.5+ method.

geerlingguy commented 4 years ago

I merged this PR but reverted the fingerprinting (at least for now), as this fixes a bug that occurs on RHEL (which I can't test in CI like I can with CentOS). I might consider the fingerprinting, but that would need to be a separate feature request/reasoning added in a separate issue or PR.