Merge pull request #5618 from inspec/nm/check-cookstyle

Integrate InSpec check with Cookstyle

.gitignore

@@ -28,6 +28,7 @@ inspec-deprecations-in-cfg.txt
 inspec-deprecations-in-lib.txt
 kitchen.local.yml
 meta-profile-0.2.0.tar.gz
+inheritance-1.0.0.tar.gz
 omnibus/.cache
 omnibus/pkg
 profile-1.0.0.tar.gz

docs-chef-io/content/inspec/cli.md

@@ -60,7 +60,7 @@ inspec automate SUBCOMMAND
 
 ## check
 
-Verify metadata in inspec.yml. Verify control data has fields (title, description, impact) defined and that all controls have visible tests.
+Verify the metadata in the inspec.yml file, verify that control blocks have the correct fields (title, description, impact) defined, that all controls have visible tests, and that controls are not using deprecated InSpec DSL code.
 
 ### Syntax
 

inspec.gemspec

@@ -34,4 +34,7 @@ Gem::Specification.new do |spec|
   spec.add_dependency "train-aws",     "~> 0.2"
   spec.add_dependency "train-winrm",   "~> 0.2"
   spec.add_dependency "mongo", "= 2.13.2" # 2.14 introduces a broken symlink in mongo-2.14.0/spec/support/ocsp
+
+  # checks code offenses with inspec check
+  spec.add_dependency "cookstyle"
 end

lib/inspec/cli.rb

@@ -122,8 +122,8 @@ class Inspec::InspecCLI < Inspec::BaseCLI
       end
       puts
 
-      if result[:errors].empty? && result[:warnings].empty?
+      if result[:errors].empty? && result[:warnings].empty? && result[:offenses].empty?
-        ui.plain_line("No errors or warnings")
+        ui.plain_line("No errors, warnings, or offenses")
       else
         item_msg = lambda { |item|
           pos = [item[:file], item[:line], item[:column]].compact.join(":")
@@ -135,11 +135,18 @@ class Inspec::InspecCLI < Inspec::BaseCLI
 
         puts
 
+        unless result[:offenses].empty?
+          puts "Offenses:\n"
+          result[:offenses].each { |item| ui.cyan(" #{Inspec::UI::GLYPHS[:script_x]} #{item_msg.call(item)}\n\n") }
+        end
+
+        offenses = ui.cyan("#{result[:offenses].length} offenses", print: false)
         errors = ui.red("#{result[:errors].length} errors", print: false)
         warnings = ui.yellow("#{result[:warnings].length} warnings", print: false)
-        ui.plain_line("Summary:     #{errors}, #{warnings}")
+        ui.plain_line("Summary:     #{errors}, #{warnings}, #{offenses}")
       end
     end
+
     ui.exit Inspec::UI::EXIT_USAGE_ERROR unless result[:summary][:valid]
   rescue StandardError => e
     pretty_handle_exception(e)

lib/inspec/profile.rb

@@ -448,6 +448,43 @@ module Inspec
       res
     end
 
+    def cookstyle_linting_check
+      msgs = []
+      output = cookstyle_rake_output.split("Offenses:").last
+      msgs = output.split("\n").select { |x| x =~ /[A-Z]:/ } unless output.nil?
+      msgs
+    end
+
+    # Cookstyle linting rake run output
+    def cookstyle_rake_output
+      require "cookstyle"
+      require "rubocop/rake_task"
+      begin
+        RuboCop::RakeTask.new(:cookstyle_lint) do |spec|
+          spec.options += [
+            "--display-cop-names",
+            "--parallel",
+            "--only=InSpec/Deprecations",
+          ]
+          spec.patterns += Dir.glob("#{@target}/**/*.rb").reject { |f| File.directory?(f) }
+          spec.fail_on_error = false
+        end
+      rescue LoadError
+        puts "Rubocop is not available. Install the rubocop gem to run the lint tests."
+      end
+      begin
+        stdout = StringIO.new
+        $stdout = stdout
+        Rake::Task["cookstyle_lint"].invoke
+        $stdout = STDOUT
+        Rake.application["cookstyle_lint"].reenable
+        stdout.string
+      rescue => e
+        puts "Cookstyle lint checks could not be performed. Error while running cookstyle - #{e}"
+        ""
+      end
+    end
+
     # Check if the profile is internally well-structured. The logger will be
     # used to print information on errors and warnings which are found.
     #
@@ -464,6 +501,7 @@ module Inspec
         },
         errors: [],
         warnings: [],
+        offenses: [],
       }
 
       entry = lambda { |file, line, column, control, msg|
@@ -486,6 +524,10 @@ module Inspec
         result[:errors].push(entry.call(file, line, column, control, msg))
       }
 
+      offense = lambda { |file, line, column, control, msg|
+        result[:offenses].push(entry.call(file, line, column, control, msg))
+      }
+
       @logger.info "Checking profile in #{@target}"
       meta_path = @source_reader.target.abs_path(@source_reader.metadata.ref)
 
@@ -548,8 +590,15 @@ module Inspec
         warn.call(sfile, sline, nil, id, "Control #{id} has no tests defined") if control[:checks].nil? || control[:checks].empty?
       end
 
-      # profile is valid if we could not find any error
+      # Running cookstyle to check for code offenses
-      result[:summary][:valid] = result[:errors].empty?
+      cookstyle_linting_check.each do |lint_output|
+        data = lint_output.split(":")
+        msg = "#{data[-2]}:#{data[-1]}"
+        offense.call(data[0], data[1], data[2], nil, msg)
+      end
+
+      # profile is valid if we could not find any error & offenses
+      result[:summary][:valid] = result[:errors].empty? && result[:offenses].empty?
 
       @logger.info "Control definitions OK." if result[:warnings].empty?
       result

test/functional/helper.rb

@@ -235,6 +235,14 @@ module FunctionalHelper
     end
   end
 
+  def prepare_profiles(dir = nil, &block)
+    Dir.mktmpdir do |tmpdir|
+      FileUtils.cp_r(profile_path, tmpdir)
+      bn = File.basename(profile_path)
+      yield(File.join(tmpdir, bn, dir.to_s))
+    end
+  end
+
   private
 
   def assemble_env_prefix(env = {})

test/functional/inspec_archive_test.rb

@@ -102,7 +102,7 @@ describe "inspec archive" do
   end
 
   it "vendors dependencies by default" do
-    prepare_examples("meta-profile") do |dir|
+    prepare_profiles("dependencies/inheritance") do |dir|
       out = inspec("archive " + dir + " --output " + dst.path)
 
       _(out.stderr).must_equal ""

test/functional/inspec_check_test.rb

@@ -117,4 +117,18 @@ describe "inspec check" do
       assert_exit_code 1, out
     end
   end
+
+  describe "inspec check also check for cookstyle offenses" do
+    it "finds no offenses in a complete profile" do
+      out = inspec("check #{profile_path}/complete-profile")
+      _(out.stdout).must_match(/No errors, warnings, or offenses/)
+      assert_exit_code 0, out
+    end
+
+    it "fails and returns offenses in a profile" do
+      out = inspec("check #{profile_path}/inputs/metadata-basic")
+      _(out.stdout).must_match(/1 offenses/)
+      assert_exit_code 1, out
+    end
+  end
 end

test/functional/inspec_vendor_test.rb

@@ -155,7 +155,7 @@ describe "example inheritance profile" do
   end
 
   it "use lockfile in tarball" do
-    prepare_examples("meta-profile") do |dir|
+    prepare_profiles("dependencies/inheritance") do |dir|
       # ensure the profile is vendored and packaged as tar
       out = inspec("vendor " + dir + " --overwrite")
 
@@ -172,7 +172,7 @@ describe "example inheritance profile" do
 
       # TODO: split
       # execute json command
-      out = inspec("json meta-profile-0.2.0.tar.gz -l debug")
+      out = inspec("json inheritance-1.0.0.tar.gz -l debug")
 
       _(out.stdout.scan(/Fetching URL:/).length).must_equal 0
       _(out.stdout).wont_match(/Fetching URL:/)

test/unit/profiles/profile_test.rb

@@ -124,6 +124,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 0
         _(result[:errors].length).must_equal 1
         _(result[:warnings].length).must_equal 5
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -148,6 +149,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 0
         _(result[:errors]).must_be_empty
         _(result[:warnings].length).must_equal 1
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -171,6 +173,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 1
         _(result[:errors]).must_be_empty
         _(result[:warnings]).must_be_empty
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -196,6 +199,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 1
         _(result[:errors]).must_be_empty
         _(result[:warnings]).must_be_empty
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -221,6 +225,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 1
         _(result[:errors]).must_be_empty
         _(result[:warnings]).must_be_empty
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -246,6 +251,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 1
         _(result[:errors]).must_be_empty
         _(result[:warnings]).must_be_empty
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -272,6 +278,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 0
         _(result[:errors].length).must_equal 1
         _(result[:warnings].length).must_equal 1
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -289,6 +296,7 @@ describe Inspec::Profile do
         logger.verify
         _(result[:warnings]).must_be_empty
         _(result[:errors].length).must_equal 1
+        _(result[:offenses]).must_be_empty
       end
     end
 
@@ -316,6 +324,7 @@ describe Inspec::Profile do
         _(result[:summary][:controls]).must_equal 0
         _(result[:errors]).must_be_empty
         _(result[:warnings].length).must_equal 2
+        _(result[:offenses]).must_be_empty
       end
 
       describe "shows no warning if license is spdx" do
@@ -341,6 +350,7 @@ describe Inspec::Profile do
           _(result[:summary][:controls]).must_equal 0
           _(result[:errors]).must_be_empty
           _(result[:warnings].length).must_equal 1
+          _(result[:offenses]).must_be_empty
         end
       end
 
@@ -367,6 +377,7 @@ describe Inspec::Profile do
           _(result[:summary][:controls]).must_equal 0
           _(result[:errors]).must_be_empty
           _(result[:warnings].length).must_equal 1
+          _(result[:offenses]).must_be_empty
         end
       end