elastic / crd-ref-docs

Generates Kubernetes CRD API reference documentation
Apache License 2.0
103 stars 53 forks source link

Support for rendering whether fields are required/optional #38

Closed jinnovation closed 1 year ago

jinnovation commented 1 year ago

CustomResourceDefinition has a required field for API versions that shows which fields must be populated for the custom resource in question. This would be useful to expose in documentation.

robbie-demuth commented 1 year ago

I'd like to render whether fields are required or optional too. I explored doing so via custom templates, but doing so isn't possible b/c field markers and tags aren't available via types.Field. I'm guessing you're looking for the built-in templates to support rendering whether fields are required or optional. My guess is this is a bit hard given that there are so many different ways to indicate whether fields are required or optional:

If we wanted to add a Required bool (or Optional bool) field to types.Field (and update the built-in templates to use it), we'd probably need to mimic controller-tool's logic (I haven't dug into it yet). That being said, I think there's a lower effort approach where we transfer tags and markers from markers.FieldInfo to types.Field and enable users to write custom templates that render whether fields are required or optional based on how they indicate so in their *_types.go files. Thoughts?

robbie-demuth commented 1 year ago

we'd probably need to mimic controller-tool's logic

This looks like the appropriate bit:

        // if no default required mode is set, default to required
        defaultMode := "required"
        if ctx.PackageMarkers.Get("kubebuilder:validation:Optional") != nil {
            defaultMode = "optional"
        }

        switch defaultMode {
        // if this package isn't set to optional default...
        case "required":
            // ...everything that's not inline, omitempty, or explicitly optional is required
            if !inline && !omitEmpty && field.Markers.Get("kubebuilder:validation:Optional") == nil && field.Markers.Get("optional") == nil {
                props.Required = append(props.Required, fieldName)
            }

        // if this package isn't set to required default...
        case "optional":
            // ...everything that isn't explicitly required is optional
            if field.Markers.Get("kubebuilder:validation:Required") != nil {
                props.Required = append(props.Required, fieldName)
            }
        }
mikaello commented 1 year ago

We have kind of solved this by adding the columns Default and Validation, which just forwards the content of the default and validation markers with minor changes, example:

image

Code:

type Ingress struct {
    Auto *AutoIngress `json:"auto,omitempty"`
    // Subdomains enables ingress for hosts in list.
    Subdomains []Subdomain `json:"subdomains,omitempty"`
    // Domains enables ingress for hosts in list.
    Domains []Domain `json:"domains,omitempty"`
    // TimeoutSeconds sets the server-side timeout for the ingress.
    // Implementation specific default value.
    //+kubebuilder:validation:Minimum=1
    //+kubebuilder:validation:Maximum=2147483647
    TimeoutSeconds *int32 `json:"timeoutSeconds,omitempty"`
    //+kubebuilder:validation:Enum={Edge,Passthrough,Reencrypt}
    //+kubebuilder:default=Edge
    Termination *common.IngressTLSTermination `json:"termination,omitempty"`
}

This is the code change in crd-ref-doc: https://github.com/mikaello/crd-ref-docs-fork/commit/889241694b63598f79ed6f5c40058ecee78e7cb1

mikaello commented 1 year ago

The showcase of the marker feature in https://github.com/elastic/crd-ref-docs/pull/49 could also solve this issue.

mowies commented 1 year ago

Any updates on this? :) We would really like to have this feature in https://github.com/keptn/lifecycle-toolkit

thbkrkr commented 1 year ago

Closed by: