Skip to content
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.

Sign up for GitHub

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

Jump to bottom

❇️ pkg/internal/codegen/parse: "Title" CRD validation annotation #182

Closed
wants to merge 2 commits into from
Closed

❇️ pkg/internal/codegen/parse: "Title" CRD validation annotation #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13d3af1
pkg/internal/codegen/parse/crd.go: add Title parse annotation
estroz Apr 10, 2019
c60f60b
testData: add Title annotations to test types
estroz Apr 10, 2019
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
50 changes: 34 additions & 16 deletions pkg/internal/codegen/parse/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,9 @@ var primitiveTemplate = template.Must(template.New("map-template").Parse(
{{ if .MinLength -}}
MinLength: getInt({{ .MinLength }}),
{{ end -}}
{{ if .Title -}}
Title: "{{ .Title }}",
{{ end -}}
}`))

// parsePrimitiveValidation returns a JSONSchemaProps object and its
Expand Down Expand Up @@ -316,14 +319,18 @@ func (b *APIs) parsePrimitiveValidation(t *types.Type, found sets.String, commen
return props, buff.String()
}

type mapTempateArgs struct {
type mapTemplateArgs struct {
v1beta1.JSONSchemaProps
Result string
SkipMapValidation bool
}

var mapTemplate = template.Must(template.New("map-template").Parse(
`v1beta1.JSONSchemaProps{
Type: "object",
{{ if .Title -}}
Title: "{{ .Title }}",
{{ end -}}
{{if not .SkipMapValidation}}AdditionalProperties: &v1beta1.JSONSchemaPropsOrBool{
Allows: true,
Schema: &{{.Result}},
Expand All @@ -335,6 +342,7 @@ var mapTemplate = template.Must(template.New("map-template").Parse(
func (b *APIs) parseMapValidation(t *types.Type, found sets.String, comments []string) (v1beta1.JSONSchemaProps, string) {
additionalProps, result := b.typeToJSONSchemaProps(t.Elem, found, comments, false)
additionalProps.Description = ""
additionalProps.Title = ""
props := v1beta1.JSONSchemaProps{
Type: "object",
Description: parseDescription(comments),
Expand All @@ -351,7 +359,7 @@ func (b *APIs) parseMapValidation(t *types.Type, found sets.String, comments []s
}

buff := &bytes.Buffer{}
if err := mapTemplate.Execute(buff, mapTempateArgs{Result: result, SkipMapValidation: parseOption.SkipMapValidation}); err != nil {
if err := mapTemplate.Execute(buff, mapTemplateArgs{props, result, parseOption.SkipMapValidation}); err != nil {
log.Fatalf("%v", err)
}
return props, buff.String()
Expand All @@ -360,6 +368,9 @@ func (b *APIs) parseMapValidation(t *types.Type, found sets.String, comments []s
var arrayTemplate = template.Must(template.New("array-template").Parse(
`v1beta1.JSONSchemaProps{
Type: "{{.Type}}",
{{ if .Title -}}
Title: "{{ .Title }}",
{{ end -}}
{{ if .Format -}}
Format: "{{.Format}}",
{{ end -}}
Expand Down Expand Up @@ -389,6 +400,7 @@ type arrayTemplateArgs struct {
func (b *APIs) parseArrayValidation(t *types.Type, found sets.String, comments []string) (v1beta1.JSONSchemaProps, string) {
items, result := b.typeToJSONSchemaProps(t.Elem, found, comments, false)
items.Description = ""
items.Title = ""
props := v1beta1.JSONSchemaProps{
Type: "array",
Items: &v1beta1.JSONSchemaPropsOrArray{Schema: &items},
Expand Down Expand Up @@ -426,17 +438,20 @@ type objectTemplateArgs struct {

var objectTemplate = template.Must(template.New("object-template").Parse(
`v1beta1.JSONSchemaProps{
{{ if not .IsRoot -}}
{{ if not .IsRoot -}}
Type: "object",
{{ end -}}
{{ end -}}
{{ if .Title -}}
Title: "{{ .Title }}",
{{ end -}}
Properties: map[string]v1beta1.JSONSchemaProps{
{{ range $k, $v := .Fields -}}
"{{ $k }}": {{ $v }},
{{ end -}}
},
{{if .Required}}Required: []string{
{{ range $k, $v := .Required -}}
"{{ $v }}",
"{{ $v }}",
{{ end -}}
},{{ end -}}
}`))
Expand Down Expand Up @@ -481,51 +496,44 @@ func getValidation(comment string, props *v1beta1.JSONSchemaProps) {
parts := strings.Split(c, "=")
if len(parts) != 2 {
log.Fatalf("Expected +kubebuilder:validation:<key>=<value> actual: %s", comment)
return
}
switch parts[0] {
case "Maximum":
f, err := strconv.ParseFloat(parts[1], 64)
if err != nil {
log.Fatalf("Could not parse float from %s: %v", comment, err)
return
}
props.Maximum = &f
case "ExclusiveMaximum":
b, err := strconv.ParseBool(parts[1])
if err != nil {
log.Fatalf("Could not parse bool from %s: %v", comment, err)
return
}
props.ExclusiveMaximum = b
case "Minimum":
f, err := strconv.ParseFloat(parts[1], 64)
if err != nil {
log.Fatalf("Could not parse float from %s: %v", comment, err)
return
}
props.Minimum = &f
case "ExclusiveMinimum":
b, err := strconv.ParseBool(parts[1])
if err != nil {
log.Fatalf("Could not parse bool from %s: %v", comment, err)
return
}
props.ExclusiveMinimum = b
case "MaxLength":
i, err := strconv.Atoi(parts[1])
v := int64(i)
if err != nil {
log.Fatalf("Could not parse int from %s: %v", comment, err)
return
}
props.MaxLength = &v
case "MinLength":
i, err := strconv.Atoi(parts[1])
v := int64(i)
if err != nil {
log.Fatalf("Could not parse int from %s: %v", comment, err)
return
}
props.MinLength = &v
case "Pattern":
Expand All @@ -536,7 +544,6 @@ func getValidation(comment string, props *v1beta1.JSONSchemaProps) {
v := int64(i)
if err != nil {
log.Fatalf("Could not parse int from %s: %v", comment, err)
return
}
props.MaxItems = &v
}
Expand All @@ -546,7 +553,6 @@ func getValidation(comment string, props *v1beta1.JSONSchemaProps) {
v := int64(i)
if err != nil {
log.Fatalf("Could not parse int from %s: %v", comment, err)
return
}
props.MinItems = &v
}
Expand All @@ -555,15 +561,13 @@ func getValidation(comment string, props *v1beta1.JSONSchemaProps) {
b, err := strconv.ParseBool(parts[1])
if err != nil {
log.Fatalf("Could not parse bool from %s: %v", comment, err)
return
}
props.UniqueItems = b
}
case "MultipleOf":
f, err := strconv.ParseFloat(parts[1], 64)
if err != nil {
log.Fatalf("Could not parse float from %s: %v", comment, err)
return
}
props.MultipleOf = &f
case "Enum":
Expand All @@ -577,6 +581,20 @@ func getValidation(comment string, props *v1beta1.JSONSchemaProps) {
}
case "Format":
props.Format = parts[1]
case "Title":
title := strings.TrimSpace(strings.Join(parts[1:], " "))
if len(title) > 2 {
// Remove beginning and ending quote from non-empty strings.
if title[0] == '"' {
Copy link
Contributor

Choose a reason for hiding this comment

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

this accepts Title=foo", which doesn't seem to be valid. Also, Title=f should probably be valid. I think we may need to hold on here a bit and define more format parsing semantics for our markers (I've got some stuff in the works that I'll post soon).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could simply require string fields be wrapped in "" and anything inside will be interpreted literally.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've got parsing logic written. We should be consistent across our different annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Has it been merged?

Copy link
Contributor

Choose a reason for hiding this comment

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

not yet, it's part of a broader overhaul being tracked on the crdgenerator feature branch

title = title[1:]
}
if title[len(title)-1] == '"' {
title = title[:len(title)-1]
}
props.Title = strings.TrimSpace(title)
} else {
log.Fatalf(`'%s' is not a valid title`, title)
}
default:
log.Fatalf("Unsupport validation: %s", comment)
}
Expand Down
4 changes: 4 additions & 0 deletions testData/config/crds/fun_v1alpha1_toy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,18 @@ spec:
type: string
maxItems: 500
minItems: 1
title: A \"List\" "of" Knights
type: array
location:
additionalProperties:
type: string
description: This is a comment on a map field.
title: A Map
type: object
name:
maxLength: 15
minLength: 1
title: A primitive Name
type: string
power:
exclusiveMinimum: true
Expand Down Expand Up @@ -131,6 +134,7 @@ spec:
type: object
template:
description: This is a comment on an object field.
title: An Object
type: object
winner:
description: This is a comment on a boolean field.
Expand Down
4 changes: 4 additions & 0 deletions testData/pkg/apis/fun/v1alpha1/toy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ type ToySpec struct {

// +kubebuilder:validation:MaxLength=15
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:Title="A primitive Name"
Name string `json:"name,omitempty"`

// This is a comment on an array field.
// +kubebuilder:validation:Title="A \"List\" "of" Knights"
// +kubebuilder:validation:MaxItems=500
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:UniqueItems=false
Expand All @@ -67,6 +69,7 @@ type ToySpec struct {
Description string `json:"description"`

// This is a comment on an object field.
// +kubebuilder:validation:Title="An Object"
Template v1.PodTemplateSpec `json:"template"`

Claim v1.PersistentVolumeClaim `json:"claim,omitempty"`
Expand All @@ -83,6 +86,7 @@ type ToySpec struct {
S3Log ToyS3LogConfig `json:"s3Log"`

// This is a comment on a map field.
// +kubebuilder:validation:Title="A Map"
Location map[string]string `json:"location"`

// This is an IPv4 address.
Expand Down