🐛 Skip unexported fields when generating CRDs (#584)

* Skip unexported fields when generating CRDs

* Add generator flag to skip unexported fields

Author: williammck

pkg/crd/gen.go

@@ -41,6 +41,11 @@ const defaultVersion = v1
 
 // Generator generates CustomResourceDefinition objects.
 type Generator struct {
+	// IgnoreUnexportedFields indicates that we should skip unexported fields.
+	//
+	// Left unspecified, the default is false.
+	IgnoreUnexportedFields *bool `marker:",optional"`
+
 	// AllowDangerousTypes allows types which are usually omitted from CRD generation
 	// because they are not recommended.
 	//
@@ -85,7 +90,8 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
 		Collector: ctx.Collector,
 		Checker:   ctx.Checker,
 		// Perform defaulting here to avoid ambiguity later
-		AllowDangerousTypes: g.AllowDangerousTypes != nil && *g.AllowDangerousTypes == true,
+		IgnoreUnexportedFields: g.IgnoreUnexportedFields != nil && *g.IgnoreUnexportedFields == true,
+		AllowDangerousTypes:    g.AllowDangerousTypes != nil && *g.AllowDangerousTypes == true,
 		// Indicates the parser on whether to register the ObjectMeta type or not
 		GenerateEmbeddedObjectMeta: g.GenerateEmbeddedObjectMeta != nil && *g.GenerateEmbeddedObjectMeta == true,
 	}

pkg/crd/parser.go

@@ -87,6 +87,9 @@ type Parser struct {
 	// TODO: Should we have a more formal mechanism for putting "type patterns" in each of the above categories?
 	AllowDangerousTypes bool
 
+	// IgnoreUnexportedFields specifies if unexported fields on the struct should be skipped
+	IgnoreUnexportedFields bool
+
 	// GenerateEmbeddedObjectMeta specifies if any embedded ObjectMeta should be generated
 	GenerateEmbeddedObjectMeta bool
 }
@@ -178,7 +181,7 @@ func (p *Parser) NeedSchemaFor(typ TypeIdent) {
 	// avoid tripping recursive schemata, like ManagedFields, by adding an empty WIP schema
 	p.Schemata[typ] = apiext.JSONSchemaProps{}
 
-	schemaCtx := newSchemaContext(typ.Package, p, p.AllowDangerousTypes)
+	schemaCtx := newSchemaContext(typ.Package, p, p.AllowDangerousTypes, p.IgnoreUnexportedFields)
 	ctxForInfo := schemaCtx.ForInfo(info)
 
 	pkgMarkers, err := markers.PackageMarkers(p.Collector, typ.Package)

pkg/crd/parser_integration_test.go

@@ -72,8 +72,9 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 		reg := &markers.Registry{}
 		Expect(crdmarkers.Register(reg)).To(Succeed())
 		parser := &crd.Parser{
-			Collector: &markers.Collector{Registry: reg},
-			Checker:   &loader.TypeChecker{},
+			Collector:              &markers.Collector{Registry: reg},
+			Checker:                &loader.TypeChecker{},
+			IgnoreUnexportedFields: true,
 		}
 		crd.AddKnownTypes(parser)
 

pkg/crd/schema.go

@@ -69,28 +69,31 @@ type schemaContext struct {
 	schemaRequester schemaRequester
 	PackageMarkers  markers.MarkerValues
 
-	allowDangerousTypes bool
+	allowDangerousTypes    bool
+	ignoreUnexportedFields bool
 }
 
 // newSchemaContext constructs a new schemaContext for the given package and schema requester.
 // It must have type info added before use via ForInfo.
-func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes bool) *schemaContext {
+func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes, ignoreUnexportedFields bool) *schemaContext {
 	pkg.NeedTypesInfo()
 	return &schemaContext{
-		pkg:                 pkg,
-		schemaRequester:     req,
-		allowDangerousTypes: allowDangerousTypes,
+		pkg:                    pkg,
+		schemaRequester:        req,
+		allowDangerousTypes:    allowDangerousTypes,
+		ignoreUnexportedFields: ignoreUnexportedFields,
 	}
 }
 
 // ForInfo produces a new schemaContext with containing the same information
 // as this one, except with the given type information.
 func (c *schemaContext) ForInfo(info *markers.TypeInfo) *schemaContext {
 	return &schemaContext{
-		pkg:                 c.pkg,
-		info:                info,
-		schemaRequester:     c.schemaRequester,
-		allowDangerousTypes: c.allowDangerousTypes,
+		pkg:                    c.pkg,
+		info:                   info,
+		schemaRequester:        c.schemaRequester,
+		allowDangerousTypes:    c.allowDangerousTypes,
+		ignoreUnexportedFields: c.ignoreUnexportedFields,
 	}
 }
 
@@ -339,6 +342,11 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON
 	}
 
 	for _, field := range ctx.info.Fields {
+		// Skip if the field is not an inline field, ignoreUnexportedFields is true, and the field is not exported
+		if field.Name != "" && ctx.ignoreUnexportedFields && !ast.IsExported(field.Name) {
+			continue
+		}
+
 		jsonTag, hasTag := field.Tag.Lookup("json")
 		if !hasTag {
 			// if the field doesn't have a JSON tag, it doesn't belong in output (and shouldn't exist in a serialized type)

pkg/crd/schema_test.go

@@ -63,7 +63,7 @@ func transform(t *testing.T, expr string) *apiext.JSONSchemaProps {
 	pkg.NeedTypesInfo()
 	failIfErrors(t, pkg.Errors)
 
-	schemaContext := newSchemaContext(pkg, nil, true).ForInfo(&markers.TypeInfo{})
+	schemaContext := newSchemaContext(pkg, nil, true, false).ForInfo(&markers.TypeInfo{})
 	// yick: grab the only type definition
 	definedType := pkg.Syntax[0].Decls[0].(*ast.GenDecl).Specs[0].(*ast.TypeSpec).Type
 	result := typeToSchema(schemaContext, definedType)

pkg/crd/testdata/cronjob_types.go

@@ -181,6 +181,13 @@ type CronJobSpec struct {
 
 	// Maps of arrays of things-that-aren’t-strings are permitted
 	MapOfArraysOfFloats map[string][]bool `json:"mapOfArraysOfFloats,omitempty"`
+
+	// This tests that unexported fields are skipped in the schema generation
+	unexportedField string
+
+	// This tests that both unexported and exported inline fields are not skipped in the schema generation
+	unexportedStruct `json:",inline"`
+	ExportedStruct `json:",inline"`
 }
 
 type ContainsNestedMap struct {
@@ -235,6 +242,22 @@ type MinMaxObject struct {
 	Baz string `json:"baz,omitempty"`
 }
 
+type unexportedStruct struct {
+	// This tests that exported fields are not skipped in the schema generation
+	Foo string `json:"foo"`
+
+	// This tests that unexported fields are skipped in the schema generation
+	bar string
+}
+
+type ExportedStruct struct {
+	// This tests that exported fields are not skipped in the schema generation
+	Baz string `json:"baz"`
+
+	// This tests that unexported fields are skipped in the schema generation
+	qux string
+}
+
 type RootObject struct {
 	Nested NestedObject `json:"nested"`
 }

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -55,6 +55,9 @@ spec:
                 - name
                 - secondary
                 x-kubernetes-list-type: map
+              baz:
+                description: This tests that exported fields are not skipped in the schema generation
+                type: string
               binaryName:
                 description: This tests byte slice schema generation.
                 format: byte
@@ -134,6 +137,9 @@ spec:
                   for having a pattern on this type.
                 pattern: ^((100|[0-9]{1,2})%|[0-9]+)$
                 x-kubernetes-int-or-string: true
+              foo:
+                description: This tests that exported fields are not skipped in the schema generation
+                type: string
               jobTemplate:
                 description: Specifies the job that will be created when executing
                   a CronJob.
@@ -7312,12 +7318,14 @@ spec:
                 x-kubernetes-preserve-unknown-fields: true
             required:
             - associativeList
+            - baz
             - binaryName
             - canBeNull
             - defaultedObject
             - defaultedSlice
             - defaultedString
             - embeddedResource
+            - foo
             - jobTemplate
             - mapOfInfo
             - patternObject