client-gen generates empty "core" group clientsets when "api" package directory is in use for input APIs

[shaneutt]

Last year we added a v4 project layout in Kubebuilder which utilizes api/ as the directory for APIs to better follow the package naming standards.

I have some kubebuilder-based projects where I am migrating from the v3 layout to the v4 layout. As part of this transition I'm moving my APIs from the apis/ directory to the api/ directory.

Using the latest main (da01854 at the time of writing) the clientset generators break, emitting empty files to a core directory. It seems using api trips some special rules in client-gen.

I dug into the code and did some dlv debugging on this and found an assumption in the code:

code-generator/cmd/client-gen/types/types.go

Line 44 in da01854

func (g Group) NonEmpty() string {

func (g Group) NonEmpty() string {
	if g == "api" {
		return "core"
	}
	return string(g)
}

I understanding why that assumption exists for upstream, but given that Kubebuilder layout v4 now defaults to an api/ directory it seems like this might be problematic for downstream users.

During my debugging I was able to hack around this a bit to get something that emitted a working client for me, but I wanted to stop and mention this before trying to submit a patch to see if anyone had any thoughts on the matter, or some keen insights?

[delthas]

Hi, I have the exact same issue with a default kubebuilder scaffolding. Actually, I was about to send this issue when I saw yours:

---

Hi,

The default kubebuilder scaffolding generated by:

kubebuilder init --domain example.com
kubebuilder create api --group group --version v1 --kind Foo

results in the following files:

./api/v1/foo_types.go
./api/v1/groupversion_info.go

(with // +groupName=group.example.com as expected.)

In order to generate a typed client for this API, I added //+genclient to ./api/v1/foo_types.go. However, running client-gen results in no output and an error:

client-gen -n client --input api/v1 --input-base example.com/project -o . -p example.com/project --trim-path-prefix example.com/project -v 10

I0220 13:12:11.969569  203278 parse.go:383] importPackage example.com/project/core/v1
I0220 13:12:11.969622  203278 parse.go:330] addDir example.com/project/core/v1
I0220 13:12:11.982155  203278 parse.go:404] unable to import "example.com/project/core/v1": no required module provides package example.com/project/core/v1; to add it:
	go get example.com/project/core/v1

Indeed, example.com/project/core/v1 does not exist; it should really be example.com/project/api/v1 (corresponding to what I passed client-gen); but apparently api is rewritten to core, then client-gen tries to look in the folder core, which does not exist.

The following workaround works, and a typed client for my resource is correctly generated:

ln -s api core
client-gen -n client --input api/v1 --input-base example.com/project -o . -p example.com/project --trim-path-prefix example.com/project

It seems this is due to this line, which rewrites api to core: https://github.com/kubernetes/code-generator/blob/master/cmd/client-gen/types/types.go#L45

[shaneutt]

Thanks for adding on, I have a feeling this is probably a common problem for anyone using layout v4.

@camilamacedo86 I just wanted to ping you in here NOT because there's anything you need to do, but because I was working with you last in kubebuilder on the layout stuff a while back it seemed reasonable to assume you might wanna be aware of this problem.

[camilamacedo86]

Hi @shaneutt

Thank you for letting me know. IHMO is a bug/something that needs to be changed in the code generator.
Any project that tries to follow the Golang standards will not work within since it seems very logical to have a path called api to store the APIs, right? Please let me know if I missed something.

Kubebuilder officially supports the controller-gen since that is what we use by default, but it would be great if the code-generator could change that and work well with Kubebuilder. I know that some users have started to use it within.

[camilamacedo86]

Hi @shaneutt,

OutOfScope of this issue

Can a project work with both code-generator and controller-gen?
If so, we could study the benefit to scaffold by default both OR give the option for users opt-in or opt-out.
WDYT?

Could you give a hand for us and raise an issue in the Kubebuilder like Add official support to code-generator, add what would need to do in the scaffold be done for we are able to integrate the project within and if both can be applied at the same time or if we should have an option to allow users opt-in for one or another as the cons/pros of each one of them?

[shaneutt]

Sure sounds good, created: kubernetes-sigs/kubebuilder#3795

[tankbusta]

Hey folks, hope this is the right spot (first kubernetes issue for me) but I think there's another related issue to this. I'm using the V4 layout and my generated code is using the wrong API Path in the typed version client

From code-generator

func setConfigDefaults(config *rest.Config) error {
	gv := v1alpha1.SchemeGroupVersion
	config.GroupVersion = &gv
	config.APIPath = "/api"
	config.NegotiatedSerializer = scheme.Codecs.WithoutConversion()

	if config.UserAgent == "" {
		config.UserAgent = rest.DefaultKubernetesUserAgent()
	}

	return nil
}

From my understanding, /api is "legacy" whereas /apis is for newer resource types and CRDs. In the code that generates this, I noticed similar logic

code-generator/cmd/client-gen/generators/generator_for_group.go

Lines 74 to 79 in 07ba736

	apiPath := func(group string) string {
	if group == "core" {
	return `"/api"`
	}
	return `"` + g.apiPath + `"`
	}

[spacewander]

If client-go needs to keep the speicial conversion, what about adding a note in the argument flag?

It's time-wasting for the user to figure it out by reading the implementation code.

[camilamacedo86]

@spacewander,

So, is not this one a bug?
Can someone add an example over how to integrate this project with Kubebuilder in order to not face the issue reported here?

Thank you a lot

[spacewander]

@spacewander,

So, is not this one a bug? Can someone add an example over how to integrate this project with Kubebuilder in order to not face the issue reported here?

Thank you a lot

I don't know why the author of code-generator added this conversion. Maybe they want to implement some compatibility. If removing this behavior is not possible, the other solution is to add a new argument to bypass this behavior.

[camilamacedo86]

Hi @wojtek-t, @sttts, @deads2k

Would be possible to change the conditional to allow users use it with Kubebuilder OR with any Golang project following the Golang Standard Layout (equally Kubebuilder) ?

See https://github.com/golang-standards/project-layout?tab=readme-ov-file#service-application-directories (it is a good practice create our APIs and keep them in dir called api)

code-generator/cmd/client-gen/generators/generator_for_group.go

Lines 74 to 79 in 07ba736

	apiPath := func(group string) string {
	if group == "core" {
	return `"/api"`
	}
	return `"` + g.apiPath + `"`
	}

In Kubebuilder we have mapped the core types in this way:

https://github.com/kubernetes-sigs/kubebuilder/blob/32e0fdc918b9a21485d7b042833402a62d0480f4/pkg/plugins/golang/options.go#L29-L52

I hope that can helps. Sorry I am not acquaintance well with this project. But could we not add this map in this part and check by the kind/group of each of them to know if is a core type or not?

Thank you for your support and understanding.

Cheers,

[sttts]

kubernetes/kubernetes#125162 fixes this.