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

Create a test to run cosign from main #105

Open
haydentherapper opened this issue Sep 1, 2023 · 7 comments
Open

Create a test to run cosign from main #105

haydentherapper opened this issue Sep 1, 2023 · 7 comments
Labels
enhancement New feature or request github_actions Pull requests that update GitHub Actions code good first issue Good for newcomers

Comments

@haydentherapper
Copy link
Contributor

Description

The current issue where attestations are no longer persisted would have been caught with this.

@haydentherapper haydentherapper added the enhancement New feature or request label Sep 1, 2023
@bobcallaway
Copy link
Member

it would be nice to do the same logical thing for each supported client/SDK - perhaps have a template for each SDK (e.g. python, java, javascript, rust, etc) to implement as a workflow and then the prober reaches out to call out to public services? we could then have a policy that it should always run:

  • latest code in main (low priority alert)
  • all releases in last 365 days (high priority alert)
  • anything else deemed of interest (low priority alert)

@bobcallaway bobcallaway added good first issue Good for newcomers github_actions Pull requests that update GitHub Actions code labels Sep 1, 2023
@cmurphy
Copy link
Contributor

cmurphy commented Jan 4, 2024

With respect to cosign at least, what's the difference between this enhancement and the e2e tests that cosign already runs on pull requests? https://github.com/sigstore/cosign/blob/286a98a4a99c1b2f32f84b0d560e324100312280/.github/workflows/tests.yaml#L86

If we had a post-merge workflow running the same tests on main, would that address the cosign part of this? Do we need to add more tests to that e2e suite to catch this attestation issue?

@bobcallaway
Copy link
Member

With respect to cosign at least, what's the difference between this enhancement and the e2e tests that cosign already runs on pull requests? https://github.com/sigstore/cosign/blob/286a98a4a99c1b2f32f84b0d560e324100312280/.github/workflows/tests.yaml#L86

If we had a post-merge workflow running the same tests on main, would that address the cosign part of this? Do we need to add more tests to that e2e suite to catch this attestation issue?

https://github.com/sigstore/cosign/blob/286a98a4a99c1b2f32f84b0d560e324100312280/test/e2e_test.sh runs against a local clone of the main branch of code, vs the version of the service running in production. The tests in this issue are about ensuring that we are actively testing the latest client code against the system running in production.

@jku
Copy link
Member

jku commented Jan 23, 2024

This issue may be aiming for something more minimal but... does cosign use sigstore-conformance at all yet?

IIRC conformance tests against both staging and production -- and you can set "expected failures" in case the client doesn't quite agree with the test suite...

@jku
Copy link
Member

jku commented Jan 23, 2024

IIRC conformance tests against both staging and production

I may have misremembered: It looks like the client-under-test is expected to use production (and right now there is now flag to change that in the "client-under-test" CLI).

@cmurphy
Copy link
Contributor

cmurphy commented Jan 26, 2024

This issue may be aiming for something more minimal but... does cosign use sigstore-conformance at all yet?

It looks like cosign doesn't use sigstore-conformance yet. The other clients do. If we implement an entrypoint and a conformance.yml workflow for cosign, and the other clients are already doing this, I think that would cover this part

  • latest code in main (low priority alert)

then we could implement similar workflows for all the clients that run on a schedule for releases to cover the next two parts

  • all releases in last 365 days (high priority alert)
  • anything else deemed of interest (low priority alert)

right?

IIRC conformance tests against both staging and production

I think for this case we only care about production anyway, based on @bobcallaway's comment

The tests in this issue are about ensuring that we are actively testing the latest client code against the system running in production.

@haydentherapper
Copy link
Contributor Author

I think we should avoid the prober tests becoming tests for all Sigstore clients. The usage of Cosign is a smoke test against signing and verification and Cosign was meant to represent all clients (which now is not the most accurate statement).

I agree that the conformance tests cover testing from HEAD against production (and staging is a WIP). The other two (releases in the last year and anything else) likely shouldn't be covered via probers. I think these could be useful as workflows for clients to run, but not as paging probes, so these aren't as high priority.

I think the best use of our time would be to get Cosign integrated with the conformance test suite (sigstore/cosign#2434).

One other thing we could do is sign and verify with both Cosign and a representative client like sigstore-python that's integrated with the conformance test suite. Once Cosign is integrated and has a minimal number of tests it has opted out of, then just using Cosign is likely sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request github_actions Pull requests that update GitHub Actions code good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants