Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ericgarff opened this issue Mar 31, 2022 · 8 comments
Closed

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

ericgarff opened this issue Mar 31, 2022 · 8 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@ericgarff
Copy link

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: 2ebe874>

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

@ericgarff ericgarff added the kind/bug Categorizes issue or PR as related to a bug. label Mar 31, 2022
@tkrishtop
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

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
Copy link
Contributor

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 komish added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 4, 2022
@komish
Copy link
Contributor

komish commented Oct 4, 2022

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

@komish komish closed this as completed Oct 4, 2022
@fhennig
Copy link

fhennig commented Feb 22, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

5 participants