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

feat: add package_keyserver parameter #746

Merged
merged 1 commit into from Mar 6, 2018

Conversation

marcdeop
Copy link
Contributor

We have noticed sometimes puppet fails to provision our servers due to failing connectivity to pgp.mit.edu.

This MR allows you to choose the keyserver in case such an event happens.

Thanks to Erasys GmbH

@juniorsysadmin juniorsysadmin added the enhancement New feature or request label Feb 1, 2018
@bastelfreak
Copy link
Member

Thanks for the pr @marcdeop !

@@ -15,6 +15,7 @@
$minimum_version = $collectd::params::minimum_version,
$package_ensure = $collectd::params::package_ensure,
$package_install_options = $collectd::params::package_install_options,
$package_keyserver = $collectd::params::package_keyserver,
Copy link
Member

Choose a reason for hiding this comment

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

please add a datatype to the new param.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tipbased and added the String datatype to the package_keyserver parameter.

I was considering using Stdlib::Fqdn but is not supported by the minimum stdlib library supported by this module (4.13.1)

@bastelfreak
Copy link
Member

@marcdeop can you bump the version of stdlib in one commit and use Stdlib::Fqdn in the existing commit? Bumping it is fine.

@hadret
Copy link

hadret commented Mar 6, 2018

@bastelfreak is there anything blocking this PR from being merged?

@bastelfreak
Copy link
Member

As written in the last comment:
Please switch from String to Stdlib::Fqdn. Bump the version boundary of stdlib in another commit (but within this PR).

@bastelfreak bastelfreak added the needs-work not ready to merge just yet label Mar 6, 2018
@marcdeop
Copy link
Contributor Author

marcdeop commented Mar 6, 2018

Thanks for the reminder about this PR @hadret

@bastelfreak stdlib supports Stdlib::Fqdn as seen here but apparently the support is only on the master branch. The relevant PR was merged after the latest release (4.24.0) was published.

Should we just go ahead and merge this PR and at the same type create an issue to handle the stdlib::fqdn type as soon as its released by puppetlabs-stdlib?

@bastelfreak
Copy link
Member

Ah shit. I thought it's already released. I will merge this one as is. Can you spin up a new PR that uses the fqdn type, but with a comment that it requires a new stdlib release?

@bastelfreak bastelfreak merged commit 16b17ec into voxpupuli:master Mar 6, 2018
@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Mar 6, 2018
@marcdeop
Copy link
Contributor Author

marcdeop commented Mar 6, 2018

Done: #759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants