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

Use sudo to read last_run_summary and last_run_report files when nece… #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bill-mcgonigle
Copy link

This implements some of the fixes proposed in #38 , which appears to be necessary in my Puppet 4/5 environment. Tested on : Debian Stretch/Buster, Fedora 26/27, CentOS 6/7, Ubuntu 16.04.

Please modify to fit your coding style or where I misunderstood what was happening. It's working here, but I wasn't able to test on old puppets.

# User_Alias NAGIOS=nagios
# Cmnd_Alias PUPPETCHECK=/usr/bin/puppet config print all, \ # puppet 2
# /usr/bin/puppet config print, \ # puppet 3
# /usr/bin/puppet config print --section agent # other puppet version
# /usr/bin/puppet config print, \ --section agent # other puppet version
Copy link

@baldurmen baldurmen Jan 8, 2019

Choose a reason for hiding this comment

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

I'm guessing the placement of the comma and the slash here is a typo?

/usr/bin/puppet config print, \ --section agent -> /usr/bin/puppet config print --section agent, \

Copy link
Owner

Choose a reason for hiding this comment

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

@bill-mcgonigle Can you please move the \ to the end of line as @baldurmen suggests?

@baldurmen
Copy link

This works on my side. Thanks!

Copy link
Owner

@aswen aswen left a comment

Choose a reason for hiding this comment

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

@bill-mcgonigle Thanks for your PR. I would love to add this to the check script. Please fix the few remarks made by me and @baldurmen (Thanks @baldurmen for your review!).

if ( sudo test -s $lastrunfile && sudo test -r $lastrunfile ); then
sudo_summary='sudo'
else
result 1 if [ -n "$SHOW_ERROR" ]
Copy link
Owner

Choose a reason for hiding this comment

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

@bill-mcgonigle Did you test this? I never saw this order in if statements in sh scripts. It reminds me of ruby style instead.
I was curious, so tested this:

$ echo yes if [ 1 -eq 1 ]
yes if [ 1 -eq 1 ]
$ echo yes if [ -z "$thing" ]
yes if [ -z  ]
$ thing=hooray
$ echo yes if [ -n "$thing" ]
yes if [ -n hooray ]

This leads me to the suspicion this would execute the result function with 1 if [ -n <the contents of $SHOW_ERROR here> ]. That is probably not what you intended.

if ( sudo test -s $lastrunreport && sudo test -r $lastrunreport ); then
sudo_report='sudo'
else
result 12 if [ -n "$SHOW_ERROR" ]
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above.

# User_Alias NAGIOS=nagios
# Cmnd_Alias PUPPETCHECK=/usr/bin/puppet config print all, \ # puppet 2
# /usr/bin/puppet config print, \ # puppet 3
# /usr/bin/puppet config print --section agent # other puppet version
# /usr/bin/puppet config print, \ --section agent # other puppet version
Copy link
Owner

Choose a reason for hiding this comment

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

@bill-mcgonigle Can you please move the \ to the end of line as @baldurmen suggests?

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.

3 participants