From a54eb549b7e385b067bdedfca4f8dbca4e90e4aa Mon Sep 17 00:00:00 2001 From: Antoine Pelisse Date: Mon, 12 Jun 2023 14:28:29 -0700 Subject: [PATCH] generators: Make sure default value is a possible enum value --- pkg/generators/enum.go | 11 ++++++ pkg/generators/openapi.go | 34 ++++++++++++++----- pkg/generators/openapi_test.go | 17 +++------- .../pkg/generated/openapi_generated.go | 6 ++-- test/integration/testdata/enumtype/enum.go | 3 +- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/pkg/generators/enum.go b/pkg/generators/enum.go index 292a3c762..3b6777dc6 100644 --- a/pkg/generators/enum.go +++ b/pkg/generators/enum.go @@ -75,6 +75,17 @@ func (et *enumType) ValueStrings() []string { return values } +// IsValue returns true if the given value is one of the possible enum +// values. +func (et *enumType) IsValid(val string) bool { + for _, value := range et.Values { + if val == fmt.Sprintf("%q", value.Value) { + return true + } + } + return false +} + // DescriptionLines returns a description of the enum in this format: // // Possible enum values: diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 4654bbe9c..072293082 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -585,26 +585,27 @@ func mustEnforceDefault(t *types.Type, omitEmpty bool) (interface{}, error) { } } -func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool) error { +func (g openAPITypeWriter) generateDefault(comments []string, t *types.Type, omitEmpty bool) (*string, error) { t = resolveAliasAndEmbeddedType(t) def, err := defaultFromComments(comments) if err != nil { - return err + return nil, err } if enforced, err := mustEnforceDefault(t, omitEmpty); err != nil { - return err + return nil, err } else if enforced != nil { if def == nil { def = enforced } else if !reflect.DeepEqual(def, enforced) { enforcedJson, _ := json.Marshal(enforced) - return fmt.Errorf("invalid default value (%#v) for non-pointer/non-omitempty. If specified, must be: %v", def, string(enforcedJson)) + return nil, fmt.Errorf("invalid default value (%#v) for non-pointer/non-omitempty. If specified, must be: %v", def, string(enforcedJson)) } } if def != nil { - g.Do("Default: $.$,\n", fmt.Sprintf("%#v", def)) + val := fmt.Sprintf("%#v", def) + return &val, nil } - return nil + return nil, nil } func (g openAPITypeWriter) generateDescription(CommentLines []string) { @@ -676,9 +677,16 @@ func (g openAPITypeWriter) generateProperty(m *types.Member, parent *types.Type) return nil } omitEmpty := strings.Contains(reflect.StructTag(m.Tags).Get("json"), "omitempty") - if err := g.generateDefault(m.CommentLines, m.Type, omitEmpty); err != nil { + def, err := g.generateDefault(m.CommentLines, m.Type, omitEmpty) + if err != nil { return fmt.Errorf("failed to generate default in %v: %v: %v", parent, m.Name, err) } + if def != nil { + g.Do("Default: $.$,\n", *def) + if enumType, isEnum := g.enumContext.EnumType(m.Type); isEnum && !enumType.IsValid(*def) { + return fmt.Errorf("Default value is not a valid enum value: %v not in %v", *def, enumType.ValueStrings()) + } + } t := resolveAliasAndPtrType(m.Type) // If we can get a openAPI type and format for this type, we consider it to be simple property typeString, format := openapi.OpenAPITypeFormat(t.String()) @@ -762,9 +770,13 @@ func (g openAPITypeWriter) generateMapProperty(t *types.Type) error { g.Do("Type: []string{\"object\"},\n", nil) g.Do("AdditionalProperties: &spec.SchemaOrBool{\nAllows: true,\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil) - if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil { + def, err := g.generateDefault(t.Elem.CommentLines, t.Elem, false) + if err != nil { return err } + if def != nil { + g.Do("Default: $.$,\n", *def) + } typeString, format := openapi.OpenAPITypeFormat(elemType.String()) if typeString != "" { g.generateSimpleProperty(typeString, format) @@ -795,9 +807,13 @@ func (g openAPITypeWriter) generateSliceProperty(t *types.Type) error { elemType := resolveAliasAndPtrType(t.Elem) g.Do("Type: []string{\"array\"},\n", nil) g.Do("Items: &spec.SchemaOrArray{\nSchema: &spec.Schema{\nSchemaProps: spec.SchemaProps{\n", nil) - if err := g.generateDefault(t.Elem.CommentLines, t.Elem, false); err != nil { + def, err := g.generateDefault(t.Elem.CommentLines, t.Elem, false) + if err != nil { return err } + if def != nil { + g.Do("Default: $.$,\n", *def) + } typeString, format := openapi.OpenAPITypeFormat(elemType.String()) if typeString != "" { g.generateSimpleProperty(typeString, format) diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index ba86a17a9..871d66c23 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -1633,8 +1633,8 @@ const EnumB EnumType = "b" // +k8s:openapi-gen=x-kubernetes-type-tag:type_test type Blah struct { // Value is the value. - Value EnumType - NoCommentEnum EnumType + // +default="b" + Value *EnumType // +optional OptionalEnum *EnumType }`) @@ -1655,16 +1655,7 @@ Properties: map[string]spec.Schema{ "Value": { SchemaProps: spec.SchemaProps{ Description: "Value is the value.\n\nPossible enum values:\n - `+"`"+`\"a\"`+"`"+` is a.\n - `+"`"+`\"b\"`+"`"+` is b.", -Default: "", -Type: []string{"string"}, -Format: "", -Enum: []interface{}{"a", "b"}, -}, -}, -"NoCommentEnum": { -SchemaProps: spec.SchemaProps{ -Description: "Possible enum values:\n - `+"`"+`\"a\"`+"`"+` is a.\n - `+"`"+`\"b\"`+"`"+` is b.", -Default: "", +Default: "b", Type: []string{"string"}, Format: "", Enum: []interface{}{"a", "b"}, @@ -1679,7 +1670,7 @@ Enum: []interface{}{"a", "b"}, }, }, }, -Required: []string{"Value","NoCommentEnum"}, +Required: []string{"Value"}, }, VendorExtensible: spec.VendorExtensible{ Extensions: spec.Extensions{ diff --git a/test/integration/pkg/generated/openapi_generated.go b/test/integration/pkg/generated/openapi_generated.go index d9dc25cbf..f6c24513f 100644 --- a/test/integration/pkg/generated/openapi_generated.go +++ b/test/integration/pkg/generated/openapi_generated.go @@ -338,10 +338,10 @@ func schema_test_integration_testdata_enumtype_FruitsBasket(ref common.Reference "content": { SchemaProps: spec.SchemaProps{ Description: "Possible enum values:\n - `\"apple\"` is the Apple\n - `\"banana\"` is the Banana\n - `\"onigiri\"` is the Rice ball that does not seem to belong to a fruits basket but has a long comment that is so long that it spans multiple lines", - Default: "", Type: []string{"string"}, Format: "", - Enum: []interface{}{"apple", "banana", "onigiri"}}, + Enum: []interface{}{"apple", "banana", "onigiri"}, + }, }, "count": { SchemaProps: spec.SchemaProps{ @@ -351,7 +351,7 @@ func schema_test_integration_testdata_enumtype_FruitsBasket(ref common.Reference }, }, }, - Required: []string{"content", "count"}, + Required: []string{"count"}, }, }, } diff --git a/test/integration/testdata/enumtype/enum.go b/test/integration/testdata/enumtype/enum.go index c285919dd..f72a33047 100644 --- a/test/integration/testdata/enumtype/enum.go +++ b/test/integration/testdata/enumtype/enum.go @@ -18,7 +18,8 @@ const FruitRiceBall FruitType = "onigiri" // FruitsBasket is the type that contains the enum type. // +k8s:openapi-gen=true type FruitsBasket struct { - Content FruitType `json:"content"` + // +optional + Content FruitType `json:"content,omitempty"` // +default=0 Count int `json:"count"`