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.

Sign up for GitHub

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

Jump to bottom

proposal: remove pkg/errors dependency #1880

Closed
zchee opened this issue May 14, 2022 · 15 comments · Fixed by sigstore/fulcio#598
Closed

proposal: remove pkg/errors dependency #1880

zchee opened this issue May 14, 2022 · 15 comments · Fixed by sigstore/fulcio#598
Labels
enhancement New feature or request

Comments

@zchee
Copy link
Contributor

zchee commented May 14, 2022 •
edited
Loading

Description

The pkg/errors package has been deprecated.
I think remove this dependency will more maintainability.

Actually, use fmt.Errorf with %w verb.

Also, I would like to point out that they are mixed in current cosign's souce code.
@zchee zchee added the enhancement New feature or request label May 14, 2022
@zchee
Copy link
Contributor Author

zchee commented May 14, 2022

cc: @dlorenc

@dlorenc
Copy link
Member

dlorenc commented May 14, 2022

cc @imjasonh

@imjasonh
Copy link
Member

+1 to this!

If you're volunteering to do the cleanup that's great, please cc me on any PRs you send. And if not that's okay too, just let me know and I'll take care of it.

@zchee
Copy link
Contributor Author

zchee commented May 16, 2022

@imjasonh @dlorenc will do cleanup.

@imjasonh
Copy link
Member

I added a quick WIP depcheck_test to help ensure we don't depend on this and uhh...we have a lot of transitive deps on it:

--- FAIL: TestNoDeps (0.71s)
    --- FAIL: TestNoDeps/github.com/sigstore/cosign/cmd/cosign (0.71s)
        depcheck.go:126: CheckNoDependency() = github.com/sigstore/cosign/cmd/cosign depends on banned dependency github.com/pkg/errors
            github.com/sigstore/cosign/cmd/cosign
            github.com/sigstore/cosign/cmd/cosign/cli  # Also: [github.com/sigstore/cosign/cmd/cosign/cli/attach github.com/sigstore/cosign/cmd/cosign/cli/attest github.com/sigstore/cosign/cmd/cosign/cli/copy github.com/sigstore/cosign/cmd/cosign/cli/download github.com/sigstore/cosign/cmd/cosign/cli/fulcio github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier github.com/sigstore/cosign/cmd/cosign/cli/generate github.com/sigstore/cosign/cmd/cosign/cli/rekor github.com/sigstore/cosign/cmd/cosign/cli/sign github.com/sigstore/cosign/cmd/cosign/cli/triangulate github.com/sigstore/cosign/cmd/cosign/cli/upload github.com/sigstore/cosign/cmd/cosign/cli/verify github.com/sigstore/cosign/pkg/policy github.com/sigstore/cosign/pkg/sget]
            github.com/sigstore/cosign/cmd/cosign/cli/options
            github.com/awslabs/amazon-ecr-credential-helper/ecr-login
            github.com/awslabs/amazon-ecr-credential-helper/ecr-login/api  # Also: [github.com/cockroachdb/apd/v2 github.com/containerd/stargz-snapshotter/estargz github.com/docker/cli/cli/config github.com/docker/cli/cli/config/configfile github.com/open-policy-agent/opa/ast github.com/open-policy-agent/opa/ast/location github.com/open-policy-agent/opa/bundle github.com/open-policy-agent/opa/internal/compiler/wasm github.com/open-policy-agent/opa/internal/jwx/buffer github.com/open-policy-agent/opa/internal/jwx/jwa github.com/open-policy-agent/opa/internal/jwx/jwk github.com/open-policy-agent/opa/internal/jwx/jws github.com/open-policy-agent/opa/internal/jwx/jws/sign github.com/open-policy-agent/opa/internal/jwx/jws/verify github.com/open-policy-agent/opa/internal/wasm/encoding github.com/open-policy-agent/opa/topdown github.com/sigstore/cosign/cmd/cosign/cli github.com/sigstore/cosign/cmd/cosign/cli/attach github.com/sigstore/cosign/cmd/cosign/cli/attest github.com/sigstore/cosign/cmd/cosign/cli/dockerfile github.com/sigstore/cosign/cmd/cosign/cli/fulcio github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioroots github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl github.com/sigstore/cosign/cmd/cosign/cli/generate github.com/sigstore/cosign/cmd/cosign/cli/manifest github.com/sigstore/cosign/cmd/cosign/cli/options github.com/sigstore/cosign/cmd/cosign/cli/publickey github.com/sigstore/cosign/cmd/cosign/cli/sign github.com/sigstore/cosign/cmd/cosign/cli/triangulate github.com/sigstore/cosign/cmd/cosign/cli/verify github.com/sigstore/cosign/pkg/cosign github.com/sigstore/cosign/pkg/cosign/attestation github.com/sigstore/cosign/pkg/cosign/git/github github.com/sigstore/cosign/pkg/cosign/git/gitlab github.com/sigstore/cosign/pkg/cosign/kubernetes github.com/sigstore/cosign/pkg/cosign/pkcs11key github.com/sigstore/cosign/pkg/cosign/tuf github.com/sigstore/cosign/pkg/oci/internal/signature github.com/sigstore/cosign/pkg/oci/layout github.com/sigstore/cosign/pkg/oci/mutate github.com/sigstore/cosign/pkg/oci/remote github.com/sigstore/cosign/pkg/policy github.com/sigstore/cosign/pkg/sget github.com/sigstore/cosign/pkg/signature github.com/sigstore/rekor/pkg/types/hashedrekord github.com/sigstore/rekor/pkg/types/hashedrekord/v0.0.1 github.com/sigstore/rekor/pkg/types/intoto github.com/sigstore/rekor/pkg/types/rekord github.com/sigstore/rekor/pkg/util github.com/sigstore/sigstore/pkg/oauthflow github.com/sigstore/sigstore/pkg/signature github.com/sigstore/sigstore/pkg/signature/kms/aws github.com/sigstore/sigstore/pkg/signature/kms/azure github.com/sigstore/sigstore/pkg/signature/kms/gcp github.com/sigstore/sigstore/pkg/signature/kms/hashivault github.com/yashtewari/glob-intersection]

The sigstore/cosign ones should be easy enough to drop, and there are also some in sigstore/sigstore and sigstore/rekor, but the rest will probably take a lot more work. At least getting them out of our own tree(s) will be nice.

@zchee
Copy link
Contributor Author

zchee commented May 16, 2022 •
edited
Loading

@imjasonh Yeah, I focus to sigstore/cosign at first. But if you want, I'll also cleanup to sigstore/xxx (actually, sigstore orgs) repositories.

And yes, remove all pkg/errors dependency would be hard, I think ok to +indirect.

@imjasonh imjasonh mentioned this issue May 16, 2022
Merged
@imjasonh
Copy link
Member

I've sent sigstore/sigstore#444 to drop it from sigstore/sigstore

@zchee zchee mentioned this issue May 16, 2022
Merged
@imjasonh
Copy link
Member

FYI, also chasing down some usages in dependencies:

@zchee
Copy link
Contributor Author

zchee commented May 19, 2022 •
edited
Loading

@imjasonh OK, finished initial implements pkgerrors vet tool.
https://github.com/zchee/go-analyzer/tree/main/pkgerrors

$ go vet -vettool=$(which pkgerrors) github.com/sigstore/rekor/...
# github.com/sigstore/rekor/pkg/types/jar
pkg/types/jar/jar.go:66:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/intoto
pkg/types/intoto/intoto.go:66:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/hashedrekord
pkg/types/hashedrekord/hashedrekord.go:67:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/rekord
pkg/types/rekord/rekord.go:66:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/rfc3161
pkg/types/rfc3161/rfc3161.go:66:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/rpm
pkg/types/rpm/rpm.go:66:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/tuf
pkg/types/tuf/tuf.go:68:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/sharding
pkg/sharding/ranges.go:58:23: found use location of the deprecated github.com/pkg/errors
pkg/sharding/ranges.go:63:24: found use location of the deprecated github.com/pkg/errors
pkg/sharding/ranges.go:96:23: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/util
pkg/util/checkpoint.go:134:10: found use location of the deprecated github.com/pkg/errors
pkg/util/checkpoint.go:138:10: found use location of the deprecated github.com/pkg/errors
pkg/util/signed_note.go:49:15: found use location of the deprecated github.com/pkg/errors
pkg/util/signed_note.go:54:15: found use location of the deprecated github.com/pkg/errors
pkg/util/signed_note.go:58:15: found use location of the deprecated github.com/pkg/errors
pkg/util/signed_note.go:161:11: found use location of the deprecated github.com/pkg/errors
pkg/util/signed_note.go:166:11: found use location of the deprecated github.com/pkg/errors
pkg/util/timestamp_note.go:164:10: found use location of the deprecated github.com/pkg/errors
pkg/util/timestamp_note.go:168:10: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/helm
pkg/types/helm/helm.go:66:15: found use location of the deprecated github.com/pkg/errors
pkg/types/helm/provenance.go:59:10: found use location of the deprecated github.com/pkg/errors
pkg/types/helm/provenance.go:76:10: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/alpine
pkg/types/alpine/alpine.go:66:15: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:97:10: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:111:10: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:122:10: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:129:10: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:148:11: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:154:12: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:175:11: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:181:12: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:186:12: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:190:12: found use location of the deprecated github.com/pkg/errors
pkg/types/alpine/apk.go:199:10: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/intoto/v0.0.1
pkg/types/intoto/v0.0.1/entry.go:190:16: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/types/hashedrekord/v0.0.1
pkg/types/hashedrekord/v0.0.1/entry.go:180:42: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/cmd/rekor-cli/app
cmd/rekor-cli/app/log_info.go:121:11: found use location of the deprecated github.com/pkg/errors
cmd/rekor-cli/app/pflags.go:119:14: found use location of the deprecated github.com/pkg/errors
cmd/rekor-cli/app/upload.go:143:16: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/pkg/api
pkg/api/api.go:71:15: found use location of the deprecated github.com/pkg/errors
pkg/api/api.go:79:15: found use location of the deprecated github.com/pkg/errors
pkg/api/api.go:87:16: found use location of the deprecated github.com/pkg/errors
pkg/api/api.go:96:15: found use location of the deprecated github.com/pkg/errors
pkg/api/api.go:100:15: found use location of the deprecated github.com/pkg/errors
pkg/api/api.go:104:15: found use location of the deprecated github.com/pkg/errors
pkg/api/trillian_client.go:330:15: found use location of the deprecated github.com/pkg/errors
pkg/api/trillian_client.go:334:15: found use location of the deprecated github.com/pkg/errors
# github.com/sigstore/rekor/cmd/rekor-server/app
cmd/rekor-server/app/watch.go:136:15: found use location of the deprecated github.com/pkg/errors
cmd/rekor-server/app/watch.go:140:15: found use location of the deprecated github.com/pkg/errors
cmd/rekor-server/app/watch.go:149:15: found use location of the deprecated github.com/pkg/errors

@imjasonh
Copy link
Member

Very cool! I'm not very familiar with go analyzers (yet!), how do I install the tool so I can call it with go vet? I can't find a package main.

It looks like you can wrap the analyzer in a singlechecker to make a tool that runs and maybe fixes?

@zchee
Copy link
Contributor Author

zchee commented May 19, 2022 •
edited
Loading

@imjasonh yeah , strange. now debug it :D

See below.

@zchee
Copy link
Contributor Author

zchee commented May 19, 2022 •
edited
Loading

note that I force pushed.

@zchee
Copy link
Contributor Author

zchee commented May 19, 2022 •
edited
Loading

@imjasonh
https://github.com/zchee/go-analyzer/tree/main/pkgerrors/cmd/pkgerrors

still can't resolve vendor. now trying to fix it.

This was referenced May 19, 2022
Merged
Merged
@imjasonh
Copy link
Member

I think we can say this is done! Thanks @zchee for taking this on, and for teaching me about Go analyzers in the process. 👍

@dlorenc dlorenc closed this as completed May 20, 2022
@zchee
Copy link
Contributor Author

zchee commented May 20, 2022

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cmd/app: remove dependency on deprecated github.com/pkg/errors zchee/sigstore-fulcio
3 participants
@imjasonh @dlorenc @zchee