✨ Generation of typed apply clients #536

Jefftree · 2021-01-19T18:58:42Z

Generate Apply clients for use with server side apply. For built in types, a hard coded mapping is done to point to the client-go/applyconfiguration types. For CRDs, we assume applyconfigurations live at <path-to-package>/ac directory which should be the case if imported CRD packages also use our generator.

k8s-ci-robot · 2021-01-19T18:58:46Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

DirectXMan12

(first pass on some high-level stuff)

pkg/applyconfigurations/gen.go

pkg/applyconfigurations/traverse.go

jpbetz

I primarily reviewed the generated types. They look mostly correct, just some corner cases that were different than k/k or that might need some extra documentation.

pkg/applyconfigurations/testdata/ac/zz_generated.applyconfigurations.go

pkg/applyconfigurations/testdata/cronjob_types.go

pkg/genall/output.go

cmd/controller-gen/main.go

pkg/applyconfigurations/gen.go

alvaroaleman · 2021-09-16T20:13:27Z

/test all

k8s-triage-robot · 2021-12-20T18:08:38Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2022-01-19T18:13:37Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot · 2022-02-18T18:27:45Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2022-02-18T18:27:56Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen

Mark this issue or PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

JoelSpeed · 2023-01-24T15:14:39Z

The only reason I could see for not merging this now would be if we are concerned that we wouldn't be able to get the markers added in this PR to work with the upstream generator

JoelSpeed · 2023-01-24T16:53:15Z

pkg/applyconfigurations/testdata/cronjob/groupversion_info.go

+
+// +groupName=testdata.kubebuilder.io
+// +versionName=v1
+// +kubebuilder:ac:generate=true


What is the expected behaviour when this is used in various combinations with the type level marker?
I've had a stab at enumerating below. I'm using - to mean it's not set

Package CRD Generated

- - No

true - Yes

false - No

- true Yes

true true Yes

false true ???

- false No

true false ???

false false No

Does this match your expectations and can we get tests that cover this logic?

I suppose the CRD tag could/should always overwrite the higher level tag.

JoelSpeed · 2023-02-02T22:34:28Z

I did start working on a branch to integrate the upstream generator and I got it pretty much working, you can see the WIP branch here. I think I was left with this comment as an open question, if we can conclude what we want to do there I could try to finish up this extension of @Jefftree's original branch

EraYaN · 2023-02-09T15:16:27Z

@JoelSpeed Did you have any luck with your new branch to generate the Extract methods? Running both the upstream tool directly and the code in your branch it doesn't seem to pick up any openapi data. So internal's schemaYAML is mostly empty and the Extract methods are not generated.

JoelSpeed · 2023-02-09T16:45:56Z

@EraYaN No, I actually deliberately omitted the Extract methods since I didn't see those being implemented as part of this PR. With the branch I linked in my previous comment, if you supply the openapi-schema to the generator then it will generate the Extract methods correctly, but it's not a requirement to have those as far as I understood.

I've been working on another branch which integrates directly the code into controller-tools and that again, doesn't generate the extract methods, but, if we could generate the openapi schema and pass it in, could do, perhaps a later extension?

EraYaN · 2023-02-10T12:10:44Z

Generating the openapi.json doesn't seem to be trivial, so if at all possible it would be great if it ends up being all "batteries included". Operators like the ~~prometheus-operator~~ (EDIT: it was cert-manager) wrote some go tools to get the OpenAPI json dumped, but I feel that shouldn't be required for users to do. But it is also kind of a shame the openapi.json from an actual API server does not seem to work as a shortcut.

I like having the Extract methods since mostly we are doing something akin to this: https://github.com/zoetrope/kubebuilder-training/blob/main/codes/50_completed/controllers/markdownview_controller.go#L221-L253 Arguably we probably don't need those Extract calls and the Gets in the first place. But since it seems like it's one of the only complete examples using SSA extensively, it might be part of the story for quite some people.

JoelSpeed · 2023-02-10T18:02:37Z

Generating the openapi.json doesn't seem to be trivial, so if at all possible it would be great if it ends up being all "batteries included".

You're right it's not the most straight forward thing to generate. By not supplying it, I think we are achieving what this PR set out to do, but, what is missing? The extract functions could possibly be generated separately, or, we can try and work out (maybe in a follow up PR), how to provide this and complete the generation

camilamacedo86 · 2023-04-19T10:49:15Z

@JoelSpeed do you agree within we moving forward with this one now?
We can improve it in follow ups. WDYT?

camilamacedo86

It seems fine for me.

/lgtm

Looking for @JoelSpeed and @vincepri and @alvaroaleman

Give their OK and remove the hold on this one.

vincepri · 2023-04-19T14:20:43Z

/approve cancel

@Jefftree Are you able to add tests and documentation before merging?

alvaroaleman · 2023-04-19T14:22:19Z

@JoelSpeed mentioned they might be able to finish their branch after kubecon, which is likely the preferred approach as it avoids a lot of duplication

JoelSpeed · 2023-05-19T15:41:05Z

I've been working on #818 today which I think is a good start on where we want to go. I would not recommend reviewing commit wise, it needs a lot of squashing, but, lets start working out what we want to see with this new integration. I'll leave a review myself with various points of UX I've thought of while working through

k8s-triage-robot · 2024-01-19T06:57:08Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

EraYaN · 2024-01-19T09:48:22Z

This is still relevant
/remove-lifecycle stale

k8s-triage-robot · 2024-04-18T10:33:00Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

joelanford · 2024-04-18T11:48:45Z

/remove-lifecycle stale

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 19, 2021

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 19, 2021

k8s-ci-robot requested review from droot and pwittrock January 19, 2021 18:58

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

Jefftree force-pushed the apply-generator-code branch from 0605102 to 9122158 Compare January 19, 2021 18:59

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 19, 2021

Jefftree force-pushed the apply-generator-code branch from 6c7bede to c9926ae Compare January 25, 2021 17:40

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2021

DirectXMan12 reviewed Feb 24, 2021

View reviewed changes

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

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

Jefftree force-pushed the apply-generator-code branch from a299774 to 1aa6beb Compare May 17, 2021 19:20

Jefftree changed the title ~~[WIP] Generation of typed apply clients~~ Generation of typed apply clients May 17, 2021

Jefftree marked this pull request as ready for review May 17, 2021 19:24

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 17, 2021

Jefftree changed the title ~~Generation of typed apply clients~~ ✨ Generation of typed apply clients May 17, 2021

jpbetz suggested changes Jun 2, 2021

View reviewed changes

zoetrope mentioned this pull request Aug 20, 2021

controller-gen feature request: support for generating DeepCopy of ApplyConfigrations #570

Closed

alvaroaleman reviewed Sep 16, 2021

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 20, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 19, 2022

k8s-ci-robot closed this Feb 18, 2022

JoelSpeed reviewed Jan 24, 2023

View reviewed changes

alvaroaleman mentioned this pull request Mar 22, 2023

Server-side Apply! kubernetes-sigs/controller-runtime#347

Open

camilamacedo86 reviewed Apr 19, 2023

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Apr 19, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 19, 2023

stevenctl mentioned this pull request May 3, 2023

crdclient: support Apply operations istio/istio#44741

Closed

camilamacedo86 mentioned this pull request Jul 7, 2023

kubebuilder make install fails kubernetes-sigs/kubebuilder#3460

Closed

yujunz mentioned this pull request Jul 27, 2023

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

Open

camilamacedo86 mentioned this pull request Aug 21, 2023

📖 Add Make install workaround kubernetes-sigs/kubebuilder#3543

Merged

erikgb mentioned this pull request Oct 29, 2023

chore: generate applyconfigurations for custom resources cert-manager/trust-manager#217

Merged

camilamacedo86 mentioned this pull request Nov 12, 2023

[Assessment and POC] - Integrate Server-Side-Apply Support in Kubebuilder via Controller-Tools and Code-Generator Enhancements kubernetes-sigs/kubebuilder#3692

Open

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 19, 2024

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 19, 2024

pedjak mentioned this pull request Apr 2, 2024

Add ObservedObjectCollection API type crossplane-contrib/provider-kubernetes#217

Merged

2 tasks

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 18, 2024

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Generation of typed apply clients #536

✨ Generation of typed apply clients #536

Jefftree commented Jan 19, 2021 •

edited

k8s-ci-robot commented Jan 19, 2021

DirectXMan12 left a comment

jpbetz left a comment

alvaroaleman commented Sep 16, 2021

k8s-triage-robot commented Dec 20, 2021

k8s-triage-robot commented Jan 19, 2022

k8s-triage-robot commented Feb 18, 2022

k8s-ci-robot commented Feb 18, 2022

JoelSpeed commented Jan 24, 2023

JoelSpeed Jan 24, 2023

EraYaN Feb 9, 2023

JoelSpeed commented Feb 2, 2023

EraYaN commented Feb 9, 2023

JoelSpeed commented Feb 9, 2023

EraYaN commented Feb 10, 2023 •

edited

JoelSpeed commented Feb 10, 2023

camilamacedo86 commented Apr 19, 2023

camilamacedo86 left a comment

vincepri commented Apr 19, 2023

alvaroaleman commented Apr 19, 2023

JoelSpeed commented May 19, 2023

k8s-triage-robot commented Jan 19, 2024

EraYaN commented Jan 19, 2024

k8s-triage-robot commented Apr 18, 2024

joelanford commented Apr 18, 2024

Package	CRD	Generated
-	-	No
true	-	Yes
false	-	No
-	true	Yes
true	true	Yes
false	true	???
-	false	No
true	false	???
false	false	No

✨ Generation of typed apply clients #536

Are you sure you want to change the base?

✨ Generation of typed apply clients #536

Conversation

Jefftree commented Jan 19, 2021 • edited

k8s-ci-robot commented Jan 19, 2021

DirectXMan12 left a comment

Choose a reason for hiding this comment

jpbetz left a comment

Choose a reason for hiding this comment

alvaroaleman commented Sep 16, 2021

k8s-triage-robot commented Dec 20, 2021

k8s-triage-robot commented Jan 19, 2022

k8s-triage-robot commented Feb 18, 2022

k8s-ci-robot commented Feb 18, 2022

JoelSpeed commented Jan 24, 2023

JoelSpeed Jan 24, 2023

Choose a reason for hiding this comment

EraYaN Feb 9, 2023

Choose a reason for hiding this comment

JoelSpeed commented Feb 2, 2023

EraYaN commented Feb 9, 2023

JoelSpeed commented Feb 9, 2023

EraYaN commented Feb 10, 2023 • edited

JoelSpeed commented Feb 10, 2023

camilamacedo86 commented Apr 19, 2023

camilamacedo86 left a comment

Choose a reason for hiding this comment

vincepri commented Apr 19, 2023

alvaroaleman commented Apr 19, 2023

JoelSpeed commented May 19, 2023

k8s-triage-robot commented Jan 19, 2024

EraYaN commented Jan 19, 2024

k8s-triage-robot commented Apr 18, 2024

joelanford commented Apr 18, 2024

Jefftree commented Jan 19, 2021 •

edited

EraYaN commented Feb 10, 2023 •

edited