Fix for --controls option is not working as expected. #5434

Vasu1105 · 2021-03-17T14:35:11Z

Signed-off-by: Vasu1105 vasundhara.jagdale@chef.io

Description

As mentioned in issue #5215 the control block also gets evaluated and due to which the --controls option is not working as expected.
The reason behind that the filtering of control here https://github.com/inspec/inspec/blob/master/lib/inspec/profile.rb#L228 is happening after evaluating the controls which are happening here https://github.com/inspec/inspec/blob/master/lib/inspec/profile.rb#L220

which calls this

inspec/lib/inspec/profile_context.rb

Line 154 in 2f41c16

load_with_context(control_eval_context, *args)

and then this

inspec/lib/inspec/profile_context.rb

Line 162 in 2f41c16

def load_with_context(context, content, source = nil, line = nil)

which then calls this control definition

inspec/lib/inspec/control_eval_context.rb

Line 54 in 2f41c16

def control(id, opts = {}, &block)

and describe definition (descrhttps://github.com/inspec/inspec/blob/2f41c16ad444eaf69f7ec77f4ff681f35aad9210/lib/inspec/control_eval_context.rb#L66

both these methods instantiate Inspec::rule

inspec/lib/inspec/rule.rb

Line 46 in 2f41c16

instance_eval(&block)

which basically causing the skipped control is getting evaluated (note not the describe block)

I have added a method in the control_eval_context.rb to filter the controls before evaluating it. I need to add the filter in describe method also because when we are having only describe block without control block the methods creates auto-generated control as described here

inspec/lib/inspec/control_eval_context.rb

Line 63 in 2f41c16

# generate a control for them automatically and then execute

so I am making sure those describe blocks are also get filtered when specifying --controls option.

Related Issue

Fixes #5215

Types of changes

Bug fix (non-breaking change which fixes an issue)
New content (non-breaking change)
Breaking change (a content change which would break existing functionality or processes)

Checklist:

I have read the CONTRIBUTING document.

netlify · 2021-03-17T14:35:15Z

Deploy preview for chef-inspec canceled.

Built with commit 513c7be

https://app.netlify.com/sites/chef-inspec/deploys/60548abb0c84330007964c14

clintoncwolfe · 2021-03-17T17:25:33Z

test/functional/inspec_exec_test.rb

@@ -178,6 +178,25 @@ def stderr
    assert_exit_code 100, out
  end

+  it "executes only specified controls when selecting the controls by literal names" do
+    inspec("exec " + File.join(profile_path, "filter_table") + " --no-create-lockfile --controls foo")


Why did you choose the filter_table fixture? I don't think this problem has anything to do with FilterTable. I would suggest either finding a different profile, or creating a new one just for this purpose.

Yes after checking the impact decided to remove this from here. I thought since we are using --controls options in test with this profile that will be good place to include. But yes definitely this is not the right place to have this. Making the required changes.

clintoncwolfe · 2021-03-17T17:28:39Z

test/functional/inspec_exec_test.rb

+Version: 0.1.0
+Target:  local://
+
+  ✔  foo: a thing


This is likely too brittle of a test - the check marks vary under Windows, for example. I suggest you write tests that look specifically for what you are looking for. For example,

_(out.stdout).wont_include "bar" _(out.stdout).wont_include "baz"

clintoncwolfe · 2021-03-18T20:23:21Z

test/functional/inspec_exec_test.rb

+    assert_exit_code 0, out
+  end
+
+  it "executes only specified controls when selecting the controls by literal names" do


This selects by regex, not by literal name

clintoncwolfe · 2021-03-18T20:24:37Z

test/fixtures/profiles/controls-option-test/controls/example.rb

+title '/ profile'
+
+# you add controls here
+control 'tmp-1.0' do                                   # A unique ID for this control
+  impact 0.7                                           # The criticality, if this control fails.
+  title 'Create / directory'                        # A human-readable title
+  desc 'An optional description...'                    # Describe why this is needed
+  desc 'label', 'An optional description with a label' # Pair a part of the description with a label
+  tag data: 'temp data'                                # A tag allows you to associate key information
+  tag 'security'                                       # to the test
+  ref 'Document A-12', url: 'http://...'               # Additional references
+
+  describe file('/') do                             # The actual test
+    it { should be_directory }
+  end
+end
+
+# you can also use plain tests
+describe file('/') do
+  it { should be_directory }
+end


This file has a lot of extra material in it that is not being tested. Either it should be removed or it should be tested - fixture files should be minimal. Thanks!

Thanks @clintoncwolfe .Did the required changes.

clintoncwolfe

Just a couple of minor notes about the test fixture file, nit-picking really. This works really well. I think users will be very happy with this improvement!

…option Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

This reverts commit 2048c4d.

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

…ing the control block of not included controls in the controls option as it was filtering the controls from the list after evaluating updated the logic so that it gets evaluated after filtering. Removed filter_controls methods as no more using that Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

… getting evaluated as filtering of the control was happening after evaluating so added the filter logic in the control_eval_context. Also when we have describe block outside control block we are we generating a control for them automatically and then execute due that also becomes a control and has to add same logic to filter the control in that mehtod Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

…_list_exist? Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

…with some random hex which might contain the number that I have used in the rgex due to which test is failing Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

klose4711 · 2021-03-19T18:08:35Z

Thanks for fixing this! 🙂

Vasu1105 requested a review from a team as a code owner March 17, 2021 14:35

Vasu1105 requested review from alexpop and Nik08 March 17, 2021 14:35

chef-expeditor bot assigned Vasu1105 Mar 17, 2021

clintoncwolfe reviewed Mar 17, 2021

View reviewed changes

Vasu1105 requested a review from IanMadd as a code owner March 18, 2021 08:43

Vasu1105 changed the title ~~[WIP] Fix for when -controls option is not working as expected.~~ Fix for when -controls option is not working as expected. Mar 18, 2021

clintoncwolfe reviewed Mar 18, 2021

View reviewed changes

clintoncwolfe approved these changes Mar 18, 2021

View reviewed changes

clintoncwolfe added Aspect: Performance Nimble is delightful. Type: Bug Feature not working as expected Component: CLI: exec labels Mar 18, 2021

Vasu1105 added 12 commits March 19, 2021 16:10

Added test to catch the control execution issue while using controls …

6853f64

…option Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Updated test to match the exact string

75ca22c

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Revert "Updated test to match the exact string"

e1e1073

This reverts commit 2048c4d.

Fixed test file for syntax issues

89e994c

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Fixing test cases

b4d3196

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Removed readme file from test profile

df8d589

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

minor code fix

2dcd174

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Fixed failing test in inputs_test.rb

9a404f5

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Updated test for --controls option

b180914

Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Added code comments and rename control_list_exist? method to controls…

428a8a7

…_list_exist? Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Vasu1105 force-pushed the vasundhara/fix-for-controls-option branch from 982ba44 to 428a8a7 Compare March 19, 2021 10:40

Added specific regex as for describe block we genrate random control …

513c7be

…with some random hex which might contain the number that I have used in the rgex due to which test is failing Signed-off-by: Vasu1105 <vasundhara.jagdale@chef.io>

Vasu1105 changed the title ~~Fix for when -controls option is not working as expected.~~ Fix for -controls option is not working as expected. Mar 19, 2021

clintoncwolfe merged commit 5378a51 into master Mar 19, 2021

clintoncwolfe deleted the vasundhara/fix-for-controls-option branch March 19, 2021 14:16

Vasu1105 restored the vasundhara/fix-for-controls-option branch March 25, 2021 14:52

This was referenced Apr 6, 2021

All controls in profile evaluated when controls are selected using --controls #2857

Closed

Fix skipping of controls to actually do what it says #5068

Open

Vasu1105 mentioned this pull request May 7, 2021

Inheritance - exec with --controls filters out required dependent controls #4318

Closed

ejaronne mentioned this pull request Aug 31, 2021

--controls does not support dependent profiles #5647

Closed

Vasu1105 changed the title ~~Fix for -controls option is not working as expected.~~ Fix for --controls option is not working as expected. Oct 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for --controls option is not working as expected. #5434

Fix for --controls option is not working as expected. #5434

Vasu1105 commented Mar 17, 2021 •

edited

netlify bot commented Mar 17, 2021 •

edited

clintoncwolfe Mar 17, 2021

Vasu1105 Mar 18, 2021 •

edited

clintoncwolfe Mar 17, 2021

clintoncwolfe Mar 18, 2021

Vasu1105 Mar 19, 2021

clintoncwolfe Mar 18, 2021

Vasu1105 Mar 19, 2021

clintoncwolfe left a comment

klose4711 commented Mar 19, 2021

Fix for --controls option is not working as expected. #5434

Fix for --controls option is not working as expected. #5434

Conversation

Vasu1105 commented Mar 17, 2021 • edited

Description

Related Issue

Types of changes

Checklist:

netlify bot commented Mar 17, 2021 • edited

clintoncwolfe Mar 17, 2021

Choose a reason for hiding this comment

Vasu1105 Mar 18, 2021 • edited

Choose a reason for hiding this comment

clintoncwolfe Mar 17, 2021

Choose a reason for hiding this comment

clintoncwolfe Mar 18, 2021

Choose a reason for hiding this comment

Vasu1105 Mar 19, 2021

Choose a reason for hiding this comment

clintoncwolfe Mar 18, 2021

Choose a reason for hiding this comment

Vasu1105 Mar 19, 2021

Choose a reason for hiding this comment

clintoncwolfe left a comment

Choose a reason for hiding this comment

klose4711 commented Mar 19, 2021

Vasu1105 commented Mar 17, 2021 •

edited

netlify bot commented Mar 17, 2021 •

edited

Vasu1105 Mar 18, 2021 •

edited