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

Allow management_port to be a string; require stdlib >= 4.25.0 #275

Merged
merged 2 commits into from
Aug 18, 2018

Conversation

marieof9
Copy link

@marieof9 marieof9 commented Apr 5, 2018

Allow the management_port parameter to be a string too. This fixed #274

@@ -478,7 +478,7 @@
String $pam_module_arguments = 'login',
Boolean $management = false,
String $management_ip = 'localhost',
Integer $management_port = 7505,
Variant[Integer,String] $management_port = 7505,
Copy link
Member

Choose a reason for hiding this comment

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

Is there any string valid, or only 'unix'? Otherwise we could do something like Variant[Integer,Enum['unix']].


Could you also enhance this by removing the Integer type and replace it with Stdlib::Port::Unprivileged from https://github.com/puppetlabs/puppetlabs-stdlib/blob/master/types/port/unprivileged.pp ? This requires a bump of the minimal stdlib version to 4.25.0 in the metadata.json:

"version_requirement": ">= 4.13.1 < 5.0.0"

@bastelfreak bastelfreak added enhancement New feature or request backwards-incompatible needs-work not ready to merge just yet and removed backwards-incompatible labels Aug 12, 2018
@bastelfreak bastelfreak changed the title Allow management_port to be a string. [Fix #274] Allow management_port to be a string. [Fix #274]; require stdlib >= 4.25.0 Aug 18, 2018
@bastelfreak bastelfreak changed the title Allow management_port to be a string. [Fix #274]; require stdlib >= 4.25.0 Allow management_port to be a string; require stdlib >= 4.25.0 Aug 18, 2018
@bastelfreak
Copy link
Member

thanks!

@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Aug 18, 2018
@bastelfreak bastelfreak merged commit 27647d4 into voxpupuli:master Aug 18, 2018
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.

Configuring management unix socket is no longer possible
3 participants