Skip to content

✨(feat): add necessary changes to support applyconfiguration gen #5237

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.

Sign up for GitHub

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

✨(feat): add necessary changes to support applyconfiguration gen #5237

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7166a19
(feat) add necessary changes to support applyconfiguration gen
yyewolf Nov 19, 2025
8d51a9f
(fix) reverts breaking change on GroupVersion
yyewolf Nov 19, 2025
473f3ca
(feat) add an option for applyconfiguration
yyewolf Nov 19, 2025
c91bfa9
(feat) add makefile and set generation to enabled by default
yyewolf Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/plugins/golang/v4/scaffolds/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func (s *apiScaffolder) Scaffold() error {
if err := scaffold.Execute(
&api.Types{Force: s.force},
&api.Group{},
&api.Doc{},
Copy link
Member

@camilamacedo86 camilamacedo86 Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should only generate the new files if the/when the flag is informed

); err != nil {
return fmt.Errorf("error scaffolding APIs: %w", err)
}
Expand Down
59 changes: 59 additions & 0 deletions pkg/plugins/golang/v4/scaffolds/internal/templates/api/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
Copy link
Member

@camilamacedo86 camilamacedo86 Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into that. 🎉

If it turns out to be a breaking change, we need to find a way to make it backwards compatible. We can’t introduce it right now.

However, if you want to go ahead and implement the changes based on how you think it should work, and then share your findings with us, we can refine it together in a follow-up.

Because of that, I wouldn’t worry about the e2e tests at this stage. Instead, focus on making the changes in a way that, when you run make generate, the testdata samples are regenerated—or at least show how a project would look with the new behavior.

Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package api

import (
log "log/slog"
"path/filepath"

"sigs.k8s.io/kubebuilder/v4/pkg/machinery"
)

var _ machinery.Template = &Doc{}

// Doc scaffolds the doc file that defines the groupName
type Doc struct {
machinery.TemplateMixin
machinery.MultiGroupMixin
machinery.BoilerplateMixin
machinery.ResourceMixin
}

// SetTemplateDefaults implements machinery.Template
func (f *Doc) SetTemplateDefaults() error {
if f.Path == "" {
if f.MultiGroup && f.Resource.Group != "" {
f.Path = filepath.Join("api", "%[group]", "%[version]", "doc.go")
} else {
f.Path = filepath.Join("api", "%[version]", "doc.go")
}
}

f.Path = f.Resource.Replacer().Replace(f.Path)
log.Info(f.Path)
f.TemplateBody = docTemplate

return nil
}

//nolint:lll
const docTemplate = `{{ .Boilerplate }}
// Package {{ .Resource.Version }} contains API Schema definitions for the {{ .Resource.Group }} {{ .Resource.Version }} API group.
// +groupName={{ .Resource.QualifiedGroup }}
package {{ .Resource.Version }}
`
7 changes: 3 additions & 4 deletions pkg/plugins/golang/v4/scaffolds/internal/templates/api/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const groupTemplate = `{{ .Boilerplate }}

// Package {{ .Resource.Version }} contains API Schema definitions for the {{ .Resource.Group }} {{ .Resource.Version }} API group.
// +kubebuilder:object:generate=true
// +groupName={{ .Resource.QualifiedGroup }}
package {{ .Resource.Version }}

import (
Expand All @@ -64,11 +63,11 @@ import (
)

var (
// GroupVersion is group version used to register these objects.
GroupVersion = schema.GroupVersion{Group: "{{ .Resource.QualifiedGroup }}", Version: "{{ .Resource.Version }}"}
// SchemeGroupVersion is group version used to register these objects.
SchemeGroupVersion = schema.GroupVersion{Group: "{{ .Resource.QualifiedGroup }}", Version: "{{ .Resource.Version }}"}
Copy link
Member

@camilamacedo86 camilamacedo86 Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would those changes?
We did not change the values. Why would we need to change the exported variable name?

Copy link
Author

@yyewolf yyewolf Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The generated code for applyconfiguration uses this variable name, it's referenced here: https://github.com/kubernetes/code-generator/blob/master/cmd/applyconfiguration-gen/generators/util.go#L129

However in order to make it backward compatible, we can easily go back and do

Suggested change
SchemeGroupVersion = schema.GroupVersion{Group: "{{ .Resource.QualifiedGroup }}", Version: "{{ .Resource.Version }}"}
GroupVersion = schema.GroupVersion{Group: "{{ .Resource.QualifiedGroup }}", Version: "{{ .Resource.Version }}"}
SchemeGroupVersion = GroupVersion


// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion}
SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion}

// AddToScheme adds the types in this group-version to the given scheme.
AddToScheme = SchemeBuilder.AddToScheme
Expand Down
3 changes: 3 additions & 0 deletions pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (f *Types) SetTemplateDefaults() error {
//nolint:lll
const typesTemplate = `{{ .Boilerplate }}

// +kubebuilder:ac:generate=true
Copy link
Member

@camilamacedo86 camilamacedo86 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the feature must be optional.
We should not promote it by default, right?
By default, people should still be working as they do today.

Copy link
Author

@yyewolf yyewolf Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'll make it optional

Copy link
Member

@camilamacedo86 camilamacedo86 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if you want to add it in a way that we can first see it working, then we can evaluate it.

Do you have a project that uses this configuration? I’ve never tried to use it myself, so looking at an example would be helpful and could give us some ideas on how we might add it.

Copy link
Author

@yyewolf yyewolf Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a project that uses this configuration, however I ran the generate testdata using it: 537f54e

Copy link
Author

@yyewolf yyewolf Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the flag, made it true by default (for now so we can see the generated code) and also generated the code 👍

package {{ .Resource.Version }}

import (
Expand Down Expand Up @@ -116,6 +117,8 @@ type {{ .Resource.Kind }}Status struct {
// +kubebuilder:resource:scope=Cluster
{{- else if not .Resource.IsRegularPlural }}
// +kubebuilder:resource:path={{ .Resource.Plural }}
{{- else }}
// +kubebuilder:resource
Copy link
Member

@camilamacedo86 camilamacedo86 Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this marker do?
Have you an example of a project using the option for we give a look?
Could you share?

Copy link
Author

@yyewolf yyewolf Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

controller-tools uses this marker to mark the type as a resource type:

Marker: https://github.com/kubernetes-sigs/controller-tools/blob/main/pkg/applyconfiguration/gen.go#L45C1-L45C127
Help message associated: https://github.com/kubernetes-sigs/controller-tools/blob/main/pkg/applyconfiguration/gen.go#L80

{{- end }}

// {{ .Resource.Kind }} is the Schema for the {{ .Resource.Plural }} API
Expand Down