kubernetes-sigs / controller-tools

Tools to use with the controller-runtime libraries
Apache License 2.0
740 stars 423 forks source link

Ability to skip descriptions per field or per package #441

Open christopherhein opened 4 years ago

christopherhein commented 4 years ago

I have a CRD which nests multiple large CRD objects, when controller-gen generates the manifests with all the metadata and descriptions this can equal well over 2mb. Looking at controller-gen crd -www it doesn’t appear there is a way to skip descriptions either at a field level or even at a package level. Is that correct?

If that is, would there be any objections to a marker like // +kubernetes:skip:description where it could be at the package or at an individual field in the CRD?

christopherhein commented 4 years ago

You can accomplish this same thing using maxDescLen=0 in the CLI options. For example:

controller-gen crd:trivialVersions=true,maxDescLen=0 rbac:roleName=manager-role paths="./..." output:crd:artifacts:config=config/crds
JohnRusk commented 4 years ago

@christopherhein Does the maxDescLen solution really solve what you were originally asking for? It truncates the descriptions for all fields, right? Wouldn't it be preferable if it was possible to suppress descriptions just at package level, like you originally asked?

For instance, I'm looking at this CRD here: https://github.com/prometheus-operator/prometheus-operator/blob/master/example/prometheus-operator-crd/monitoring.coreos.com_prometheuses.yaml

It's over 6,000 lines, and many of them are descriptions of things from the package k8s.io/apimachinery/pkg/apis/meta/v1. Everybody who uses K8s is familiar with those things already (selectors etc) and so we don't need verbose docs for them in CRDs that happen to use them. So I really like your original idea of including or excluding descriptions based on namespace. It would be a way to keep documentation on the main (outer) types in the CRD, but omit it from the commonly known nested inner types.

What do you think?

christopherhein commented 4 years ago

/reopen

k8s-ci-robot commented 4 years ago

@christopherhein: Reopened this issue.

In response to [this](https://github.com/kubernetes-sigs/controller-tools/issues/441#issuecomment-695875584): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
christopherhein commented 4 years ago

@JohnRusk correct it's really just a patch to allow apply to work. I definitely would still be interested in a cleaner solution.

JohnRusk commented 4 years ago

Hopefully the example CRD I've linked to above will help to illustrate the value of your suggestion.

christopherhein commented 4 years ago

It does, now it's just a matter of finding time 😛

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

JohnRusk commented 3 years ago

/remove-lifecycle stale

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

christopherhein commented 3 years ago

/remove-lifecycle stale

fejta-bot commented 3 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

christopherhein commented 3 years ago

/remove-lifecycle stale

k8s-triage-robot commented 3 years ago

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:

You can:

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

/lifecycle stale

christopherhein commented 3 years ago

/remove-lifecycle stale /lifecycle frozen

armsnyder commented 3 years ago

This would be very useful for CRDs that embed other resource definition types like DeploymentSpec.

When CRDs get too large, it becomes impossible to manage them with tools like Helm which store a compressed manifest in a Secret, which has a 1 MB limit.

Ideally as a controller-tools user, I could keep descriptions on all my fields except for specifically marked embedded ones. I could add a comment to the embedded fields referring to existing documentation, and strip off all documentation within the embedded types.

A second use case would be removing descriptions at the package level, so I could exclude descriptions for alpha APIs to reduce the size of my manifest bundle while keeping the alpha schemas for CRD compatibility.