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

Enable Inspec caching #297

Merged
merged 3 commits into from Dec 6, 2017

Conversation

Projects
None yet
4 participants
@jquick
Contributor

jquick commented Dec 5, 2017

Description

Add inspec caching attribute which defaults to true. This enables the new inspec caching when used by the Audit cookbook.

Issues Resolved

Fixes #296

Signed-off-by: Jared Quick jquick@chef.io

Add option to enable inspec caching.
Signed-off-by: Jared Quick <jquick@chef.io>

@jquick jquick requested a review from chef-cookbooks/audit-cookbook-maintainers as a code owner Dec 5, 2017

@adamleff

Great first pass. We should also modify the metadata.rb file and bump the major version. Because of the new InSpec constraint and a change in default behavior, this will require a major version bump.

@@ -124,6 +129,7 @@ def get_opts(format, quiet, attributes)
'format' => format,
'output' => output,
'logger' => Chef::Log, # Use chef-client log level for inspec run,
:backend_cache => node['audit']['inspec_backend_cache'],

This comment has been minimized.

@adamleff

adamleff Dec 5, 2017

Collaborator

Can we change this to use the new hash syntax instead of hash-rockets?

backend_cache: node['audit']['inspec_backend_cache']

... the stuff above this apparently needs to be strings (but maybe it doesn't, we should probably check on that as some point), but if this key is expected to be a symbol, we should be consistent. 🙂

@@ -38,6 +38,11 @@ def report
# load inspec, supermarket bundle and compliance bundle
load_needed_dependencies
# check if we have a valid version for backend caching
if node['audit']['inspec_backend_cache'] && (Gem::Version.new(::Inspec::VERSION) < Gem::Version.new('1.47.0'))

This comment has been minimized.

@adamleff

adamleff Dec 5, 2017

Collaborator

We're now checking for a specific version constraint in two different places within this file... here, and:

https://github.com/chef-cookbooks/audit/blob/master/files/default/handler/audit_report.rb#L134-L137

I think we should move the MIN_INSPEC_VERSION one to be right above this, and out of the #call method. Or, possibly better yet, extract both to a validate_inspec_version! method and call it here in the #report method in place of this stanza. What do you think?

@@ -38,6 +38,11 @@ def report
# load inspec, supermarket bundle and compliance bundle
load_needed_dependencies
# check if we have a valid version for backend caching
if node['audit']['inspec_backend_cache'] && (Gem::Version.new(::Inspec::VERSION) < Gem::Version.new('1.47.0'))
Chef::Log.warn 'inspec_backend_cache requires Inspec version >= 1.47.0'

This comment has been minimized.

@adamleff

adamleff Dec 5, 2017

Collaborator

InSpec - capital S, please :)

let(:mynode) { Chef::Node.new }
before :each do
mynode.default['audit']['inspec_backend_cache'] = true

This comment has been minimized.

@adamleff

adamleff Dec 5, 2017

Collaborator

Should we add a test that assures that opts[:backend_cache] gets set to false if the node attribute calls for it? I hate to only have a test for the default path.

metadata.rb Outdated
@@ -5,7 +5,7 @@
license 'Apache-2.0'
description 'Allows for fetching and executing compliance profiles, and reporting its results'
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
version '5.0.4'
version '5.0.5'

This comment has been minimized.

@adamleff

adamleff Dec 5, 2017

Collaborator

Gotta be a major bump for a change like this.

jquick added some commits Dec 5, 2017

Bump version
Signed-off-by: Jared Quick <jquick@chef.io>
Clean up tests and update arrangement of caching logic.
Signed-off-by: Jared Quick <jquick@chef.io>
@jquick

This comment has been minimized.

Contributor

jquick commented Dec 5, 2017

These issues have been resolved.

@adamleff

Nice work, @jquick. Looks good to me.

@arlimus

arlimus approved these changes Dec 6, 2017

Fantastic to see the new caching mechanism, thank you @jquick !!

@arlimus arlimus merged commit 3434c9e into master Dec 6, 2017

2 checks passed

DCO This commit has a DCO Signed-off-by
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@arlimus arlimus deleted the jq/enable_inspec_caching branch Dec 6, 2017

miah added a commit to miah/audit that referenced this pull request Apr 11, 2018

Enable Inspec caching (chef-cookbooks#297)
* Add option to enable inspec caching.

Signed-off-by: Jared Quick <jquick@chef.io>

* Bump version

Signed-off-by: Jared Quick <jquick@chef.io>

* Clean up tests and update arrangement of caching logic.

Signed-off-by: Jared Quick <jquick@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment