Container ssa annotate success #15031

enoodle · 2017-05-09T08:18:59Z

Fixes #15013

This should be merged together with ManageIQ/manageiq-providers-openshift#41

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1458678

simon3z · 2017-05-10T15:32:35Z

@miq-bot assign enoodle

simon3z · 2017-05-17T09:55:33Z

@moolitayer can you review this?
@enoodle what does it take to finalize this?

moolitayer · 2017-05-17T12:52:49Z

app/models/miq_action.rb

@@ -730,7 +730,19 @@ def action_container_image_analyze(action, rec, inputs)
    rec.scan
  end

+  def action_container_image_annotate_allow_execution(action, rec, inputs)
+    _log.info("EREZ DEBUG:: in function #{__method__}")


moolitayer · 2017-05-17T12:54:00Z

app/models/container_image.rb

+  def annotate_allow_execution(causing_policy)
+    annotate_image({
+      "security.manageiq.org/successful-policy" => causing_policy,
+      "images.openshift.io/allow-execution"     => "true"


See question:
#15013 (comment)

I see what you're asking now. It seems you don't want to overwrite an annotation. Once it's non-compliant it should stay that way. I don't see how adding another annotation helps. Now you could have both allow and deny annotations co-existing. A single boolean is cleanest implementation. How about a single method that accepts a pass/fail boolean?

def annotate_execution(causing_policy, deny_execution) annotate_image({ "security.manageiq.org/policy" => causing_policy, "images.openshift.io/deny-execution" => deny_execution })

Is allow-execution annotation even a thing? Will openshift admission plugin look at it? https://docs.openshift.com/container-platform/latest/admin_guide/image_policy.html gives me the impression only deny-execution is a thing, with value true or false.

moolitayer · 2017-05-17T12:55:49Z

app/models/container_image.rb

      "security.manageiq.org/failed-policy" => causing_policy,
      "images.openshift.io/deny-execution"  => "true"
-    )
+    })


@aweiteka can you review these annotations?

moolitayer · 2017-05-17T13:01:42Z

db/fixtures/miq_policy_sets.yml

+          enabled:
+        MiqAction:
+          name: container_image_annotate_allow_execution
+          description: Prevent container image from running on OpenShift


fix the description

moolitayer · 2017-05-17T13:01:56Z

app/models/miq_action.rb

  def action_container_image_annotate_deny_execution(action, rec, inputs)
+    _log.info("EREZ DEBUG:: in function #{__method__}")


moolitayer

Code seems good, a couple of comments and making sure an image compliancy cannot change
(this code assumes it can't)

aweiteka · 2017-05-17T13:08:53Z

app/models/container_image.rb

+  def annotate_allow_execution(causing_policy)
+    annotate_image({
+      "security.manageiq.org/successful-policy" => causing_policy,
+      "images.openshift.io/allow-execution"     => "true"


I see what you're asking now. It seems you don't want to overwrite an annotation. Once it's non-compliant it should stay that way. I don't see how adding another annotation helps. Now you could have both allow and deny annotations co-existing. A single boolean is cleanest implementation. How about a single method that accepts a pass/fail boolean?

def annotate_execution(causing_policy, deny_execution) annotate_image({ "security.manageiq.org/policy" => causing_policy, "images.openshift.io/deny-execution" => deny_execution })

moolitayer · 2017-05-17T13:10:39Z

I think compliancy cannot change from this point of view, since we only annotate openshift images, so that's fine:
https://github.com/ManageIQ/manageiq/pull/15031/files#diff-b65b69c5edaddcf64ef4a666f394489eL746

aweiteka · 2017-05-17T13:27:14Z

we only annotate openshift images

Technically that doesn't matter because other scanning tools have the same privilege to the cluster and may annotate all images in any namespace. We'll deal with this later once this PR is merged, but I wanted to clarify.

enoodle · 2017-05-23T13:57:53Z

My main problem with this PR is that it is not working currently.. I must have not defined something important for the policy. @moolitayer @cben if you could take a look and maybe give me a hint it would be nice. Also, is there a point of contact in ManageIQ for policies that we could ask for advice?

moolitayer · 2017-05-24T15:44:40Z

What happens when you call the action method directly, does it work?
I can probably help with the policy, also take a look at log/policy.log

enoodle · 2017-06-08T07:50:26Z

This is now successfully annotating compliant images and also adds the quality summary from #15212

This is still WIP becuase I need to understand how to map the severity levels critical, important, moderate, low to the ones returned from OpenSCAP High, Medium, Low. Any advice?

moolitayer · 2017-06-08T08:08:51Z

@enoodle Is there a documentation for what the logic is behind High, Medium, Low ?

@isimluk can you please remind me who is our contact for OpenSCAP?

aweiteka · 2017-06-08T14:21:31Z

can you please remind me who is our contact for OpenSCAP?

@mpreisler

documentation for what the logic is behind High, Medium, Low ?

This is a good question. I missed that this mapping is happening. I don't see why we don't just pass the results through to the MIQ user unless we are trying to simplify (3 states vs 4?). But any interpretation we do moves us away from an "OpenSCAP provider" to an "MIQ provider (fed by OpenSCAP results)".

Any change here would break backwards compatibility with the policy. Workaround would be to carry logic "critical OR high" in the policy.

enoodle · 2017-06-11T14:11:01Z

@aweiteka

I missed that this mapping is happening.

I missed it too, Sorry.

Do you agree to {"Low" => "low", "Medium" => "moderate", "High" => "critical"} mapping? We have to skip one state, because we are coming out of the 3-state OpenSCAP results, and I feel like everything might be considered "important" if your definition of important is wide enough.

mpreisler · 2017-06-20T18:49:13Z

The severity is not coming from @OpenSCAP but from the NIST XCCDF specification. There are actually 5 possible states. See https://scap.nist.gov/specifications/xccdf/xccdf_element_dictionary.html#severityEnumType

Could we just use this as it is?

enoodle · 2017-06-21T08:53:13Z

@mpreisler We need to send it as detailed here: https://github.com/adellape/openshift-docs/blob/master/security/container_content.adoc#container-content-scanning

I will use your link to make a 1-1 map of those 5 seventies that both have. Thank you.

enoodle · 2017-06-21T14:33:38Z

@aweiteka @moolitayer Can you please take another look?

miq-bot · 2017-10-25T15:11:38Z

Checked commits enoodle/manageiq@803fbfd~...7334216 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

enoodle · 2017-10-29T14:10:13Z

@gtanzillo Can you review this please? It can be changed with respect to #16213 if needed.

moolitayer · 2017-10-29T16:00:27Z

@aweiteka Please take a look

enoodle · 2017-10-30T12:35:39Z

ping @Fryguy @agrare can you please take a look?
@miq-bot add_label bug gaprindashvili/yes

miq-bot · 2017-10-30T12:36:11Z

@enoodle Cannot apply the following label because they are not recognized: bug gaprindashvili/yes

enoodle · 2017-10-31T13:20:21Z

@miq-bot add_label gaprindashvili/yes bug

miq-bot · 2017-10-31T13:21:11Z

@enoodle Cannot apply the following label because they are not recognized: gaprindashvili/yes bug

moolitayer · 2017-11-02T06:50:03Z

@enoodle the bot was missing a comma

mjudeikis · 2017-11-07T12:27:50Z

What release is this planned for? Any instructions how to port it back to old version of CF?

enoodle · 2017-11-07T12:35:03Z

@mjudeikis It is aimed for gaprindashvili. As for backports, because it is based on ManageIQ/manageiq-providers-openshift#41 which itself is based on other gaprindashvili PRs since the repositories split, backporting will not be trivial.

chessbyte · 2017-11-14T08:05:39Z

@enoodle Is this replacing action_container_image_annotate_deny_execution with action_container_image_annotate_scan_results?

enoodle · 2017-11-14T08:19:21Z

@chessbyte Yes, it is based on a change in
ManageIQ/manageiq-providers-openshift#41

bazulay · 2017-11-19T13:35:03Z

@gtanzillo who can take a look

enoodle · 2017-11-20T10:10:08Z

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1458678

gtanzillo

This looks good to me 👍 Is this change covered in existing tests?

enoodle · 2017-11-20T14:20:12Z

@gtanzillo I updated the miqAction tests that were effected by this, but couldn't find any tests for miq_policy_sets.yml , do you know of any?

moolitayer · 2017-11-20T14:41:04Z

🎉

Container ssa annotate success (cherry picked from commit 9835af3) https://bugzilla.redhat.com/show_bug.cgi?id=1515438

simaishi · 2017-11-20T19:31:41Z

Gaprindashvili backport details:

$ git log -1
commit 3ec3b2e1852e8a7883a96eaeb6607e1d158bc3f6
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Nov 20 09:24:08 2017 -0500

    Merge pull request #15031 from enoodle/container_ssa_annotate_success
    
    Container ssa annotate success
    (cherry picked from commit 9835af3b3e53525a3dbb6745b4718866890edd4f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1515438

enoodle changed the title ~~Container ssa annotate success~~ [WIP] Container ssa annotate success May 9, 2017

miq-bot added the wip label May 9, 2017

miq-bot assigned enoodle May 10, 2017

moolitayer reviewed May 17, 2017

View reviewed changes

moolitayer suggested changes May 17, 2017

View reviewed changes

aweiteka suggested changes May 17, 2017

View reviewed changes

enoodle force-pushed the container_ssa_annotate_success branch 2 times, most recently from 80f8b37 to df8b918 Compare June 8, 2017 07:48

enoodle force-pushed the container_ssa_annotate_success branch from df8b918 to ae8017c Compare June 8, 2017 08:02

enoodle force-pushed the container_ssa_annotate_success branch from ae8017c to 6d8b936 Compare June 8, 2017 08:15

enoodle force-pushed the container_ssa_annotate_success branch from 6d8b936 to be96afc Compare June 21, 2017 14:18

enoodle changed the title ~~[WIP] Container ssa annotate success~~ Container ssa annotate success Jun 21, 2017

miq-bot removed the wip label Jun 21, 2017

Erez Freiberger added 2 commits October 25, 2017 17:31

annotating container images with successful compliance

803fbfd

move to one annotating function for policy results

7334216

enoodle force-pushed the container_ssa_annotate_success branch from d2346d8 to 7334216 Compare October 25, 2017 14:59

agrare added bug gaprindashvili/yes providers/containers labels Oct 31, 2017

gtanzillo reviewed Nov 20, 2017

View reviewed changes

gtanzillo approved these changes Nov 20, 2017

View reviewed changes

gtanzillo added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 20, 2017

gtanzillo merged commit 9835af3 into ManageIQ:master Nov 20, 2017

simaishi pushed a commit that referenced this pull request Nov 20, 2017

Merge pull request #15031 from enoodle/container_ssa_annotate_success

3ec3b2e

Container ssa annotate success (cherry picked from commit 9835af3) https://bugzilla.redhat.com/show_bug.cgi?id=1515438

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Nov 20, 2017

		def action_container_image_annotate_deny_execution(action, rec, inputs)
		_log.info("EREZ DEBUG:: in function #{__method__}")

Container ssa annotate success #15031

Container ssa annotate success #15031

Conversation

enoodle commented May 9, 2017 • edited Loading

simon3z commented May 10, 2017

simon3z commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moolitayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moolitayer commented May 17, 2017

aweiteka commented May 17, 2017

enoodle commented May 23, 2017

moolitayer commented May 24, 2017

enoodle commented Jun 8, 2017

moolitayer commented Jun 8, 2017 • edited Loading

aweiteka commented Jun 8, 2017

enoodle commented Jun 11, 2017

mpreisler commented Jun 20, 2017

enoodle commented Jun 21, 2017

enoodle commented Jun 21, 2017

miq-bot commented Oct 25, 2017

enoodle commented Oct 29, 2017

moolitayer commented Oct 29, 2017

enoodle commented Oct 30, 2017

miq-bot commented Oct 30, 2017

enoodle commented Oct 31, 2017

miq-bot commented Oct 31, 2017

moolitayer commented Nov 2, 2017

mjudeikis commented Nov 7, 2017

enoodle commented Nov 7, 2017

chessbyte commented Nov 14, 2017

enoodle commented Nov 14, 2017

bazulay commented Nov 19, 2017

enoodle commented Nov 20, 2017

gtanzillo left a comment

Choose a reason for hiding this comment

enoodle commented Nov 20, 2017

moolitayer commented Nov 20, 2017

simaishi commented Nov 20, 2017

enoodle commented May 9, 2017 •

edited

Loading

moolitayer commented Jun 8, 2017 •

edited

Loading