search

redhat-openshift-ecosystem / openshift-preflight

Operator and container preflight certification tests
Apache License 2.0
58 stars 65 forks source link

Exit code doesn't change based on success or failure #535

Closed ericgarff closed 1 year ago

ericgarff commented 2 years ago

Bug Description

When running preflight, regardless of the result the exit code is 0 (shown via echo $?).

Version and Command Invocation

preflight version 1.1.0-beta6 <commit: 2ebe87409fcc9333149511466fdb452ccb88ad44>

Steps to Reproduce:

(How can we reproduce this?) 1) Run preflight on an image that passes all checks and echo the exit code, then run preflight on an image that fails and echo the exit code, you'll get 0 either way.

Expected Result

In the case of a failure, a non-zero exit code

Actual Result

Always gets an zero exit code

tkrishtop commented 2 years ago

I don't think that a failing test is a reason for a non-zero exit code. The exit code should be different in two situations:

  1. Exit 0: All tests were running, some of them passed, and some failed.
  2. Exit 1: Preflight encountered an error during the execution and this error prevented Preflight from running all the tests.

We could probably introduce the third exit code to distinguish the situations "Global error in Preflight / Local errors while running a particular test":

  1. Exit 0: All tests were running, some of them passed, and some failed.
  2. Exit 1: Preflight encountered a global error during the execution and this error prevented Preflight from running all the tests.
  3. Exit 2: Local errors while running a particular test / non-empty stderr
ericgarff commented 2 years ago

From a programmatic/pipeline standpoint, having any sort of non-zero exit code would be beneficial. If I'm reading it correctly, exit 2 would be if a particular test or tests failed?

ericgarff commented 2 years ago

As a point, not sure if this was expected, but if it fails the supported red hat image check it fails with a non-zero exit code, so this is potentially inconsistent?

acornett21 commented 2 years ago

@ericgarff Which check specifically are you talking about? And do you have a publicly available container that does not pass that check to test with that you saw a different exit code?

ericgarff commented 2 years ago

root@1ceeb6233927:/# preflight check container gcr.io/kasten-images/grafana:k10-8.1.8 --pyxis-api-token=XXXXXXX --certification-project-id=XXXXXX --pyxis-host=catalog.uat.redhat.com/api/containers ; echo $? time="2022-04-11T16:50:11Z" level=info msg="certification library version 1.1.0-beta6 <commit: 2ebe87409fcc9333149511466fdb452ccb88ad44>" Error: unsupported image. only RHEL/UBI or scratch base images are supported time="2022-04-11T16:50:40Z" level=fatal msg="unsupported image. only RHEL/UBI or scratch base images are supported" 1

vs.

root@1ceeb6233927:/# preflight check container gcr.io/kasten-images/kanister:4.5.11 --pyxis-api-token=XXXXXX --certification-project-id=XXXXXXXX --pyxis-host=catalog.uat.redhat.com/api/containers ; echo $? time="2022-04-11T16:51:39Z" level=info msg="certification library version 1.1.0-beta6 <commit: 2ebe87409fcc9333149511466fdb452ccb88ad44>" time="2022-04-11T16:52:38Z" level=info msg="check completed: BasedOnUbi" result=PASSED time="2022-04-11T16:53:05Z" level=info msg="check completed: HasModifiedFiles" result=PASSED time="2022-04-11T16:53:05Z" level=info msg="check completed: HasLicense" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasUniqueTag" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: LayerCountAcceptable" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasNoProhibitedPackagesMounted" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasRequiredLabel" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: RunAsNonRoot" result=FAILED { "image": "gcr.io/kasten-images/kanister:4.5.11", "passed": false, "test_library": { "name": "github.com/redhat-openshift-ecosystem/openshift-preflight", "version": "1.1.0-beta6", "commit": "2ebe87409fcc9333149511466fdb452ccb88ad44" }, "results": { "passed": [ { "name": "BasedOnUbi", "elapsed_time": 1060, "description": "Checking if the container's base image is based upon the Red Hat Universal Base Image (UBI)" }, { "name": "HasModifiedFiles", "elapsed_time": 27382, "description": "Checks that no files installed via RPM in the base Red Hat layer have been modified" }, { "name": "HasLicense", "elapsed_time": 0, "description": "Checking if terms and conditions applicable to the software including open source licensing information are present. The license must be at /licenses" }, { "name": "HasUniqueTag", "elapsed_time": 1259, "description": "Checking if container has a tag other than 'latest', so that the image can be uniquely identified." }, { "name": "LayerCountAcceptable", "elapsed_time": 0, "description": "Checking if container has less than 40 layers. Too many layers within the container images can degrade container performance." }, { "name": "HasNoProhibitedPackagesMounted", "elapsed_time": 100, "description": "Checks to ensure that the image in use does not include prohibited packages, such as Red Hat Enterprise Linux (RHEL) kernel packages." }, { "name": "HasRequiredLabel", "elapsed_time": 0, "description": "Checking if the required labels (name, vendor, version, release, summary, description) are present in the container metadata." } ], "failed": [ { "name": "RunAsNonRoot", "elapsed_time": 0, "description": "Checking if container runs as the root user because a container that does not specify a non-root user will fail the automatic certification, and will be subject to a manual review before the container can be approved for publication", "help": "Check RunAsNonRoot encountered an error. Please review the preflight.log file for more information.", "suggestion": "Indicate a specific USER in the dockerfile or containerfile", "knowledgebase_url": "https://connect.redhat.com/zones/containers/container-certification-policy-guide", "check_url": "https://connect.redhat.com/zones/containers/container-certification-policy-guide" } ], "errors": [] } } 0

acornett21 commented 2 years ago

root@1ceeb6233927:/# preflight check container gcr.io/kasten-images/grafana:k10-8.1.8 --pyxis-api-token=XXXXXXX --certification-project-id=XXXXXX --pyxis-host=catalog.uat.redhat.com/api/containers ; echo $? time="2022-04-11T16:50:11Z" level=info msg="certification library version 1.1.0-beta6 <commit: 2ebe87409fcc9333149511466fdb452ccb88ad44>" Error: unsupported image. only RHEL/UBI or scratch base images are supported time="2022-04-11T16:50:40Z" level=fatal msg="unsupported image. only RHEL/UBI or scratch base images are supported" 1

vs.

root@1ceeb6233927:/# preflight check container gcr.io/kasten-images/kanister:4.5.11 --pyxis-api-token=XXXXXX --certification-project-id=XXXXXXXX --pyxis-host=catalog.uat.redhat.com/api/containers ; echo $? time="2022-04-11T16:51:39Z" level=info msg="certification library version 1.1.0-beta6 <commit: 2ebe87409fcc9333149511466fdb452ccb88ad44>" time="2022-04-11T16:52:38Z" level=info msg="check completed: BasedOnUbi" result=PASSED time="2022-04-11T16:53:05Z" level=info msg="check completed: HasModifiedFiles" result=PASSED time="2022-04-11T16:53:05Z" level=info msg="check completed: HasLicense" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasUniqueTag" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: LayerCountAcceptable" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasNoProhibitedPackagesMounted" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: HasRequiredLabel" result=PASSED time="2022-04-11T16:53:07Z" level=info msg="check completed: RunAsNonRoot" result=FAILED { "image": "gcr.io/kasten-images/kanister:4.5.11", "passed": false, "test_library": { "name": "github.com/redhat-openshift-ecosystem/openshift-preflight", "version": "1.1.0-beta6", "commit": "2ebe87409fcc9333149511466fdb452ccb88ad44" }, "results": { "passed": [ { "name": "BasedOnUbi", "elapsed_time": 1060, "description": "Checking if the container's base image is based upon the Red Hat Universal Base Image (UBI)" }, { "name": "HasModifiedFiles", "elapsed_time": 27382, "description": "Checks that no files installed via RPM in the base Red Hat layer have been modified" }, { "name": "HasLicense", "elapsed_time": 0, "description": "Checking if terms and conditions applicable to the software including open source licensing information are present. The license must be at /licenses" }, { "name": "HasUniqueTag", "elapsed_time": 1259, "description": "Checking if container has a tag other than 'latest', so that the image can be uniquely identified." }, { "name": "LayerCountAcceptable", "elapsed_time": 0, "description": "Checking if container has less than 40 layers. Too many layers within the container images can degrade container performance." }, { "name": "HasNoProhibitedPackagesMounted", "elapsed_time": 100, "description": "Checks to ensure that the image in use does not include prohibited packages, such as Red Hat Enterprise Linux (RHEL) kernel packages." }, { "name": "HasRequiredLabel", "elapsed_time": 0, "description": "Checking if the required labels (name, vendor, version, release, summary, description) are present in the container metadata." } ], "failed": [ { "name": "RunAsNonRoot", "elapsed_time": 0, "description": "Checking if container runs as the root user because a container that does not specify a non-root user will fail the automatic certification, and will be subject to a manual review before the container can be approved for publication", "help": "Check RunAsNonRoot encountered an error. Please review the preflight.log file for more information.", "suggestion": "Indicate a specific USER in the dockerfile or containerfile", "knowledgebase_url": "https://connect.redhat.com/zones/containers/container-certification-policy-guide", "check_url": "https://connect.redhat.com/zones/containers/container-certification-policy-guide" } ], "errors": [] } } 0

The first error is an internal application error and fails before any checks are even run. The 2nd all the checks are run and there were no internal application errors. This has been cleaned up in main to avoid confusion, and the first case should no longer error, and will yield the same results (exit code wise) as the 2nd case.

komish commented 1 year ago

Closing this as a stale issue. If this issue needs additional work, please feel free to re-open.

fhennig commented 1 year ago

@komish I've just come across a need for this as well, can we re-open the issue?

If I use the preflight in CI, it would be useful to have a non-zero exit code on any failure. It is common for other testing tools to indicate test failure with a non-zero exit code.

Now, I have to instead parse the json, which is additional overhead and more error prone than a simple non-zero exit code.