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

Limit the number of 3rd-party dependencies #3072

Open
mimowo opened this issue Jan 15, 2025 · 10 comments
Open

Limit the number of 3rd-party dependencies #3072

mimowo opened this issue Jan 15, 2025 · 10 comments

Comments

@mimowo
Copy link

mimowo commented Jan 15, 2025

controller-runtime is a foundational library for many projects, such as Kueue: https://github.com/kubernetes-sigs/kueue.

As such it is very important for the future of the projects, and the large net of dependencies raises some concerns:

  • the 3rd party projects also need to pull tons of dependencies themselves, potentially risking conflicts
  • it is hard to review the dependencies as reliable given their number. In particular, there are a couple of 0.0.0 libraries

Would eliminating at least some of the 0.0.0 dependencies be feasible? This is more of an open discussion question than concrete call to action.

@mimowo
Copy link
Author

mimowo commented Jan 15, 2025

cc @tenzen-y

@tenzen-y
Copy link
Member

Here, what is 3rd party dependencies? Does that indicate non-K8s libraries (sigs.k8s.io and k8s.io)?
Or the 3rd party contains K8s libraries as well?

@mimowo
Copy link
Author

mimowo commented Jan 17, 2025

I'm thinking about the non-k8s dependencies.

@sbueringer
Copy link
Member

Are there specific ones you have in mind? In general I would assume that there are not many that k8s.io/* itself doesn't depend on

@mimowo
Copy link
Author

mimowo commented Jan 17, 2025

Not really, I was mostly hoping there are some low hanging fruits in the net that could be eliminated.

To make the issue a little bit more actionable, let me focus on the couple of 0.0.0 ones:

  • github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a // indirect
  • github.com/google/pprof v0.0.0-20241029153458-d1b30febd7db // indirect
  • github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
  • github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect

I use the goda tree ./...:all > out.txt to look at them, and it seems that:

  • github.com/modern-go/concurrent comes from sigs.k8s.io/structured-merge-diff/v4/value which comes from k8s.io/apimachinery/pkg/runtime, so that's ok
  • github.com/munnerz/goautoneg comes from k8s.io/kube-openapi/pkg/handler3 so that's ok

However, using goda tree I didn't see where github.com/asaskevich/govalidator and github.com/google/pprof come from - I might be missing something, as I'm trying to learn how to wrap my head around the dependencies, and learn the tooling here.

@sbueringer
Copy link
Member

I don't think we can drop indirect dependencies without dropping direct dependencies. The only reason they are there is because some of the direct dependencies depend on them.

So wouldn't it make more sense to only look at direct dependencies?

@mimowo
Copy link
Author

mimowo commented Jan 17, 2025

Right, that is another valid perspective for sure. I just took the bottom-up approach first to start from the ones which looked suspicious, but indeed most of that turns out to be indirect dependency of k8s.io anyway.

However, this one still puzzles me what it is needed for: go mod why -m github.com/asaskevich/govalidator does not find anything.

As for the top-down scan, this catches my attention:

  1. experimentals:
	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect

here:

> go mod why -m golang.org/x/exp                                 
# golang.org/x/exp
sigs.k8s.io/controller-runtime/pkg/cache
golang.org/x/exp/maps

But, maps is already built-in in new golang, so maybe we can drop the experimenal dep?

  1. libraries for json patches
	github.com/evanphx/json-patch/v5 v5.9.0
	gomodules.xyz/jsonpatch/v2 v2.4.0
	gopkg.in/evanphx/json-patch.v4 v4.12.0 // Using v4 to match upstream

Do you know if we still need v4 and v5, and why we have gomodules.xyz/jsonpatch/v2 on top of that?

@sbueringer
Copy link
Member

But, maps is already built-in in new golang, so maybe we can drop the experimenal dep?

I think we already did: #3012 (released with CR v0.20)

@sbueringer
Copy link
Member

Do you know if we still need v4 and v5, and why we have gomodules.xyz/jsonpatch/v2 on top of that?

v4 is to match upstream k/client-go. Otherwise our fake client would behave different than the one from client-go

v5 is what we use in pkg/client. If we change that we potentially break folks.

I don't know why we use gomodules.xyz/jsonpatch/v2, you have to check the imports. We just have to be careful to not break folks by changing behavior.

@sbueringer
Copy link
Member

Regarding govalidator:

$ go mod graph | grep govalidator
sigs.k8s.io/controller-runtime github.com/asaskevich/govalidator@v0.0.0-20190424111038-f61b66f89f4a
k8s.io/apiextensions-apiserver@v0.32.0 github.com/asaskevich/govalidator@v0.0.0-20190424111038-f61b66f89f4a
k8s.io/apiserver@v0.32.0 github.com/asaskevich/govalidator@v0.0.0-20190424111038-f61b66f89f4a
k8s.io/kube-openapi@v0.0.0-20241105132330-32ad38e42d3f github.com/asaskevich/govalidator@v0.0.0-20190424111038-f61b66f89f4a

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

No branches or pull requests

3 participants