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

Add validate_augeas command #114

Merged
merged 5 commits into from Jan 23, 2013
Merged

Add validate_augeas command #114

merged 5 commits into from Jan 23, 2013

Conversation

raphink
Copy link
Contributor

@raphink raphink commented Dec 6, 2012

This PR adds a new validate_augeas function, which takes a string and an Augeas lens, and validates the string against the Augeas lens. Example:

validate_augeas("root ALL=(ALL) ALL\n", "Sudoers.lns")

The function optionally takes an array of paths which should not be found in the content.

For example, if you want to make sure your passwd content never contains a user foo, you could write:

validate_augeas($passwdcontent, 'Passwd.lns', ['$file/foo'])

Or if you wanted to ensure that no users used the '/bin/barsh' shell, you could use:

validate_augeas($passwdcontent, 'Passwd.lns', ['$file/*[shell="/bin/barsh"]']

Like all validate_* functions, it fails compilation if the check fails. It also takes an optional 4rd argument to specify a nicer message.


Or if you wanted to ensure that no users used the '/bin/barsh' shell,
you could use:

Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out for whitespace errors. I found this with git diff --check master.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I'll fix it up in my own topic branch, no action required.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

@jeffmccune
Copy link
Contributor

@raphink Does this function require augeaus? If so, I'd like to discuss the pros and cons of merging it into stdlib rather than another module. As it currently stands, stdlib does not have any dependencies other than Ruby and Puppet AFAIK, so this would be a big change from a dependency and requirements stand point.

@raphink
Copy link
Contributor Author

raphink commented Jan 18, 2013

The function does require augeas indeed. That said, it will not make stdlib depend on augeas per se. Rather, you will need ruby-augeas to use this function only. I could probably add a check for augeas in the function to issue a clean warning if it is not available.

@jeffmccune
Copy link
Contributor

@raphink Yes, if the function disabled itself if Augeas isn't present then I think it would be OK to include. It may also be worth checking how the Augeas types and providers perform this check. If it's a public API then this parser function could re-use the behavior.

@raphink
Copy link
Contributor Author

raphink commented Jan 18, 2013

@jeffmccune I added changes to:

  • Ensure the augeas handler is always closed
  • Ensure the tmpfile is always closed and unlinked
  • Ensure Puppet has the augeas feature


ENDHEREDOC
unless Puppet.features.augeas?
raise Puppet::ParseError, ("validate_augeas(): requires the ruby augeas bindings")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good, but I think it would be great if the error message helped the end user with next steps to make this work. For example, perhaps a URL to a document that describes how to enable the augeas feature in Puppet.

Hopefully we have this documented at http://docs.puppetlabs.com/

Maybe something like, "requires the ruby augeas bindings. Information about how to satisfy this requirements is available at: http://..."

I think this would be great because a ParseError aborts catalog compilation entirely, so the user is sort of "stuck" if they encounter this error.

@jeffmccune
Copy link
Contributor

I think this is ready to go once the one comment about "next steps" when the augeas feature is not available is addressed and the tests are passing.

-Jeff


ENDHEREDOC
unless Puppet.features.augeas?
raise Puppet::ParseError, ("validate_augeas(): this function requires the augeas feature. See http://projects.puppetlabs.com/projects/puppet/wiki/Puppet_Augeas#Pre-requisites for how to activate it.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeffmccune This URL is where augeas installation for puppet is documented.

@jeffmccune jeffmccune merged commit c5f0309 into puppetlabs:master Jan 23, 2013
@jeffmccune
Copy link
Contributor

Merged into master as 4d24bd3.

This should be released in 3.3.0.

Thanks again for the contribution!

-Jeff

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

Successfully merging this pull request may close these issues.

None yet

3 participants