Skip to content
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

Panic in stable version #754

Closed
jrsmroz opened this issue Aug 5, 2022 · 6 comments · Fixed by #756
Closed

Panic in stable version #754

jrsmroz opened this issue Aug 5, 2022 · 6 comments · Fixed by #756
Assignees
Labels
blocker Indicates that a partner is blocked from submitting results kind/bug Categorizes issue or PR as related to a bug.

Comments

@jrsmroz
Copy link

jrsmroz commented Aug 5, 2022

Bug Description

When running preflight stable image the panic can be seen during a submit

https://github.com/jrsmroz/gateway-operator/runs/7689619774?check_suite_focus=true

$ docker run -v /var/run/docker.sock:/var/run/docker.sock -v /home/runner/.docker/:/docker quay.io/opdev/preflight:stable check container scan.connect.redhat.com/***/gateway-operator:0.0.0-test3-redhat --docker-config=/docker/config.json --submit --certification-project-id=*** --pyxis-api-token=
Unable to find image 'quay.io/opdev/preflight:stable' locally
stable: Pulling from opdev/preflight
9849471674a8: Pulling fs layer
21ca3853a314: Pulling fs layer
d3dd878ef08b: Pulling fs layer
21ca3853a314: Download complete
9849471674a8: Verifying Checksum
9849471674a8: Download complete
d3dd878ef08b: Verifying Checksum
d3dd878ef08b: Download complete
9849471674a8: Pull complete
21ca3853a314: Pull complete
d3dd878ef08b: Pull complete
Digest: sha256:6670809cbe8de9495325cbae12d565388b772c6773d6537fad74e7202c2b2403
Status: Downloaded newer image for quay.io/opdev/preflight:stable
time="2022-08-05T10:44:59Z" level=info msg="certification library version 1.3.3 <commit: 6c30f7caae731ecc2faeaebd78c6d3981c708fe6>"
time="2022-08-05T10:45:09Z" level=info msg="check completed: HasLicense" result=PASSED
time="2022-08-05T10:45:09Z" level=info msg="check completed: HasUniqueTag" result=PASSED
time="2022-08-05T10:45:09Z" level=info msg="check completed: LayerCountAcceptable" result=PASSED
time="2022-08-05T10:45:09Z" level=info msg="check completed: HasNoProhibitedPackages" result=PASSED
time="2022-08-05T10:45:09Z" level=info msg="check completed: HasRequiredLabel" result=PASSED
time="2022-08-05T10:45:09Z" level=info msg="USER 1000 specified that is non-root"
time="2022-08-05T10:45:09Z" level=info msg="check completed: RunAsNonRoot" result=PASSED
time="2022-08-05T10:45:14Z" level=info msg="check completed: HasModifiedFiles" result=PASSED
time="2022-08-05T10:45:14Z" level=info msg="check completed: BasedOnUbi" result=PASSED
{
    "image": "scan.connect.redhat.com/***/gateway-operator:0.0.0-test3-redhat",
    "passed": true,
    "test_library": {
        "name": "github.com/redhat-openshift-ecosystem/openshift-preflight",
        "version": "1.3.3",
        "commit": "6c30f7caae731ecc2faeaebd78c6d3981c708fe6"
    },
    "results": {
        "passed": [
            {
                "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": 275,
                "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": "HasNoProhibitedPackages",
                "elapsed_time": 83,
                "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."
            },
            {
                "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"
            },
            {
                "name": "HasModifiedFiles",
                "elapsed_time": 4549,
                "description": "Checks that no files installed via RPM in the base Red Hat layer have been modified"
            },
            {
                "name": "BasedOnUbi",
                "elapsed_time": 567,
                "description": "Checking if the container's base image is based upon the Red Hat Universal Base Image (UBI)"
            }
        ],
        "failed": [],
        "errors": []
    }
}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x2c pc=0x533e7d]

goroutine 1 [running]:
github.com/sirupsen/logrus.(*Logger).Log(0x19046fb, 0x2a, {0xc000c097b8, 0x4ee66e, 0xc0001e7520})
	/go/pkg/mod/github.com/sirupsen/logrus@v1.8.1/logger.go:196 +0x1d
github.com/sirupsen/logrus.(*Logger).Info(...)
	/go/pkg/mod/github.com/sirupsen/logrus@v1.8.1/logger.go:220
github.com/redhat-openshift-ecosystem/openshift-preflight/cmd.(*noopSubmitter).Submit(0x1b0d0e0, {0xc00054c0c0, 0xc000c098e8})
	/go/src/preflight/cmd/types.go:187 +0x105
github.com/redhat-openshift-ecosystem/openshift-preflight/cmd.preflightCheck({0x1b37078, 0xc000044030}, 0xc000515440, {0x0, 0x0}, {0x1b1fed8, 0xc0000ae140}, {0x1b25010, 0xc0001aede0}, {0x1b25070, ...}, ...)
	/go/src/preflight/cmd/preflight_check.go:63 +0x448
github.com/redhat-openshift-ecosystem/openshift-preflight/cmd.checkContainerRunE(0x2774700, {0xc00010eaa0, 0x1, 0x5})
	/go/src/preflight/cmd/check_container.go:99 +0x32a
github.com/spf13/cobra.(*Command).execute(0x2774700, {0xc00010ea50, 0x5, 0x5})
	/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:856 +0x60e
github.com/spf13/cobra.(*Command).ExecuteC(0x2774e80)
	/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:974 +0x3bc
github.com/spf13/cobra.(*Command).Execute(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:902
github.com/spf13/cobra.(*Command).ExecuteContext(...)
	/go/pkg/mod/github.com/spf13/cobra@v1.4.0/command.go:[89](https://github.com/jrsmroz/gateway-operator/runs/7689619774?check_suite_focus=true#step:17:91)5
github.com/redhat-openshift-ecosystem/openshift-preflight/cmd.Execute()
	/go/src/preflight/cmd/root.go:38 +0x58
main.main()
	/go/src/preflight/main.go:8 +0x17

Version and Command Invocation

Extracted from the logs:
time="2022-08-05T10:44:59Z" level=info msg="certification library version 1.3.3 <commit: 6c30f7caae731ecc2faeaebd78c6d3981c708fe6>"

Steps to Reproduce:

(How can we reproduce this?)

  1. We've run the preflight against our operator in the github CI pipeline. Seems to be random, as most of the time the panic does not occur

Expected Result

I would expect the preflight not to panic.

Actual Result

Preflight have panicked

Additional Context

https://github.com/jrsmroz/gateway-operator/runs/7689619774?check_suite_focus=true

@jrsmroz jrsmroz added the kind/bug Categorizes issue or PR as related to a bug. label Aug 5, 2022
@bcrochet bcrochet self-assigned this Aug 8, 2022
@komish
Copy link
Contributor

komish commented Aug 8, 2022

@bcrochet the stack trace seems to indicate that the noopSubmitter is being used even though the --submit flag was provided. I believe that's not expected.

github.com/redhat-openshift-ecosystem/openshift-preflight/cmd.(*noopSubmitter).Submit(0x1b0d0e0, {0xc00054c0c0, 0xc000c098e8})
	/go/src/preflight/cmd/types.go:187 +0x105

@jomkz jomkz added the blocker Indicates that a partner is blocked from submitting results label Aug 8, 2022
@bcrochet
Copy link
Contributor

bcrochet commented Aug 8, 2022

I noticed that.

@jrsmroz Can you verify that you actually have a pyxis api token set up? It appears that's not being set. As in, I see no '***' after --pyxis-api-token. I'm assuming you'd have that stored in a secret. I will look to see if that would somehow trigger the noopSubmitter to be used.

@bcrochet
Copy link
Contributor

bcrochet commented Aug 8, 2022

I was able to reproduce this locally by leaving off the pyxis api key. So, the command line finishes with:

--certification-id=*** --pyxis-api-token

So, without the pyxis api token, it will end up with the noopSubmitter, and panic. I will look to why it is panicing, but I will also make a fix that exits early if the flag is given, but no value is supplied.

bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 8, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
@jrsmroz
Copy link
Author

jrsmroz commented Aug 8, 2022

I used the correct pyxis token. Actually when I rerun the same pipeline it worked fine. So it's seems to be random.
It was a forked repo for my tests, so maybe the pyxis token was set up before after the pipeline picked up? Frankly speaking, I don't remember that now.

@bcrochet
Copy link
Contributor

bcrochet commented Aug 8, 2022

I used the correct pyxis token. Actually when I rerun the same pipeline it worked fine. So it's seems to be random. It was a forked repo for my tests, so maybe the pyxis token was set up before the pipeline picked up? Frankly speaking, I don't remember that now.

It definitely looks like the token wasn't present when it ran. If this happens again, after this PR is merged, it should at least give us an error with better info, and no panic.

@jrsmroz
Copy link
Author

jrsmroz commented Aug 9, 2022

You're right the pyxis token was not set.

bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 9, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 11, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 15, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit to bcrochet/openshift-preflight that referenced this issue Aug 15, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes redhat-openshift-ecosystem#754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
bcrochet added a commit that referenced this issue Aug 16, 2022
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.

Fixes #754

Signed-off-by: Brad P. Crochet <brad@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Indicates that a partner is blocked from submitting results kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants