Fixes #22687 - Add openscap report check and procedure #150

xprazak2 · 2018-03-01T14:16:53Z

wip as I need to test in production

iNecas · 2018-03-01T15:30:25Z

definitions/checks/foreman_openscap/report_associations.rb

+  module ForemanOpenscap
+    class ReportAssociations < ForemanMaintain::Check
+      metadata do
+        label :report_associations


I would suggest using :openscap_ prefix in the label: it makes it easier when listing the checks in fm health list command

iNecas · 2018-03-01T15:30:49Z

definitions/checks/foreman_openscap/report_associations.rb

+        description 'Check whether reports have correct associations'
+        tags :pre_upgrade, :foreman_openscap, :report_associations
+
+        confine do


When using for_feature the confine is not needed

iNecas · 2018-03-01T15:32:49Z

definitions/checks/foreman_openscap/report_associations.rb

@@ -0,0 +1,29 @@
+module Checks
+  module ForemanOpenscap
+    class ReportAssociations < ForemanMaintain::Check


The check should describe what's wrong about the state, if it fails: what about something like InvalidReportAssociations

iNecas · 2018-03-01T15:35:14Z

definitions/procedures/foreman_openscap/report_associations.rb

@@ -0,0 +1,15 @@
+module Procedures::ForemanOpenscap
+  class ReportAssociations < ForemanMaintain::Procedure


Similarly to the checks naming, I would suggest ClearReportAssociations

iNecas

Made some initial generic comments. Rubocop is failing currently. I was not paying attention to the openscap part: leaving this for somebody from the scap area @ares?

ntkathole · 2018-03-05T10:09:18Z

 ./bin/foreman-maintain advanced procedure run foreman-openscap-report-associations
Running ForemanMaintain::Scenario
================================================================================
Delete reports with association issues:                               [FAIL]
undefined method `join' for nil:NilClass
--------------------------------------------------------------------------------
Scenario [ForemanMaintain::Scenario] failed.

The following steps ended up in failing state:

  [foreman-openscap-report-associations]

Resolve the failed steps and rerun
the command. In case the failures are false positives,
use --whitelist="foreman-openscap-report-associations"

xprazak2 · 2018-03-16T13:56:29Z

I made changes as suggested. I am not entirely sure what causes the nil issue as I cannot reproduce, but I made an extra precaution for that as well.

ntkathole · 2018-03-18T07:43:29Z

@xprazak2 I still see the same issue on sat62 and sat63. Let me know if you need setup.

E, [2018-03-18 03:39:41-0400 #25968] ERROR -- : undefined method `join' for nil:NilClass (NoMethodError)
/root/foreman_maintain/definitions/features/foreman_openscap.rb:37:in `delete_reports'
/root/foreman_maintain/definitions/procedures/foreman_openscap/invalid_report_associations.rb:12:in `run'
/root/foreman_maintain/lib/foreman_maintain/executable.rb:119:in `__run__'
/root/foreman_maintain/lib/foreman_maintain/runner/execution.rb:76:in `block (2 levels) in run'
/root/foreman_maintain/lib/foreman_maintain/runner/execution.rb:99:in `capture_errors'
/root/foreman_maintain/lib/foreman_maintain/runner/execution.rb:75:in `block in run'
/root/foreman_maintain/lib/foreman_maintain/runner/execution.rb:93:in `with_metadata_calculation'
/root/foreman_maintain/lib/foreman_maintain/runner/execution.rb:74:in `run'
/root/foreman_maintain/lib/foreman_maintain/runner.rb:109:in `run_step'
/root/foreman_maintain/lib/foreman_maintain/runner.rb:98:in `run_steps'
/root/foreman_maintain/lib/foreman_maintain/runner.rb:91:in `execute_scenario_steps'
/root/foreman_maintain/lib/foreman_maintain/runner.rb:45:in `run_scenario'

kgaikwad · 2018-03-19T13:52:47Z

definitions/procedures/foreman_openscap/invalid_report_associations.rb

+module Procedures::ForemanOpenscap
+  class InvalidReportAssociations < ForemanMaintain::Procedure
+    metadata do
+      param :ids_to_remove, 'Ids of reports to remove'


@xprazak2 ,

param :ids_to_remove is required to run this procedure so you could mention :required => true to param declaration.

xprazak2 · 2018-03-19T14:24:12Z

The nil issue was caused by running the procedure directly through advanced procedure run, but it requires a list of ids which are supplied by check. I added advanced_run false to disable running it directly. Thanks @kgaikwad!

ntkathole · 2018-04-02T09:15:59Z

Tested on sat 6.2 :
Steps:

Created scap report with policy
deleted policy

Before performing upgrade, it asks to delete orphaned reports. But my concern is if we are handling the same in installer as well https://bugzilla.redhat.com/show_bug.cgi?id=1547607#c16 , @xprazak2 @iNecas shall we still have this check in foreman-maintain ?

xprazak2 · 2018-04-03T06:59:55Z

Partially, the migration in bz just removes reports without policy. Here we check for other associations as well. The migration runs as a part of upgrade and only once, having this in foreman_maintain provides value to users outside of upgrade scenario.

xprazak2 · 2018-06-20T06:52:48Z

@iNecas, any additional comments?

iNecas · 2018-06-20T07:21:33Z

Last question: should this apply to any upgrade with 'tfm-rubygem-foreman_openscap' >= '0.5.3', or is this specific let's say for sat6.3 -> sat6.4 but would not be applicable for sat6.4 -> sat6.5?

xprazak2 · 2018-06-21T08:47:07Z

People were having problems on 6.2, so this is mostly relevant for 6.2 -> 6.3. I do not expect this problem in later versions, steps were taken to prevent this from happening again. So not too relevant for the 6.3 -> 6.4 and later, but the check and procedure can be run manually as well if the users need it.

iNecas · 2018-06-21T11:20:37Z

Ok, let's pull it in then. Thanks @xprazak2

theforeman-bot added Needs testing Not yet reviewed labels Mar 1, 2018

iNecas reviewed Mar 1, 2018

View reviewed changes

iNecas suggested changes Mar 1, 2018

View reviewed changes

theforeman-bot added Waiting on contributor and removed Not yet reviewed labels Mar 1, 2018

xprazak2 force-pushed the oscap_clenaup branch from fbe6a36 to c5c6422 Compare March 16, 2018 13:31

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Mar 16, 2018

kgaikwad reviewed Mar 19, 2018

View reviewed changes

xprazak2 force-pushed the oscap_clenaup branch 2 times, most recently from d269e3d to 1640f11 Compare March 19, 2018 14:21

xprazak2 force-pushed the oscap_clenaup branch 3 times, most recently from 7477374 to 992ad5b Compare March 20, 2018 08:57

Fixes #22687 - Add openscap report check and procedure

1846643

xprazak2 force-pushed the oscap_clenaup branch from 992ad5b to 1846643 Compare March 21, 2018 14:27

iNecas merged commit 4624c41 into theforeman:master Jun 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #22687 - Add openscap report check and procedure #150

Fixes #22687 - Add openscap report check and procedure #150

xprazak2 commented Mar 1, 2018

iNecas Mar 1, 2018

iNecas Mar 1, 2018

iNecas Mar 1, 2018

iNecas Mar 1, 2018

iNecas left a comment

ntkathole commented Mar 5, 2018

xprazak2 commented Mar 16, 2018

ntkathole commented Mar 18, 2018 •

edited

Loading

kgaikwad Mar 19, 2018

xprazak2 commented Mar 19, 2018

ntkathole commented Apr 2, 2018

xprazak2 commented Apr 3, 2018

xprazak2 commented Jun 20, 2018

iNecas commented Jun 20, 2018

xprazak2 commented Jun 21, 2018

iNecas commented Jun 21, 2018

		@@ -0,0 +1,15 @@
		module Procedures::ForemanOpenscap
		class ReportAssociations < ForemanMaintain::Procedure

Fixes #22687 - Add openscap report check and procedure #150

Fixes #22687 - Add openscap report check and procedure #150

Conversation

xprazak2 commented Mar 1, 2018

iNecas Mar 1, 2018

Choose a reason for hiding this comment

iNecas Mar 1, 2018

Choose a reason for hiding this comment

iNecas Mar 1, 2018

Choose a reason for hiding this comment

iNecas Mar 1, 2018

Choose a reason for hiding this comment

iNecas left a comment

Choose a reason for hiding this comment

ntkathole commented Mar 5, 2018

xprazak2 commented Mar 16, 2018

ntkathole commented Mar 18, 2018 • edited Loading

kgaikwad Mar 19, 2018

Choose a reason for hiding this comment

xprazak2 commented Mar 19, 2018

ntkathole commented Apr 2, 2018

xprazak2 commented Apr 3, 2018

xprazak2 commented Jun 20, 2018

iNecas commented Jun 20, 2018

xprazak2 commented Jun 21, 2018

iNecas commented Jun 21, 2018

ntkathole commented Mar 18, 2018 •

edited

Loading