From 2b4e2b0ee917a9e210b2ac10104982765f32535e Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:46:20 -0700 Subject: [PATCH 1/3] bugfix: avoid non-deterministic output for extensions --- pkg/generators/openapi.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 745bac6b7..d6f699234 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -644,7 +644,15 @@ func (g openAPITypeWriter) emitExtensions(extensions []extension, unions []union } if len(otherExtensions) > 0 { - for k, v := range otherExtensions { + // Sort extension keys to generate deterministic output + keys := []string{} + for k := range otherExtensions { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + v := otherExtensions[k] g.Do("$.key$: $.value$,\n", map[string]interface{}{ "key": fmt.Sprintf("%#v", k), "value": fmt.Sprintf("%#v", v), From 631461ff3bc13d892136b98213da68dca5336318 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:49:46 -0700 Subject: [PATCH 2/3] Add support for adding validations for nested subschemas through marker comments adds powerful feature to marker comments to be able to add validations attributed to subschemas. Does not support removing validations from subschemas --- pkg/generators/markers.go | 187 +++++++++++++++++++++++++--- pkg/generators/markers_test.go | 219 ++++++++++++++++++++++++++++++++- pkg/generators/openapi.go | 105 ++++++++++++++++ pkg/generators/openapi_test.go | 183 ++++++++++++++++++++++++++- 4 files changed, 666 insertions(+), 28 deletions(-) diff --git a/pkg/generators/markers.go b/pkg/generators/markers.go index 7f0fe985a..c4dd67d3b 100644 --- a/pkg/generators/markers.go +++ b/pkg/generators/markers.go @@ -75,7 +75,29 @@ func (c *CELTag) Validate() error { // - +listMapKeys // - +mapType type commentTags struct { - spec.SchemaProps + Nullable *bool `json:"nullable,omitempty"` + Format *string `json:"format,omitempty"` + Maximum *float64 `json:"maximum,omitempty"` + ExclusiveMaximum *bool `json:"exclusiveMaximum,omitempty"` + Minimum *float64 `json:"minimum,omitempty"` + ExclusiveMinimum *bool `json:"exclusiveMinimum,omitempty"` + MaxLength *int64 `json:"maxLength,omitempty"` + MinLength *int64 `json:"minLength,omitempty"` + Pattern *string `json:"pattern,omitempty"` + MaxItems *int64 `json:"maxItems,omitempty"` + MinItems *int64 `json:"minItems,omitempty"` + UniqueItems *bool `json:"uniqueItems,omitempty"` + MultipleOf *float64 `json:"multipleOf,omitempty"` + Enum []interface{} `json:"enum,omitempty"` + MaxProperties *int64 `json:"maxProperties,omitempty"` + MinProperties *int64 `json:"minProperties,omitempty"` + + // Nested commentTags for extending the schemas of subfields at point-of-use + // when you cant annotate them directly. Cannot be used to add properties + // or remove validations on the overridden schema. + Items *commentTags `json:"items,omitempty"` + Properties map[string]*commentTags `json:"properties,omitempty"` + AdditionalProperties *commentTags `json:"additionalProperties,omitempty"` CEL []CELTag `json:"cel,omitempty"` @@ -86,9 +108,75 @@ type commentTags struct { // Returns the schema for the given CommentTags instance. // This is the final authoritative schema for the comment tags -func (c commentTags) ValidationSchema() (*spec.Schema, error) { +func (c *commentTags) ValidationSchema() (*spec.Schema, error) { + if c == nil { + return nil, nil + } + + isNullable := c.Nullable != nil && *c.Nullable + format := "" + if c.Format != nil { + format = *c.Format + } + isExclusiveMaximum := c.ExclusiveMaximum != nil && *c.ExclusiveMaximum + isExclusiveMinimum := c.ExclusiveMinimum != nil && *c.ExclusiveMinimum + isUniqueItems := c.UniqueItems != nil && *c.UniqueItems + pattern := "" + if c.Pattern != nil { + pattern = *c.Pattern + } + + var transformedItems *spec.SchemaOrArray + var transformedProperties map[string]spec.Schema + var transformedAdditionalProperties *spec.SchemaOrBool + + if c.Items != nil { + items, err := c.Items.ValidationSchema() + if err != nil { + return nil, fmt.Errorf("failed to transform items: %w", err) + } + transformedItems = &spec.SchemaOrArray{Schema: items} + } + + if c.Properties != nil { + properties := make(map[string]spec.Schema) + for key, value := range c.Properties { + property, err := value.ValidationSchema() + if err != nil { + return nil, fmt.Errorf("failed to transform property %q: %w", key, err) + } + properties[key] = *property + } + transformedProperties = properties + } + + if c.AdditionalProperties != nil { + additionalProperties, err := c.AdditionalProperties.ValidationSchema() + if err != nil { + return nil, fmt.Errorf("failed to transform additionalProperties: %w", err) + } + transformedAdditionalProperties = &spec.SchemaOrBool{Schema: additionalProperties, Allows: true} + } + res := spec.Schema{ - SchemaProps: c.SchemaProps, + SchemaProps: spec.SchemaProps{ + Nullable: isNullable, + Format: format, + Maximum: c.Maximum, + ExclusiveMaximum: isExclusiveMaximum, + Minimum: c.Minimum, + ExclusiveMinimum: isExclusiveMinimum, + MaxLength: c.MaxLength, + MinLength: c.MinLength, + Pattern: pattern, + MaxItems: c.MaxItems, + MinItems: c.MinItems, + UniqueItems: isUniqueItems, + MultipleOf: c.MultipleOf, + Enum: c.Enum, + MaxProperties: c.MaxProperties, + MinProperties: c.MinProperties, + }, } if len(c.CEL) > 0 { @@ -105,6 +193,18 @@ func (c commentTags) ValidationSchema() (*spec.Schema, error) { res.VendorExtensible.AddExtension("x-kubernetes-validations", celTagMap) } + // Dont add structural properties directly to this schema. This schema + // is used only for validation. + if transformedItems != nil || len(transformedProperties) > 0 || transformedAdditionalProperties != nil { + res.AllOf = append(res.AllOf, spec.Schema{ + SchemaProps: spec.SchemaProps{ + Items: transformedItems, + Properties: transformedProperties, + AdditionalProperties: transformedAdditionalProperties, + }, + }) + } + return &res, nil } @@ -134,7 +234,7 @@ func (c commentTags) Validate() error { if c.Minimum != nil && c.Maximum != nil && *c.Minimum > *c.Maximum { err = errors.Join(err, fmt.Errorf("minimum %f is greater than maximum %f", *c.Minimum, *c.Maximum)) } - if (c.ExclusiveMinimum || c.ExclusiveMaximum) && c.Minimum != nil && c.Maximum != nil && *c.Minimum == *c.Maximum { + if (c.ExclusiveMinimum != nil || c.ExclusiveMaximum != nil) && c.Minimum != nil && c.Maximum != nil && *c.Minimum == *c.Maximum { err = errors.Join(err, fmt.Errorf("exclusiveMinimum/Maximum cannot be set when minimum == maximum")) } if c.MinLength != nil && c.MaxLength != nil && *c.MinLength > *c.MaxLength { @@ -146,10 +246,10 @@ func (c commentTags) Validate() error { if c.MinProperties != nil && c.MaxProperties != nil && *c.MinProperties > *c.MaxProperties { err = errors.Join(err, fmt.Errorf("minProperties %d is greater than maxProperties %d", *c.MinProperties, *c.MaxProperties)) } - if c.Pattern != "" { - _, e := regexp.Compile(c.Pattern) + if c.Pattern != nil { + _, e := regexp.Compile(*c.Pattern) if e != nil { - err = errors.Join(err, fmt.Errorf("invalid pattern %q: %v", c.Pattern, e)) + err = errors.Join(err, fmt.Errorf("invalid pattern %q: %v", *c.Pattern, e)) } } if c.MultipleOf != nil && *c.MultipleOf == 0 { @@ -175,10 +275,23 @@ func (c commentTags) ValidateType(t *types.Type) error { typeString, _ := openapi.OpenAPITypeFormat(resolvedType.String()) // will be empty for complicated types // Structs and interfaces may dynamically be any type, so we cant validate them - // easily. We may be able to if we check that they don't implement all the - // override functions, but for now we just skip them. + // easily. if resolvedType.Kind == types.Interface || resolvedType.Kind == types.Struct { - return nil + // Skip validation for structs and interfaces which implement custom + // overrides + // + // Only check top-level t type without resolving alias to mirror generator + // behavior. Generator only checks the top level type without resolving + // alias. The `has*Method` functions can be changed to add this behavior in the + // future if needed. + elemT := resolvePtrType(t) + if hasOpenAPIDefinitionMethod(elemT) || + hasOpenAPIDefinitionMethods(elemT) || + hasOpenAPIV3DefinitionMethod(elemT) || + hasOpenAPIV3OneOfMethod(elemT) { + + return nil + } } isArray := resolvedType.Kind == types.Slice || resolvedType.Kind == types.Array @@ -186,6 +299,7 @@ func (c commentTags) ValidateType(t *types.Type) error { isString := typeString == "string" isInt := typeString == "integer" isFloat := typeString == "number" + isStruct := resolvedType.Kind == types.Struct if c.MaxItems != nil && !isArray { err = errors.Join(err, fmt.Errorf("maxItems can only be used on array types")) @@ -193,13 +307,13 @@ func (c commentTags) ValidateType(t *types.Type) error { if c.MinItems != nil && !isArray { err = errors.Join(err, fmt.Errorf("minItems can only be used on array types")) } - if c.UniqueItems && !isArray { + if c.UniqueItems != nil && !isArray { err = errors.Join(err, fmt.Errorf("uniqueItems can only be used on array types")) } - if c.MaxProperties != nil && !isMap { + if c.MaxProperties != nil && !(isMap || isStruct) { err = errors.Join(err, fmt.Errorf("maxProperties can only be used on map types")) } - if c.MinProperties != nil && !isMap { + if c.MinProperties != nil && !(isMap || isStruct) { err = errors.Join(err, fmt.Errorf("minProperties can only be used on map types")) } if c.MinLength != nil && !isString { @@ -208,7 +322,7 @@ func (c commentTags) ValidateType(t *types.Type) error { if c.MaxLength != nil && !isString { err = errors.Join(err, fmt.Errorf("maxLength can only be used on string types")) } - if c.Pattern != "" && !isString { + if c.Pattern != nil && !isString { err = errors.Join(err, fmt.Errorf("pattern can only be used on string types")) } if c.Minimum != nil && !isInt && !isFloat { @@ -220,16 +334,57 @@ func (c commentTags) ValidateType(t *types.Type) error { if c.MultipleOf != nil && !isInt && !isFloat { err = errors.Join(err, fmt.Errorf("multipleOf can only be used on numeric types")) } - if c.ExclusiveMinimum && !isInt && !isFloat { + if c.ExclusiveMinimum != nil && !isInt && !isFloat { err = errors.Join(err, fmt.Errorf("exclusiveMinimum can only be used on numeric types")) } - if c.ExclusiveMaximum && !isInt && !isFloat { + if c.ExclusiveMaximum != nil && !isInt && !isFloat { err = errors.Join(err, fmt.Errorf("exclusiveMaximum can only be used on numeric types")) } + if c.AdditionalProperties != nil && !isMap { + err = errors.Join(err, fmt.Errorf("additionalProperties can only be used on map types")) + + if err == nil { + err = errors.Join(err, c.AdditionalProperties.ValidateType(t)) + } + } + if c.Items != nil && !isArray { + err = errors.Join(err, fmt.Errorf("items can only be used on array types")) + + if err == nil { + err = errors.Join(err, c.Items.ValidateType(t)) + } + } + if c.Properties != nil { + if !isStruct && !isMap { + err = errors.Join(err, fmt.Errorf("properties can only be used on struct types")) + } else if isStruct && err == nil { + for key, tags := range c.Properties { + if member := memberWithJSONName(resolvedType, key); member == nil { + err = errors.Join(err, fmt.Errorf("property used in comment tag %q not found in struct %s", key, resolvedType.String())) + } else if nestedErr := tags.ValidateType(member.Type); nestedErr != nil { + err = errors.Join(err, fmt.Errorf("failed to validate property %q: %w", key, nestedErr)) + } + } + } + } return err } +func memberWithJSONName(t *types.Type, key string) *types.Member { + for _, member := range t.Members { + tags := getJsonTags(&member) + if len(tags) > 0 && tags[0] == key { + return &member + } else if member.Embedded { + if embeddedMember := memberWithJSONName(member.Type, key); embeddedMember != nil { + return embeddedMember + } + } + } + return nil +} + // Parses the given comments into a CommentTags type. Validates the parsed comment tags, and returns the result. // Accepts an optional type to validate against, and a prefix to filter out markers not related to validation. // Accepts a prefix to filter out markers not related to validation. diff --git a/pkg/generators/markers_test.go b/pkg/generators/markers_test.go index 36f0ff10d..28f6e551a 100644 --- a/pkg/generators/markers_test.go +++ b/pkg/generators/markers_test.go @@ -59,20 +59,88 @@ func TestParseCommentTags(t *testing.T) { "exclusiveMaximum=true", "not+k8s:validation:Minimum=0.0", }, + expectedError: `invalid marker comments: maxItems can only be used on array types +minItems can only be used on array types +uniqueItems can only be used on array types +minLength can only be used on string types +maxLength can only be used on string types +pattern can only be used on string types +minimum can only be used on numeric types +maximum can only be used on numeric types +multipleOf can only be used on numeric types`, + }, + { + t: arrayType, + name: "basic array example", + comments: []string{ + "comment", + "another + comment", + "+k8s:validation:minItems=1", + "+k8s:validation:maxItems=2", + "+k8s:validation:uniqueItems=true", + }, expected: &spec.Schema{ SchemaProps: spec.SchemaProps{ - Maximum: ptr.To(20.0), - Minimum: ptr.To(10.0), - MinLength: ptr.To[int64](20), - MaxLength: ptr.To[int64](30), - Pattern: "asdf", - MultipleOf: ptr.To(1.0), MinItems: ptr.To[int64](1), MaxItems: ptr.To[int64](2), UniqueItems: true, }, }, }, + { + t: mapType, + name: "basic map example", + comments: []string{ + "comment", + "another + comment", + "+k8s:validation:minProperties=1", + "+k8s:validation:maxProperties=2", + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + MinProperties: ptr.To[int64](1), + MaxProperties: ptr.To[int64](2), + }, + }, + }, + { + t: types.String, + name: "basic string example", + comments: []string{ + "comment", + "another + comment", + "+k8s:validation:minLength=20", + "+k8s:validation:maxLength=30", + `+k8s:validation:pattern="asdf"`, + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + MinLength: ptr.To[int64](20), + MaxLength: ptr.To[int64](30), + Pattern: "asdf", + }, + }, + }, + { + t: types.Int, + name: "basic int example", + comments: []string{ + "comment", + "another + comment", + "+k8s:validation:minimum=10.0", + "+k8s:validation:maximum=20.0", + "+k8s:validation:multipleOf=1.0", + "exclusiveMaximum=true", + "not+k8s:validation:Minimum=0.0", + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Maximum: ptr.To(20.0), + Minimum: ptr.To(10.0), + MultipleOf: ptr.To(1.0), + }, + }, + }, { t: structKind, name: "empty", @@ -494,6 +562,44 @@ func TestParseCommentTags(t *testing.T) { }, expectedError: `failed to parse marker comments: concatenations to key 'cel[0]:message' must be consecutive with its assignment`, }, + { + name: "nested cel", + comments: []string{ + `+k8s:validation:items:cel[0]:rule="self.length() % 2 == 0"`, + `+k8s:validation:items:cel[0]:message="must be even"`, + }, + t: &types.Type{ + Kind: types.Alias, + Underlying: &types.Type{ + Kind: types.Slice, + Elem: types.String, + }, + }, + expected: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + VendorExtensible: spec.VendorExtensible{ + Extensions: map[string]interface{}{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": "self.length() % 2 == 0", + "message": "must be even", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, } for _, tc := range cases { @@ -753,6 +859,107 @@ func TestCommentTags_Validate(t *testing.T) { t: types.String, errorMessage: "", }, + { + name: "additionalProperties on non-map", + comments: []string{ + `+k8s:validation:additionalProperties:pattern=".*"`, + }, + t: types.String, + errorMessage: "additionalProperties can only be used on map types", + }, + { + name: "properties on non-struct", + comments: []string{ + `+k8s:validation:properties:name:pattern=".*"`, + }, + t: types.String, + errorMessage: "properties can only be used on struct types", + }, + { + name: "items on non-array", + comments: []string{ + `+k8s:validation:items:pattern=".*"`, + }, + t: types.String, + errorMessage: "items can only be used on array types", + }, + { + name: "property missing from struct", + comments: []string{ + `+k8s:validation:properties:name:pattern=".*"`, + }, + t: &types.Type{ + Kind: types.Struct, + Name: types.Name{Name: "struct"}, + Members: []types.Member{ + { + Name: "notname", + Type: types.String, + Tags: `json:"notname"`, + }, + }, + }, + errorMessage: `property used in comment tag "name" not found in struct struct`, + }, + { + name: "nested comments also type checked", + comments: []string{ + `+k8s:validation:properties:name:items:pattern=".*"`, + }, + t: &types.Type{ + Kind: types.Struct, + Name: types.Name{Name: "struct"}, + Members: []types.Member{ + { + Name: "name", + Type: types.String, + Tags: `json:"name"`, + }, + }, + }, + errorMessage: `failed to validate property "name": items can only be used on array types`, + }, + { + name: "nested comments also type checked - passing", + comments: []string{ + `+k8s:validation:properties:name:pattern=".*"`, + }, + t: &types.Type{ + Kind: types.Struct, + Name: types.Name{Name: "struct"}, + Members: []types.Member{ + { + Name: "name", + Type: types.String, + Tags: `json:"name"`, + }, + }, + }, + }, + { + name: "nested marker type checking through alias", + comments: []string{ + `+k8s:validation:properties:name:pattern=".*"`, + }, + t: &types.Type{ + Kind: types.Struct, + Name: types.Name{Name: "struct"}, + Members: []types.Member{ + { + Name: "name", + Tags: `json:"name"`, + Type: &types.Type{ + Kind: types.Alias, + Underlying: &types.Type{ + Kind: types.Slice, + Elem: types.String, + }, + }, + }, + }, + }, + errorMessage: `failed to validate property "name": pattern can only be used on string types`, + }, } for _, tc := range testCases { diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index d6f699234..95a08560c 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -362,6 +362,88 @@ func (g openAPITypeWriter) generateCall(t *types.Type) error { return g.Error() } +// Generates Go code to represent an OpenAPI schema. May be refactored in +// the future to take more responsibility as we transition from an on-line +// approach to parsing the comments to spec.Schema +func (g openAPITypeWriter) generateSchema(s *spec.Schema) error { + if !reflect.DeepEqual(s.SchemaProps, spec.SchemaProps{}) { + g.Do("SchemaProps: spec.SchemaProps{\n", nil) + err := g.generateValueValidations(&s.SchemaProps) + if err != nil { + return err + } + + if len(s.Properties) > 0 { + g.Do("Properties: map[string]spec.Schema{\n", nil) + + // Sort property names to generate deterministic output + keys := []string{} + for k := range s.Properties { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + v := s.Properties[k] + g.Do("$.$: {\n", fmt.Sprintf("%#v", k)) + err := g.generateSchema(&v) + if err != nil { + return err + } + g.Do("},\n", nil) + } + g.Do("},\n", nil) + } + + if s.AdditionalProperties != nil && s.AdditionalProperties.Schema != nil { + g.Do("AdditionalProperties: &spec.SchemaOrBool{\n", nil) + g.Do("Allows: true,\n", nil) + g.Do("Schema: &spec.Schema{\n", nil) + err := g.generateSchema(s.AdditionalProperties.Schema) + if err != nil { + return err + } + g.Do("},\n", nil) + g.Do("},\n", nil) + } + + if s.Items != nil && s.Items.Schema != nil { + g.Do("Items: &spec.SchemaOrArray{\n", nil) + g.Do("Schema: &spec.Schema{\n", nil) + err := g.generateSchema(s.Items.Schema) + if err != nil { + return err + } + g.Do("},\n", nil) + g.Do("},\n", nil) + } + + g.Do("},\n", nil) + } + + if len(s.Extensions) > 0 { + g.Do("VendorExtensible: spec.VendorExtensible{\nExtensions: spec.Extensions{\n", nil) + + // Sort extension keys to generate deterministic output + keys := []string{} + for k := range s.Extensions { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + v := s.Extensions[k] + g.Do("$.key$: $.value$,\n", map[string]interface{}{ + "key": fmt.Sprintf("%#v", k), + "value": fmt.Sprintf("%#v", v), + }) + } + g.Do("},\n},\n", nil) + } + + return nil +} + func (g openAPITypeWriter) generateValueValidations(vs *spec.SchemaProps) error { if vs == nil { @@ -420,6 +502,18 @@ func (g openAPITypeWriter) generateValueValidations(vs *spec.SchemaProps) error g.Do("UniqueItems: true,\n", nil) } + if len(vs.AllOf) > 0 { + g.Do("AllOf: []spec.Schema{\n", nil) + for _, s := range vs.AllOf { + g.Do("{\n", nil) + if err := g.generateSchema(&s); err != nil { + return err + } + g.Do("},\n", nil) + } + g.Do("},\n", nil) + } + return nil } @@ -942,6 +1036,17 @@ func (g openAPITypeWriter) generateReferenceProperty(t *types.Type) { g.Do("Ref: ref(\"$.$\"),\n", t.Name.String()) } +func resolvePtrType(t *types.Type) *types.Type { + var prev *types.Type + for prev != t { + prev = t + if t.Kind == types.Pointer { + t = t.Elem + } + } + return t +} + func resolveAliasAndPtrType(t *types.Type) *types.Type { var prev *types.Type for prev != t { diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index 3fa753d14..84eced7f7 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -2360,8 +2360,6 @@ func TestMarkerComments(t *testing.T) { // +k8s:openapi-gen=true // +k8s:validation:maxProperties=10 // +k8s:validation:minProperties=1 - // +k8s:validation:exclusiveMinimum - // +k8s:validation:exclusiveMaximum type Blah struct { // Integer with min and max values @@ -2421,10 +2419,8 @@ func TestMarkerComments(t *testing.T) { Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ Type: []string{"object"}, - ExclusiveMinimum: true, - ExclusiveMaximum: true, - MinProperties: ptr.To[int64](1), - MaxProperties: ptr.To[int64](10), + MinProperties: ptr.To[int64](1), + MaxProperties: ptr.To[int64](10), Properties: map[string]spec.Schema{ "IntValue": { SchemaProps: spec.SchemaProps{ @@ -2919,3 +2915,178 @@ MaxLength: ptr.To[int64](10), }`) }) } + +func TestNestedMarkers(t *testing.T) { + inputFile := ` + package foo + + // +k8s:openapi-gen=true + // +k8s:validation:properties:field:items:maxLength=10 + // +k8s:validation:properties:aliasMap:additionalProperties:pattern>^foo$ + type Blah struct { + // +k8s:validation:items:cel[0]:rule="self.length() % 2 == 0" + Field MyAlias ` + "`json:\"field,omitempty\"`" + ` + + // +k8s:validation:additionalProperties:maxLength=10 + AliasMap MyAliasMap ` + "`json:\"aliasMap,omitempty\"`" + ` + } + + type MyAliasMap map[string]MyAlias + type MyAlias []string` + + packagestest.TestAll(t, func(t *testing.T, x packagestest.Exporter) { + e := packagestest.Export(t, x, []packagestest.Module{{ + Name: "example.com/base/foo", + Files: map[string]interface{}{ + "foo.go": inputFile, + }, + }}) + defer e.Cleanup() + + callErr, funcErr, _, funcBuffer, imports := testOpenAPITypeWriter(t, e.Config) + if funcErr != nil { + t.Fatalf("Unexpected funcErr: %v", funcErr) + } + if callErr != nil { + t.Fatalf("Unexpected callErr: %v", callErr) + } + expImports := []string{ + `foo "example.com/base/foo"`, + `common "k8s.io/kube-openapi/pkg/common"`, + `spec "k8s.io/kube-openapi/pkg/validation/spec"`, + `ptr "k8s.io/utils/ptr"`, + } + if !cmp.Equal(imports, expImports) { + t.Errorf("wrong imports:\n%s", cmp.Diff(expImports, imports)) + } + + if formatted, err := format.Source(funcBuffer.Bytes()); err != nil { + t.Fatalf("%v\n%v", err, funcBuffer.String()) + } else { + formatted_expected, ree := format.Source([]byte(`func schema_examplecom_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + Properties: map[string]spec.Schema{ + "aliasMap": { + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Pattern: "^foo$", + }, + }, + }, + }, + }, + }, + }, + }, + "field": { + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + MaxLength: ptr.To[int64](10), + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + Properties: map[string]spec.Schema{ + "field": { + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-validations": []interface{}{map[string]interface{}{"rule": "self.length() % 2 == 0"}}, + }, + }, + }, + }, + }, + }, + }, + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + "aliasMap": { + SchemaProps: spec.SchemaProps{ + AllOf: []spec.Schema{ + { + SchemaProps: spec.SchemaProps{ + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + MaxLength: ptr.To[int64](10), + }, + }, + }, + }, + }, + }, + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + }`)) + if ree != nil { + t.Fatal(ree) + } + assertEqual(t, string(formatted), string(formatted_expected)) + } + }) + +} From 47d23e80961a8052717cee7dd9a5d8e0bd8b6e88 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Mon, 29 Apr 2024 14:19:25 -0700 Subject: [PATCH 3/3] add context to error message for comment tags --- pkg/generators/openapi.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 95a08560c..9ffbd5d91 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -523,7 +523,7 @@ func (g openAPITypeWriter) generate(t *types.Type) error { case types.Struct: validationSchema, err := ParseCommentTags(t, t.CommentLines, markerPrefix) if err != nil { - return err + return fmt.Errorf("failed parsing comment tags for %v: %w", t.String(), err) } hasV2Definition := hasOpenAPIDefinitionMethod(t)