Fixes #11872 - Compliance status for hosts #131

xprazak2 · 2015-09-23T07:11:28Z

Current behaviour:

When Arf report is uploaded to Foreman, Compliance status is updated/created for a host
We have 3 compliance states: compliant, incompliant, inconclusive.
When any latest arf report for any of the policies that are assigned to the host has a failed rule, the status is incompliant
When latest arf reports for all policies assigned to the host have only passed rules, the status is compliant
When the latest reports have passed and othered rules, the status is inconclusive

Changes in UI:

I removed the square icon with letter from host list
I removed the Compliance table from host details, added a button to the details

Questions and thoughts:

Do we want to add Compliance status to API?
We have one status for all policies on the host. If we wanted greater detail, we could create 'partial' scap status for 1 policy and then build 'global scap status' for a host. This would probably require changes in how Foreman displays statuses on host details page in properties table. A suitable place to display these 'partial statuses' could be a policy dashboard page

shlomizadok · 2015-09-23T23:08:35Z

@xprazak2 - please rebase

xprazak2 · 2015-09-24T07:57:02Z

@shlomizadok: I rebased

ares · 2015-09-24T08:38:39Z

Quick scan looks good, I'd like to give it another round and test later. Don't forget to either bump https://github.com/theforeman/foreman_openscap/blob/master/lib/foreman_openscap/engine.rb#L38 to 1.10 or wrap the status registration in a condition to keep 1.9- compatibility.

Do we want to add Compliance status to API?

It should be added automatically, did you check the host detail output?

ares · 2015-09-24T10:04:53Z

app/models/concerns/foreman_openscap/host_extensions.rb

@@ -8,6 +8,7 @@ module HostExtensions
      has_many :asset_policies, :through => :asset, :class_name => "::ForemanOpenscap::AssetPolicy"
      has_many :policies, :through => :asset_policies, :class_name => "::ForemanOpenscap::Policy"
      has_many :arf_reports, :through => :asset, :class_name => '::ForemanOpenscap::ArfReport'
+      has_one :scap_status, :class_name => 'HostStatus::ScapStatus', :foreign_key => 'host_id', :dependent => :destroy


does :dependent => :destroy play nicely with host_statuses that also defines this dependency? (https://github.com/theforeman/foreman/blob/develop/app/models/host/managed.rb#L17) this has_one is subset of host_statuses

yes, it would be probably better to remove it

xprazak2 · 2015-09-24T12:35:37Z

@ares, I removed :dependent => :destroy, changed latest_report_for_policy to last_report_for_policy, added a check into engine for compatibility, and made sure status gets picked up by API. There may be something wrong in how the global status gets rendered by core, it has the correct "Error" label but the number code should be '2'. Could you please check if it is only local?

ares · 2015-10-08T06:17:12Z

app/models/concerns/foreman_openscap/host_extensions.rb

@@ -8,6 +8,7 @@ module HostExtensions
      has_many :asset_policies, :through => :asset, :class_name => "::ForemanOpenscap::AssetPolicy"
      has_many :policies, :through => :asset_policies, :class_name => "::ForemanOpenscap::Policy"
      has_many :arf_reports, :through => :asset, :class_name => '::ForemanOpenscap::ArfReport'
+      has_one :compliance_status, :class_name => 'HostStatus::ComplianceStatus', :foreign_key => 'host_id'


this should be named compliace_status_object, you override it by method with same name below https://github.com/theforeman/foreman_openscap/pull/131/files#diff-c08b2f64bbed0d7a15b0f7241af53d04R50

xprazak2 · 2015-10-08T06:33:35Z

@ares: I changed compliance_status to compliance_status_object

ares · 2015-10-08T06:45:04Z

app/models/concerns/foreman_openscap/host_extensions.rb

@@ -8,6 +8,7 @@ module HostExtensions
      has_many :asset_policies, :through => :asset, :class_name => "::ForemanOpenscap::AssetPolicy"
      has_many :policies, :through => :asset_policies, :class_name => "::ForemanOpenscap::Policy"
      has_many :arf_reports, :through => :asset, :class_name => '::ForemanOpenscap::ArfReport'
+      has_one :compliance_status_object, :class_name => 'HostStatus::ComplianceStatus', :foreign_key => 'host_id'


this association is usually used in scoped search, but I don't see any new definitions in the PR, could you add some? see https://github.com/theforeman/foreman_remote_execution/pull/42/files#diff-e3b1be04f41d99a66e199b0060b84e63R14 for an example

@ares: I added scoped search on compliance status

next time pls write a comment to original one so we can see that it's addressed

EDIT: unless it's outdated by git change, then it's optional...

xprazak2 · 2015-10-08T07:15:19Z

@ares: I added scoped search on compliance status

ares · 2015-10-08T07:17:56Z

app/models/host_status/compliance_status.rb

@@ -0,0 +1,46 @@
+module HostStatus


this should be also under ForemanOpenscap module and hence in app/models/foreman_openscap/host_status (host status might not be needed, but leaving this up to you)

ares · 2015-10-08T08:01:46Z

There may be something wrong in how the global status gets rendered by core, it has the correct "Error" label but the number code should be '2'. Could you please check if it is only local?

This can happen in development if rails autoloader reloads the code. Restart Foreman and see if it helps, it seems to work for me. The reason is that we refer to class name in engine.rb in initializer (the way that Foreman plugin currently works) but we should do it in config.to_prepare block... Anyway this should be OK in production env.

ares · 2015-10-08T12:44:31Z

app/models/concerns/foreman_openscap/host_extensions.rb

+      scoped_search :in => :compliance_status_object, :on => :status, :rename => :compliance_status,
+                    :complete_value => {:compliant => ForemanOpenscap::::ComplianceStatus::COMPLIANT,
+                                        :incompliant => ForemanOpenscap::::ComplianceStatus::INCOMPLIANT,
+                                        :inconclusive => ForemanOpenscap::::ComplianceStatus::INCONCLUSIVE}


/home/ares/Projekty/Zdrojaky/foreman_openscap/app/models/concerns/foreman_openscap/host_extensions.rb:18: syntax error, unexpected ::, expecting '(' (SyntaxError) ...ompliant => ForemanOpenscap::::ComplianceStatus::COMPLIANT, ...

ares · 2015-10-09T08:18:31Z

app/models/concerns/foreman_openscap/host_extensions.rb

@@ -8,11 +8,16 @@ module HostExtensions
      has_many :asset_policies, :through => :asset, :class_name => "::ForemanOpenscap::AssetPolicy"
      has_many :policies, :through => :asset_policies, :class_name => "::ForemanOpenscap::Policy"
      has_many :arf_reports, :through => :asset, :class_name => '::ForemanOpenscap::ArfReport'
+      has_one :compliance_status_object, :class_name => 'ForemanOpenscap::::ComplianceStatus', :foreign_key => 'host_id'


another syntax error

shlomizadok · 2015-10-12T11:37:39Z

@xprazak2 - Not sure if it's this PR or previous, yet my dashboard now looks like:

Also, rubocop is failing.

Thanks

shlomizadok · 2015-10-13T10:35:01Z

Merging. Thanks @xprazak2 !

Fixes #11872 - Compliance status for hosts

theforeman-bot added Needs testing Not yet reviewed labels Sep 23, 2015

xprazak2 force-pushed the global-status branch from d488d21 to c05ada0 Compare September 24, 2015 07:52

ares reviewed Sep 24, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch 3 times, most recently from 5dee2d2 to d0f7c01 Compare September 24, 2015 11:14

ares reviewed Oct 8, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch from d0f7c01 to e82eeb9 Compare October 8, 2015 06:27

ares reviewed Oct 8, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch from e82eeb9 to d7bf3ff Compare October 8, 2015 07:14

ares reviewed Oct 8, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch 3 times, most recently from defafd3 to 890a913 Compare October 8, 2015 12:34

ares reviewed Oct 8, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch 2 times, most recently from 8a30df7 to d8a2730 Compare October 9, 2015 06:07

ares reviewed Oct 9, 2015
View reviewed changes

xprazak2 force-pushed the global-status branch from d8a2730 to 8758cb8 Compare October 9, 2015 13:47

shlomizadok added Waiting on contributor and removed Needs testing Not yet reviewed labels Oct 12, 2015

xprazak2 force-pushed the global-status branch from 8758cb8 to c7a975f Compare October 12, 2015 16:42

theforeman-bot added Needs testing Needs re-review and removed Waiting on contributor labels Oct 12, 2015

Fixes #11872 - Compliance status for hosts

94a859b

xprazak2 force-pushed the global-status branch from c7a975f to 94a859b Compare October 13, 2015 08:13

shlomizadok added a commit that referenced this pull request Oct 13, 2015

Merge pull request #131 from xprazak2/global-status

f4fd397

Fixes #11872 - Compliance status for hosts

shlomizadok merged commit f4fd397 into theforeman:master Oct 13, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #11872 - Compliance status for hosts #131

Fixes #11872 - Compliance status for hosts #131

xprazak2 commented Sep 23, 2015

shlomizadok commented Sep 23, 2015

xprazak2 commented Sep 24, 2015

ares commented Sep 24, 2015

ares Sep 24, 2015

xprazak2 Sep 24, 2015

xprazak2 commented Sep 24, 2015

ares Oct 8, 2015

xprazak2 commented Oct 8, 2015

ares Oct 8, 2015

ares Oct 8, 2015

xprazak2 commented Oct 8, 2015

ares Oct 8, 2015

ares commented Oct 8, 2015

ares Oct 8, 2015

ares Oct 9, 2015

shlomizadok commented Oct 12, 2015

shlomizadok commented Oct 13, 2015

Fixes #11872 - Compliance status for hosts #131

Fixes #11872 - Compliance status for hosts #131

Conversation

xprazak2 commented Sep 23, 2015

shlomizadok commented Sep 23, 2015

xprazak2 commented Sep 24, 2015

ares commented Sep 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xprazak2 commented Sep 24, 2015

Choose a reason for hiding this comment

xprazak2 commented Oct 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xprazak2 commented Oct 8, 2015

Choose a reason for hiding this comment

ares commented Oct 8, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shlomizadok commented Oct 12, 2015

shlomizadok commented Oct 13, 2015