search

kubernetes-sigs / zeitgeist

Zeitgeist: the language-agnostic dependency checker
https://godoc.org/sigs.k8s.io/zeitgeist
Apache License 2.0
181 stars 23 forks source link

Massive numbers of transitive dependencies (including github.com/hashicorp/go-retryablehttp) required to build #543

Closed liggitt closed 5 months ago

liggitt commented 1 year ago

What happened:

Cannot build zeitgeist without pulling in MPL-licensed projects not in the CNCF allowlist.

zeitgeist depends on github.com/xanzy/go-gitlab which pulls in github.com/hashicorp/go-retryablehttp which is MPL-licensed and not included in the CNCF allowlist

https://github.com/cncf/foundation/blob/main/license-exceptions/

https://github.com/cncf/foundation/blob/main/allowed-third-party-license-policy.md#cncf-allowlist-license-policy

https://github.com/cncf/foundation/issues/138

What you expected to happen:

No dependencies on MPL-licensed projects not explicitly allowlisted

Remote client libraries like aws/github/gitlab are not required to build/run the local-only dependency file checks

How to reproduce it (as minimally and precisely as possible):

run go mod vendor to see code actually used/linked by zeitgeist and observe the go-gitlab and go-retryablehttp code is required to build.

Anything else we need to know?:

Environment:

saschagrunert commented 1 year ago

github.com/xanzy/go-gitlab seems to be the main reason here.

saschagrunert commented 1 year ago

@cpanato since you added the GitLab support: What would you prefer here? We could either search for another dependency, add the API calls manually or drop the support for GitLab at all. Any thoughts on that?

liggitt commented 1 year ago

For consumers which don't use any of the remote functionality, just the scanning of local files, it's unfortunate this pulls in so many transitive dependencies. Dropping zeitgeist from the kubernetes tools submodule cleaned out all of these:

diff --git a/hack/tools/go.mod b/hack/tools/go.mod
index 257b8eedd32..1f22dc044db 100644
--- a/hack/tools/go.mod
+++ b/hack/tools/go.mod
@@ -13,7 +13,6 @@ require (
    gotest.tools/gotestsum v1.6.4
    honnef.co/go/tools v0.4.2
    sigs.k8s.io/logtools v0.5.0
-   sigs.k8s.io/zeitgeist v0.2.0
 )

 require (
@@ -31,10 +30,8 @@ require (
    github.com/alingse/asasalint v0.0.11 // indirect
    github.com/ashanbrown/forbidigo v1.4.0 // indirect
    github.com/ashanbrown/makezero v1.1.1 // indirect
-   github.com/aws/aws-sdk-go v1.37.6 // indirect
    github.com/beorn7/perks v1.0.1 // indirect
    github.com/bkielbasa/cyclop v1.2.0 // indirect
-   github.com/blang/semver v3.5.1+incompatible // indirect
    github.com/blizzy78/varnamelen v0.8.0 // indirect
    github.com/bombsimon/wsl/v3 v3.4.0 // indirect
    github.com/breml/bidichk v0.2.3 // indirect
@@ -48,7 +45,6 @@ require (
    github.com/davecgh/go-spew v1.1.1 // indirect
    github.com/denis-tingaikin/go-header v0.4.3 // indirect
    github.com/dnephin/pflag v1.0.7 // indirect
-   github.com/emirpasic/gods v1.12.0 // indirect
    github.com/esimonov/ifshort v1.0.4 // indirect
    github.com/ettle/strcase v0.1.1 // indirect
    github.com/fatih/color v1.14.1 // indirect
@@ -57,9 +53,6 @@ require (
    github.com/fsnotify/fsnotify v1.5.4 // indirect
    github.com/fzipp/gocyclo v0.6.0 // indirect
    github.com/go-critic/go-critic v0.6.7 // indirect
-   github.com/go-git/gcfg v1.5.0 // indirect
-   github.com/go-git/go-billy/v5 v5.0.0 // indirect
-   github.com/go-git/go-git/v5 v5.2.0 // indirect
    github.com/go-toolsmith/astcast v1.1.0 // indirect
    github.com/go-toolsmith/astcopy v1.0.3 // indirect
    github.com/go-toolsmith/astequal v1.1.0 // indirect
@@ -81,8 +74,6 @@ require (
    github.com/golangci/revgrep v0.0.0-20220804021717-745bb2f7c2e6 // indirect
    github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 // indirect
    github.com/google/go-cmp v0.5.9 // indirect
-   github.com/google/go-github/v33 v33.0.0 // indirect
-   github.com/google/go-querystring v1.0.0 // indirect
    github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
    github.com/gordonklaus/ineffassign v0.0.0-20230107090616-13ace0543b28 // indirect
    github.com/gostaticanalysis/analysisutil v0.7.1 // indirect
@@ -90,23 +81,17 @@ require (
    github.com/gostaticanalysis/forcetypeassert v0.1.0 // indirect
    github.com/gostaticanalysis/nilerr v0.1.1 // indirect
    github.com/hashicorp/errwrap v1.0.0 // indirect
-   github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
    github.com/hashicorp/go-multierror v1.1.1 // indirect
-   github.com/hashicorp/go-retryablehttp v0.6.4 // indirect
    github.com/hashicorp/go-version v1.6.0 // indirect
    github.com/hashicorp/hcl v1.0.0 // indirect
    github.com/hexops/gotextdiff v1.0.3 // indirect
-   github.com/imdario/mergo v0.3.9 // indirect
    github.com/inconshreveable/mousetrap v1.0.1 // indirect
-   github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
    github.com/jgautheron/goconst v1.5.1 // indirect
    github.com/jingyugao/rowserrcheck v1.1.1 // indirect
    github.com/jirfag/go-printf-func-name v0.0.0-20200119135958-7558a9eaa5af // indirect
-   github.com/jmespath/go-jmespath v0.4.0 // indirect
    github.com/jonboulle/clockwork v0.2.2 // indirect
    github.com/julz/importas v0.1.0 // indirect
    github.com/junk1tm/musttag v0.4.5 // indirect
-   github.com/kevinburke/ssh_config v0.0.0-20190725054713-01f96b0aa0cd // indirect
    github.com/kisielk/errcheck v1.6.3 // indirect
    github.com/kisielk/gotool v1.0.0 // indirect
    github.com/kkHAIKE/contextcheck v1.1.3 // indirect
@@ -155,7 +140,6 @@ require (
    github.com/sashamelentyev/interfacebloat v1.1.0 // indirect
    github.com/sashamelentyev/usestdlibvars v1.23.0 // indirect
    github.com/securego/gosec/v2 v2.15.0 // indirect
-   github.com/sergi/go-diff v1.1.0 // indirect
    github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c // indirect
    github.com/sirupsen/logrus v1.9.0 // indirect
    github.com/sivchari/containedctx v1.0.2 // indirect
@@ -184,8 +168,6 @@ require (
    github.com/ultraware/funlen v0.0.3 // indirect
    github.com/ultraware/whitespace v0.0.5 // indirect
    github.com/uudashr/gocognit v1.0.6 // indirect
-   github.com/xanzy/go-gitlab v0.43.0 // indirect
-   github.com/xanzy/ssh-agent v0.2.1 // indirect
    github.com/yagipy/maintidx v1.0.0 // indirect
    github.com/yeya24/promlinter v0.2.0 // indirect
    gitlab.com/bosi/decorder v0.2.3 // indirect
@@ -196,18 +178,13 @@ require (
    golang.org/x/exp v0.0.0-20220722155223-a9213eeb770e // indirect
    golang.org/x/exp/typeparams v0.0.0-20230203172020-98cc5a0785f9 // indirect
    golang.org/x/mod v0.8.0 // indirect
-   golang.org/x/net v0.6.0 // indirect
-   golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 // indirect
    golang.org/x/sync v0.1.0 // indirect
    golang.org/x/sys v0.5.0 // indirect
    golang.org/x/term v0.5.0 // indirect
    golang.org/x/text v0.7.0 // indirect
-   golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 // indirect
    golang.org/x/tools v0.6.0 // indirect
-   google.golang.org/appengine v1.6.7 // indirect
    google.golang.org/protobuf v1.28.0 // indirect
    gopkg.in/ini.v1 v1.67.0 // indirect
-   gopkg.in/warnings.v0 v0.1.2 // indirect
    gopkg.in/yaml.v2 v2.4.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
    mvdan.cc/gofumpt v0.4.0 // indirect
diff --git a/hack/tools/tools.go b/hack/tools/tools.go
index dc7fb413d2d..9a639e6d910 100644
--- a/hack/tools/tools.go
+++ b/hack/tools/tools.go
@@ -32,7 +32,7 @@ import (
    _ "gotest.tools/gotestsum"

    // dependencies
-   _ "sigs.k8s.io/zeitgeist"
+   // _ "sigs.k8s.io/zeitgeist"

    // mockgen
    _ "github.com/golang/mock/mockgen"
liggitt commented 1 year ago

looking at our use of sigs.k8s.io/zeitgeist:

None of those appear to be using the upstream capabilities at all... am I missing a place this is required in-project?

I'd probably recommend the upstreams functionality be dropped or isolated so our local-scanning uses don't need to pick up so many transitive deps

Pluies commented 1 year ago

@liggitt Zeitgeist is used by a bunch of other people besides the k8s project (selfishly, me included 😅 ) who do rely on the remote scanning functionality a lot. I'm not against splitting up into a separate local-only subproject, but it'd strongly argue against removing the upstream functionality altogether.

liggitt commented 1 year ago

splitting the remote stuff into a separate command / submodule seems like the best approach to me ... we probably shouldn't have added that level of scope / additional dependencies / remote functionality into an existing local-only command with existing users

liggitt commented 1 year ago

Opened https://github.com/kubernetes-sigs/zeitgeist/pull/547 as a POC of splitting the remote capabilities into a separate binary / package

dims commented 1 year ago

+1 to splitting the remote stuff into a separate command and holding the line on what gets into the submodule that we end up using in k/k

saschagrunert commented 1 year ago

Is there any reason for k/k to not just pull in the zeitgeist binary from the releases page? We could avoid the rate limiting by moving them to a google cloud bucket as well.

cpanato commented 1 year ago

+1 on sascha's comment

liggitt commented 1 year ago

Is there any reason for k/k to not just pull in the zeitgeist binary from the releases page?

By representing the dependency in hack/tools/go.mod, you can go mod download everything you need for offline dev. Making the verify scripts do ad-hoc downloads of binaries takes hard dependencies on external systems, which we want to avoid

If zeitgeist won't isolate these dependencies, we'll probably end up doing something like https://github.com/kubernetes/kubernetes/pull/118076 instead (to avoid all the per-platform download/extract logic required to pull pre-built deps), which has the same downsides of making the tools go.mod file not represent all the external tools required to run verify scripts

liggitt commented 1 year ago

There's also still the original issue of zeitgeist building in github.com/hashicorp/go-retryablehttp which needs the license issue resolved

saschagrunert commented 1 year ago

If zeitgeist won't isolate these dependencies

We can isolate them, I'm searching for alternatives of shipping two binaries. Would a build tag (default off) work as well as per https://github.com/kubernetes-sigs/zeitgeist/pull/547#discussion_r1196039451?

liggitt commented 1 year ago

unfortunately, build tags don't inform version resolution, transitive dependency calculations in the module graph, or vendoring. None of those operations have build tags available so they all assume any build tag is possible and pull everything in.

saschagrunert commented 1 year ago

Makes sense, then let's move forward with https://github.com/kubernetes-sigs/zeitgeist/pull/547

BenTheElder commented 1 year ago

Let's also avoid more direct dependencies on paid SaaS instead of project controlled domains 🙃

saschagrunert commented 10 months ago

Can this one be closed @cpanato @liggitt ?

cpanato commented 10 months ago

no, i need to rebase liggitt pr, will do this week

k8s-triage-robot commented 7 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

BenTheElder commented 7 months ago

/lifecycle frozen

cpanato commented 6 months ago

/remove-lifecycle frozen