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

hack,install: Enable CRD description generation #10750

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timflannagan
Copy link
Member

Description

Enable the kubebuilder CRD field description functionality. Previously, this was set to
zero, which disabled description field documentation for the rendered CRD manifests.
Without this, users that rely on kubectl explain ... have a degraded UX as they cannot
easily determine the API surface behavior.

Note, I kept a maximum for the CRD description length. This was primarily to prevent
any scenario where the embedded fields within the GWParams API would exceed the
etcd size limit.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@yuval-k
Copy link
Contributor

yuval-k commented Mar 5, 2025

Can we disable description for gwparams?

@@ -38,7 +38,7 @@ API_INPUT_DIRS_SPACE="${API_INPUT_DIRS_SPACE%,}" # drop trailing space
API_INPUT_DIRS_COMMA="${API_INPUT_DIRS_COMMA%,}" # drop trailing comma

go run k8s.io/code-generator/cmd/register-gen --output-file zz_generated.register.go ${API_INPUT_DIRS_SPACE}
go run sigs.k8s.io/controller-tools/cmd/controller-gen crd:maxDescLen=0 object rbac:roleName=kgateway paths="${APIS_PKG}/api/${VERSION}" \
go run sigs.k8s.io/controller-tools/cmd/controller-gen crd:maxDescLen=1000 object rbac:roleName=kgateway paths="${APIS_PKG}/api/${VERSION}" \
Copy link
Contributor

Choose a reason for hiding this comment

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

historically we've excluded descriptions from the crds because customers have run into size limits applying the crds. if we are using a max length, can we make it smaller? (1000 characters per description still seems pretty long)

(but thinking out loud, if we are truncating some descriptions, then maybe the descriptions won't be that useful after all? users will still need to consult the docs)
do you know if kubectl explain is widely relied upon?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can reduce the max length to something more reasonable. re:truncating the description: even if the description is truncated and there's some missing information, it's still a better UX imo being able to surface this information via kubectl explain than forcing users to context switch and always go to the docs to fix API documentation.

@timflannagan
Copy link
Member Author

Can we disable description for gwparams?

This isn't possible with kubebuilder/controller-gen today. There's no per-file, per-field, etc. knob those tools expose that allow us to selectively trim descriptions from certain APIs.

If we don't want to enable descriptions, then we have a couple of options:

  1. Add a Go helper function that trims the descriptions that runs after codegen
  2. Limit the max description length for each CRD field
  3. Continue with the current approach where we avoid generating descriptions for all CRDs

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.

3 participants