zeitgeist icon indicating copy to clipboard operation
zeitgeist copied to clipboard

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

Open liggitt opened this issue 2 years ago • 21 comments

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:

  • Cloud provider or hardware configuration:
  • OS (e.g: cat /etc/os-release):
  • Kernel (e.g. uname -a):
  • Others:

liggitt avatar May 16 '23 01:05 liggitt

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

saschagrunert avatar May 16 '23 07:05 saschagrunert

@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?

saschagrunert avatar May 16 '23 07:05 saschagrunert

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 avatar May 16 '23 15:05 liggitt

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

  • https://github.com/search?q=sigs.k8s.io%2Fzeitgeist+org%3Akubernetes&type=code&p=1
    • kubernetes/kubernetes: https://github.com/kubernetes/kubernetes/blob/master/build/dependencies.yaml
    • kubernetes/release: https://github.com/kubernetes/release/blob/master/dependencies.yaml
  • https://github.com/search?q=sigs.k8s.io%2Fzeitgeist+org%3Akubernetes-sigs&type=code&p=1
    • kubernetes-sigs/promo-tools: https://github.com/kubernetes-sigs/promo-tools/blob/main/dependencies.yaml
    • kubernetes-sigs/release-utils: https://github.com/kubernetes-sigs/release-utils/blob/main/dependencies.yaml

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

liggitt avatar May 16 '23 15:05 liggitt

@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.

Pluies avatar May 16 '23 15:05 Pluies

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 avatar May 16 '23 16:05 liggitt

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

liggitt avatar May 16 '23 18:05 liggitt

+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

dims avatar May 16 '23 18:05 dims

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.

saschagrunert avatar May 17 '23 07:05 saschagrunert

+1 on sascha's comment

cpanato avatar May 17 '23 07:05 cpanato

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 avatar May 17 '23 12:05 liggitt

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

liggitt avatar May 17 '23 12:05 liggitt

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?

saschagrunert avatar May 17 '23 12:05 saschagrunert

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.

liggitt avatar May 17 '23 13:05 liggitt

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

saschagrunert avatar May 17 '23 13:05 saschagrunert

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

BenTheElder avatar May 17 '23 23:05 BenTheElder

Can this one be closed @cpanato @liggitt ?

saschagrunert avatar Nov 15 '23 11:11 saschagrunert

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

cpanato avatar Nov 15 '23 11:11 cpanato

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

k8s-triage-robot avatar Feb 13 '24 12:02 k8s-triage-robot

/lifecycle frozen

BenTheElder avatar Feb 13 '24 19:02 BenTheElder

/remove-lifecycle frozen

cpanato avatar Feb 20 '24 17:02 cpanato