Move fulcioroots and tuf packages from cosign #435

imjasonh · 2022-05-11T19:08:11Z

Summary

This moves these packages from sigstore/cosign into sigstore/sigstore.

pkg/fulcioroots comes from cosign's cmd/cosign/cli/fulcio/fulcioroots@2ccdb3, and drops that package's behavior when the SIGSTORE_ROOT_FILE env var is set -- this will remain in sigstore/cosign.
pkg/tuf comes from cosign's pkg/cosign/tuf@2ccdb3 and is otherwise largely unchanged. Some methods were unexported that aren't used outside of this package.

Release Note

pkg/fulcioroots and pkg/tuf are moved from cosign repo

imjasonh · 2022-05-11T19:22:42Z

The lint check is failing due to un-doc-commented exported methods, but I think I'd rather just leave those as-is than balloon the changes in this PR. If folks feel strongly about having these commented I think we should do that in a separate PR.

imjasonh · 2022-05-11T19:28:10Z

@haydentherapper @asraa

pkg/fulcioroots/fulcioroots.go

pkg/tuf/policy.go

pkg/tuf/signer.go

haydentherapper · 2022-05-11T20:24:33Z

go.mod

@@ -33,6 +33,11 @@ require (
 	gopkg.in/square/go-jose.v2 v2.6.0
 )

+require (
+	cloud.google.com/go/storage v1.10.0


Were there concerns about pulling in this library? Was it just due to size?

Yeah I'd like to replace it with regular HTTP calls instead of taking a dependency on this GCS package. It's not huge, it's just mostly unnecessary.

imjasonh · 2022-05-13T15:38:50Z

Any more concerns here? If not I can merge and un-WIP sigstore/cosign#1866 to start using this.

haydentherapper · 2022-05-13T15:43:17Z

This is a large pr, I want to double check nothing has changed. I don’t have a lot of confidence that a break would be detected once this is used in cosign due to the lack of tests.
I’ll take a look later today or early Monday.

haydentherapper · 2022-05-13T21:27:57Z

This LGTM - If you're able to wait a few days, it'd be nice to also get @asraa's LGTM since she's the author of the TUF changes.

asraa · 2022-05-17T14:39:34Z

I'm back in office and taking a look!

pkg/fulcioroots/fulcioroots.go

pkg/tuf/repository/root.json

pkg/tuf/client.go

haydentherapper · 2022-05-19T00:25:20Z

There's active debugging on the TUF client from a recent postmortem, so we may want to hold off on submitting this until that's resolved, as we'd like to get the bug fixed before 1.0

asraa · 2022-05-19T15:12:40Z

In particular this is probably worth waiting for the fix for: sigstore/cosign#1899

imjasonh · 2022-05-19T16:58:31Z

Sounds good, I've converted to a draft for now. After the issue is resolved I can pick it back up.

znewman01 · 2022-05-31T14:30:41Z

After we do this: should probably consider sigstore/cosign#1935

imjasonh · 2022-05-31T14:59:42Z

After we do this: should probably consider sigstore/cosign#1935

I'd be fine cleaning it up first then moving it, too. There's no real rush to move it, especially if it causes confusion during cleanups or bug fixes.

imjasonh · 2022-06-08T12:42:17Z

I'd like to pick this up again after the TUF changes that blocked us last time, and after sigstore/cosign#1967 -- let me know if there's anything else I should wait for, otherwise I'll update this PR.

* fix: fix fetching updated targets from TUF root Signed-off-by: Asra Ali <asraa@google.com> add comment Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> possible fix windows Signed-off-by: Asra Ali <asraa@google.com> lint Signed-off-by: Asra Ali <asraa@google.com> fix windows maybe Signed-off-by: Asra Ali <asraa@google.com> fix close Signed-off-by: Asra Ali <asraa@google.com> * update zack comments Signed-off-by: Asra Ali <asraa@google.com> update fix Signed-off-by: Asra Ali <asraa@google.com> update and add some debug Signed-off-by: Asra Ali <asraa@google.com> add debug Signed-off-by: Asra Ali <asraa@google.com> no cache Signed-off-by: Asra Ali <asraa@google.com> remove debug Signed-off-by: Asra Ali <asraa@google.com> * try haydens comments Signed-off-by: Asra Ali <asraa@google.com> * Use Rekor API for pubkeys before TUF if so specified. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * Address PR feedback, bump golangci-lint from 1.46.0 to 1.46.2 Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * Add comments for the env variables. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * Use path instead of filepath, basically revert to what it was before. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * ho hum, really just use the path. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * When interacting with fs do not use OS specific separators. Signed-off-by: Ville Aikas <vaikas@chainguard.dev> * fix windows line endings Signed-off-by: Asra Ali <asraa@google.com> * pass embedded into initialization Signed-off-by: Asra Ali <asraa@google.com> Co-authored-by: Ville Aikas <vaikas@chainguard.dev> Signed-off-by: Jason Hall <jason@chainguard.dev>

* move rekor public key fetch inside GetRekorPubs Signed-off-by: Asra Ali <asraa@google.com> * use in-memory metadata and targets, sync to disk on start and updates Signed-off-by: Asra Ali <asraa@google.com> update Signed-off-by: Asra Ali <asraa@google.com> * Use TUF singleton. Co-authored-by: Ville Aikas <vaikas@chainguard.dev> Signed-off-by: Asra Ali <asraa@google.com> * hayden comment, sync.Once used Signed-off-by: Asra Ali <asraa@google.com> * return global error Signed-off-by: Asra Ali <asraa@google.com> Co-authored-by: Ville Aikas <vaikas@chainguard.dev> Signed-off-by: Jason Hall <jason@chainguard.dev>

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com> Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com> Signed-off-by: Jason Hall <jason@chainguard.dev>

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

znewman01 · 2022-06-09T13:11:40Z

Lint failures seemingly related

imjasonh · 2022-06-09T13:57:01Z

Lint failures seemingly related

Yeah, there's a lot of un-godoc'ed code in the tuf and fulcioroots packages. I suspect some of them can be addressed by more aggressively unexporting types that don't need to be exported, and godoc'ing the rest, but I don't feel confident enough to document this code well.

I did a little bit of lint-hunting in a previous iteration of this work, which I've done now on this one, but I think the rest should be addressed in a separate PR.

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

znewman01 · 2022-06-09T14:04:09Z

but I think the rest should be addressed in a separate PR.

Works for me, but is there any way to disable the linter on this code for now so we don't break HEAD?

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

imjasonh · 2022-06-09T14:15:01Z

Works for me, but is there any way to disable the linter on this code for now so we don't break HEAD?

I've updated the lint action to only complain about findings in changed lines, that will at least let other PRs get merged while these are un-godoc'd.

We'll still need someone to force-merge this over the complaints of the linter in this change though. Or make the check not-required.

znewman01 · 2022-06-09T14:24:45Z

Can you exclude whole directories/specific files instead? IMO that makes more sense, then we can remove that restriction in a follow-up PR and doesn't require any kind of override.

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

imjasonh · 2022-06-09T14:32:04Z

Can you exclude whole directories/specific files instead? IMO that makes more sense, then we can remove that restriction in a follow-up PR and doesn't require any kind of override.

Good call, done!

imjasonh · 2022-06-09T20:18:46Z

Gentle ping @haydentherapper @asraa -- I'd love to get this in before anything else touches these packages.

haydentherapper · 2022-06-09T20:25:37Z

Ooo today, will take a look tomorrow!

dlorenc · 2022-06-10T13:52:19Z

Here we goooo!

imjasonh mentioned this pull request May 11, 2022

Use pkg/fulcioroots and pkg/tuf from sigstore/sigstore sigstore/cosign#1866

Merged

imjasonh marked this pull request as draft May 11, 2022 19:28

imjasonh changed the title ~~Move fulcioroots and pkg/tuf from cosign~~ [WIP] Move fulcioroots and pkg/tuf from cosign May 11, 2022

imjasonh changed the title ~~[WIP] Move fulcioroots and pkg/tuf from cosign~~ Move fulcioroots and pkg/tuf from cosign May 11, 2022

imjasonh marked this pull request as ready for review May 11, 2022 19:28

imjasonh changed the title ~~Move fulcioroots and pkg/tuf from cosign~~ Move fulcioroots, test and tuf packages from cosign May 11, 2022

haydentherapper reviewed May 11, 2022

View reviewed changes

pkg/fulcioroots/fulcioroots.go Outdated Show resolved Hide resolved

pkg/fulcioroots/fulcioroots.go Outdated Show resolved Hide resolved

pkg/tuf/policy.go Show resolved Hide resolved

pkg/tuf/signer.go Show resolved Hide resolved

haydentherapper previously approved these changes May 11, 2022

View reviewed changes

imjasonh mentioned this pull request May 11, 2022

Followups from #435 #436

Open

4 tasks

haydentherapper reviewed May 11, 2022

View reviewed changes

imjasonh dismissed haydentherapper’s stale review via a1c5180 May 12, 2022 14:53

imjasonh mentioned this pull request May 12, 2022

[WIP] Use fulcioroots from sigstore/sigstore wlynch/smimecosign#1

Closed

asraa reviewed May 17, 2022

View reviewed changes

pkg/fulcioroots/fulcioroots.go Outdated Show resolved Hide resolved

pkg/tuf/repository/root.json Show resolved Hide resolved

pkg/tuf/client.go Show resolved Hide resolved

pkg/tuf/client.go Outdated Show resolved Hide resolved

imjasonh marked this pull request as draft May 19, 2022 16:58

vaikas mentioned this pull request May 31, 2022

Cleanup for TUF code sigstore/cosign#1935

Open

imjasonh force-pushed the fulcio-tuf branch from 0f493cb to d9a9344 Compare June 8, 2022 13:02

imjasonh changed the title ~~Move fulcioroots, test and tuf packages from cosign~~ Move fulcioroots and tuf packages from cosign Jun 8, 2022

imjasonh marked this pull request as ready for review June 8, 2022 13:03

asraa and others added 5 commits June 8, 2022 14:48

move fulcioroots to pkg/fulcioroots

2f25a72

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

remove alternate root behavior, rm fulcioroots_test.go, use pkg/tuf

5fd48b6

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

imjasonh force-pushed the fulcio-tuf branch from d7fbd7d to 5fd48b6 Compare June 8, 2022 18:48

imjasonh added 4 commits June 8, 2022 14:57

base on latest go.sum

5356b9b

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

go mod tidy

dd93120

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

base on latest go.sum, again somehow?

ca71d87

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

Merge branch 'main' into fulcio-tuf

5bb8650

imjasonh mentioned this pull request Jun 9, 2022

Remove cosign pkg, take a dep on it. sigstore/policy-controller#23

Merged

vaikas mentioned this pull request Jun 9, 2022

Remove dependency on cosign/cmd for fulcioroots sigstore/policy-controller#24

Closed

address some low-hanging lint fruit

dc6ea5c

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

imjasonh force-pushed the fulcio-tuf branch from ae1750d to dc6ea5c Compare June 9, 2022 13:57

lint: only fail PRs on new findings

eb6ee04

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

lint: ignore revive lint findings in pkg/tuf

a32a80c

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

znewman01 approved these changes Jun 9, 2022

View reviewed changes

dlorenc approved these changes Jun 9, 2022

View reviewed changes

asraa approved these changes Jun 10, 2022

View reviewed changes

dlorenc merged commit ec8f2d4 into sigstore:main Jun 10, 2022

imjasonh mentioned this pull request Jun 10, 2022

Use pkg/fulcioroots from sigstore/sigstore sigstore/gitsign#67

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move fulcioroots and tuf packages from cosign #435

Move fulcioroots and tuf packages from cosign #435

imjasonh commented May 11, 2022 •

edited

imjasonh commented May 11, 2022

imjasonh commented May 11, 2022

haydentherapper May 11, 2022

imjasonh May 11, 2022

imjasonh commented May 13, 2022

haydentherapper commented May 13, 2022

haydentherapper commented May 13, 2022

asraa commented May 17, 2022

haydentherapper commented May 19, 2022

asraa commented May 19, 2022

imjasonh commented May 19, 2022

znewman01 commented May 31, 2022

imjasonh commented May 31, 2022

imjasonh commented Jun 8, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

imjasonh commented Jun 9, 2022

haydentherapper commented Jun 9, 2022

dlorenc commented Jun 10, 2022

Move fulcioroots and tuf packages from cosign #435

Move fulcioroots and tuf packages from cosign #435

Conversation

imjasonh commented May 11, 2022 • edited

Summary

Release Note

imjasonh commented May 11, 2022

imjasonh commented May 11, 2022

haydentherapper May 11, 2022

Choose a reason for hiding this comment

imjasonh May 11, 2022

Choose a reason for hiding this comment

imjasonh commented May 13, 2022

haydentherapper commented May 13, 2022

haydentherapper commented May 13, 2022

asraa commented May 17, 2022

haydentherapper commented May 19, 2022

asraa commented May 19, 2022

imjasonh commented May 19, 2022

znewman01 commented May 31, 2022

imjasonh commented May 31, 2022

imjasonh commented Jun 8, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

znewman01 commented Jun 9, 2022

imjasonh commented Jun 9, 2022

imjasonh commented Jun 9, 2022

haydentherapper commented Jun 9, 2022

dlorenc commented Jun 10, 2022

imjasonh commented May 11, 2022 •

edited