Merge pull request #1006 from dhaiducek/update-description-handling

✨ Handle word boundaries and add ellipsis for `MaxDescLen`

Author: k8s-ci-robot

pkg/crd/desc_visitor.go

@@ -33,7 +33,7 @@ func TruncateDescription(schema *apiext.JSONSchemaProps, maxLen int) {
 // descVisitor recursively visits all fields in the schema and truncates the
 // description of the fields to specified maxLen.
 type descVisitor struct {
-	// maxLen is the maximum allowed length for decription of a field
+	// maxLen is the maximum allowed length for description of a field
 	maxLen int
 }
 
@@ -60,19 +60,31 @@ func (v descVisitor) Visit(schema *apiext.JSONSchemaProps) SchemaVisitor {
 // exceeds maxLen because it tries to chop off the desc at the closest sentence
 // boundary to avoid incomplete sentences.
 func truncateString(desc string, maxLen int) string {
+	if len(desc) <= maxLen {
+		return desc
+	}
+
 	desc = desc[0:maxLen]
 
 	// Trying to chop off at closest sentence boundary.
 	if n := strings.LastIndexFunc(desc, isSentenceTerminal); n > 0 {
 		return desc[0 : n+1]
 	}
-	// TODO(droot): Improve the logic to chop off at closest word boundary
-	// or add elipses (...) to indicate that it's chopped incase no closest
-	// sentence found within maxLen.
-	return desc
+
+	// Trying to chop off at closest word boundary (i.e. whitespace).
+	if n := strings.LastIndexFunc(desc, isWhiteSpace); n > 0 {
+		return desc[0 : n] + "..."
+	}
+
+	return desc[0:maxLen] + "..."
 }
 
 // helper function to determine if given rune is a sentence terminal or not.
 func isSentenceTerminal(r rune) bool {
 	return unicode.Is(unicode.STerm, r)
 }
+
+// helper function to determine if given rune is whitespace or not.
+func isWhiteSpace(r rune) bool {
+	return unicode.Is(unicode.White_Space, r)
+}

pkg/crd/desc_visitor_test.go

@@ -75,7 +75,7 @@ var _ = Describe("TruncateDescription", func() {
 		}
 		crd.TruncateDescription(schema, len(schema.Description)-2)
 		Expect(schema).To(Equal(&apiext.JSONSchemaProps{
-			Description: `This is top level description of the root obje`,
+			Description: `This is top level description of the root...`,
 		}))
 	})
 })

pkg/crd/gen.go

@@ -93,7 +93,7 @@ type Generator struct {
 	// This value can only be specified for CustomResourceDefinitions that were created with
 	// `apiextensions.k8s.io/v1beta1`.
 	//
-	// The field can be set for compatiblity reasons, although strongly discouraged, resource
+	// The field can be set for compatibility reasons, although strongly discouraged, resource
 	// authors should move to a structural OpenAPI schema instead.
 	//
 	// See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#field-pruning

pkg/crd/gen_integration_test.go

@@ -156,6 +156,24 @@ var _ = Describe("CRD Generation proper defaulting", func() {
 		By("searching preserveUnknownFields")
 		Expect(out.buf.String()).NotTo(ContainSubstring("preserveUnknownFields"))
 	})
+
+	It("should truncate CRD descriptions", func() {
+		By("calling Generate")
+		var fifty int = 50
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1"},
+			MaxDescLen: &fifty,
+		}
+		Expect(gen.Generate(ctx)).NotTo(HaveOccurred())
+
+		By("loading the desired YAML")
+		expectedFile, err := os.ReadFile(filepath.Join(genDir, "bar.example.com_foos_maxdesclen.yaml"))
+		Expect(err).NotTo(HaveOccurred())
+		expectedFile = fixAnnotations(expectedFile)
+
+		By("comparing the two")
+		Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile)))
+	})
 })
 
 // fixAnnotations fixes the attribution annotation for tests.

pkg/crd/testdata/gen/bar.example.com_foos_maxdesclen.yaml

@@ -0,0 +1,45 @@
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (devel)
+  name: foos.bar.example.com
+spec:
+  group: bar.example.com
+  names:
+    kind: Foo
+    listKind: FooList
+    plural: foos
+    singular: foo
+  scope: Namespaced
+  versions:
+  - name: foo
+    schema:
+      openAPIV3Schema:
+        properties:
+          apiVersion:
+            description: APIVersion defines the versioned schema of this...
+            type: string
+          kind:
+            description: Kind is a string value representing the REST...
+            type: string
+          metadata:
+            type: object
+          spec:
+            description: Spec comments SHOULD appear in the CRD spec
+            properties:
+              defaultedString:
+                default: fooDefaultString
+                description: This tests that defaulted fields are stripped for...
+                example: fooExampleString
+                type: string
+            required:
+            - defaultedString
+            type: object
+          status:
+            description: Status comments SHOULD appear in the CRD spec
+            type: object
+        type: object
+    served: true
+    storage: true