Skip to content

Commit

Permalink
Merge pull request #449 from matthchr/feature/float-support
Browse files Browse the repository at this point in the history
✨ Add support for AllowDangerousTypes crd flag (enables float support)
  • Loading branch information
k8s-ci-robot authored Jun 16, 2020
2 parents b243151 + ed1c4c9 commit b45abdb
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 10 deletions.
12 changes: 12 additions & 0 deletions pkg/crd/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ type Generator struct {
// It's required to be false for v1 CRDs.
PreserveUnknownFields *bool `marker:",optional"`

// AllowDangerousTypes allows types which are usually omitted from CRD generation
// because they are not recommended.
//
// Currently the following additional types are allowed when this is true:
// float32
// float64
//
// Left unspecified, the default is false
AllowDangerousTypes *bool `marker:",optional"`

// MaxDescLen specifies the maximum description length for fields in CRD's OpenAPI schema.
//
// 0 indicates drop the description for all fields completely.
Expand All @@ -78,6 +88,8 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
parser := &Parser{
Collector: ctx.Collector,
Checker: ctx.Checker,
// Perform defaulting here to avoid ambiguity later
AllowDangerousTypes: g.AllowDangerousTypes != nil && *g.AllowDangerousTypes == true,
}

AddKnownTypes(parser)
Expand Down
16 changes: 15 additions & 1 deletion pkg/crd/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,20 @@ type Parser struct {
packages map[*loader.Package]struct{}

flattener *Flattener

// AllowDangerousTypes controls the handling of non-recommended types such as float. If
// false (the default), these types are not supported.
// There is a continuum here:
// 1. Types that are always supported.
// 2. Types that are allowed by default, but not recommended (warning emitted when they are encountered as per PR #443).
// Possibly they are allowed by default for historical reasons and may even be "on their way out" at some point in the future.
// 3. Types that are not allowed by default, not recommended, but there are some legitimate reasons to need them in certain corner cases.
// Possibly these types should also emit a warning as per PR #443 even when they are "switched on" (an integration point between
// this feature and #443 if desired). This is the category that this flag deals with.
// 4. Types that are not allowed and will not be allowed, possibly because it just "doesn't make sense" or possibly
// because the implementation is too difficult/clunky to promote them to category 3.
// TODO: Should we have a more formal mechanism for putting "type patterns" in each of the above categories?
AllowDangerousTypes bool
}

func (p *Parser) init() {
Expand Down Expand Up @@ -162,7 +176,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)
schemaCtx := newSchemaContext(typ.Package, p, p.AllowDangerousTypes)
ctxForInfo := schemaCtx.ForInfo(info)

pkgMarkers, err := markers.PackageMarkers(p.Collector, typ.Package)
Expand Down
24 changes: 15 additions & 9 deletions pkg/crd/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,25 +68,29 @@ type schemaContext struct {

schemaRequester schemaRequester
PackageMarkers markers.MarkerValues

allowDangerousTypes 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) *schemaContext {
func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes bool) *schemaContext {
pkg.NeedTypesInfo()
return &schemaContext{
pkg: pkg,
schemaRequester: req,
pkg: pkg,
schemaRequester: req,
allowDangerousTypes: allowDangerousTypes,
}
}

// 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,
pkg: c.pkg,
info: info,
schemaRequester: c.schemaRequester,
allowDangerousTypes: c.allowDangerousTypes,
}
}

Expand Down Expand Up @@ -206,7 +210,7 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiext.JSONSchema
return &apiext.JSONSchemaProps{}
}
if basicInfo, isBasic := typeInfo.(*types.Basic); isBasic {
typ, fmt, err := builtinToType(basicInfo)
typ, fmt, err := builtinToType(basicInfo, ctx.allowDangerousTypes)
if err != nil {
ctx.pkg.AddError(loader.ErrFromNode(err, ident))
}
Expand Down Expand Up @@ -398,8 +402,8 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON

// builtinToType converts builtin basic types to their equivalent JSON schema form.
// It *only* handles types allowed by the kubernetes API standards. Floats are not
// allowed.
func builtinToType(basic *types.Basic) (typ string, format string, err error) {
// allowed unless allowDangerousTypes is true
func builtinToType(basic *types.Basic, allowDangerousTypes bool) (typ string, format string, err error) {
// NB(directxman12): formats from OpenAPI v3 are slightly different than those defined
// in JSONSchema. This'll use the OpenAPI v3 ones, since they're useful for bounding our
// non-string types.
Expand All @@ -411,6 +415,8 @@ func builtinToType(basic *types.Basic) (typ string, format string, err error) {
typ = "string"
case basicInfo&types.IsInteger != 0:
typ = "integer"
case basicInfo&types.IsFloat != 0 && allowDangerousTypes:
typ = "number"
default:
// NB(directxman12): floats are *NOT* allowed in kubernetes APIs
return "", "", fmt.Errorf("unsupported type %q", basic.String())
Expand Down
4 changes: 4 additions & 0 deletions pkg/crd/zz_generated.markerhelp.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b45abdb

Please sign in to comment.