Merge pull request #258 from xprazak2/missing-arf

Fixes #19552 - Fix search for hosts without arf report

app/models/concerns/foreman_openscap/host_extensions.rb

@@ -35,15 +35,11 @@ module HostExtensions
       }
 
       scope :policy_reports_missing, lambda { |policy|
-        where("id NOT IN (SELECT host_id
-                          FROM reports INNER JOIN foreman_openscap_policy_arf_reports
-                              ON reports.id = foreman_openscap_policy_arf_reports.arf_report_id
-                          WHERE policy_id = #{policy.id})
-              AND id IN (SELECT assetable_id
-                         FROM foreman_openscap_asset_policies INNER JOIN foreman_openscap_assets
-                              ON foreman_openscap_asset_policies.asset_id = foreman_openscap_assets.id
-                         WHERE foreman_openscap_assets.assetable_type = 'Host::Base'
-                               AND foreman_openscap_asset_policies.policy_id = '#{policy.id}')")
+        search_for("compliance_report_missing_for = \"#{policy.name}\"")
+      }
+
+      scope :assigned_to_policy, lambda { |policy|
+        search_for("compliance_policy = \"#{policy.name}\"")
       }
 
       alias_method_chain :inherited_attributes, :openscap
@@ -104,20 +100,44 @@ def compliance_status_label(options = {})
     module ClassMethods
       def search_by_policy_name(key, operator, policy_name)
         cond = sanitize_sql_for_conditions(["foreman_openscap_policies.name #{operator} ?", value_to_sql(operator, policy_name)])
-        { :conditions => Host::Managed.arel_table[:id].in(Host::Managed.select(Host::Managed.arel_table[:id]).joins(:policies).where(cond).pluck(:id)).to_sql }
+
+        host_group_host_ids = policy_assigned_using_hostgroup_host_ids cond, []
+        host_group_cond = if host_group_host_ids.any?
+                            ' OR ' + sanitize_sql_for_conditions("hosts.id IN (#{host_group_host_ids.join(',')})")
+                          else
+                            ''
+                          end
+        { :conditions => Host::Managed.arel_table[:id].in(Host::Managed.select(Host::Managed.arel_table[:id]).joins(:policies).where(cond).pluck(:id)).to_sql + host_group_cond }
       end
 
       def search_by_missing_arf(key, operator, policy_name)
         cond = sanitize_sql_for_conditions(["foreman_openscap_policies.name #{operator} ?", value_to_sql(operator, policy_name)])
-        { :conditions => Host::Managed.arel_table[:id].in(Host::Managed.select(Host::Managed.arel_table[:id])
-            .joins(:policies)
-            .where(cond)
-            .where("foreman_openscap_assets.id NOT IN (
-                     SELECT DISTINCT foreman_openscap_arf_reports.asset_id
-                     FROM foreman_openscap_arf_reports
-                     WHERE foreman_openscap_arf_reports.asset_id = foreman_openscap_assets.id
-                         AND foreman_openscap_arf_reports.policy_id = foreman_openscap_policies.id)")
-            .pluck(:id)).to_sql}
+
+        host_ids_from_arf_of_policy = ForemanOpenscap::ArfReport.joins(:policy).where(cond).pluck(:host_id).uniq
+
+        direct_result = policy_assigned_directly_host_ids cond, host_ids_from_arf_of_policy
+
+        hg_result = policy_assigned_using_hostgroup_host_ids cond, host_ids_from_arf_of_policy
+
+        result = (direct_result + hg_result).uniq
+        { :conditions => "hosts.id IN (#{result.empty? ? 'NULL' : result.join(',')})" }
+      end
+
+      def policy_assigned_directly_host_ids(condition, host_ids_from_arf)
+        ForemanOpenscap::Asset.where(:assetable_type => 'Host::Base')
+                              .joins(:policies)
+                              .where(condition)
+                              .where.not(:assetable_id => host_ids_from_arf)
+                              .pluck(:assetable_id)
+      end
+
+      def policy_assigned_using_hostgroup_host_ids(condition, host_ids_from_arf)
+        hostgroup_with_policy_ids = ForemanOpenscap::Asset.where(:assetable_type => 'Hostgroup')
+                                                          .joins(:policies)
+                                                          .where(condition)
+                                                          .pluck(:assetable_id)
+        subtree_ids = Hostgroup.where(:id => hostgroup_with_policy_ids).flat_map(&:subtree_ids).uniq
+        Host.where(:hostgroup_id => subtree_ids).where.not(:id => host_ids_from_arf).pluck(:id)
       end
     end
   end

app/services/foreman_openscap/policy_dashboard/data.rb

@@ -17,13 +17,14 @@ def hosts
     end
 
     def fetch_data
+      assigned_count = Host::Managed.assigned_to_policy(@policy).count
       report.update(
         {:compliant_hosts => Host::Managed.comply_with(@policy).count,
          :incompliant_hosts => Host::Managed.incomply_with(@policy).count,
          :inconclusive_hosts => Host::Managed.inconclusive_with(@policy).count,
          :report_missing => Host::Managed.policy_reports_missing(@policy).count,
-         :assigned_hosts => @policy.assets.hosts.count,
-         :unassigned_hosts => hosts.count - @policy.assets.hosts.count}
+         :assigned_hosts => assigned_count,
+         :unassigned_hosts => hosts.count - assigned_count}
       )
     end
   end

app/views/policy_dashboard/_policy_status_widget.html.erb

@@ -1,12 +1,12 @@
 <div id='status-table'>
   <h4 class="header"><%= _('Hosts Breakdown') %></h4>
   <ul>
-    <%= status_link _('Compliant with the policy'), :compliant_hosts, arf_reports_path(:search => "comply_with = #{@policy.name}") %>
-    <%= status_link _('Not compliant with the policy'), :incompliant_hosts, arf_reports_path(:search => "not_comply_with = #{@policy.name}") %>
-    <%= status_link _('Inconclusive results'), :inconclusive_hosts, arf_reports_path(:search => " inconclusive_with = #{@policy.name}") %>
-    <%= status_link _('Never audited'), :report_missing, hosts_path(:search => "compliance_report_missing_for = #{@policy.name}") %>
+    <%= status_link _('Compliant with the policy'), :compliant_hosts, arf_reports_path(:search => "comply_with = \"#{@policy.name}\"") %>
+    <%= status_link _('Not compliant with the policy'), :incompliant_hosts, arf_reports_path(:search => "not_comply_with = \"#{@policy.name}\"") %>
+    <%= status_link _('Inconclusive results'), :inconclusive_hosts, arf_reports_path(:search => " inconclusive_with = \"#{@policy.name}\"") %>
+    <%= status_link _('Never audited'), :report_missing, hosts_path(:search => "compliance_report_missing_for = \"#{@policy.name}\"") %>
     <h4 class="total">
-      <%= link_to(_("Total hosts: %s") % @report[:assigned_hosts], hosts_path(:search => "compliance_policy = #{@policy.name}")) %>
+      <%= link_to(_("Total hosts: %s") % @report[:assigned_hosts], hosts_path(:search => "compliance_policy = \"#{@policy.name}\"")) %>
     </h4>
   </ul>
 </div>

test/factories/asset_factory.rb

@@ -3,4 +3,9 @@
     f.assetable_id { FactoryGirl.create(:host).id }
     f.assetable_type 'Host::Base'
   end
+
+  factory :asset_policy, :class => ForemanOpenscap::AssetPolicy do |f|
+    f.asset_id nil
+    f.policy_id nil
+  end
 end

test/unit/concerns/host_extensions_test.rb

@@ -22,4 +22,64 @@ class HostExtensionsTest < ActiveSupport::TestCase
     enc_out = JSON.parse @host.policies_enc
     assert_equal 6, enc_out.first['download_path'].split('/').length
   end
+
+  test "should find hosts with direct policy assignment that were never audited" do
+    policy, host, host_2 = setup_hosts_with_policy.values_at(:policy, :host, :host_2)
+    report = FactoryGirl.create(:arf_report, :host_id => host_2.id)
+    FactoryGirl.create(:policy_arf_report, :policy_id => policy.id, :arf_report_id => report.id)
+
+    res = Host.policy_reports_missing policy
+    assert_equal res.count, 1
+    assert_include res, host
+  end
+
+  test "should find hosts with inherited policy that were never audited" do
+    policy, host, host_2 = setup_hosts_with_inherited_policy.values_at(:policy, :host, :host_2)
+    report = FactoryGirl.create(:arf_report, :host_id => host_2.id)
+    FactoryGirl.create(:policy_arf_report, :policy_id => policy.id, :arf_report_id => report.id)
+
+    res = Host.policy_reports_missing policy
+    assert_equal res.count, 1
+    assert_include res, host
+  end
+
+  test "should find hosts that are assigned to policy directly" do
+    policy, host, host_2 = setup_hosts_with_policy.values_at(:policy, :host, :host_2)
+    res = Host.assigned_to_policy(policy)
+    assert_equal 2, res.count
+    assert_include res, host
+    assert_include res, host_2
+  end
+
+  test "should find hosts with policy inherited from hostgroup" do
+    policy, host, host_2 = setup_hosts_with_inherited_policy.values_at(:policy, :host, :host_2)
+    res = Host.assigned_to_policy(policy)
+    assert_equal 2, res.count
+    assert_include res, host
+    assert_include res, host_2
+  end
+
+  private
+
+  def setup_hosts_with_policy
+    policy = FactoryGirl.create(:policy)
+    host = FactoryGirl.create(:compliance_host)
+    host_2 = FactoryGirl.create(:compliance_host)
+    asset = FactoryGirl.create(:asset, :assetable_id => host.id, :assetable_type => 'Host::Base')
+    asset_2 = FactoryGirl.create(:asset, :assetable_id => host_2.id, :assetable_type => 'Host::Base')
+    FactoryGirl.create(:asset_policy, :asset_id => asset.id, :policy_id => policy.id)
+    FactoryGirl.create(:asset_policy, :asset_id => asset_2.id, :policy_id => policy.id)
+    { :host => host, :policy => policy, :host_2 => host_2 }
+  end
+
+  def setup_hosts_with_inherited_policy
+    policy = FactoryGirl.create(:policy)
+    parent = FactoryGirl.create(:hostgroup)
+    child = FactoryGirl.create(:hostgroup, :ancestry => parent.id.to_s)
+    asset = FactoryGirl.create(:asset, :assetable_id => parent.id, :assetable_type => 'Hostgroup')
+    FactoryGirl.create(:asset_policy, :asset_id => asset.id, :policy_id => policy.id)
+    host = FactoryGirl.create(:compliance_host, :hostgroup_id => child.id)
+    host_2 = FactoryGirl.create(:compliance_host, :hostgroup_id => child.id)
+    { :policy => policy, :host => host, :host_2 => host_2 }
+  end
 end