Skip to content

Kubebuilder APIs should import minimal dependencies #4467

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
JoelSpeed opened this issue Jan 3, 2025 · 6 comments
Closed

Kubebuilder APIs should import minimal dependencies #4467

JoelSpeed opened this issue Jan 3, 2025 · 6 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@JoelSpeed
Copy link

What do you want to happen?

Kubebuilder commonly provides a "standard" layout for controllers and API types across many Kubernetes related projects.

In particular, one of things that Kubebuilder does is generate basic patterns like, containing conversion code or validation webhook code within within the API package.

If you look at repositories like kuberntes/api, you will notice that these packages do not contain additional code. They are clean and only include the types themselves.

Importing an API from k/api, typically results in bringing in the API types itself, and the metav1 package from apimachinery, and dependencies of this.

Importing an API from a kubebuilder based project, typically involves also importing controller-runtime, either for conversion, or for webhook validation code.

By importing controller-runtime within the API packages of projects, this makes consuming APIs from the kubebuilder based projects difficult. Any project that relies on another projects API types, and wants to vendor those, now has to choose a compatible version of controller-runtime based on their upstream projects choice, and not on their own cadence.

Since controller-runtime is fast moving, and things change frequently, this can cause lots of toil, where a project might want to update an API it depends on to make use of a new field, but then has to fix lots of other controller-runtime related issues while doing so.

In the past I've seen projects that rely on multiple different APIs, from multiple kubebuilder based projects, and they could not update as not all of their dependencies had updated to the latest controller-runtime, they were blocked on older versions until their final dependency was updated.

It would be preferable if Kubebuilder could start promotion a pattern of creating conversion code and webhook code in other packages, and keeping the imports for kubebuilder based projects to a minimum, to make it easier and cleaner to consume APIs from other projects, and remove the tie between APIs and specific controller runtime versions.

We went through and made similar changes in the Cluster API community in 2023, but we really should try and promote this wider IMO.

Extra Labels

No response

@JoelSpeed JoelSpeed added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 3, 2025
@camilamacedo86
Copy link
Member

camilamacedo86 commented Jan 3, 2025

Hi @JoelSpeed,

What you’re asking about seems to have already been addressed to ensure projects built with controller-runtime v0.20+ remain buildable and compatible since the release of Kubebuilder v4.3.0.

Controller-runtime has deprecated the webhook.Validator and webhook.Defaulter interfaces, and they will no longer be provided in future versions. Therefore, projects must adopt the new CustomValidator and CustomDefaulter interfaces to remain compatible with controller-runtime v0.20.0 and upper versions. For more details, refer to kubernetes-sigs/controller-runtime#2641.

The changes you’re advocating for—a separation of API definitions from webhook and conversion logic—have already been implemented. With the updates in controller-runtime and Kubebuilder starting from 4.3.0:

  • Webhook and conversion code is scaffolded in the internal directory instead of being included directly within the API package.
  • This ensures that API packages in Kubebuilder-based projects remain clean and decoupled from controller-runtime, aligning with the pattern used in repositories like kubernetes/api.

This approach eliminates the need for downstream projects to import controller-runtime when consuming APIs from Kubebuilder-based projects, addressing the dependency conflict issue you described.

You can confirm these updates by reviewing the three samples under testdata for the master branch:
https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata.

Additionally, the tutorial samples in the documentation provide practical examples of the current scaffold:

Given these changes, I believe we can consider this issue resolved as of the 4.3.0 release. However, if I’ve misunderstood or overlooked any aspect of your concern, please let me know!

@camilamacedo86
Copy link
Member

I @JoelSpeed

I am closing this one since it seems done already.
However, if you think that should be re-opened, please feel free to do so.

@JoelSpeed
Copy link
Author

@camilamacedo86 No, I don't think this is necessarily fixed, looking at the latest kubebuilder v4, API v2 testdata package

If I just import one of the types from that package, the go.mod still relies on controller-runtime, which I think makes the issue still a risk. The conversion elements don't seem to have been moved out of the API packages yet?

require sigs.k8s.io/kubebuilder/testdata/project-v4 v0.0.0-20250102061453-18df53806e97

require (
	github.com/fxamacker/cbor/v2 v2.7.0 // indirect
	github.com/go-logr/logr v1.4.2 // indirect
	github.com/gogo/protobuf v1.3.2 // indirect
	github.com/google/gofuzz v1.2.0 // indirect
	github.com/json-iterator/go v1.1.12 // indirect
	github.com/kr/text v0.2.0 // indirect
	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
	github.com/modern-go/reflect2 v1.0.2 // indirect
	github.com/x448/float16 v0.8.4 // indirect
	golang.org/x/net v0.29.0 // indirect
	golang.org/x/text v0.18.0 // indirect
	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
	gopkg.in/inf.v0 v0.9.1 // indirect
	gopkg.in/yaml.v2 v2.4.0 // indirect
	k8s.io/apimachinery v0.31.1 // indirect
	k8s.io/klog/v2 v2.130.1 // indirect
	k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 // indirect
	sigs.k8s.io/controller-runtime v0.19.1 // indirect
	sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
	sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
)

@JoelSpeed
Copy link
Author

Discussing with @camilamacedo86 offline, the only two imports from controller-runtime now are "sigs.k8s.io/controller-runtime/pkg/scheme" and "sigs.k8s.io/controller-runtime/pkg/conversion". The public interfaces here have not changed in 6 years, and, while controller-runtime is still not stable, and therefore there is no guarantee that things won't change, we believe the risk here is minimal.

Kubebuilder based APIs will continue to rely on these two packages, but, unless controller-runtime introduces a change to either of these interfaces, this should not cause the import and go.mod contention issue described in my original issue.

@camilamacedo86
Copy link
Member

Hi @JoelSpeed,

We discussed this further, and you made a good point: the controller-runtime dependency is still required.

Looking at the code, it is still invoked at the following points:

These interfaces haven’t changed for years, but as you mentioned, they could still be modified. Since controller-runtime itself isn’t stable, this dependency might introduce challenges for users.

One potential solution could be to use the API machinery scheme instead of controller-runtime. However, that would still leave us with a dependency—it would just shift the problem elsewhere.

For now, I agree that the issue isn’t fully resolved. While it’s unlikely to occur often, having no dependency at all may not be achievable. A reasonable approach might be to see if controller-runtime can release a stable v1.0.0 to avoid breaking changes. We’ve raised this here: controller-runtime/issues/3065.

That said, if you feel this issue should be re-opened, please feel free to do so. I’m not entirely sure how we could resolve this 100%, but I’m happy to collaborate further.

Best regards,

@dtantsur
Copy link

These interfaces haven’t changed for years, but as you mentioned, they could still be modified. Since controller-runtime itself isn’t stable, this dependency might introduce challenges for users.

Found this bug to leave exactly this comment (per discussion with controller-runtime folks)

One potential solution could be to use the API machinery scheme instead of controller-runtime. However, that would still leave us with a dependency—it would just shift the problem elsewhere.

The dependency on apimachinery is more or less impossible to avoid: it's pulled in by TypeMeta and ObjectMeta in any API resource. In the discussion above, someone suggested me to adapt what CAPI is doing, and it worked well for us. Maybe Kubebuilder can follow it as well?

I haven't looked into conversion yet. I wonder if the controller-runtime folks will change how it works similarly to how they treated webhooks (move from API closer to controllers).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants