Skip to content

Commit a54eb54

Browse files
committed
generators: Make sure default value is a possible enum value
1 parent 3e8f62f commit a54eb54

File tree

5 files changed

+45
-26
lines changed

5 files changed

+45
-26
lines changed

Diff for: pkg/generators/enum.go

+11
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,17 @@ func (et *enumType) ValueStrings() []string {
7575
return values
7676
}
7777

78+
// IsValue returns true if the given value is one of the possible enum
79+
// values.
80+
func (et *enumType) IsValid(val string) bool {
81+
for _, value := range et.Values {
82+
if val == fmt.Sprintf("%q", value.Value) {
83+
return true
84+
}
85+
}
86+
return false
87+
}
88+
7889
// DescriptionLines returns a description of the enum in this format:
7990
//
8091
// Possible enum values:

Diff for: pkg/generators/openapi.go

+25-9
Original file line numberDiff line numberDiff line change
@@ -585,26 +585,27 @@ func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) {
585585
}
586586
}
587587

588-
func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool) error {
588+
func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool) (*string, error) {
589589
t = resolveAliasAndEmbeddedType(t)
590590
def, err := defaultFromComments(comments)
591591
if err != nil {
592-
return err
592+
return nil, err
593593
}
594594
if enforced, err := mustEnforceDefault(t, omitEmpty); err != nil {
595-
return err
595+
return nil, err
596596
} else if enforced != nil {
597597
if def == nil {
598598
def = enforced
599599
} else if !reflect.DeepEqual(def, enforced) {
600600
enforcedJson, _ := json.Marshal(enforced)
601-
return fmt.Errorf("invalid default value (%#v) for non-pointer/non-omitempty. If specified, must be: %v", def, string(enforcedJson))
601+
return nil, fmt.Errorf("invalid default value (%#v) for non-pointer/non-omitempty. If specified, must be: %v", def, string(enforcedJson))
602602
}
603603
}
604604
if def != nil {
605-
g.Do("Default: $.$,\n", fmt.Sprintf("%#v", def))
605+
val := fmt.Sprintf("%#v", def)
606+
return &val, nil
606607
}
607-
return nil
608+
return nil, nil
608609
}
609610

610611
func (g openAPITypeWriter) generateDescription(CommentLines []string) {
@@ -676,9 +677,16 @@ func (g openAPITypeWriter) generateProperty(m *types.Member, parent *types.Type)
676677
return nil
677678
}
678679
omitEmpty := strings.Contains(reflect.StructTag(m.Tags).Get("json"), "omitempty")
679-
if err := g.generateDefault(m.CommentLines, m.Type, omitEmpty); err != nil {
680+
def, err := g.generateDefault(m.CommentLines, m.Type, omitEmpty)
681+
if err != nil {
680682
return fmt.Errorf("failed to generate default in %v: %v: %v", parent, m.Name, err)
681683
}
684+
if def != nil {
685+
g.Do("Default: $.$,\n", *def)
686+
if enumType, isEnum := g.enumContext.EnumType(m.Type); isEnum && !enumType.IsValid(*def) {
687+
return fmt.Errorf("Default value is not a valid enum value: %v not in %v", *def, enumType.ValueStrings())
688+
}
689+
}
682690
t := resolveAliasAndPtrType(m.Type)
683691
// If we can get a openAPI type and format for this type, we consider it to be simple property
684692
typeString, format := openapi.OpenAPITypeFormat(t.String())
@@ -762,9 +770,13 @@ func (g openAPITypeWriter) generateMapProperty(t *types.Type) error {
762770

763771
g.Do("Type: []string{\"object\"},\n", nil)
764772
g.Do("AdditionalProperties: &spec.SchemaOrBool{\nAllows: true,\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil)
765-
if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil {
773+
def, err := g.generateDefault(t.Elem.CommentLines, t.Elem, false)
774+
if err != nil {
766775
return err
767776
}
777+
if def != nil {
778+
g.Do("Default: $.$,\n", *def)
779+
}
768780
typeString, format := openapi.OpenAPITypeFormat(elemType.String())
769781
if typeString != "" {
770782
g.generateSimpleProperty(typeString, format)
@@ -795,9 +807,13 @@ func (g openAPITypeWriter) generateSliceProperty(t *types.Type) error {
795807
elemType := resolveAliasAndPtrType(t.Elem)
796808
g.Do("Type: []string{\"array\"},\n", nil)
797809
g.Do("Items: &spec.SchemaOrArray{\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil)
798-
if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil {
810+
def, err := g.generateDefault(t.Elem.CommentLines, t.Elem, false)
811+
if err != nil {
799812
return err
800813
}
814+
if def != nil {
815+
g.Do("Default: $.$,\n", *def)
816+
}
801817
typeString, format := openapi.OpenAPITypeFormat(elemType.String())
802818
if typeString != "" {
803819
g.generateSimpleProperty(typeString, format)

Diff for: pkg/generators/openapi_test.go

+4-13
Original file line numberDiff line numberDiff line change
@@ -1633,8 +1633,8 @@ const EnumB EnumType = "b"
16331633
// +k8s:openapi-gen=x-kubernetes-type-tag:type_test
16341634
type Blah struct {
16351635
// Value is the value.
1636-
Value EnumType
1637-
NoCommentEnum EnumType
1636+
// +default="b"
1637+
Value *EnumType
16381638
// +optional
16391639
OptionalEnum *EnumType
16401640
}`)
@@ -1655,16 +1655,7 @@ Properties: map[string]spec.Schema{
16551655
"Value": {
16561656
SchemaProps: spec.SchemaProps{
16571657
Description: "Value is the value.\n\nPossible enum values:\n - `+"`"+`\"a\"`+"`"+` is a.\n - `+"`"+`\"b\"`+"`"+` is b.",
1658-
Default: "",
1659-
Type: []string{"string"},
1660-
Format: "",
1661-
Enum: []interface{}{"a", "b"},
1662-
},
1663-
},
1664-
"NoCommentEnum": {
1665-
SchemaProps: spec.SchemaProps{
1666-
Description: "Possible enum values:\n - `+"`"+`\"a\"`+"`"+` is a.\n - `+"`"+`\"b\"`+"`"+` is b.",
1667-
Default: "",
1658+
Default: "b",
16681659
Type: []string{"string"},
16691660
Format: "",
16701661
Enum: []interface{}{"a", "b"},
@@ -1679,7 +1670,7 @@ Enum: []interface{}{"a", "b"},
16791670
},
16801671
},
16811672
},
1682-
Required: []string{"Value","NoCommentEnum"},
1673+
Required: []string{"Value"},
16831674
},
16841675
VendorExtensible: spec.VendorExtensible{
16851676
Extensions: spec.Extensions{

Diff for: test/integration/pkg/generated/openapi_generated.go

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: test/integration/testdata/enumtype/enum.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ const FruitRiceBall FruitType = "onigiri"
1818
// FruitsBasket is the type that contains the enum type.
1919
// +k8s:openapi-gen=true
2020
type FruitsBasket struct {
21-
Content FruitType `json:"content"`
21+
// +optional
22+
Content FruitType `json:"content,omitempty"`
2223

2324
// +default=0
2425
Count int `json:"count"`

0 commit comments

Comments
 (0)