Only install InSpec if not installed or version provided #203
Conversation
resources/inspec_gem.rb
Outdated
|
|
||
| action_class do | ||
| def install_inspec_gem(gem_version, gem_source) | ||
| chef_gem 'inspec' do |
There was a problem hiding this comment.
Do you think it makes sense to remove inspec and train before reinstall? Especially if a version is set, we may get conflicts, since newer versions of the dependencies are not installed.
There was a problem hiding this comment.
Yup, since a user may downgrade an image, etc... I think it's fine to do that. I'll write that in.
| @@ -17,7 +17,7 @@ | |||
|
|
|||
| # controls inspec gem version to install | |||
| # example values: '1.1.0', 'latest' | |||
| default['audit']['inspec_version'] = '1.15.0' | |||
| default['audit']['inspec_version'] = nil | |||
There was a problem hiding this comment.
we should document that we use latest by default now
There was a problem hiding this comment.
I'll update the README.
| default_action :install | ||
|
|
||
| action :install do | ||
| if !version.nil? |
There was a problem hiding this comment.
That is a great improvement!
resources/inspec_gem.rb
Outdated
|
|
||
| action :install do | ||
| if !version.nil? | ||
| converge_by "install InSpec version #{version}" do |
There was a problem hiding this comment.
Should we double-check the existing version? Since it tries to install it every time once a version is set.
There was a problem hiding this comment.
I'm partially against that since we should be trusting chef (and chef_gem) to do what we tell it to. But I'm all for setting us up for greater success... to your next comment about removing-then-installing, I think that's a fine idea and will test that.
| @@ -11,7 +11,7 @@ | |||
|
|
|||
| # TODO: change once gem resource handles alternate path to `gem` command | |||
| describe command('/opt/chef/embedded/bin/gem list --local -a -q inspec | grep \'^inspec\' | awk -F"[()]" \'{printf $2}\'') do | |||
There was a problem hiding this comment.
we should use the new method once its merged to inspec, see inspec/inspec#1596
|
@alexpop I wrote this change in a way that we should hopefully not have to wait until Chef 13, but assuming my Chef change is merged, this should only be better in Chef 13, eliminating most of the gem installs. I'm fine bumping the major version. The only major change IMO is that we'll install the latest InSpec instead of being hard-locked to 1.15.0. If we feel this is a breaking change, I'm fine with it. However, I'd venture to say hard-locking to 1.15 is semi-bug-worthy :) |
|
I agree with @adamleff Since inspec is still installed by default, it is not a breaking change. This would also mean, that we remove https://github.com/chef-cookbooks/audit/blob/master/recipes/default.rb#L20 once Chef 13 is out and bump the major version. |
adamleff
force-pushed
the
adamleff/install-only-if-needed
branch
from
73ff67f to
f32fd93
Compare
|
I thought about this some more, and I think the change in install behavior (i.e. don't install if it's already installed) could technically be perceived as a breaking change, so I'm going to leave the change I introduced to bump the major version. Upgrading will only help most of our customers, but if anyone was expecting a version other than 1.15.0 to be installed every time, they're going to be caught off-guard. Bumping the major version still feels like the right thing to do here. I expect most people aren't pinning this cookbook anyways but I'll post to discourse prior to the release. |
adamleff
force-pushed
the
adamleff/install-only-if-needed
branch
from
da6ca00 to
a3682a5
Compare
To assist with air-gapped installations, the audit cookbook is being modified to only install InSpec if it's not already installed or if the user has explicitly stated a version to be installed. This will allow users to use other methods of installing the gem in the chef gem path, or take advantage of InSpec being included in chef client in the future. Signed-off-by: Adam Leff <[email protected]>
adamleff
force-pushed
the
adamleff/install-only-if-needed
branch
from
a3682a5 to
4714d08
Compare
|
This would also resolve #200 |
| @@ -0,0 +1,6 @@ | |||
| #inspec (1.16.1) | |||
There was a problem hiding this comment.
v1.16.1 needs updating or just not mention the version in the comments
Signed-off-by: Christoph Hartmann <[email protected]>
There was a problem hiding this comment.
Thank you @adamleff for this improvement and the preparation for Chef 13
To assist with air-gapped installations, the audit cookbook is
being modified to only install InSpec if it's not already installed
or if the user has explicitly stated a version to be installed.
This will allow users to use other methods of installing the gem
in the chef gem path, or take advantage of InSpec being included
in chef client in the future.
Signed-off-by: Adam Leff [email protected]