Skip to content
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.

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

chore: generate applyconfigurations for APIs (SSA) #106

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

erikgb
Copy link
Contributor

@erikgb erikgb commented Nov 11, 2023

This PR adds generation of applyconfigurations to facilitate type-safe SSA for custom resources. I have made the generated types internal for now, and only for the newer version of our API. The upstream generator is built for Kubernetes, requiring minor tweaks to the project layout.

I plan to migrate the SubNamespace controller to SSA when introducing the new API version (v2alpha1).

Please take a look and tell me what you think! I'll be happy to reduce the number of changes in this PR.

CC @zoetrope @ymmt2005

@erikgb erikgb force-pushed the generate-ac branch 2 times, most recently from a2d9cc9 to d680a31 Compare November 12, 2023 18:08
@erikgb erikgb changed the title WIP: chore: generate applyconfigurations for APIs (SSA) chore: generate applyconfigurations for APIs (SSA) Nov 12, 2023
@zoetrope
Copy link
Member

@erikgb
I think introducing SSA is a great idea.
However, when we update SubNamespace, we only update the status.
Do you think there is an opportunity to use the generated ApplyConfiguration?

@erikgb
Copy link
Contributor Author

erikgb commented Nov 14, 2023

Do you think there is an opportunity to use the generated ApplyConfiguration?

Oh yes, that is my plan in the next PR.

applyconfiguration-gen: $(APPLYCONFIGURATION_GEN) ## Download applyconfiguration-gen locally if necessary
$(APPLYCONFIGURATION_GEN):
# see https://github.com/kubernetes/code-generator/tree/master/cmd/applyconfiguration-gen
GOBIN=$(shell pwd)/bin go install k8s.io/code-generator/cmd/[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

We are using aqua to manage packages for accurate to automate package updates using renovate.
We would like to manage applyconfiguration-gen with aqua as well.
But that is a bit complicated. I will create a PR for that later.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@zoetrope zoetrope left a comment

Choose a reason for hiding this comment

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

LGTM

@zoetrope zoetrope merged commit b30b46b into cybozu-go:main Nov 15, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants