-
Notifications
You must be signed in to change notification settings - Fork 136
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 a certbot_version
fact
#322
Add a certbot_version
fact
#322
Conversation
b9ca07d
to
33e65ef
Compare
lib/facter/certbot_version.rb
Outdated
# frozen_string_literal: true | ||
|
||
Facter.add(:certbot_version) do | ||
confine kernel: %w[FreeBSD Linux OpenBSD] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should confine on Facter::Util::Resolution.which('certbot')
and not on the kernel I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest I copied that over from letsencrypt_directory.rb, assuming it would limit the fact to the supported operation systems only.
lib/facter/certbot_version.rb
Outdated
value = nil | ||
certbot = Facter::Util::Resolution.which('certbot') | ||
if certbot | ||
output = Facter::Util::Resolution.exec("#{certbot} --version 2>/dev/null") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exec
is deprecated and execute
should now be used: https://github.com/puppetlabs/facter/blob/b0d04b9aae82c1b7f58c92326667b4e2488eaf35/lib/facter/custom_facts/core/execution.rb#L90-L101
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah thanks, I always mix those two up.
3cd3e05
to
169738a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I would just reword a bit the fact description (see in-line comment: "default" vs "current").
|
||
setcode do | ||
output = Facter::Core::Execution.execute('certbot --version 2>/dev/null') | ||
output[%r{^certbot (.*)$}, 1] if output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL this syntax.
169738a
to
daa8363
Compare
certbot_version
fact
This Pull Request (PR) adds the certbot version as a fact.
This fact can be used to add support for new certbot features while maintaining compatibility with older version.
Note that it does not take a custom
$package_command
into account.