Skip to content

+enum validation marker #1179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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?

// It should.
//
// +enum
// +kubebuilder:validation:Enum=Allow;Forbid;Replace
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