diff --git a/pkg/crd/desc_visitor.go b/pkg/crd/desc_visitor.go index cba36c46c..e064be03d 100644 --- a/pkg/crd/desc_visitor.go +++ b/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) +} diff --git a/pkg/crd/desc_visitor_test.go b/pkg/crd/desc_visitor_test.go index a0f989ea0..0edd8a5d0 100644 --- a/pkg/crd/desc_visitor_test.go +++ b/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...`, })) }) }) diff --git a/pkg/crd/gen.go b/pkg/crd/gen.go index ac8eb566d..2c68e15a0 100644 --- a/pkg/crd/gen.go +++ b/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 diff --git a/pkg/crd/gen_integration_test.go b/pkg/crd/gen_integration_test.go index d59e3e923..aea253f0e 100644 --- a/pkg/crd/gen_integration_test.go +++ b/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. diff --git a/pkg/crd/testdata/gen/bar.example.com_foos_maxdesclen.yaml b/pkg/crd/testdata/gen/bar.example.com_foos_maxdesclen.yaml new file mode 100644 index 000000000..d0371eb5d --- /dev/null +++ b/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