From 3ddbc3fe79f9650eb4d058ff0ab184afca0dc1ef Mon Sep 17 00:00:00 2001 From: Alexander Zielenski <351783+alexzielenski@users.noreply.github.com> Date: Tue, 13 Feb 2024 14:54:44 -0800 Subject: [PATCH] feature: allow overriding comment tags through aliases Allows more advanced inheritences to be specified for when base Go struct types are shared. A transparent alias can be used to easily apply extra validations --- pkg/generators/markers.go | 108 ++++++++++++++++++++++++++++----- pkg/generators/markers_test.go | 63 +++++++++++++++++++ pkg/generators/openapi.go | 2 +- 3 files changed, 156 insertions(+), 17 deletions(-) diff --git a/pkg/generators/markers.go b/pkg/generators/markers.go index d51c4760c..5e18da183 100644 --- a/pkg/generators/markers.go +++ b/pkg/generators/markers.go @@ -240,26 +240,27 @@ func (c commentTags) ValidateType(t *types.Type) error { // Accepts a prefix to filter out markers not related to validation. // Returns any errors encountered while parsing or validating the comment tags. func ParseCommentTags(t *types.Type, comments []string, prefix string) (*spec.Schema, error) { - - markers, err := parseMarkers(comments, prefix) + // Parse the comment tags + commentTags, err := parseCommentTagsFromLines(comments, prefix) if err != nil { - return nil, fmt.Errorf("failed to parse marker comments: %w", err) - } - nested, err := nestMarkers(markers) - if err != nil { - return nil, fmt.Errorf("invalid marker comments: %w", err) + return nil, err } - // Parse the map into a CommentTags type by marshalling and unmarshalling - // as JSON in leiu of an unstructured converter. - out, err := json.Marshal(nested) - if err != nil { - return nil, fmt.Errorf("failed to marshal marker comments: %w", err) - } + // If t is an alias then parse each of the underlying types' comment tags + // and merge them into a single schema. Aliases closest to t should take + // precedence (e.g. minLength specified in the alias closest to t should override + // any minLength specified in an alias further away from t). + currentT := t + for currentT != nil { + aliasCommentTags, err := parseCommentTagsFromLines(currentT.CommentLines, prefix) + if err != nil { + return nil, err + } - var commentTags commentTags - if err = json.Unmarshal(out, &commentTags); err != nil { - return nil, fmt.Errorf("failed to unmarshal marker comments: %w", err) + mergeCommentTags(&commentTags, aliasCommentTags) + + // Move to the next type in the chain + currentT = currentT.Underlying } // Validate the parsed comment tags @@ -283,6 +284,81 @@ var ( valueRawString = regexp.MustCompile(fmt.Sprintf(`^(%s*)>(.*)$`, allowedKeyCharacterSet)) ) +func parseCommentTagsFromLines(comments []string, prefix string) (commentTags, error) { + if len(comments) == 0 { + return commentTags{}, nil + } + + markers, err := parseMarkers(comments, prefix) + if err != nil { + return commentTags{}, fmt.Errorf("failed to parse marker comments: %w", err) + } + nested, err := nestMarkers(markers) + if err != nil { + return commentTags{}, fmt.Errorf("invalid marker comments: %w", err) + } + + // Parse the map into a CommentTags type by marshalling and unmarshalling + // as JSON in leiu of an unstructured converter. + out, err := json.Marshal(nested) + if err != nil { + return commentTags{}, fmt.Errorf("failed to marshal marker comments: %w", err) + } + + var result commentTags + if err = json.Unmarshal(out, &result); err != nil { + return commentTags{}, fmt.Errorf("failed to unmarshal marker comments: %w", err) + } + return result, nil +} + +// Writes src values into dst if dst is nil, and src is non-nil +// Does not override anythng already set in dst +func mergeCommentTags(dst *commentTags, src commentTags) { + if src.MinLength != nil && dst.MinLength == nil { + dst.MinLength = src.MinLength + } + if src.MaxLength != nil && dst.MaxLength == nil { + dst.MaxLength = src.MaxLength + } + if src.MinItems != nil && dst.MinItems == nil { + dst.MinItems = src.MinItems + } + if src.MaxItems != nil && dst.MaxItems == nil { + dst.MaxItems = src.MaxItems + } + if src.MinProperties != nil && dst.MinProperties == nil { + dst.MinProperties = src.MinProperties + } + if src.MaxProperties != nil && dst.MaxProperties == nil { + dst.MaxProperties = src.MaxProperties + } + if src.Minimum != nil && dst.Minimum == nil { + dst.Minimum = src.Minimum + } + if src.Maximum != nil && dst.Maximum == nil { + dst.Maximum = src.Maximum + } + if src.MultipleOf != nil && dst.MultipleOf == nil { + dst.MultipleOf = src.MultipleOf + } + if src.Pattern != "" && dst.Pattern == "" { + dst.Pattern = src.Pattern + } + if src.ExclusiveMinimum && !dst.ExclusiveMinimum { + dst.ExclusiveMinimum = true + } + if src.ExclusiveMaximum && !dst.ExclusiveMaximum { + dst.ExclusiveMaximum = true + } + if src.UniqueItems && !dst.UniqueItems { + dst.UniqueItems = true + } + if len(src.CEL) > 0 { + dst.CEL = append(src.CEL, dst.CEL...) + } +} + // extractCommentTags parses comments for lines of the form: // // 'marker' + "key=value" diff --git a/pkg/generators/markers_test.go b/pkg/generators/markers_test.go index 284996ca2..d8dba0183 100644 --- a/pkg/generators/markers_test.go +++ b/pkg/generators/markers_test.go @@ -494,6 +494,69 @@ func TestParseCommentTags(t *testing.T) { }, expectedError: `failed to parse marker comments: concatenations to key 'cel[0]:message' must be consecutive with its assignment`, }, + { + name: "alias type without any comments", + t: &types.Type{Kind: types.Slice, Elem: &types.Type{Kind: types.Alias, Name: types.Name{Name: "PersistentVolumeAccessMode"}, Underlying: types.String}}, + comments: []string{ + `+k8s:validation:cel[0]:rule>!self.exists(c, c == "ReadWriteOncePod") || self.size() == 1`, + `+k8s:validation:cel[0]:message>may not use ReadWriteOncePod with other access modes`, + `+k8s:validation:cel[0]:reason>FieldForbidden`, + }, + expected: &spec.Schema{ + VendorExtensible: spec.VendorExtensible{ + Extensions: map[string]interface{}{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": `!self.exists(c, c == "ReadWriteOncePod") || self.size() == 1`, + "message": "may not use ReadWriteOncePod with other access modes", + "reason": "FieldForbidden", + }, + }, + }, + }, + }, + }, + { + name: "alias type with comments", + t: &types.Type{ + Kind: types.Alias, + Name: types.Name{Name: "PersistentVolumeAccessModeList"}, + CommentLines: []string{ + `+k8s:validation:cel[0]:rule>self.all(c, ["ReadWriteOncePod","ReadOnlyMany","ReadWriteMany","ReadWriteOnce"].contains(c))`, + `+k8s:validation:cel[0]:message>must follow enum`, + }, + Underlying: &types.Type{ + Kind: types.Slice, + Elem: &types.Type{ + Kind: types.Alias, + Name: types.Name{Name: "PersistentVolumeAccessMode"}, + Underlying: types.String, + }, + }, + }, + comments: []string{ + `+k8s:validation:cel[0]:rule>!self.exists(c, c == "ReadWriteOncePod") || self.size() == 1`, + `+k8s:validation:cel[0]:message>may not use ReadWriteOncePod with other access modes`, + `+k8s:validation:cel[0]:reason>FieldForbidden`, + }, + expected: &spec.Schema{ + VendorExtensible: spec.VendorExtensible{ + Extensions: map[string]interface{}{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": `self.all(c, ["ReadWriteOncePod","ReadOnlyMany","ReadWriteMany","ReadWriteOnce"].contains(c))`, + "message": "must follow enum", + }, + map[string]interface{}{ + "rule": `!self.exists(c, c == "ReadWriteOncePod") || self.size() == 1`, + "message": "may not use ReadWriteOncePod with other access modes", + "reason": "FieldForbidden", + }, + }, + }, + }, + }, + }, } for _, tc := range cases { diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index d5c19746c..93395eb5f 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -435,7 +435,7 @@ func (g openAPITypeWriter) generate(t *types.Type) error { // Only generate for struct type and ignore the rest switch t.Kind { case types.Struct: - validationSchema, err := ParseCommentTags(t, t.CommentLines, markerPrefix) + validationSchema, err := ParseCommentTags(t, nil, markerPrefix) if err != nil { return err }