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

Fix assert that a gem is not installed #1844

Merged
merged 1 commit into from
May 29, 2017

Conversation

cattywampus
Copy link
Contributor

Description

There are some compliance situations where we want to enforce that a gem is not installed on a system. The following spec code (which is shown in the documentation) does not pass:

describe gem("gem_name") do
  it { should_not be_installed }
end

I originally spotted this bug in v1.18.0 which threw the following error:

  gem package
     ∅  undefined method `[]' for nil:NilClass

Since v1.20.0, now the test is skipped completely because the stdout result of the gem command is blank, which is to be expected for any gem that is not installed.

The gem resource is determining if a gem is installed based on the exit status of the gem command, however that command will return zero if the package was found or not.

InSpec and Platform Version

Inspec: v1.25.1
Platform: RedHat 7 & OS/X Sierra

Looking at the code, this appears it might apply to all platforms, assuming the return code behavior of the gem command doesn't differ across different platforms.

Replication Case

This issue can be replicated with a simple describe spec:

# test.rb
describe gem("gem_i_do_not_want") do
  it { should_not be_installed }
end

Then run:

$ gem install inspec --version v1.25.1
$ inspec exec test.rb
  gem package
     ↺  Unable to retrieve gem information

Test Summary: 0 successful, 0 failures, 1 skipped

Possible Solutions

  1. Remove support for writing tests for gems that are not installed. The documentation for the gem resource currently includes a sample test which suggests that this feature should be supported. This solution would just remove that test case and add a note that this feature is no longer supported.

  2. Check the return of the gem list --local -a -q <gemname> stdout to see if the output contains the name of the gem. If it does then the gem is installed, otherwise the gem is not installed. This check would be added in addition to the existing check for the zero return status to ensure that the command also completed successfully. This is necessary because the gem list command will return zero if the gem exists or not.

This pull request implements the second option. One thing I wasn't sure about was what should happen if the command exit status is not zero. For the time being, I left the logic in place to trigger the skip_resource call in the constructor, but I think the better solution should be to raise an error. I just wasn't sure what error class to use for that.

The gem resource used to determine if a gem is installed based on the exit
status of the `gem` command, however that command will return zero
if the package was found or not. This patch checks to ensure that the
`gem list` command actually includes the gem name or is empty to
determine if the gem is in fact installed.

If the gem command returns something other than a `0` exit code, then
it'll skip the resource.

Signed-off-by: Keith Walters <keith.walters@cattywamp.us>
Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

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

Hi @cattywampus! Thank you so much for your contribution, and thanks for your first contribution to InSpec!

This change looks great to me - thank you for your well-written PR. I tested the fix locally as well and it works great.

Thank you again!

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

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

Great improvement @cattywampus

@chris-rock chris-rock merged commit 45afca2 into inspec:master May 29, 2017
@adamleff adamleff added the Type: Bug Feature not working as expected label May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants