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

Use pkg/fulcioroots from sigstore/sigstore #67

Merged
merged 1 commit into from Jun 15, 2022

Conversation

imjasonh
Copy link
Member

Signed-off-by: Jason Hall jason@chainguard.dev

These methods were moved from cosign->sigstore in sigstore/sigstore#435

After this change, remaining dependencies on cosign are:

$ git grep sigstore/cosign
...
internal/fulcio/identity.go:    "github.com/sigstore/cosign/pkg/providers"
internal/rekor/rekor.go:        "github.com/sigstore/cosign/pkg/cosign"
main.go:        _ "github.com/sigstore/cosign/pkg/providers/all"

Release Note

NONE

@imjasonh imjasonh requested a review from wlynch June 10, 2022 20:13
Copy link
Member

@wlynch wlynch left a comment

Choose a reason for hiding this comment

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

LGTM! Just need to fix up the test.

@imjasonh
Copy link
Member Author

LGTM! Just need to fix up the test.

Hmm, I didn't realize this test was using the fake roots code from cosign. We had decided to keep that in cosign for now, since we thought nobody but cosign would need it, but yet here we are 🙃.

We'll either need to find a way to get this test not to need this fake CA setup, or to move the fallback logic into the version in sigstore/sigstore. WDYT?

@wlynch
Copy link
Member

wlynch commented Jun 14, 2022

SGTM!

Leaning towards making this change within gitsign instead of sigstore/sigstore - I'm thinking it might be valuable to have the VerifySignature have configurable roots (either as a variadic option or moving func to a struct that has the roots / clients configured) instead of depending on global state from fulcioroots. 🤔

Signed-off-by: Jason Hall <jason@chainguard.dev>
@wlynch wlynch merged commit 3c72400 into sigstore:main Jun 15, 2022
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.

None yet

2 participants