Refactor Automate reporter.

Signed-off-by: Pete Higgins <pete@peterhiggins.org>

2 files changed, 200 insertions, 140 deletions

diff --git a/lib/chef/audit/reporter/automate.rb b/lib/chef/audit/reporter/automate.rb
index e90325f8b7..9921faf7b7 100644
--- a/lib/chef/audit/reporter/automate.rb
+++ b/lib/chef/audit/reporter/automate.rb
@@ -47,14 +47,11 @@ class Chef
             "x-data-collector-token" => @token,
           }
 
-          all_report_shas = report.fetch(:profiles, []).map { |p| p[:sha256] }
+          all_report_shas = report[:profiles].map { |p| p[:sha256] }
           missing_report_shas = missing_automate_profiles(headers, all_report_shas)
 
           full_report = truncate_controls_results(enriched_report(report), @control_results_limit)
-
-          # If the Automate backend has the profile metadata for at least one profile, proceed with metadata stripping
-          # TODO: Fix hardcoded 1
-          full_report = strip_profiles_meta(full_report, missing_report_shas, 1) if missing_report_shas.length < all_report_shas.length
+          full_report = strip_profiles_meta(full_report, missing_report_shas, @run_time_limit)
           json_report = Chef::JSONCompat.to_json(full_report, validate_utf8: false)
 
           # Automate GRPC currently has a message limit of ~4MB
@@ -167,33 +164,28 @@ class Chef
           report_shas
         end
 
-        # TODO: cleanup
+        # Profile 'name' is a required property.
+        # By not sending it in the report, we make it clear to the ingestion backend that the profile metadata has been stripped from this profile in the report.
+        # Profile 'title' and 'version' are still kept for troubleshooting purposes in the backend.
+        SEEN_PROFILE_UNNECESSARY_FIELDS = %i{ copyright copyright_email groups license maintainer name summary supports}.freeze
+
+        SEEN_PROFILE_UNNECESSARY_CONTROL_FIELDS = %i{ code desc descriptions impact refs source_location tags title }.freeze
+
+        # TODO: This mutates the report and probably doesn't need to.
         def strip_profiles_meta(report, missing_report_shas, run_time_limit)
-          return report unless report.is_a?(Hash) && report[:profiles].is_a?(Array)
           report[:profiles].each do |p|
             next if missing_report_shas.include?(p[:sha256])
-            # Profile 'name' is a required property. By not sending it in the report, we make it clear to the ingestion backend that the profile metadata has been stripped from this profile in the report.
-            # Profile 'title' and 'version' are still kept for troubleshooting purposes in the backend.
-            p.delete(:name)
-            p.delete(:groups)
-            p.delete(:copyright_email)
-            p.delete(:copyright)
-            p.delete(:summary)
-            p.delete(:supports)
-            p.delete(:license)
-            p.delete(:maintainer)
+
+            p.delete_if { |f| SEEN_PROFILE_UNNECESSARY_FIELDS.include?(f) }
+
             next unless p[:controls].is_a?(Array)
+
             p[:controls].each do |c|
-              c.delete(:code)
-              c.delete(:desc)
-              c.delete(:descriptions)
-              c.delete(:impact)
-              c.delete(:refs)
-              c.delete(:tags)
-              c.delete(:title)
-              c.delete(:source_location)
+              c.delete_if { |f| SEEN_PROFILE_UNNECESSARY_CONTROL_FIELDS.include?(f) }
               c.delete(:waiver_data) if c[:waiver_data] == {}
+
               next unless c[:results].is_a?(Array)
+
               c[:results].each do |r|
                 if r[:run_time].is_a?(Float) && r[:run_time] < run_time_limit
                   r.delete(:start_time)
diff --git a/spec/unit/audit/reporter/automate_spec.rb b/spec/unit/audit/reporter/automate_spec.rb
index 4c6e379392..d55bea282f 100644
--- a/spec/unit/audit/reporter/automate_spec.rb
+++ b/spec/unit/audit/reporter/automate_spec.rb
@@ -2,14 +2,6 @@ require "spec_helper"
 require "json" # For .to_json
 
 describe Chef::Audit::Reporter::Automate do
-  before :each do
-    WebMock.disable_net_connect!
-
-    Chef::Config[:data_collector] = { token: token, server_url: "https://automate.test/data_collector" }
-  end
-
-  let(:token) { "fake_token" }
-
   let(:reporter) { Chef::Audit::Reporter::Automate.new(opts) }
 
   let(:opts) do
@@ -29,7 +21,7 @@ describe Chef::Audit::Reporter::Automate do
         ipaddress: "192.168.56.33",
         fqdn: "lb1.prod.example.com",
       },
-      run_time_limit: 1.0,
+      run_time_limit: 1.1,
       control_results_limit: 2,
       timestamp: Time.parse("2016-07-19T19:19:19+01:00"),
     }
@@ -83,83 +75,17 @@ describe Chef::Audit::Reporter::Automate do
     }
   end
 
-  let(:enriched_report) do
-    {
-      "version": "1.2.1",
-      "profiles": [
-        {
-          "name": "tmp_compliance_profile",
-          "title": "/tmp Compliance Profile",
-          "summary": "An Example Compliance Profile",
-          "sha256": "7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd",
-          "version": "0.1.1",
-          "maintainer": "Nathen Harvey <nharvey@chef.io>",
-          "license": "Apache 2.0 License",
-          "copyright": "Nathen Harvey <nharvey@chef.io>",
-          "supports": [],
-          "controls": [
-            {
-              "title": "A /tmp directory must exist",
-              "desc": "A /tmp directory must exist",
-              "impact": 0.3,
-              "refs": [],
-              "tags": {},
-              "code": "control 'tmp-1.0' do\n  impact 0.3\n  title 'A /tmp directory must exist'\n  desc 'A /tmp directory must exist'\n  describe file '/tmp' do\n    it { should be_directory }\n  end\nend\n",
-              "source_location": { "ref": "/Users/vjeffrey/code/delivery/insights/data_generator/chef-client/cache/cookbooks/test-cookbook/recipes/../files/default/compliance_profiles/tmp_compliance_profile/controls/tmp.rb", "line": 3 },
-              "id": "tmp-1.0",
-              "results": [
-                { "status": "passed", "code_desc": "File /tmp should be directory", "run_time": 0.002312, "start_time": "2016-10-19 11:09:43 -0400" },
-              ],
-            },
-            {
-              "title": "/tmp directory is owned by the root user",
-              "desc": "The /tmp directory must be owned by the root user",
-              "impact": 0.3,
-              "refs": [
-                { "url": "https://pages.chef.io/rs/255-VFB-268/images/compliance-at-velocity2015.pdf", "ref": "Compliance Whitepaper" },
-              ],
-              "tags": { "production": nil, "development": nil, "identifier": "value", "remediation": "https://github.com/chef-cookbooks/audit" },
-              "code": "control 'tmp-1.1' do\n  impact 0.3\n  title '/tmp directory is owned by the root user'\n  desc 'The /tmp directory must be owned by the root user'\n  tag 'production','development'\n  tag identifier: 'value'\n  tag remediation: 'https://github.com/chef-cookbooks/audit'\n  ref 'Compliance Whitepaper', url: 'https://pages.chef.io/rs/255-VFB-268/images/compliance-at-velocity2015.pdf'\n  describe file '/tmp' do\n    it { should be_owned_by 'root' }\n  end\nend\n",
-              "source_location": { "ref": "/Users/vjeffrey/code/delivery/insights/data_generator/chef-client/cache/cookbooks/test-cookbook/recipes/../files/default/compliance_profiles/tmp_compliance_profile/controls/tmp.rb", "line": 12 },
-              "id": "tmp-1.1",
-              "results": [
-                { "status": "failed", "code_desc": "File /etc/hosts is expected to be directory", "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400", "message": "expected `File /etc/hosts.directory?` to return true, got false" },
-                { "status": "skipped", "code_desc": 'File /tmp should be owned by "root"', "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400" },
-              ],
-              "removed_results_counts": { "failed": 0, "skipped": 0, "passed": 1 },
-            },
-          ],
-          "groups": [
-            { "title": "/tmp Compliance Profile", "controls": ["tmp-1.0", "tmp-1.1"], "id": "controls/tmp.rb" },
-          ],
-          "attributes": [
-            { "name": "syslog_pkg", "options": { "default": "rsyslog", "description": "syslog package..." } },
-          ],
-        },
-      ],
-      "other_checks": [],
-      "statistics": { "duration": 0.032332 },
-      "type": "inspec_report",
-      "node_name": "chef-client.solo",
-      "end_time": "2016-07-19T18:19:19Z",
-      "node_uuid": "aaaaaaaa-709a-475d-bef5-zzzzzzzzzzzz",
-      "environment": "My Prod Env",
-      "roles": %w{base_linux apache_linux},
-      "recipes": ["some_cookbook::some_recipe", "some_cookbook"],
-      "report_uuid": "3f0536f7-3361-4bca-ae53-b45118dceb5d",
-      "source_fqdn": "api.chef.io",
-      "organization_name": "test_org",
-      "policy_group": "test_policy_group",
-      "policy_name": "test_policy_name",
-      "chef_tags": ["mylinux", "my.tag", "some=tag"],
-      "ipaddress": "192.168.56.33",
-      "fqdn": "lb1.prod.example.com",
-    }
-  end
-
   describe "#send_report" do
-    let!(:metasearch_stub) do
-      stub_request(:post, "https://automate.test/compliance/profiles/metasearch")
+    before :each do
+      WebMock.disable_net_connect!
+
+      Chef::Config[:data_collector] = { token: token, server_url: "https://automate.test/data_collector" }
+    end
+
+    let(:token) { "fake_token" }
+
+    it "sends report successfully to ChefAutomate with missing profiles" do
+      metasearch_stub = stub_request(:post, "https://automate.test/compliance/profiles/metasearch")
         .with(
           body: '{"sha256": ["7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd"]}',
           headers: {
@@ -172,12 +98,81 @@ describe Chef::Audit::Reporter::Automate do
           status: 200,
           body: '{"missing_sha256": ["7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd"]}'
         )
-    end
 
-    let!(:report_stub) do
-      stub_request(:post, "https://automate.test/data_collector")
+      report_stub = stub_request(:post, "https://automate.test/data_collector")
         .with(
-          body: enriched_report,
+          body: {
+            "version": "1.2.1",
+            "profiles": [
+              {
+                "name": "tmp_compliance_profile",
+                "title": "/tmp Compliance Profile",
+                "summary": "An Example Compliance Profile",
+                "sha256": "7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd",
+                "version": "0.1.1",
+                "maintainer": "Nathen Harvey <nharvey@chef.io>",
+                "license": "Apache 2.0 License",
+                "copyright": "Nathen Harvey <nharvey@chef.io>",
+                "supports": [],
+                "controls": [
+                  {
+                    "title": "A /tmp directory must exist",
+                    "desc": "A /tmp directory must exist",
+                    "impact": 0.3,
+                    "refs": [],
+                    "tags": {},
+                    "code": "control 'tmp-1.0' do\n  impact 0.3\n  title 'A /tmp directory must exist'\n  desc 'A /tmp directory must exist'\n  describe file '/tmp' do\n    it { should be_directory }\n  end\nend\n",
+                    "source_location": { "ref": "/Users/vjeffrey/code/delivery/insights/data_generator/chef-client/cache/cookbooks/test-cookbook/recipes/../files/default/compliance_profiles/tmp_compliance_profile/controls/tmp.rb", "line": 3 },
+                    "id": "tmp-1.0",
+                    "results": [
+                      { "status": "passed", "code_desc": "File /tmp should be directory", "run_time": 0.002312, "start_time": "2016-10-19 11:09:43 -0400" },
+                    ],
+                  },
+                  {
+                    "title": "/tmp directory is owned by the root user",
+                    "desc": "The /tmp directory must be owned by the root user",
+                    "impact": 0.3,
+                    "refs": [
+                      { "url": "https://pages.chef.io/rs/255-VFB-268/images/compliance-at-velocity2015.pdf", "ref": "Compliance Whitepaper" },
+                    ],
+                    "tags": { "production": nil, "development": nil, "identifier": "value", "remediation": "https://github.com/chef-cookbooks/audit" },
+                    "code": "control 'tmp-1.1' do\n  impact 0.3\n  title '/tmp directory is owned by the root user'\n  desc 'The /tmp directory must be owned by the root user'\n  tag 'production','development'\n  tag identifier: 'value'\n  tag remediation: 'https://github.com/chef-cookbooks/audit'\n  ref 'Compliance Whitepaper', url: 'https://pages.chef.io/rs/255-VFB-268/images/compliance-at-velocity2015.pdf'\n  describe file '/tmp' do\n    it { should be_owned_by 'root' }\n  end\nend\n",
+                    "source_location": { "ref": "/Users/vjeffrey/code/delivery/insights/data_generator/chef-client/cache/cookbooks/test-cookbook/recipes/../files/default/compliance_profiles/tmp_compliance_profile/controls/tmp.rb", "line": 12 },
+                    "id": "tmp-1.1",
+                    "results": [
+                      { "status": "failed", "code_desc": "File /etc/hosts is expected to be directory", "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400", "message": "expected `File /etc/hosts.directory?` to return true, got false" },
+                      { "status": "skipped", "code_desc": 'File /tmp should be owned by "root"', "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400" },
+                    ],
+                    "removed_results_counts": { "failed": 0, "skipped": 0, "passed": 1 },
+                  },
+                ],
+                "groups": [
+                  { "title": "/tmp Compliance Profile", "controls": ["tmp-1.0", "tmp-1.1"], "id": "controls/tmp.rb" },
+                ],
+                "attributes": [
+                  { "name": "syslog_pkg", "options": { "default": "rsyslog", "description": "syslog package..." } },
+                ],
+              },
+            ],
+            "other_checks": [],
+            "statistics": { "duration": 0.032332 },
+            "type": "inspec_report",
+            "node_name": "chef-client.solo",
+            "end_time": "2016-07-19T18:19:19Z",
+            "node_uuid": "aaaaaaaa-709a-475d-bef5-zzzzzzzzzzzz",
+            "environment": "My Prod Env",
+            "roles": %w{base_linux apache_linux},
+            "recipes": ["some_cookbook::some_recipe", "some_cookbook"],
+            "report_uuid": "3f0536f7-3361-4bca-ae53-b45118dceb5d",
+            "source_fqdn": "api.chef.io",
+            "organization_name": "test_org",
+            "policy_group": "test_policy_group",
+            "policy_name": "test_policy_name",
+            "chef_tags": ["mylinux", "my.tag", "some=tag"],
+            "ipaddress": "192.168.56.33",
+            "fqdn": "lb1.prod.example.com",
+            "run_time_limit": 1.1,
+          },
           headers: {
             "Accept-Encoding" => "identity",
             "X-Chef-Version" => Chef::VERSION,
@@ -185,9 +180,85 @@ describe Chef::Audit::Reporter::Automate do
             "X-Data-Collector-Token" => token,
           }
         ).to_return(status: 200)
+
+      expect(reporter.send_report(inspec_report)).to eq(true)
+
+      expect(metasearch_stub).to have_been_requested
+      expect(report_stub).to have_been_requested
     end
 
-    it "sends report successfully to ChefAutomate" do
+    it "sends report successfully to ChefAutomate with seen profiles" do
+      metasearch_stub = stub_request(:post, "https://automate.test/compliance/profiles/metasearch")
+        .with(
+          body: '{"sha256": ["7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd"]}',
+          headers: {
+            "Accept-Encoding" => "identity",
+            "X-Chef-Version" => Chef::VERSION,
+            "X-Data-Collector-Auth" => "version=1.0",
+            "X-Data-Collector-Token" => token,
+          }
+        ).to_return(
+          status: 200,
+          body: '{"missing_sha256": []}'
+        )
+
+      report_stub = stub_request(:post, "https://automate.test/data_collector")
+        .with(
+          body: {
+            "version": "1.2.1",
+            "profiles": [
+              {
+                "title": "/tmp Compliance Profile",
+                "sha256": "7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd",
+                "version": "0.1.1",
+                "controls": [
+                  {
+                    "id": "tmp-1.0",
+                    "results": [
+                      { "status": "passed", "code_desc": "File /tmp should be directory" },
+                    ],
+                  },
+                  {
+                    "id": "tmp-1.1",
+                    "results": [
+                      { "status": "failed", "code_desc": "File /etc/hosts is expected to be directory", "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400", "message": "expected `File /etc/hosts.directory?` to return true, got false" },
+                      { "status": "skipped", "code_desc": 'File /tmp should be owned by "root"', "run_time": 1.228845, "start_time": "2016-10-19 11:09:43 -0400" },
+                    ],
+                    "removed_results_counts": { "failed": 0, "skipped": 0, "passed": 1 },
+                  },
+                ],
+                "attributes": [
+                  { "name": "syslog_pkg", "options": { "default": "rsyslog", "description": "syslog package..." } },
+                ],
+              },
+            ],
+            "other_checks": [],
+            "statistics": { "duration": 0.032332 },
+            "type": "inspec_report",
+            "node_name": "chef-client.solo",
+            "end_time": "2016-07-19T18:19:19Z",
+            "node_uuid": "aaaaaaaa-709a-475d-bef5-zzzzzzzzzzzz",
+            "environment": "My Prod Env",
+            "roles": %w{base_linux apache_linux},
+            "recipes": ["some_cookbook::some_recipe", "some_cookbook"],
+            "report_uuid": "3f0536f7-3361-4bca-ae53-b45118dceb5d",
+            "source_fqdn": "api.chef.io",
+            "organization_name": "test_org",
+            "policy_group": "test_policy_group",
+            "policy_name": "test_policy_name",
+            "chef_tags": ["mylinux", "my.tag", "some=tag"],
+            "ipaddress": "192.168.56.33",
+            "fqdn": "lb1.prod.example.com",
+            "run_time_limit": 1.1,
+          },
+          headers: {
+            "Accept-Encoding" => "identity",
+            "X-Chef-Version" => Chef::VERSION,
+            "X-Data-Collector-Auth" => "version=1.0",
+            "X-Data-Collector-Token" => token,
+          }
+        ).to_return(status: 200)
+
       expect(reporter.send_report(inspec_report)).to eq(true)
 
       expect(metasearch_stub).to have_been_requested
@@ -198,9 +269,6 @@ describe Chef::Audit::Reporter::Automate do
       opts.delete(:entity_uuid)
       reporter = Chef::Audit::Reporter::Automate.new(opts)
       expect(reporter.send_report(inspec_report)).to eq(false)
-
-      expect(metasearch_stub).not_to have_been_requested
-      expect(report_stub).not_to have_been_requested
     end
   end
 
@@ -288,72 +356,72 @@ describe Chef::Audit::Reporter::Automate do
   end
 
   describe "#strip_profiles_meta" do
-    it 'removes the metadata from seen profiles' do
+    it "removes the metadata from seen profiles" do
       expected = {
         other_checks: [],
         profiles: [
           {
             attributes: [
               {
-                name: 'syslog_pkg',
+                name: "syslog_pkg",
                 options: {
-                  default: 'rsyslog',
-                  description: 'syslog package...',
+                  default: "rsyslog",
+                  description: "syslog package...",
                 },
               },
             ],
             controls: [
               {
-                id: 'tmp-1.0',
+                id: "tmp-1.0",
                 results: [
                   {
-                    code_desc: 'File /tmp should be directory',
-                    status: 'passed',
+                    code_desc: "File /tmp should be directory",
+                    status: "passed",
                   },
                 ],
               },
               {
-                id: 'tmp-1.1',
+                id: "tmp-1.1",
                 results: [
                   {
                     code_desc: 'File /tmp should be owned by "root"',
                     run_time: 1.228845,
-                    start_time: '2016-10-19 11:09:43 -0400',
-                    status: 'passed',
+                    start_time: "2016-10-19 11:09:43 -0400",
+                    status: "passed",
                   },
                   {
                     code_desc: 'File /tmp should be owned by "root"',
                     run_time: 1.228845,
-                    start_time: '2016-10-19 11:09:43 -0400',
-                    status: 'skipped',
+                    start_time: "2016-10-19 11:09:43 -0400",
+                    status: "skipped",
                   },
                   {
-                    code_desc: 'File /etc/hosts is expected to be directory',
-                    message: 'expected `File /etc/hosts.directory?` to return true, got false',
+                    code_desc: "File /etc/hosts is expected to be directory",
+                    message: "expected `File /etc/hosts.directory?` to return true, got false",
                     run_time: 1.228845,
-                    start_time: '2016-10-19 11:09:43 -0400',
-                    status: 'failed',
+                    start_time: "2016-10-19 11:09:43 -0400",
+                    status: "failed",
                   },
                 ],
               },
             ],
-            sha256: '7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd',
-            title: '/tmp Compliance Profile',
-            version: '0.1.1',
+            sha256: "7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd",
+            title: "/tmp Compliance Profile",
+            version: "0.1.1",
           },
         ],
         run_time_limit: 1.1,
         statistics: {
           duration: 0.032332,
         },
-        version: '1.2.1',
+        version: "1.2.1",
       }
       expect(reporter.strip_profiles_meta(inspec_report, [], 1.1)).to eq(expected)
     end
 
-    it 'does not remove the metadata from missing profiles' do
+    it "does not remove the metadata from missing profiles" do
       expected = inspec_report.merge(run_time_limit: 1.1)
-      expect(reporter.strip_profiles_meta(inspec_report, ['7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd'], 1.1)).to eq(expected)
+      expect(reporter.strip_profiles_meta(inspec_report, ["7bd598e369970002fc6f2d16d5b988027d58b044ac3fa30ae5fc1b8492e215cd"], 1.1)).to eq(expected)
     end
   end
 end