fedora-ci / rpminspect-runner

Container image for the rpminspect pipeline
Apache License 2.0
3 stars 9 forks source link

Various improvements for rpminspect_runner.sh #2

Closed dcantrell closed 1 year ago

dcantrell commented 3 years ago

Fix up warnings from shellcheck. Change get_name_from_nvr() to speak to the Koji hub using xmlrpc. Since Koji already knows the package name, that's the better option to use IMHO and less risk for failures for package names we have not seen yet.

What I want to really do is send a PR to koji for the buildinfo command to extend what it outputs. But later.

jimbair commented 3 years ago

Looks good to me - I like the addition of {} around the vars

dcantrell commented 3 years ago

Even better, after digging in to koji I learned it can output JSON now. So I've updated the PR to do:

koji call --json-output getBuild NVR | jq -r '.name'

To get the package name. I was going to update the other koji calls to make them run through the JSON API if it makes string handling more reliable.

NOTE: This does require an active Fedora krb5 ticket, so the runner will need that in place. I imagine we already have that since we're talking to so many Fedora things anyway.

msrb commented 3 years ago

Even better, after digging in to koji I learned it can output JSON now. So I've updated the PR to do:

koji call --json-output getBuild NVR | jq -r '.name'

To get the package name. I was going to update the other koji calls to make them run through the JSON API if it makes string handling more reliable.

NOTE: This does require an active Fedora krb5 ticket, so the runner will need that in place. I imagine we already have that since we're talking to so many Fedora things anyway.

Hmm, does it really require an active kerberos ticket? Because if it does, then we are in trouble. This runner scripts runs in Testing Farm, and there is no support for any credentials yet (afaik).

dcantrell commented 3 years ago

I have filed a ticket with Koji:

https://pagure.io/koji/issue/2718

I do not think 'koji call' should require a ticket here. I can do this:

xmlrpc https://koji.fedoraproject.org/kojihub getBuild nfs-utils-2.5.3-0.fc35

And it returns build information and I do not need a ticket. In fact, the fact that the XML-RPC API is read-only without authentication is how rpminspect even works in the first place. I think this is a bug in Koji and the 'koji call' command. I can change up the patch to use xmlrpc which is still more reliable than assuming the package name fits a specific format.

dcantrell commented 3 years ago

I modified the PR so that get_name_from_nvr() does an xmlrpc call using the command line 'xmlrpc' program. I returned get_after_build() to use the 'koji taskinfo' call it was making.

Hopefully the upstream koji project can fix up 'koji call' to not require auth for read-only calls. koji call can give JSON output or Python variable output which is nice.

msrb commented 3 years ago

@dcantrell thanks for the pull request David. I am doing some bigger changes in the script now, so please don't worry about rebases right now. Once I am done, I will rebase this PR for you ;)

dcantrell commented 3 years ago

Noted, thanks.