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

tests: Add sigstore-rs client test #1434

Merged
merged 2 commits into from
Feb 6, 2025
Merged

Conversation

jku
Copy link
Member

@jku jku commented Feb 3, 2025

  • This test is not in root-signing-staging since the client does not support staging
  • Uses current main HEAD since last release does not have the "bundle" example we use
  • Does not test signing since the client does not support signing with the GitHub Actions workflow identity

Testing this is an issue:

  • as mentioned, we can't test this in staging
  • Here the test currently fails because of key_id mismatch for root v11 #1431: sigstore-rs expects specifically computed TUF keyids which current root metadata does not provide.
  • we can do a signing event to fix the keyid but can only test the result after the signing event is merged (but not necessarily published to production)

Setting as draft for now: let's figure out what we do with 1431.

@jku jku linked an issue Feb 3, 2025 that may be closed by this pull request
@jku jku mentioned this pull request Feb 3, 2025
@jku
Copy link
Member Author

jku commented Feb 4, 2025

I think we could leave this after the repository has been fixed: theupdateframework/tuf-on-ci#536 should be enough to guaranteethat the signing event is good.

jku added 2 commits February 6, 2025 10:19
* This test is not in root-signing-staging since the client does
  not support staging
* Uses current main HEAD since last release does not have the
  "bundle" example we use
* Does not test signing since the client does not support signing
  with the GitHub Actions workflow identity

This test currently fails because of sigstore#1431: sigstore-rs expects
specifically computed TUF keyids which current root
metadata does not provide.

Signed-off-by: Jussi Kukkonen <[email protected]>
This sigstore-rs branch contains the v12 root so allows testing
when v12 is getting published

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku force-pushed the rust-client-test branch from e4095f8 to 89158ba Compare February 6, 2025 08:25
@jku jku changed the base branch from main to sign/root-v12 February 6, 2025 08:26
@jku jku marked this pull request as ready for review February 6, 2025 08:26
@jku
Copy link
Member Author

jku commented Feb 6, 2025

Changes:

  • rebased on sign/root-v12, set base to sign/root-v12: I'm proposing we merge this with the signing event to get immediate testing with sigstore-rs
  • Switched to a branch of sigstore-rs that uses the v12 root as the embedded root (since v11 is incompatible): we should start using sigstore-rs release when possible

The idea here is that if everything works well, we get sigstore-rs tests running in the publishing pipeline. If something fails during publish, we can fix issues in the test workflow or even remove the sigstore-rs test if we decide so without redoing the signing event -- but we won't have another root update that breaks something without our knowledge.

@jku
Copy link
Member Author

jku commented Feb 6, 2025

If something fails during publish, we can fix issues in the test workflow or even remove the sigstore-rs test if we decide so

I guess it's reasonable to mention why I said that: the workflow has never been tested because it can only possibly work after the next online signing. So there is a chance of bugs here.

# TODO: Just use a released sigstore-rs when possible
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
repository: "jku/sigstore-rs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would your change need to be merged into upstream sigstore-rs after this so we can depend on that instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I would expect

  • root v12 change gets merged in sigstore-rs after we publish
  • sigstore-rs makes a release
  • we can start using the released sigstore-rs (maybe even via cargo and not actions checkout)

Copy link
Member

@kommendorkapten kommendorkapten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets try this out. The key id mismatch is the last known issue to me for sigstore-rs. Ans as it's quite complex to organize and test as @jku explains. This is the option with the fewest steps.

@jku jku mentioned this pull request Feb 6, 2025
@jku jku merged commit 0a60b8c into sigstore:sign/root-v12 Feb 6, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add test for sigstore-rs
2 participants