@mtrmac code wise I just rebased your PR. And I won't pretend I understand most of this sigstore/rekor code so I could be missing something obvious but the basic test seems to work. Not sure what other test cases I could add?

Also the thing I wasn't sure about should I setup the podman containers from the go test like it is right now or would it be better to require the test infra to set it up outside of go test and then set some env var with the local rekor URL.

Thanks! Just a very minimal skim for now:

Also the thing I wasn't sure about should I setup the podman containers from the go test like it is right now or would it be better to require the test infra to set it up outside of go test and then set some env var with the local rekor URL.

I’d rather prefer for ordinary go test in a subpackage, by default, to only run unit tests that don’t affect any global state, and that can run cross-platform (e.g. so that IDE “run this unit test function” features work without surprises).

So, from that point of view, I think the new test should either be in a separate integration test directory (which makes it hard to use internal.VerifyRekorSET), or an opt-in via a build tag — the latter seems easier. In that case, contrib/cirrus/runner.sh:_run_image_tests.sh could be modified to call the new shell script and to add the opt-in build tag.

(Cc: @lsm5 in case TMT meant more things to consider.)

Well technically assuming you have podman with a working machine it should work cross platform I suppose. But yeah we still modify the global state. My main issue with the external setup is that at some point the CI setup changes, and then the test is kipped so nobody notices that we loose coverage unless there is someone reading all skip messages in the CI logs which from past experience is not happing (or at least not something I can count on).

But I agree it is a bit much for a go unit test setup. I will rework it so the CI setup spin up the containers beforehand

ok github.com/containers/image/v5/signature/sigstore/rekor 64.513s coverage: 59.6% of statements

64s is not to bad I suppose so I guess we don't need to worry about the CI time here to much. The skopeo task is still much slower so this would not hold up total time to merge.

64s is not to bad … The skopeo task is still much slower so this would not hold up total time to merge.

ACK, that’s perfectly fine.

My main issue with the external setup is that at some point the CI setup changes, and then the test is kipped so nobody notices that we loose coverage unless there is someone reading all skip messages in the CI logs

I’m not too worried — if a new build tag enables a test, we can outright fail in that test if the server is not running, with no auto-skipping.

(It’s also possible to over-engineer this, with the integration test writing a “yes, I really did run” marker file, and the Cirrus runner verifying that the file was created, to rule out typos in the tag name or something like that. I don’t think that’s really necessary.)

Moved rekor setup into the CI script, CI logs show it running

=== RUN   Test_rekorUploadKeyOrCert
2025/06/24 12:25:31 [DEBUG] GET http://127.0.0.1:3000/api/v1/log/publicKey
=== RUN   Test_rekorUploadKeyOrCert/upload_valid_signature
=== RUN   Test_rekorUploadKeyOrCert/invalid_key
=== RUN   Test_rekorUploadKeyOrCert/invalid_signature
=== RUN   Test_rekorUploadKeyOrCert/invalid_payload
--- PASS: Test_rekorUploadKeyOrCert (0.53s)
    --- PASS: Test_rekorUploadKeyOrCert/upload_valid_signature (0.46s)
    --- PASS: Test_rekorUploadKeyOrCert/invalid_key (0.03s)
    --- PASS: Test_rekorUploadKeyOrCert/invalid_signature (0.01s)
    --- PASS: Test_rekorUploadKeyOrCert/invalid_payload (0.01s)
PASS
coverage: 59.6% of statements
ok  	github.com/containers/image/v5/signature/sigstore/rekor	0.549s	coverage: 59.6% of statements

Not sure how important it is but since the test run rootless I some

storage_reference_test.go:156: test requires root privileges on Linux

lines in the log now so I wonder if you are aware of it? (Of course outside of this PR discussion) Do we need to run unit tests as root as well?

test requires root privileges on Linux

The history of that is #2147 (comment) , and, again, the unit tests somewhat prioritize usability during ~interactive development.

Do we need to run unit tests as root as well?

IIRC some of them don’t succeed as root, so it would require a bit more fine-tuning. (Also we don’t need to run most of them twice.) Nice to have, and help would be appreciated, but perhaps not a top priority for anyone.

Nice to have, and help would be appreciated, but perhaps not a top priority for anyone.

I did a quick pass over it #2878, maybe that is good enough.

(I just rebased)

Ok I checked in the log the test did run properly so this should be good now.