✨ Generation of typed apply clients using upstream generator #818

JoelSpeed · 2023-05-19T08:34:04Z

This integrates the upstream applyconfig generator into controller-tools to allow generation of ApplyConfig style types from custom API types.

The aim here is to provide the pointer style structs that could then be passed to an Apply style method for using server side apply.

Setting this up as a WIP PR for now, current TODO list:

Work out if there's any extra stuff here that's not needed
Work out how to test this without the hack adding cronjob testdata to the go.mod
Do we need OpenAPI schema/do we need the extract functions?
- Future work, after this PR has merged
General review of code and cleanup - this was quite hacky when I started the branch
How would a user want this integrated into their project?
- For now they can add the correct markers and it will generate as a subfolder of their API

k8s-ci-robot · 2023-05-19T08:34:06Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

pkg/applyconfigurations/gen.go

JoelSpeed · 2023-05-19T09:44:24Z

pkg/applyconfigurations/gen.go

+	enableTypeMarker = markers.Must(markers.MakeDefinition("kubebuilder:ac:generate", markers.DescribesType, false))
+)
+
+var importMapping = map[string]string{


Should we allow additional import mappings, I know the upstream applyconfiguration gen does

I think that makes sense, yeah (But we can do that in a follow-up)

JoelSpeed · 2023-05-19T09:46:27Z

/test all

Need to work out how to test this, but want to see what the existing tests do with this so I can fix any failures introduced

JoelSpeed · 2023-05-19T15:39:01Z

/test all

JoelSpeed · 2023-05-19T15:52:25Z

pkg/applyconfigurations/applyconfiguration_integration_test.go

+		filesInMaster := make(map[string][]byte)
+		masterFileNames := sets.New[string]()
+		cronJobFS := os.DirFS(".")
+		masterPath := "applyconfiguration-master"


Because I can't change the directory to which we are generating the assets currently, this is what I came up with, create a copy that we keep checked in and then diff against that, if there are genuine changes we can update both copies.

Will look at gengo to see if there's a backwards compatible way to allow it to output to different places like OutputRule does for these generators

JoelSpeed · 2023-05-19T15:56:13Z

/assign @alvaroaleman

EraYaN · 2023-05-22T07:49:50Z

As an option I would love to have the Extract functions, we use them extensively in our operator (for better or for worse).

pkg/applyconfigurations/gen.go

sbueringer · 2023-05-23T09:29:10Z

@JoelSpeed Can we get rid of the vendor folder? It's probably on my side but the performance of the file tab is pretty bad with 1700 files.

pkg/applyconfigurations/testdata/cronjob/applyconfiguration-master/internal/internal.go

alvaroaleman · 2023-05-23T20:55:23Z

@JoelSpeed Can we get rid of the vendor folder? It's probably on my side but the performance of the file tab is pretty bad with 1700 files.

Second this, I can literally not review this through the GitHub UI

JoelSpeed · 2023-05-26T14:23:47Z

@JoelSpeed Can we get rid of the vendor folder? It's probably on my side but the performance of the file tab is pretty bad with 1700 files.

Second this, I can literally not review this through the GitHub UI

Sorry about that, must have committed during my experimenting, dropped it now, commit history in need of a good clean though

alvaroaleman

For an internal project, I added the +kubebuilder:ac:generate=true comment both on the package and on my type, running controller-gen with apply paths=./pkg/api/v1/ succeeds but doesn't generate anything - any idea what I am am doing wrong?

alvaroaleman · 2023-05-23T20:49:02Z

pkg/applyconfigurations/gen.go

+	enableTypeMarker = markers.Must(markers.MakeDefinition("kubebuilder:ac:generate", markers.DescribesType, false))
+)
+
+var importMapping = map[string]string{


I think that makes sense, yeah (But we can do that in a follow-up)

alvaroaleman

For an internal project, I added the +kubebuilder:ac:generate=true comment both on the package and on my type, running controller-gen with apply paths=./pkg/api/v1/ succeeds but doesn't generate anything - any idea what I am am doing wrong?

sbueringer · 2023-05-30T17:49:57Z

For an internal project, I added the +kubebuilder:ac:generate=true comment both on the package and on my type, running controller-gen with apply paths=./pkg/api/v1/ succeeds but doesn't generate anything - any idea what I am am doing wrong?

Potentially could be: #818 (comment)

sbueringer · 2025-03-26T14:23:49Z

Thx!

/lgtm

k8s-ci-robot · 2025-03-26T14:23:57Z

LGTM label has been added.

Git tree hash: 51056a1fd6f10be8a831975e810ceb82afa1b23d

alvaroaleman

Looks good, thank you so much for your work on this!

k8s-ci-robot · 2025-03-29T15:37:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, JoelSpeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…tes-sigs#818) * Stub out generator * Add apply gen to controller-tools * Add code-generator to go.mod * Integrate upstream apply config generator * Cleanup unused code from previous implementation * Convert controller-tools tags to genclient * Add generated clients * Cleanup markers, ensure correct path for non GOPATH * Add test to check generated content against generated master * chore: resolve applyconfig gen correctly * Migrate to gengo v2 * Update generated test data * Ignore testdata for the purposes of go generate * Rename apply command to applyconfiguration * Rename package to applyconfiguration * Rework tests to use temporary directory for output files * Update cronjob_types with the latest version * Update groupversion inline with latest kubebuilder * Update go.mod to match latest of top level * Move test apis into an API v1 folder to be more accurate * Add test for generating client outside of API directory * Address feedback on gen.go --------- Co-authored-by: Joe Betz <[email protected]> Co-authored-by: Jefftree <[email protected]> Co-authored-by: Jakob Möller <[email protected]>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 19, 2023

k8s-ci-robot requested review from joelanford and vincepri May 19, 2023 08:34

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 19, 2023

JoelSpeed force-pushed the applyconfig-gen-integration branch 2 times, most recently from 18594e4 to 74642c2 Compare May 19, 2023 09:38

JoelSpeed commented May 19, 2023

View reviewed changes

JoelSpeed mentioned this pull request May 19, 2023

✨ Generation of typed apply clients #536

Closed

JoelSpeed marked this pull request as ready for review May 19, 2023 15:46

JoelSpeed commented May 19, 2023

View reviewed changes

k8s-ci-robot assigned alvaroaleman May 19, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2023

sbueringer reviewed May 23, 2023

View reviewed changes

pkg/applyconfigurations/gen.go Outdated Show resolved Hide resolved

sbueringer reviewed May 23, 2023

View reviewed changes

pkg/applyconfigurations/testdata/cronjob/applyconfiguration-master/internal/internal.go Outdated Show resolved Hide resolved

alvaroaleman reviewed May 30, 2023

View reviewed changes

joelanford mentioned this pull request Jun 8, 2023

add support for handling plain+v0 bundle types operator-framework/operator-controller#242

Merged

4 tasks

yujunz mentioned this pull request Jul 27, 2023

Opt-in to tolerate ad-hoc modification in resources alexandrevilain/temporal-operator#418

Open

jakobmoellerdev mentioned this pull request Aug 23, 2023

OCPVE-613: feat: switch to SSA for resource reconciliation openshift/lvm-operator#395

Closed

5 tasks

JoelSpeed force-pushed the applyconfig-gen-integration branch from 607e978 to 0aa6676 Compare September 18, 2023 09:18

JoelSpeed and others added 16 commits March 26, 2025 13:44

Add generated clients

ba99f7a

Cleanup markers, ensure correct path for non GOPATH

d93d1cb

Add test to check generated content against generated master

0a5f81a

chore: resolve applyconfig gen correctly

8d0a899

Migrate to gengo v2

740e0ae

Update generated test data

5647ec5

Ignore testdata for the purposes of go generate

9aec904

Rename apply command to applyconfiguration

8a7c5ec

Rename package to applyconfiguration

c343c7d

Rework tests to use temporary directory for output files

326a20e

Update cronjob_types with the latest version

0aae757

Update groupversion inline with latest kubebuilder

c8ef095

Update go.mod to match latest of top level

fe610e2

Move test apis into an API v1 folder to be more accurate

46f9f4f

Add test for generating client outside of API directory

0023b01

Address feedback on gen.go

804b842

JoelSpeed force-pushed the applyconfig-gen-integration branch from c599c42 to 804b842 Compare March 26, 2025 13:47

k8s-ci-robot assigned sbueringer Mar 26, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 26, 2025

alvaroaleman approved these changes Mar 29, 2025

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2025

k8s-ci-robot merged commit 06ca07b into kubernetes-sigs:main Mar 29, 2025
8 checks passed

sbueringer mentioned this pull request Mar 30, 2025

Feature: Native SSA Support kubernetes-sigs/controller-runtime#3183

Open

9 tasks

This was referenced Apr 11, 2025

Remove outdated and non-generated apply configurations cert-manager/trust-manager#589

Closed

Add k8s.io/client-go/applyconfigurations style *ApplyConfigurations for the included CRDs cert-manager/cert-manager#5783

Open

terinjokes mentioned this pull request Apr 16, 2025

applyconfiguration generator uses a different variable name #1193

Open

✨ Generation of typed apply clients using upstream generator #818

✨ Generation of typed apply clients using upstream generator #818

Uh oh!

Conversation

JoelSpeed commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented May 19, 2023

Uh oh!

Uh oh!

JoelSpeed May 19, 2023

Choose a reason for hiding this comment

Uh oh!

alvaroaleman May 23, 2023

Choose a reason for hiding this comment

Uh oh!

JoelSpeed commented May 19, 2023

Uh oh!

JoelSpeed commented May 19, 2023

Uh oh!

JoelSpeed May 19, 2023

Choose a reason for hiding this comment

Uh oh!

JoelSpeed commented May 19, 2023

Uh oh!

EraYaN commented May 22, 2023

Uh oh!

Uh oh!

sbueringer commented May 23, 2023

Uh oh!

Uh oh!

alvaroaleman commented May 23, 2023

Uh oh!

JoelSpeed commented May 26, 2023

Uh oh!

alvaroaleman left a comment

Choose a reason for hiding this comment

Uh oh!

alvaroaleman May 23, 2023

Choose a reason for hiding this comment

Uh oh!

alvaroaleman left a comment

Choose a reason for hiding this comment

Uh oh!

sbueringer commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Mar 26, 2025

Uh oh!

k8s-ci-robot commented Mar 26, 2025

Uh oh!

alvaroaleman left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Mar 29, 2025

Uh oh!

Uh oh!

Uh oh!

JoelSpeed commented May 19, 2023 •

edited

Loading

sbueringer commented May 30, 2023 •

edited

Loading