-
Notifications
You must be signed in to change notification settings - Fork 682
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
Yum resource fix for non-existent repos and repo info #1593
Conversation
lib/resources/yum.rb
Outdated
@@ -33,7 +32,7 @@ | |||
module Inspec::Resources | |||
class Yum < Inspec.resource(1) | |||
name 'yum' | |||
desc 'Use the yum InSpec audit resource to test packages in the Yum repository.' | |||
desc 'Use the yum InSpec audit resource to the configuration of Yum repositories.' |
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.
This is missing the word "test" and should be:
desc 'Use the yum InSpec audit resource to test the configuration of Yum repositories.'
Sorry for being pedantic :)
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.
Pedants welcome. I'm the club president.
a5ade36
to
3b7332f
Compare
lib/resources/yum.rb
Outdated
info['status'] == 'enabled' | ||
end | ||
|
||
def method_missing(name) |
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 try to avoid method_missing whenever possible. Do we know all possible properties? Maybe we use something like:
%w{
baseurl
}.each do |m|
define_method m.to_sym do |*args|
info[m.to_s]
end
end
@@ -22,6 +20,7 @@ | |||
# describe yum.repo('epel') do | |||
# it { should exist } | |||
# it { should be_enabled } | |||
# its('baseurl') { should include 'mycompany.biz' } |
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.
should this be baseurl
or ruby style base_url
?
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.
The key in the yum output and in the *.repo
files is baseurl
so I vote for keeping it consistent.
def to_s | ||
"YumRepo #{shortname(info['id'])}" | ||
"YumRepo #{@reponame}" |
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.
Thank you 👍
If a repo did not exist, running matchers against it (such as `exist`) were failing due to a bug in `#to_s` when fetching the repo name. The `info` method would return nil and we'd still try to treat it as a hash. This change ensures that info is always a hash, possibly empty if the repo doesn't exist, and uses the repo name provided by the user rather than shortening it to be consistent with our other resources which don't manipulate the user input in the formatter. Also added a method_missing to allow users to interrogate repo options, such as baseurl or gpgcheck. Signed-off-by: Adam Leff <adam@leff.co>
Instead of method_missing, methods for each output item from `yum repolist` are provided. Signed-off-by: Adam Leff <adam@leff.co>
b120a57
to
7df9674
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.
Great update and more usefull methods here, kudos @adamleff
end | ||
|
||
def enabled? | ||
repo = info | ||
return false if repo.nil? | ||
return false unless exist? |
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.
Nice simplification
If a repo did not exist, running matchers against it (such as
exist
)were failing due to a bug in
#to_s
when fetching the repo name. Theinfo
method would return nil and we'd still try to treat it as a hash.This change ensures that info is always a hash, possibly empty if the
repo doesn't exist, and uses the repo name provided by the user rather
than shortening it to be consistent with our other resources which don't
manipulate the user input in the formatter.
Also added a method_missing to allow users to interrogate repo options,
such as baseurl or gpgcheck.
Fixes #1553