From f613a4de6db40570d04bccb2aed2d42d7d6c09fb Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Tue, 23 Jan 2024 14:46:32 +0200 Subject: [PATCH] Strip comments from CRD descriptions Kubernetes has some conventions for type descriptions: - Line that only contains --- separates the description from comments. The leading lines are part of public documentation, the trailing lines are internal instructions such as code examples etc. - Line that begins with TODO are internal notes for implementers, and not part of public documentation of the type. This change strips above informational text away from the generated CRDs while keeping them if they appear inside markdown fenced code block. Signed-off-by: Tero Saarni --- .../testdata.kubebuilder.io_cronjobs.yaml | 79 +------------------ pkg/markers/collect_test.go | 4 +- pkg/markers/markers_suite_test.go | 2 + pkg/markers/zip.go | 37 +++++++-- 4 files changed, 38 insertions(+), 84 deletions(-) diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index 83d11978b..10a24e6c9 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -992,7 +992,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -1059,7 +1058,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -1092,7 +1090,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the ConfigMap @@ -1111,7 +1108,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the Secret @@ -1219,7 +1215,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -1324,7 +1319,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -1448,7 +1442,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -1634,7 +1627,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -1811,7 +1803,6 @@ spec: type indicates which kind of seccomp profile will be applied. Valid options are: - Localhost - a profile defined in a file on the node should be used. RuntimeDefault - the container runtime default profile should be used. Unconfined - no profile should be applied. @@ -1952,7 +1943,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -2235,7 +2225,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -2302,7 +2291,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -2335,7 +2323,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the ConfigMap @@ -2354,7 +2341,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the Secret @@ -2459,7 +2445,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -2564,7 +2549,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -2685,7 +2669,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -2857,7 +2840,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -3031,7 +3013,6 @@ spec: type indicates which kind of seccomp profile will be applied. Valid options are: - Localhost - a profile defined in a file on the node should be used. RuntimeDefault - the container runtime default profile should be used. Unconfined - no profile should be applied. @@ -3165,7 +3146,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -3372,7 +3352,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object type: array @@ -3459,7 +3438,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -3526,7 +3504,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -3559,7 +3536,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the ConfigMap @@ -3578,7 +3554,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the Secret @@ -3686,7 +3661,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -3791,7 +3765,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name @@ -3915,7 +3888,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -4101,7 +4073,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -4278,7 +4249,6 @@ spec: type indicates which kind of seccomp profile will be applied. Valid options are: - Localhost - a profile defined in a file on the node should be used. RuntimeDefault - the container runtime default profile should be used. Unconfined - no profile should be applied. @@ -4419,7 +4389,6 @@ spec: description: |- TCPSocket specifies an action involving a TCP port. TCP hooks not yet supported - TODO: implement a realistic TCP lifecycle hook properties: host: description: 'Optional: Host name to @@ -4671,12 +4640,10 @@ spec: Some volume types allow the Kubelet to change the ownership of that volume to be owned by the pod: - 1. The owning GID will be the FSGroup 2. The setgid bit is set (new files created in the volume will be owned by FSGroup) 3. The permission bits are OR'd with rw-rw---- - If unset, the Kubelet will not modify the ownership and permissions of any volume. format: int64 type: integer @@ -4757,7 +4724,6 @@ spec: type indicates which kind of seccomp profile will be applied. Valid options are: - Localhost - a profile defined in a file on the node should be used. RuntimeDefault - the container runtime default profile should be used. Unconfined - no profile should be applied. @@ -5033,7 +4999,6 @@ spec: Tip: Ensure that the filesystem type is supported by the host operating system. Examples: "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. More info: https://kubernetes.io/docs/concepts/storage/volumes#awselasticblockstore - TODO: how do we prevent errors in the filesystem from compromising the machine type: string partition: description: |- @@ -5153,7 +5118,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object user: @@ -5191,7 +5155,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object volumeID: @@ -5259,7 +5222,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the ConfigMap @@ -5294,7 +5256,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object readOnly: @@ -5437,7 +5398,6 @@ spec: The volume's lifecycle is tied to the pod that defines it - it will be created before the pod starts, and deleted when the pod is removed. - Use this if: a) the volume is only needed while the pod runs, b) features of normal volumes like restoring from snapshot or capacity @@ -5448,17 +5408,14 @@ spec: information on the connection between this volume type and PersistentVolumeClaim). - Use PersistentVolumeClaim or one of the vendor-specific APIs for volumes that persist for longer than the lifecycle of an individual pod. - Use CSI for light-weight local ephemeral volumes if the CSI driver is meant to be used that way - see the documentation of the driver for more information. - A pod can use both types of ephemeral volumes and persistent volumes at the same time. properties: @@ -5477,7 +5434,6 @@ spec: entry. Pod validation will reject the pod if the concatenated name is not valid for a PVC (for example, too long). - An existing PVC with that name that is not owned by the pod will *not* be used for the pod to avoid using an unrelated volume by mistake. Starting the pod is then blocked until @@ -5487,11 +5443,9 @@ spec: this should not be necessary, but it may be useful when manually reconstructing a broken cluster. - This field is read-only and no changes will be made by Kubernetes to the PVC after it has been created. - Required, must not be nil. properties: metadata: @@ -5654,7 +5608,6 @@ spec: Filesystem type to mount. Must be a filesystem type supported by the host operating system. Ex. "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. - TODO: how do we prevent errors in the filesystem from compromising the machine type: string lun: description: 'Optional: FC target lun number' @@ -5717,7 +5670,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object required: @@ -5751,7 +5703,6 @@ spec: Tip: Ensure that the filesystem type is supported by the host operating system. Examples: "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. More info: https://kubernetes.io/docs/concepts/storage/volumes#gcepersistentdisk - TODO: how do we prevent errors in the filesystem from compromising the machine type: string partition: description: |- @@ -5832,9 +5783,6 @@ spec: used for system agents or other privileged things that are allowed to see the host machine. Most containers will NOT need this. More info: https://kubernetes.io/docs/concepts/storage/volumes#hostpath - --- - TODO(jonesdl) We need to restrict who can use host directory mounts and who can/can not - mount host directories as read/write. properties: path: description: |- @@ -5871,7 +5819,6 @@ spec: Tip: Ensure that the filesystem type is supported by the host operating system. Examples: "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. More info: https://kubernetes.io/docs/concepts/storage/volumes#iscsi - TODO: how do we prevent errors in the filesystem from compromising the machine type: string initiatorName: description: |- @@ -5911,7 +5858,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object targetPortal: @@ -6083,7 +6029,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -6229,7 +6174,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string optional: description: Specify whether the @@ -6319,7 +6263,6 @@ spec: Tip: Ensure that the filesystem type is supported by the host operating system. Examples: "ext4", "xfs", "ntfs". Implicitly inferred to be "ext4" if unspecified. More info: https://kubernetes.io/docs/concepts/storage/volumes#rbd - TODO: how do we prevent errors in the filesystem from compromising the machine type: string image: description: |- @@ -6362,7 +6305,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object user: @@ -6409,7 +6351,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object sslEnabled: @@ -6528,7 +6469,6 @@ spec: description: |- Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names - TODO: Add other useful fields. apiVersion, kind, uid? type: string type: object volumeName: @@ -6853,22 +6793,8 @@ spec: active: description: A list of pointers to currently running jobs. items: - description: |- - ObjectReference contains enough information to let you inspect or modify the referred object. - --- - New uses of this type are discouraged because of difficulty describing its usage when embedded in APIs. - 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion and FieldPath are both very rarely valid in actual usage. - 2. Invalid usage help. It is impossible to add specific help for individual usage. In most embedded usages, there are particular - restrictions like, "must refer only to types A and B" or "UID not honored" or "name must be restricted". - Those cannot be well described when embedded. - 3. Inconsistent validation. Because the usages are different, the validation rules are different by usage, which makes it hard for users to predict what will happen. - 4. The fields are both imprecise and overly precise. Kind is not a precise mapping to a URL. This can produce ambiguity - during interpretation and require a REST mapping. In most cases, the dependency is on the group,resource tuple - and the version of the actual struct is irrelevant. - 5. We cannot easily change it. Because this type is embedded in many locations, updates to this type - will affect numerous schemas. Don't make new APIs embed an underspecified API type they do not control. - Instead of using this type, create a locally provided and used type that is well-focused on your reference. - For example, ServiceReferences for admission registration: https://github.com/kubernetes/api/blob/release-1.17/admissionregistration/v1/types.go#L533 . + description: ObjectReference contains enough information to let + you inspect or modify the referred object. properties: apiVersion: description: API version of the referent. @@ -6882,7 +6808,6 @@ spec: the event) or if no container name is specified "spec.containers[2]" (container with index 2 in this pod). This syntax is chosen only to have some well-defined way of referencing a part of an object. - TODO: this design is not final and this field is subject to change in the future. type: string kind: description: |- diff --git a/pkg/markers/collect_test.go b/pkg/markers/collect_test.go index 3dadbf120..8ac0ce26b 100644 --- a/pkg/markers/collect_test.go +++ b/pkg/markers/collect_test.go @@ -141,12 +141,14 @@ var _ = Describe("Collecting", func() { Expect(docsByType).To(HaveKeyWithValue("HasNonAsteriskDocWithYamlEmbeeded", `This is a description this is an example as yaml: +~~~yaml --- foo: bar: dar: - value1 - - value2`)) + - value2 +~~~`)) }) }) diff --git a/pkg/markers/markers_suite_test.go b/pkg/markers/markers_suite_test.go index 36bd21e32..e977094c5 100644 --- a/pkg/markers/markers_suite_test.go +++ b/pkg/markers/markers_suite_test.go @@ -101,12 +101,14 @@ var _ = BeforeSuite(func() { // This is a description // this is an example as yaml: + // ~~~yaml // --- // foo: // bar: // dar: // - value1 // - value2 + // ~~~ type HasNonAsteriskDocWithYamlEmbeeded struct { } diff --git a/pkg/markers/zip.go b/pkg/markers/zip.go index fd5b0035a..5abedb4ac 100644 --- a/pkg/markers/zip.go +++ b/pkg/markers/zip.go @@ -69,13 +69,15 @@ func extractDoc(node ast.Node, decl *ast.GenDecl) string { // split lines, and re-join together as a single // paragraph, respecting double-newlines as // paragraph markers. - outLines := strings.Split(outGroup.Text(), "\n") - if outLines[len(outLines)-1] == "" { + lines := strings.Split(outGroup.Text(), "\n") + if lines[len(lines)-1] == "" { // chop off the extraneous last part - outLines = outLines[:len(outLines)-1] + lines = lines[:len(lines)-1] } - for i, line := range outLines { + var outLines []string + var insideCodeBlock bool + for i, line := range lines { if isAsteriskComment { // Trim any extranous whitespace, // for handling /*…*/-style comments, @@ -86,10 +88,33 @@ func extractDoc(node ast.Node, decl *ast.GenDecl) string { // Respect that double-newline means // actual newline: if line == "" { - outLines[i] = "\n" + lines[i] = "\n" } else { - outLines[i] = line + lines[i] = line } + + // Recognize markdown code blocks (``` or ~~~) + // https://spec.commonmark.org/0.27/#fenced-code-blocks + if strings.HasPrefix(line, "```") || strings.HasPrefix(line, "~~~") { + insideCodeBlock = !insideCodeBlock + } + + if !insideCodeBlock { + // If we are not inside markdown code black, follow the Kubernetes formatting conventions: + // - Lines after --- are comments and should be ignored. + // - Lines starting with TODO are comments and should be ignored. + // See function fmtRawDoc() in https://github.com/kubernetes/apimachinery/blob/master/pkg/runtime/swagger_doc_generator.go + + if strings.HasPrefix(line, "TODO") { + continue + } + + if strings.HasPrefix(line, "---") { + break + } + } + + outLines = append(outLines, line) } return strings.Join(outLines, "\n") }