Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions pkg/crd/markers/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,12 @@ type SelectableField struct {
JSONPath string `marker:"JSONPath"`
}

// +controllertools:marker:generateHelp:category="CRD validation"

// Enum marker marks a string alias type as an enum.
// It infers the members from constants declared of that type.
type InferredEnum struct{}

func (s SelectableField) ApplyToCRD(crd *apiext.CustomResourceDefinitionSpec, version string) error {
var selectableFields *[]apiext.SelectableField
for i := range crd.Versions {
Expand Down
2 changes: 2 additions & 0 deletions pkg/crd/markers/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ var ValidationIshMarkers = []*definitionWithHelp{
WithHelp(XPreserveUnknownFields{}.Help()),
must(markers.MakeDefinition("kubebuilder:pruning:PreserveUnknownFields", markers.DescribesType, XPreserveUnknownFields{})).
WithHelp(XPreserveUnknownFields{}.Help()),
must(markers.MakeDefinition("enum", markers.DescribesType, InferredEnum{})).
WithHelp(InferredEnum{}.Help()),
}

func init() {
Expand Down
11 changes: 11 additions & 0 deletions pkg/crd/markers/zz_generated.markerhelp.go

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

17 changes: 17 additions & 0 deletions pkg/crd/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package crd

import (
"fmt"
"go/ast"

apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -122,6 +123,21 @@ func (p *Parser) init() {
}
}

func (p *Parser) indexEnumMemberConstantDeclarations(pkg *loader.Package) {
loader.EachConstDecl(pkg, func(spec *ast.ValueSpec) {
if id, ok := spec.Type.(*ast.Ident); ok {
if typeinfo, ok := p.Types[TypeIdent{
pkg, id.Name,
}]; ok {
typeinfo.EnumValues = append(typeinfo.EnumValues, markers.EnumMemberInfo{
Name: spec.Names[0].Name,
ValueSpec: spec,
})
}
}
})
}

// indexTypes loads all types in the package into Types.
func (p *Parser) indexTypes(pkg *loader.Package) {
// autodetect
Expand Down Expand Up @@ -220,6 +236,7 @@ func (p *Parser) AddPackage(pkg *loader.Package) {
return
}
p.indexTypes(pkg)
p.indexEnumMemberConstantDeclarations(pkg)
p.Checker.Check(pkg)
p.packages[pkg] = struct{}{}
}
Expand Down
38 changes: 33 additions & 5 deletions pkg/crd/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ limitations under the License.
package crd

import (
"encoding/json"
"errors"
"fmt"
"go/ast"
"go/token"
"go/types"
"sort"
"strconv"
"strings"

apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand All @@ -36,7 +38,8 @@ import (

const (
// defPrefix is the prefix used to link to definitions in the OpenAPI schema.
defPrefix = "#/definitions/"
defPrefix = "#/definitions/"
typeString = "string"
)

// byteType is the types.Type for byte (see the types documention
Expand Down Expand Up @@ -126,9 +129,9 @@ func infoToSchema(ctx *schemaContext) *apiext.JSONSchemaProps {

// If the obj implements a text marshaler, encode it as a string.
case implements(obj.Type(), textMarshaler):
schema := &apiext.JSONSchemaProps{Type: "string"}
schema := &apiext.JSONSchemaProps{Type: typeString}
applyMarkers(ctx, ctx.info.Markers, schema, ctx.info.RawSpec.Type)
if schema.Type != "string" {
if schema.Type != typeString {
err := fmt.Errorf("%q implements encoding.TextMarshaler but schema type is not string: %q", ctx.info.RawSpec.Name, schema.Type)
ctx.pkg.AddError(loader.ErrFromNode(err, ctx.info.RawSpec.Type))
}
Expand Down Expand Up @@ -274,6 +277,29 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiext.JSONSchema
if err != nil {
ctx.pkg.AddError(loader.ErrFromNode(err, ident))
}
var enumMembers []apiext.JSON
if ctx.info.Markers.Get("enum") != nil && typ == typeString {
enumMembers = make([]apiext.JSON, 0, len(ctx.info.EnumValues))
var ok bool
for i := range ctx.info.EnumValues {
var member = &ctx.info.EnumValues[i]
var v *ast.BasicLit
if v, ok = member.Values[0].(*ast.BasicLit); !ok {
ctx.pkg.AddError(loader.ErrFromNode(errors.New("constants for a +enum decorated type should be strings"), ident))
}
var value string
if value, err = strconv.Unquote(v.Value); err != nil {
ctx.pkg.AddError(loader.ErrFromNode(err, ident))
continue
}
var j apiext.JSON
if j.Raw, err = json.Marshal(value); err != nil {
ctx.pkg.AddError(loader.ErrFromNode(err, ident))
continue
}
enumMembers = append(enumMembers, j)
}
}
// Check for type aliasing to a basic type for gotypesalias=0. See more
// in documentation https://pkg.go.dev/go/types#Alias:
// > For gotypesalias=1, alias declarations produce an Alias type.
Expand All @@ -284,12 +310,14 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiext.JSONSchema
link := TypeRefLink("", ident.Name)
return &apiext.JSONSchemaProps{
Type: typ,
Enum: enumMembers,
Format: fmt,
Ref: &link,
}
}
return &apiext.JSONSchemaProps{
Type: typ,
Enum: enumMembers,
Format: fmt,
}
}
Expand Down Expand Up @@ -334,7 +362,7 @@ func arrayToSchema(ctx *schemaContext, array *ast.ArrayType) *apiext.JSONSchemaP
// byte slices are represented as base64-encoded strings
// (the format is defined in OpenAPI v3, but not JSON Schema)
return &apiext.JSONSchemaProps{
Type: "string",
Type: typeString,
Format: "byte",
}
}
Expand Down Expand Up @@ -502,7 +530,7 @@ func builtinToType(basic *types.Basic, allowDangerousTypes bool) (typ string, fo
case basicInfo&types.IsBoolean != 0:
typ = "boolean"
case basicInfo&types.IsString != 0:
typ = "string"
typ = typeString
case basicInfo&types.IsInteger != 0:
typ = "integer"
case basicInfo&types.IsFloat != 0:
Expand Down
28 changes: 28 additions & 0 deletions pkg/crd/testdata/cronjob_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,29 @@ import (

const DefaultRefValue = "defaultRefValue"

// +enum
type EnumType string

const (
EnumType_One EnumType = "one"
EnumType_Two EnumType = "two"
EnumType_Three EnumType = "three"
)

// This enum type tests for whether when both "+enum" and
// "+kubebuilder:validation:Enum" are defined the former takes precedence.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter should be taking precedence no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. The tests show - Allow - Forbid - Replace.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MishimaPorte Can we get this comment updated please

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

// It should.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this last sentence.

//
// +enum
// +kubebuilder:validation:Enum=Allow;Forbid;Replace

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about forbidding this behavior?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would break users who have this already, for example on a type they don't actually control but have imported and then added the +kubebuilder:validation:Enum to, to allow compatibility with the validation of the type they imported

type AnotherEnumType string

const (
AnotherEnumType_One AnotherEnumType = "another_one"
AnotherEnumType_Two AnotherEnumType = "another_two"
AnotherEnumType_Three AnotherEnumType = "another_three"
)

// CronJobSpec defines the desired state of CronJob
// +kubebuilder:validation:XValidation:rule="has(oldSelf.forbiddenInt) || !has(self.forbiddenInt)",message="forbiddenInt is not allowed",fieldPath=".forbiddenInt",reason="FieldValueForbidden"
type CronJobSpec struct {
Expand Down Expand Up @@ -332,6 +355,11 @@ type CronJobSpec struct {
// +kubebuilder:validation:items:Enum=0;1;3
EnumSlice []int `json:"enumSlice,omitempty"`

EnumValue EnumType `json:"enumValue,omitempty"`
AnotherEnumValue AnotherEnumType `json:"anotherEnumValue,omitempty"`
// +kubebuilder:validation:Enum=Allow;Forbid;Replace
OneMoreEnumValue EnumType `json:"oneMoreEnumValue,omitempty"`

HostsAlias Hosts `json:"hostsAlias,omitempty"`

// This tests that alias imported from a package is handled correctly. The
Expand Down
Loading