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

Enabled usage of the $ntpsigndsocket parameter, for socket signing. #304

Merged
merged 1 commit into from Dec 21, 2015

Conversation

powertoaster
Copy link

I need to be able to sign NTP packets and this enables the parameter.

@bmjen
Copy link
Contributor

bmjen commented Dec 18, 2015

@powertoaster This looks good but can you please remove the updates to the CHANGELOG and the metadata version bump? We will take care of that part when we prep the release. Thanks!

@powertoaster
Copy link
Author

Done, thank you all for your help and patience with me.

@@ -225,6 +225,10 @@ Tells Puppet to use non-standard minimal poll interval of upstream servers. Vali

Tells Puppet to use non-standard maximal poll interval of upstream servers. Valid options: 3 to 16. Default option: undef, except FreeBSD (on FreeBSD `maxpoll` set 9 by default).

####`ntpsigndsocket`

Tells NTP to sign packets using the socket in the ntpsigndsocket path. NTP must be configured to sign sockets for this to work.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a line here describing what the parameter should take and what the Default is? Similar to the descriptions provided by the other parameters around it.

@bmjen
Copy link
Contributor

bmjen commented Dec 18, 2015

@powertoaster No problem. Thank you for contributing! I'm sorry I didn't catch this before my last comment. But I've made one more request in the README. Thanks again!

@powertoaster
Copy link
Author

Added the requested comments, I want to thank everyone at Puppet for letting me flail around here a little. I have learned a great deal about GIT and contributing to a project in the process. Way more than I would have had one of you cleaned up my little mistakes for me.

Oh, and puppet rules.

@bmjen
Copy link
Contributor

bmjen commented Dec 21, 2015

@powertoaster The README still needs a blurb about the Parameter input and default. Sorry for the back and forth. :(

@powertoaster
Copy link
Author

I pushed it to my repo, I can see the changes there but for some reason they are not appearing here. I frankly am not sure how to force them, all of my other changes to the pull request code came over automatically. All I could do at this point is close the pull and redo it.

@bmjen
Copy link
Contributor

bmjen commented Dec 21, 2015

@powertoaster Actually.. they appear to have updated, but apparently didn't resolve my comment. weird.

####`ntpsigndsocket`

Tells NTP to sign packets using the socket in the ntpsigndsocket path. NTP must be configured to sign sockets for this to work.
Valid options a path to the socket directory, in the case of Samba it would be: ntpsigndsocket = usr/local/samba/var/lib/ntp_signd, there is no default.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typically.. you'd say that Default value: undef as is the case here.

@bmjen
Copy link
Contributor

bmjen commented Dec 21, 2015

@powertoaster Apologies for nitpicking the docs, I just want to maintain consistency through the README when new parameters or features are added. Thanks for your patience!

@powertoaster
Copy link
Author

Should be good to go

@bmjen
Copy link
Contributor

bmjen commented Dec 21, 2015

Thanks @powertoaster !

bmjen added a commit that referenced this pull request Dec 21, 2015
Enabled usage of the $ntpsigndsocket parameter, for socket signing.
@bmjen bmjen merged commit 16cad50 into puppetlabs:master Dec 21, 2015
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

2 participants