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 the ability to install inspec as package when specified #165

Closed
wants to merge 3 commits into from

Conversation

vjeffrey
Copy link

@vjeffrey vjeffrey commented Nov 17, 2016

Description

@stephenlauck came up with a nice recipe to do this (https://github.com/stephenlauck/audit/blob/lauck/inspec_package/recipes/inspec_package.rb), just filling in the details to get it working with attributes, etc

Adds the ability to install inspec as package when specified. @stephenlauck does this fit what you were thinking??

Issues Resolved

fixes: #164

Check List

action :upgrade
end

load_inspec_libs
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this with the package?

The package seems to have /opt/inspec/embedded/lib as its base path. I don't think that's in Chef-client's $LOAD_PATH..

Copy link
Contributor

Choose a reason for hiding this comment

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

That appears to be the case:

root@node:/tmp# /opt/chef/embedded/bin/irb
irb(main):001:0> require 'inspec'
=> true
irb(main):002:0>
root@node:/tmp# /opt/chef/embedded/bin/gem which inspec
/opt/chef/embedded/lib/ruby/gems/2.3.0/gems/inspec-1.4.1/lib/inspec.rb
root@node:/tmp# /opt/chef/embedded/bin/gem uninstall inspec
Remove executables:
    inspec

in addition to the gem? [Yn]  y
Removing inspec
Successfully uninstalled inspec-1.4.1
root@node:/tmp# /opt/chef/embedded/bin/irb
irb(main):001:0> require 'inspec'
LoadError: cannot load such file -- inspec
    from /opt/chef/embedded/lib/ruby/site_ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from /opt/chef/embedded/lib/ruby/site_ruby/2.3.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from (irb):1
    from /opt/chef/embedded/bin/irb:11:in `<main>'
irb(main):002:0> libpath = '/opt/inspec/embedded/lib/ruby/gems/2.3.0/gems/inspec-1.4.1/lib'
=> "/opt/inspec/embedded/lib/ruby/gems/2.3.0/gems/inspec-1.4.1/lib"
irb(main):003:0> $LOAD_PATH.unshift(libpath) unless $LOAD_PATH.include?(libpath)
=> ["/opt/inspec/embedded/lib/ruby/gems/2.3.0/gems/inspec-1.4.1/lib", "/opt/chef/embedded/lib/ruby/gems/2.3.0/gems/did_you_mean-1.0.0/lib", "/opt/chef/embedded/lib/ruby/gems/2.3.0/gems/rb-readline-0.5.3/lib", "/opt/chef/embedded/lib/ruby/site_ruby/2.3.0", "/opt/chef/embedded/lib/ruby/site_ruby/2.3.0/x86_64-linux", "/opt/chef/embedded/lib/ruby/site_ruby", "/opt/chef/embedded/lib/ruby/vendor_ruby/2.3.0", "/opt/chef/embedded/lib/ruby/vendor_ruby/2.3.0/x86_64-linux", "/opt/chef/embedded/lib/ruby/vendor_ruby", "/opt/chef/embedded/lib/ruby/2.3.0", "/opt/chef/embedded/lib/ruby/2.3.0/x86_64-linux"]
irb(main):004:0> require 'inspec'
=> true
irb(main):005:0>

Copy link
Author

Choose a reason for hiding this comment

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

oh shoot i think i had some leftover gem cruft in there when i tested. right, ok, so we would need to set the load path like you did there. sounds like there are some other issues too (looking in compliance-support) in that chef-ingredient uses mixlib install gem. so we could go the remote file and execute route. and then there's the problem of having gem and package installed....we could do a check to get around that i suppose

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 17, 2016

I see the value in installing via package. I am pretty sure though that enterprises are not going to pull the package down from our own repos but will want to host it internally on theirs. Specifying the package source would need to be a capability in my opinion, as we are for the gem.

@vjeffrey
Copy link
Author

oh, yup -- definitely @jeremymv2 good idea, thank you!

@vjeffrey
Copy link
Author

vjeffrey commented Nov 17, 2016

ok so things to do here are:

  1. change from using chef_ingredient to remote file execute?
  2. make sure path is set
  3. ensure there's no gem already installed if doing package install?
  4. give ability to specify package source

does that sound right @stephenlauck @jeremymv2 ??

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 18, 2016

Here's a time proven example of chef_ingredient installing supermarket rpm including potentially from custom repo: https://github.com/chef-cookbooks/supermarket-omnibus-cookbook/blob/master/resources/supermarket_server.rb

The node['chef-ingredient']['custom-repo-recipe']: https://github.com/chef-cookbooks/chef-ingredient#chef_ingredient

@vjeffrey vjeffrey changed the title add the ability to install gem as package when specified add the ability to install inspec as package when specified Nov 18, 2016
@stephenlauck
Copy link

Would it be possible for the Inspec package install to configure itself and be ready to be loaded so the recipe is only does a package install and Inspec is ready to go?

@vjeffrey vjeffrey changed the title add the ability to install inspec as package when specified wip: add the ability to install inspec as package when specified Nov 18, 2016
@vjeffrey
Copy link
Author

from discussion: it might be preferable to vendor the gem in the cookbook. 4+ big customers would benefit from a vendored setup

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 18, 2016

I would like to keep this in mind: #112
Vendoring a gem is not without its own caveats that also need to be considered.

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 22, 2016

Vendoring a gem and its dependencies in a cookbook is not really a pattern that is commonly used. If hosting the gem is internally is not an option, then this could be an alternative strategy you might consider:

You can bundle up the gems you need into a tarball, host that as an artifact internally, then download and extract via recipe in your wrapper cookbook:

# example with mixlib-install gem and dependencies
$ gem install --no-rdoc --no-ri --install-dir tmp/ --no-user-install mixlib-install
Fetching: artifactory-2.3.3.gem (100%)
Successfully installed artifactory-2.3.3
Fetching: mixlib-versioning-1.1.0.gem (100%)
Successfully installed mixlib-versioning-1.1.0
Fetching: mixlib-shellout-2.2.6.gem (100%)
Successfully installed mixlib-shellout-2.2.6
Fetching: mixlib-install-1.1.0.gem (100%)
Successfully installed mixlib-install-1.1.0
4 gems installed

$ ls -l tmp/
drwxr-xr-x  2 jmiller  staff   68 Jul 19 19:01 build_info
drwxr-xr-x  6 jmiller  staff  204 Jul 19 19:01 cache
drwxr-xr-x  2 jmiller  staff   68 Jul 19 19:01 doc
drwxr-xr-x  2 jmiller  staff   68 Jul 19 19:01 extensions
drwxr-xr-x  6 jmiller  staff  204 Jul 19 19:02 gems
drwxr-xr-x  6 jmiller  staff  204 Jul 19 19:02 specifications
$ cd tmp
$ tar cvf gems.tar *
Use it later in a recipe:

download_location = ::File.join(Chef::Config[:file_cache_path], 'gems.tar')

remote_file download_location do
  source 'https://your.artifact.server/gems.tar'
  action :create
end

execute 'extract gems' do
  command "tar -xvf #{download_location} -C #{node[:languages][:ruby][:gems_dir]}"
  not_if do
    ::File.exist?("#{node[:languages][:ruby][:gems_dir]}/somegem")
  end
end

@vjeffrey
Copy link
Author

@@ -17,7 +17,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

if node['audit']['inspec_package_source']
if node['audit']['inspec_package']['source']
include_recipe 'audit::inspec_package'
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious if there is a reason to break out inspec installation into two separate recipes? Would it be tighter to just include_recipe 'audit::inspec' here and let the inspec recipe have the conditional logic to decide whether to install from package or from gem within an if statement?

Copy link
Author

Choose a reason for hiding this comment

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

oh, ya, that makes sense @jeremymv2 i like that.

@@ -72,33 +72,26 @@
context 'When a package install is specified' do
let(:chef_run) do
ChefSpec::ServerRunner.new do |node|
node.override['audit']['inspec_package'] = true
node.override['audit']['inspec_package']['source'] = 'http://path/to/fake.rpm'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the inspec package installation tests should be moved into a inspec_spec.rb file under spec/unit/recipes since the best practice is for the unit test file name to be <recipe_name>_spec.rb.

Copy link
Author

Choose a reason for hiding this comment

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

👍

@vjeffrey vjeffrey mentioned this pull request Nov 23, 2016
4 tasks
@vjeffrey
Copy link
Author

pushed a commit to address your comments @jeremymv2 ; thank you for reviewing. Also went ahead and added info to readme.

@vjeffrey vjeffrey changed the title wip: add the ability to install inspec as package when specified add the ability to install inspec as package when specified Nov 23, 2016
@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 23, 2016

This doesn't work on Ubuntu since the default package provider selected is apt_package which cannot handle source.

           Chef::Exceptions::Package
           -------------------------
           apt package provider cannot handle source attribute. Use dpkg provider instead

@vjeffrey
Copy link
Author

vjeffrey commented Nov 23, 2016

huh....shouldn't the chef package resource be doing that for us? package tells the chef-client to manage a package; the chef-client will determine the correct package provider to use based on the platform running on the node :(

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 23, 2016

The package resource is making a decision, since each platform could potentially have any number of valid package providers, it needs to make an assumption and select the "best" one to make default when the recipe isn't specific. It's choosing the wrong one in this case, when source property is specified. I suppose the Package Resource should be smart enough to detect that, but that doesn't appear to be the case 😢 apt_package seems to be the default rather than dpkg_package. A quick test of changing the resource to dpkg_package works.

@vjeffrey
Copy link
Author

vjeffrey commented Nov 23, 2016

k...so doing something along these lines is working for me:

  case node[:platform]
  when 'ubuntu'
    dpkg_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  when 'centos'
    package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  end

....it's not awesome...and this is all feeling more and more hacky by the moment...but not sure what we could do that would be better..

@jeremymv2
Copy link
Contributor

Cool. Need to handle when !ubuntu && !centos
Perhaps just package is a sane default for (Windows, Mac, ..)

@vjeffrey
Copy link
Author

vjeffrey commented Nov 23, 2016

k...so how much is a reasonable scope? we can make the default one package, as is, which covers centos...then dpkg package for ubuntu, bff package for aix, freebsd package for freebsd, solaris package for solaris, windows package for windows....so we end up with something like:

  case node[:platform]
  when 'ubuntu'
    dpkg_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  when 'aix'
    bff_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  when 'freebsd'
    freebsd_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  when 'solaris'
    solaris_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  when 'windows'
    windows_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  else
    package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  end

:/ anything from that list that 'isn't really worth it' or anything that should be added?

@jeremymv2
Copy link
Contributor

Browsing the chef-client code base under chef/lib/chef/provider/package I'm only seeing where source is an issue for apt_package. That being the case, I think this would suffice and handle all scenarios:

case node[:platform]
  when 'ubuntu'
    dpkg_package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  else
    package 'inspec' do
      source package_path
      action :nothing
    end.run_action(:install)
  end

@vjeffrey
Copy link
Author

oh good, that's much nicer!

@jeremymv2
Copy link
Contributor

jeremymv2 commented Nov 23, 2016

Great @vjeffrey Awesome improvement to the cookbook! 👍

Victoria Jeffrey and others added 2 commits November 23, 2016 15:05
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
Load inspec lib from package
Signed-off-by: Victoria Jeffrey <vjeffrey@chef.io>
@chris-rock
Copy link
Contributor

InSpec gem is part of Chef 13, therefore we do not need to work on alternatives anymore.

@chris-rock chris-rock closed this Apr 25, 2017
@chris-rock
Copy link
Contributor

Thank you @vjeffrey and @jeremymv2 for raising the issue. This helped us to bring inspec into chef-client!

@vjeffrey vjeffrey deleted the lauck-vj/inspec-package branch April 16, 2019 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ability to install inspec as a package
4 participants