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

Test against staging too #121

Closed
jku opened this issue Jan 24, 2024 · 8 comments · Fixed by #124 or #127
Closed

Test against staging too #121

jku opened this issue Jan 24, 2024 · 8 comments · Fixed by #124 or #127
Labels
enhancement New feature or request

Comments

@jku
Copy link
Member

jku commented Jan 24, 2024

It eould be useful if this project also ensured that the client works with current staging infra:

  • client developers would find out ASAP if their client breaks when something changes in staging: these changes are likely to propagate to production later
    • in case of infrastructure bug, client developer complaints could prevent the propagation of this bug
    • in case of client bug or missing feature this gives client developer more time to fix issues (or ask for delay in infrastructure change propagation)

There might be several ways to accomplish this but a simple one might be to

  • Add a --staging flag to the client-under-test CLI spec
  • add a single new test test_simple_staging() that does exactly what test_simple() does but against staging

cc @loosebazooka

@jku jku added the enhancement New feature or request label Jan 24, 2024
@woodruffw
Copy link
Member

Adding --staging works for me, but thinking even more generally: it'd be nice to make this parametric, i.e. allow every test to run on both staging and production (if supported by the client).

(But that's a more generic, long-term thought. I'm okay with having this be one-off for now.)

@loosebazooka
Copy link
Member

I can add a staging flag into the java "cli".

I also think it might make sense to run this on a cron (if people aren't already). Currently java only runs it on PRs.

@woodruffw
Copy link
Member

I also think it might make sense to run this on a cron (if people aren't already). Currently java only runs it on PRs.

Agreed -- I believe we run it on a schedule on sigstore-python, and it's helped us with regressions/availability issues before!

@jku
Copy link
Member Author

jku commented Feb 21, 2024

I'm reopening since the staging support is not really usable yet.

I have a plan that allows us to get to running staging tests soon but is still compatible with the idea of running "the whole suite" against testing. This plan assumes the workflows running the suite will add a new step (essentially will run the action a second time for staging).

  1. currently all tests supposedly run against staging: this does not actually work as all of the embedded assets are for production infra
    • let's add a custom pytest marker "staging" to tests that we think are going to work with staging as is
    • then modify pytest config so it only runs those marked tests when run with --staging
    • later on we can add assets for staging, figure out asset loading, and add "staging" markers to more tests
  2. Let's add a boolean action input "staging" to action.yml which sets --staging in the command line
  3. Let's add support for --staging in action.py
  4. Let's modify sigstore-python-conformance to accept --staging
    • this already kind of works but emits a warning because we do sigstore sign --staging instead of the correct sigstore --staging sign
  5. Let's add a new step into selftest workflow that calls the action with staging: true

After these steps we'll know the staging support works, can maybe improve cli_protocol.md a bit and then ask client devs to implement --staging in their conformance clients and add the new workflow step.

@woodruffw how does this sound?

@jku
Copy link
Member Author

jku commented Feb 21, 2024

This plan assumes the workflows running the suite will add a new step (essentially will run the action a second time for staging).

I suppose alternatively action.py could call pytest twice if --staging is given to it, once with staging and once with prod.

@steiza
Copy link
Member

steiza commented Feb 22, 2024

currently all tests supposedly run against staging: this does not actually work as all of the embedded assets are for production infra

I believe sigstore-go, sigstore-java, sigstore-js, and sigstore-python all now support providing a custom trust root, so an alternative here would be to ensure the tests with embedded assets include a trust root for production, if they don't currently have one.

@woodruffw
Copy link
Member

I suppose alternatively action.py could call pytest twice if --staging is given to it, once with staging and once with prod.

I slightly prefer this, as a matter of keeping changes/configuration effort on each downstream's CI to a minimum. Otherwise your proposal SGTM @jku!

I believe sigstore-go, sigstore-java, sigstore-js, and sigstore-python all now support providing a custom trust root, so an alternative here would be to ensure the tests with embedded assets include a trust root for production, if they don't currently have one.

We aren't quite there yet on the sigstore-python side yet, unfortunately: sigstore/sigstore-python#821

(Taking a step back: I suppose we need to define how --staging and --trusted-root will interact in the conformance suite. We could say that they're mutually exclusive, I think?)

@jku
Copy link
Member Author

jku commented Feb 23, 2024

Supporting trusted-root seems like a good plan but since AFAIK that is not possible for a signing client at the moment, I think that's a future feature.

Current plan is for enable-staging to be an "additional feature" (in other words enable-staging=true does not disable testing production). With trusted-root that's probably different: I would expect staging and production to not be tested in that case, just the infrastructure defined by the trustedroot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
4 participants