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

(SIMP-8582) Add params to override default gpgkey #242

Merged
merged 3 commits into from Oct 21, 2020

Conversation

op-ct
Copy link
Member

@op-ct op-ct commented Oct 15, 2020

This patch introduces the new parameters baseurl and gpgkey to
simp::yum::repo::local_simp and simp::yum::repo::local_os_updates.

These parameters provide a way to completely override ther respective
yumrepo settings, which makes it possible to host the packages' GPG key
in a non-repo location, as CentOS 8 now requires.

SIMP-8582 #close
SIMP-8470 #close

This patch introduces the new parameters `baseurl` and `gpgkey` to
`simp::yum::repo::local_simp` and `simp::yum::repo::local_os_updates`.

These parameters provide a way to completely override ther respective
yumrepo settings, which makes it possible to host the packages' GPG key
in a non-repo location, as CentOS 8 now requires.

[SIMP-8582] #close
[SIMP-8470] #close
#
# @param gpgkey
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the documentation is confusing. Without delving into the code to see that gpgkeys are set for you, I do not see why is this parameter is needed when extra_gpgkey_urls could be used. Are intending to deprecate extra_gpgkey_urls? Also, why isn't gpgkey an array? It could be an array of Simplib::URI types.

Copy link
Member

Choose a reason for hiding this comment

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

@lnemsick-simp Would you prefer something like disable_defaults or the like? That would make extra_gpgkey_urls the only thing that works. Looking back at this, it's sort of confusing across the board but we needed some way to make things "just work".

Copy link
Contributor

Choose a reason for hiding this comment

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

@trevor-vaughan How about default_gpgkey_urls as an Array[Simplib::URI]. This would expose the parameter for what it really is and the documentation in the CHANGELOG could explain that. Then the user could override it. When they need to override it they are unlikely to set extra_gpgkey_urls. But I do not see the need to make them mutually exclusive.

Copy link
Member

Choose a reason for hiding this comment

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

After re-reading the original, as well as all of this, I think that it makes sense to push the new gpgkey parameter back into the code.

It looks like using servers (with URLs as confusing as that is) combined with extra_gpgkeys should give people everything that they need.

Changing servers to default_gpgkeys probably makes sense but is API breaking so we can figure out if we want to flip it with a compat option or just leave it for now.

@trevor-vaughan
Copy link
Member

As a note, if people need to get fancy, they can just disable ALL of this and use yum::repos to stuff in whatever they want via Hiera.

@trevor-vaughan
Copy link
Member

After posting that comment, I'm starting to wonder if we shouldn't push our default repos into Hiera so that it's more obvious and easier to change 🤔

@trevor-vaughan
Copy link
Member

Ok, I see why this is necessary now. Fine with this since it's largely internal but we should probably move this to pure Hiera at some point.

@lnemsick-simp
Copy link
Contributor

REFERENCE.md needs to be regenerated for docs changes.

@trevor-vaughan trevor-vaughan merged commit e9a75df into simp:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants