Repository: redhat-openshift-ecosystem/openshift-preflight
Pull request #747 (2022-07-27 15:47:13)

Only add results that are not marked as "optional"
The "optional" level will treat the check as normal, but will not
add it's outcome to the actual results. A user will still see the
log message with the status, but it will not cause a failure, nor
will it be reported in the output.

Signed-off-by: Brad P. Crochet <brad@redhat.com>

openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#" title="Author self-approved">bcrochet</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [bcrochet]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
coveralls: 
[![Coverage Status](https://coveralls.io/builds/51205780/badge)](https://coveralls.io/builds/51205780)

Coverage decreased (-0.08%) to 84.267% when pulling **46ad7f54fb08531cd3138166dad7954076864767 on bcrochet:autopass** into **31407374afd32a1eeb94bf80130ef4906e776ca2 on redhat-openshift-ecosystem:main**.

dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/298ddf01-ae65-48fa-95ba-92eb1e4200e1/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747
dcibot: -  SUCCESS https://www.distributed-ci.io/jobs/85d9dc4e-c407-4e0a-9ce5-3959914e8dc3/jobStates
 from https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#" title="Author self-approved">bcrochet</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#pullrequestreview-1053070980" title="LGTM">komish</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [bcrochet,komish]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#pullrequestreview-1057488482" title="Approved">acornett21</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#" title="Author self-approved">bcrochet</a>*, *<a href="https://github.com/redhat-openshift-ecosystem/openshift-preflight/pull/747#pullrequestreview-1053070980" title="LGTM">komish</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-openshift-ecosystem%2Fopenshift-preflight).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-openshift-ecosystem/openshift-preflight/blob/main/OWNERS)~~ [acornett21,bcrochet,komish]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/certification/internal/engine/engine.go b/certification/internal/engine/engine.go
index 31ecec6..d847dff 100644
--- a/certification/internal/engine/engine.go
+++ b/certification/internal/engine/engine.go
@@ -174,6 +174,9 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error {
 		c.results.TestedImage = c.Image
 
 		log.Debug("running check: ", check.Name())
+		if check.Metadata().Level == "optional" {
+			log.Infof("Check %s is not currently being enforced.", check.Name())
+		}
 
 		// run the validation
 		checkStartTime := time.Now()
@@ -182,18 +185,18 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error {
 
 		if err != nil {
 			log.WithFields(log.Fields{"result": "ERROR", "err": err}).Info("check completed: ", check.Name())
-			c.results.Errors = append(c.results.Errors, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
+			c.results.Errors = appendUnlessOptional(c.results.Errors, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
 			continue
 		}
 
 		if !checkPassed {
 			log.WithFields(log.Fields{"result": "FAILED"}).Info("check completed: ", check.Name())
-			c.results.Failed = append(c.results.Failed, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
+			c.results.Failed = appendUnlessOptional(c.results.Failed, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
 			continue
 		}
 
 		log.WithFields(log.Fields{"result": "PASSED"}).Info("check completed: ", check.Name())
-		c.results.Passed = append(c.results.Passed, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
+		c.results.Passed = appendUnlessOptional(c.results.Passed, runtime.Result{Check: check, ElapsedTime: checkElapsedTime})
 	}
 
 	if len(c.results.Errors) > 0 || len(c.results.Failed) > 0 {
@@ -224,6 +227,13 @@ func (c *CraneEngine) ExecuteChecks(ctx context.Context) error {
 	return nil
 }
 
+func appendUnlessOptional(results []runtime.Result, result runtime.Result) []runtime.Result {
+	if result.Check.Metadata().Level == "optional" {
+		return results
+	}
+	return append(results, result)
+}
+
 // tagDigestBindingInfo emits a log line describing tag and digest binding semantics.
 // The providedIdentifer is the tag or digest of the image as the user gave it at the commandline.
 // resolvedDigest
diff --git a/certification/internal/policy/operator/certified_images.go b/certification/internal/policy/operator/certified_images.go
index 6c49d99..49d7c9a 100644
--- a/certification/internal/policy/operator/certified_images.go
+++ b/certification/internal/policy/operator/certified_images.go
@@ -89,7 +89,7 @@ func (p *certifiedImagesCheck) Name() string {
 
 func (p *certifiedImagesCheck) Metadata() certification.Metadata {
 	return certification.Metadata{
-		Description:      "Checking that all images referenced in the CSV are certified",
+		Description:      "Checking that all images referenced in the CSV are certified. Currently, this check is not enforced.",
 		Level:            "optional",
 		KnowledgeBaseURL: "https://access.redhat.com/documentation/en-us/red_hat_software_certification/8.45/html/red_hat_openshift_software_certification_policy_guide/assembly-products-managed-by-an-operator_openshift-sw-cert-policy-container-images#con-operand-requirements_openshift-sw-cert-policy-products-managed",
 		CheckURL:         "https://access.redhat.com/documentation/en-us/red_hat_software_certification/8.45/html/red_hat_openshift_software_certification_policy_guide/assembly-products-managed-by-an-operator_openshift-sw-cert-policy-container-images#con-operand-requirements_openshift-sw-cert-policy-products-managed",
diff --git a/certification/internal/policy/operator/related_images.go b/certification/internal/policy/operator/related_images.go
index b1adffd..08969bc 100644
--- a/certification/internal/policy/operator/related_images.go
+++ b/certification/internal/policy/operator/related_images.go
@@ -74,7 +74,7 @@ func (p *RelatedImagesCheck) Name() string {
 
 func (p *RelatedImagesCheck) Metadata() certification.Metadata {
 	return certification.Metadata{
-		Description:      "Check that all images in the CSV are listed in RelatedImages section",
+		Description:      "Check that all images in the CSV are listed in RelatedImages section. Currently, this check is not enforced.",
 		Level:            "optional",
 		KnowledgeBaseURL: "https://access.redhat.com/documentation/en-us/red_hat_software_certification/8.45/html/red_hat_openshift_software_certification_policy_guide/assembly-products-managed-by-an-operator_openshift-sw-cert-policy-container-images#con-operator-requirements_openshift-sw-cert-policy-products-managed",
 		CheckURL:         "https://access.redhat.com/documentation/en-us/red_hat_software_certification/8.45/html/red_hat_openshift_software_certification_policy_guide/assembly-products-managed-by-an-operator_openshift-sw-cert-policy-container-images#con-operator-requirements_openshift-sw-cert-policy-products-managed",


Test patch:
diff --git a/certification/internal/engine/engine_test.go b/certification/internal/engine/engine_test.go
index b5edd5e..1327ebf 100644
--- a/certification/internal/engine/engine_test.go
+++ b/certification/internal/engine/engine_test.go
@@ -73,6 +73,24 @@ var _ = Describe("Execute Checks tests", func() {
 			certification.HelpText{},
 		)
 
+		optionalCheckPassing := certification.NewGenericCheck(
+			"optionalCheckPassing",
+			func(context.Context, certification.ImageReference) (bool, error) {
+				return true, nil
+			},
+			certification.Metadata{Level: "optional"},
+			certification.HelpText{},
+		)
+
+		optionalCheckFailing := certification.NewGenericCheck(
+			"optionalCheckFailing",
+			func(context.Context, certification.ImageReference) (bool, error) {
+				return false, fmt.Errorf("optionalError")
+			},
+			certification.Metadata{Level: "optional"},
+			certification.HelpText{},
+		)
+
 		emptyConfig := runtime.Config{}
 		engine = CraneEngine{
 			Config: emptyConfig.ReadOnly(), // must pass a config to avoid nil pointer errors
@@ -81,6 +99,8 @@ var _ = Describe("Execute Checks tests", func() {
 				goodCheck,
 				errorCheck,
 				failedCheck,
+				optionalCheckPassing,
+				optionalCheckFailing,
 			},
 			IsBundle:  false,
 			IsScratch: false,