Merge pull request #4427 from inspec/cw/waivers-overlay

waivers: functional test fixture and eval-time skipping

lib/inspec/formatters/base.rb

@@ -158,6 +158,7 @@ module Inspec::Formatters
         start_time: example.execution_result.started_at.to_datetime.rfc3339.to_s,
         resource_title: example.metadata[:described_class] || example.metadata[:example_group][:description],
         expectation_message: format_expectation_message(example),
+        waiver_data: example.metadata[:waiver_data],
       }
 
       unless (pid = example.metadata[:profile_id]).nil?

lib/inspec/rule.rb

@@ -1,6 +1,7 @@
 # copyright: 2015, Dominik Richter
 
 require "method_source"
+require "date"
 require "inspec/describe"
 require "inspec/expect"
 require "inspec/resource"
@@ -28,6 +29,7 @@ module Inspec
       @resource_dsl
     end
 
+    attr_reader :__waiver_data
     def initialize(id, profile_id, opts, &block)
       @impact = nil
       @title = nil
@@ -42,7 +44,7 @@ module Inspec
       @__rule_id = id
       @__profile_id = profile_id
       @__checks = []
-      @__skip_rule = {}
+      @__skip_rule = {} # { result: true, message: "Why", type: [:only_if, :waiver] }
       @__merge_count = 0
       @__merge_changes = []
       @__skip_only_if_eval = opts[:skip_only_if_eval]
@@ -52,6 +54,11 @@ module Inspec
 
       begin
         instance_eval(&block)
+
+        # By applying waivers *after* the instance eval, we assure that
+        # waivers have higher precedence than only_if.
+        __apply_waivers
+
       rescue StandardError => e
         # We've encountered an exception while trying to eval the code inside the
         # control block. We need to prevent the exception from bubbling up, and
@@ -141,6 +148,7 @@ module Inspec
       return if @__skip_only_if_eval == true
 
       @__skip_rule[:result] ||= !yield
+      @__skip_rule[:type] = :only_if
       @__skip_rule[:message] = message
     end
 
@@ -193,9 +201,9 @@ module Inspec
       rule.instance_variable_get(:@__skip_rule)
     end
 
-    def self.set_skip_rule(rule, value, message = nil)
+    def self.set_skip_rule(rule, value, message = nil, type = :only_if)
       rule.instance_variable_set(:@__skip_rule,
-        { result: value, message: message })
+        { result: value, message: message, type: type })
     end
 
     def self.merge_count(rule)
@@ -206,14 +214,16 @@ module Inspec
       rule.instance_variable_get(:@__merge_changes)
     end
 
+    # If a rule is marked to be skipped, this
+    # creates a dummay array of "checks" with a skip outcome
     def self.prepare_checks(rule)
       skip_check = skip_status(rule)
       return checks(rule) unless skip_check[:result].eql?(true)
 
       if skip_check[:message]
-        msg = "Skipped control due to only_if condition: #{skip_check[:message]}"
+        msg = "Skipped control due to #{skip_check[:type]} condition: #{skip_check[:message]}"
       else
-        msg = "Skipped control due to only_if condition."
+        msg = "Skipped control due to #{skip_check[:type]} condition."
       end
 
       # TODO: we use os as the carrier here, but should consider
@@ -251,7 +261,8 @@ module Inspec
       skip_check = skip_status(src)
       sr = skip_check[:result]
       msg = skip_check[:message]
-      set_skip_rule(dst, sr, msg) unless sr.nil?
+      skip_type = skip_check[:type]
+      set_skip_rule(dst, sr, msg, skip_type) unless sr.nil?
 
       # Save merge history
       dst.instance_variable_set(:@__merge_count, merge_count(dst) + 1)
@@ -267,6 +278,56 @@ module Inspec
       @__checks.push([describe_or_expect, values, block])
     end
 
+    # Look for an input with a matching ID, and if found, apply waiver
+    # skipping logic. Basically, if we have a current waiver, and it says
+    # to skip, we'll replace all the checks with a dummy check (same as
+    # only_if mechanism)
+    # Double underscore: not intended to be called as part of the DSL
+    def __apply_waivers
+      input_name = @__rule_id # TODO: control ID slugging
+      registry = Inspec::InputRegistry.instance
+      input = registry.inputs_by_profile.dig(@__profile_id, input_name)
+      return unless input
+
+      # An InSpec Input is a datastructure that tracks a profile parameter
+      # over time. Its value can be set by many sources, and it keeps a
+      # log of each "set" event so that when it is collapsed to a value,
+      # it can determine the correct (highest priority) value.
+      # Store in an instance variable for.. later reading???
+      @__waiver_data = input.value
+      __waiver_data["skipped_due_to_waiver"] = false
+      __waiver_data["message"] = ""
+
+      # Waivers should have a hash value with keys possibly including skip and
+      # expiration_date. We only care here if it has a skip key and it
+      # is yes-like, since all non-skipped waiver operations are handled
+      # during reporting phase.
+      return unless __waiver_data.key?("skip") && __waiver_data["skip"]
+
+      # OK, the intent is to skip. Does it have an expiration date, and
+      # if so, is it in the future?
+      expiry = __waiver_data["expiration_date"]
+      if expiry
+        if expiry.is_a?(Date)
+          # It appears that yaml.rb automagically parses dates for us
+          if expiry < Date.today # If the waiver expired, return - no skip applied
+            __waiver_data["message"] = "Waiver expired on #{expiry}, evaluating control normally"
+            return
+          end
+        else
+          ui = Inspec::UI.new
+          ui.error("Unable to parse waiver expiration date '#{expiry}' for control #{@__rule_id}")
+          ui.exit(:usage_error)
+        end
+      end
+
+      # OK, apply a skip.
+      @__skip_rule[:result] = true
+      @__skip_rule[:type] = :waiver
+      @__skip_rule[:message] = __waiver_data["justification"]
+      __waiver_data["skipped_due_to_waiver"] = true
+    end
+
     #
     # Takes a block and returns a block that will run the given block
     # with access to the resource_dsl of the current class. This is to

lib/inspec/runner_rspec.rb

@@ -171,6 +171,7 @@ module Inspec
       metadata[:descriptions] = rule.descriptions
       metadata[:code] = rule.instance_variable_get(:@__code)
       metadata[:source_location] = rule.instance_variable_get(:@__source_location)
+      metadata[:waiver_data] = rule.__waiver_data
     end
   end
 end

test/functional/waivers_test.rb

@@ -0,0 +1,104 @@
+require "functional/helper"
+
+describe "waivers" do
+  include FunctionalHelper
+  let(:waivers_profiles_path) { "#{profile_path}/waivers" }
+  let(:run_result) { run_inspec_process(cmd, json: true) }
+  let(:controls_by_id) { run_result.payload.json.dig("profiles", 0, "controls").map { |c| [c["id"], c] }.to_h }
+  let(:cmd) { "exec #{waivers_profiles_path}/#{profile_name} --input-file #{waivers_profiles_path}/#{profile_name}/files/#{waiver_file}" }
+
+  def assert_test_outcome(expected, control_id)
+    assert_equal "#{control_id}_#{expected}", "#{control_id}_#{controls_by_id.dig(control_id, "results", 0, "status")}"
+  end
+
+  def assert_waiver_annotation(control_id)
+    # TODO - test JSON for waiver annotation
+  end
+
+  def refute_waiver_annotation(control_id)
+    # TODO - test JSON for waiver annotation
+    # Don't suppose we get this for free by defining assert_waiver_annotation ...
+  end
+
+  describe "a fully pre-slugged control file" do
+    let(:profile_name) { "basic" }
+    let(:waiver_file) { "waivers.yaml" }
+
+    it "has all of the expected outcomes" do
+      [
+        "01_not_waivered_passes",
+        "03_waivered_no_expiry_not_skipped_passes", # this had a waiver but still passed - no annotation?
+        "06_waivered_expiry_in_past_not_skipped_passes", # a stale waiver
+        "08_waivered_expiry_in_past_skipped", # another stale waiver
+        "09_waivered_expiry_in_future_not_skipped_passes", # unneeded waiver
+      ].each do |control_id|
+        assert_test_outcome "passed", control_id
+        refute_waiver_annotation control_id
+      end
+
+      [
+        "02_not_waivered_fails",
+        "07_waivered_expiry_in_past_not_skipped_fails", # Should this give a special waiver expired message?
+      ].each do |control_id|
+        assert_test_outcome "failed", control_id
+        refute_waiver_annotation control_id
+      end
+
+      # Each of these should have been forced to skip by the waiver system
+      %w{
+        05_waivered_no_expiry_skipped
+        11_waivered_expiry_in_future_skipped
+      }.each do |control_id|
+        assert_test_outcome "skipped", control_id
+        assert_waiver_annotation control_id
+      end
+
+      # Each of these should have had a failure, but had a waiver annotation
+      # added to the output.
+      %w{
+        04_waivered_no_expiry_not_skipped_fails
+        10_waivered_expiry_in_future_not_skipped_fails
+      }.each do |control_id|
+        assert_test_outcome "failed", control_id
+        assert_waiver_annotation control_id
+      end
+    end
+  end
+
+  # describe "an inherited profile"
+  # describe "a profile whose control ids require transformation"
+
+  describe "a waiver file with invalid dates" do
+    let(:profile_name) { "short" }
+    let(:waiver_file) { "bad-date.yaml" }
+    it "gracefully errors" do
+      result = run_result
+      assert_includes "ERROR", result.stdout # the error level
+      assert_includes "01_small", result.stdout # the offending control ID
+      assert_includes "never", result.stdout # The bad value
+      assert_equal 1, result.exit_status
+    end
+  end
+
+  describe "waivers and only_if" do
+    let(:profile_name) { "only_if" }
+
+    describe "when an only_if is used with no waiver" do
+      let(:waiver_file) { "empty.yaml" }
+      it "skips the control with an only_if message" do
+        msg = controls_by_id.dig("01_only_if", "results", 0, "skip_message")
+        assert_includes msg, "due to only_if"
+        refute_includes msg, "waiver"
+      end
+    end
+
+    describe "when both a skipping waiver and an only_if are present" do
+      let(:waiver_file) { "waiver.yaml" }
+      it "skips the control with a waiver message" do
+        msg = controls_by_id.dig("01_only_if", "results", 0, "skip_message")
+        refute_includes msg, "due to only_if"
+        assert_includes msg, "waiver"
+      end
+    end
+  end
+end

test/unit/mock/profiles/waivers/basic/controls/basic.rb

@@ -0,0 +1,43 @@
+control "01_not_waivered_passes" do
+  describe(true) { it { should eq true } }
+end
+
+control "02_not_waivered_fails" do
+  describe(true) { it { should eq false } }
+end
+
+control "03_waivered_no_expiry_not_skipped_passes" do
+  describe(true) { it { should eq true } }
+end
+
+control "04_waivered_no_expiry_not_skipped_fails" do
+  describe(true) { it { should eq false } }
+end
+
+control "05_waivered_no_expiry_skipped" do
+  describe(true) { it { should eq true } }
+end
+
+control "06_waivered_expiry_in_past_not_skipped_passes" do
+  describe(true) { it { should eq true } }
+end
+
+control "07_waivered_expiry_in_past_not_skipped_fails" do
+  describe(true) { it { should eq false } }
+end
+
+control "08_waivered_expiry_in_past_skipped" do
+  describe(true) { it { should eq true } }
+end
+
+control "09_waivered_expiry_in_future_not_skipped_passes" do
+  describe(true) { it { should eq true } }
+end
+
+control "10_waivered_expiry_in_future_not_skipped_fails" do
+  describe(true) { it { should eq false } }
+end
+
+control "11_waivered_expiry_in_future_skipped" do
+  describe(true) { it { should eq true } }
+end

test/unit/mock/profiles/waivers/basic/files/waivers.yaml

@@ -0,0 +1,41 @@
+03_waivered_no_expiry_not_skipped_passes:
+  justification: Sound reasoning
+  skip: no
+
+04_waivered_no_expiry_not_skipped_fails:
+  justification: Unassailable thinking
+  skip: no
+
+05_waivered_no_expiry_skipped:
+  justification: Sheer cleverness
+  skip: yes
+
+06_waivered_expiry_in_past_not_skipped_passes:
+  expiration_date: 1977-06-01
+  justification: Necessity
+  skip: no
+
+07_waivered_expiry_in_past_not_skipped_fails:
+  expiration_date: 1977-06-01
+  justification: Whimsy
+  skip: no
+
+08_waivered_expiry_in_past_skipped:
+  expiration_date: 1977-06-01
+  justification: Contrariness
+  skip: yes
+
+09_waivered_expiry_in_future_not_skipped_passes:
+  expiration_date: 2077-06-01
+  justification: Handwaving
+  skip: no
+
+10_waivered_expiry_in_future_not_skipped_fails:
+  expiration_date: 2077-06-01
+  justification: Didn't feel like it
+  skip: no
+
+11_waivered_expiry_in_future_skipped:
+  expiration_date: 2077-06-01
+  justification: Lack of imagination
+  skip: yes

test/unit/mock/profiles/waivers/basic/inspec.yml

@@ -0,0 +1,6 @@
+name: basic
+license: Apache-2.0
+summary: A profile that demonstrates basic usage of the waiver system
+version: 0.1.0
+supports:
+  platform: os

test/unit/mock/profiles/waivers/only_if/controls/only_if_controls.rb

@@ -0,0 +1,9 @@
+
+# This fixture will cause a "skip due to only if" if waivers are
+# not working correctly (should be waivered)
+control "01_only_if" do
+  only_if("test_message_from_dsl") { false }
+  describe true do
+    it { should eq true }
+  end
+end

test/unit/mock/profiles/waivers/only_if/files/waiver.yaml

@@ -0,0 +1,3 @@
+01_only_if:
+  skip: true
+  justification: test_message_from_waiver

test/unit/mock/profiles/waivers/only_if/inspec.yml

@@ -0,0 +1,5 @@
+name: only_if
+summary: Verifies waiver/only_if precedence
+version: 0.1.0
+supports:
+  platform: os

test/unit/mock/profiles/waivers/small/controls/small.rb

@@ -0,0 +1,5 @@
+control "01_small" do
+  describe true do
+    it { should eq true }
+  end
+end

test/unit/mock/profiles/waivers/small/files/bad-date.yaml

@@ -0,0 +1,4 @@
+01_small:
+  expiration_date: never
+  skip: true
+  justification: Callous disregard

test/unit/mock/profiles/waivers/small/inspec.yml

@@ -0,0 +1,5 @@
+name: small
+summary: Test profile for running bad waiver files through InSpec
+version: 0.1.0
+supports:
+  platform: os