diff --git a/pkg/generators/markers.go b/pkg/generators/markers.go index 75acab94e..2236ea13d 100644 --- a/pkg/generators/markers.go +++ b/pkg/generators/markers.go @@ -231,66 +231,171 @@ func ParseCommentTags(t *types.Type, comments []string, prefix string) (CommentT return commentTags, nil } -// lintArrayIndices attempts to prevent: -// 1. Skipped indices -// 2. Non-consecutive indices -func lintArrayIndices(markerComments []string, prefix string) error { +var ( + allowedKeyCharacterSet = `[:_a-zA-Z0-9\[\]\-]` + valueEmpty = regexp.MustCompile(fmt.Sprintf(`^(%s*)$`, allowedKeyCharacterSet)) + valueAssign = regexp.MustCompile(fmt.Sprintf(`^(%s*)=(.*)$`, allowedKeyCharacterSet)) + valueRawString = regexp.MustCompile(fmt.Sprintf(`^(%s*)>(.*)$`, allowedKeyCharacterSet)) +) + +// extractCommentTags parses comments for lines of the form: +// +// 'marker' + "key=value" +// +// or to specify truthy boolean keys: +// +// 'marker' + "key" +// +// Values are optional; "" is the default. A tag can be specified more than +// one time and all values are returned. Returns a map with an entry for +// for each key and a value. +// +// Similar to version from gengo, but this version support only allows one +// value per key (preferring explicit array indices), supports raw strings +// with concatenation, and limits the usable characters allowed in a key +// (for simpler parsing). +// +// Assignments and empty values have the same syntax as from gengo. Raw strings +// have the syntax: +// +// 'marker' + "key>value" +// 'marker' + "key>value" +// +// Successive usages of the same raw string key results in concatenating each +// line with `\n` in between. It is an error to use `=` to assing to a previously +// assigned key +// (in contrast to types.ExtractCommentTags which allows array-typed +// values to be specified using `=`). +func extractCommentTags(marker string, lines []string) (map[string]string, error) { + out := map[string]string{} + + // Used to track the the line immediately prior to the one being iterated. + // If there was an invalid or ignored line, these values get reset. + lastKey := "" + lastIndex := -1 + lastArrayKey := "" + var lintErrors []error - prevLine := "" - prevIndex := -1 - for _, line := range markerComments { + + for _, line := range lines { line = strings.Trim(line, " ") - // If there was a non-prefixed marker inserted in between, then - // reset the previous line and index. - if !strings.HasPrefix(line, "+"+prefix) { - prevLine = "" - prevIndex = -1 - continue - } + // Track the current value of the last vars to use in this loop iteration + // before they are reset for the next iteration. + previousKey := lastKey + previousArrayKey := lastArrayKey + previousIndex := lastIndex - // There will always be at least the first element in the split. - // If there is no = sign, we want to use the whole line as the key in the - // case of boolean marker comments. - key := strings.Split(line, "=")[0] - subscriptIdx := strings.Index(key, "[") - if subscriptIdx == -1 { - prevLine = "" - prevIndex = -1 + // Make sure last vars gets reset if we `continue` + lastIndex = -1 + lastArrayKey = "" + lastKey = "" + + if len(line) == 0 { + continue + } else if !strings.HasPrefix(line, marker) { continue } - arrayPath := key[:subscriptIdx] - subscript := strings.Split(key[subscriptIdx+1:], "]")[0] + line = strings.TrimPrefix(line, marker) + + key := "" + value := "" + + if matches := valueAssign.FindStringSubmatch(line); matches != nil { + key = matches[1] + value = matches[2] + + // If key exists, throw error. + // Some of the old kube open-api gen marker comments like + // `+listMapKeys` allowed a list to be specified by writing key=value + // multiple times. + // + // This is not longer supported for the prefixed marker comments. + // This is to prevent confusion with the new array syntax which + // supports lists of objects. + // + // The old marker comments like +listMapKeys will remain functional, + // but new markers will not support it. + if _, ok := out[key]; ok { + return nil, fmt.Errorf("cannot have multiple values for key '%v'", key) + } - if len(subscript) == 0 { - lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: empty subscript not allowed", line)) - prevLine = "" - prevIndex = -1 - continue - } + } else if matches := valueEmpty.FindStringSubmatch(line); matches != nil { + key = matches[1] + value = "" + + } else if matches := valueRawString.FindStringSubmatch(line); matches != nil { + toAdd := strings.Trim(string(matches[2]), " ") - index, err := strconv.Atoi(subscript) + key = matches[1] - // If index is non-zero, check that that previous line was for the same - // key and either the same or previous index - if err != nil { - lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: expected integer index in key '%v'", line, line[:subscriptIdx])) - } else if prevLine != arrayPath && index != 0 { - lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: non-consecutive index %v for key '%v'", line, index, arrayPath)) - } else if index != prevIndex+1 && index != prevIndex { - lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: non-consecutive index %v for key '%v'", line, index, arrayPath)) + // First usage as a raw string. + if existing, exists := out[key]; !exists { + + // Encode the raw string as JSON to ensure that it is properly escaped. + valueBytes, err := json.Marshal(toAdd) + if err != nil { + return nil, fmt.Errorf("invalid value for key %v: %w", key, err) + } + + value = string(valueBytes) + } else if key != previousKey { + // Successive usages of the same key of a raw string must be + // consecutive + return nil, fmt.Errorf("concatenations to key '%s' must be consecutive with its assignment", key) + } else { + // If it is a consecutive repeat usage, concatenate to the + // existing value. + // + // Decode JSON string, append to it, re-encode JSON string. + // Kinda janky but this is a code-generator... + var unmarshalled string + if err := json.Unmarshal([]byte(existing), &unmarshalled); err != nil { + return nil, fmt.Errorf("invalid value for key %v: %w", key, err) + } else { + unmarshalled += "\n" + toAdd + valueBytes, err := json.Marshal(unmarshalled) + if err != nil { + return nil, fmt.Errorf("invalid value for key %v: %w", key, err) + } + + value = string(valueBytes) + } + } + } else { + // Comment has the correct prefix, but incorrect syntax, so it is an + // error + return nil, fmt.Errorf("invalid marker comment does not match expected `+key=` pattern: %v", line) } - prevLine = arrayPath - prevIndex = index + out[key] = value + lastKey = key + + // Lint the array subscript for common mistakes. This only lints the last + // array index used, (since we do not have a need for nested arrays yet + // in markers) + if arrayPath, index, hasSubscript, err := extractArraySubscript(key); hasSubscript { + // If index is non-zero, check that that previous line was for the same + // key and either the same or previous index + if err != nil { + lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: expected integer index in key '%v'", line, key)) + } else if previousArrayKey != arrayPath && index != 0 { + lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: non-consecutive index %v for key '%v'", line, index, arrayPath)) + } else if index != previousIndex+1 && index != previousIndex { + lintErrors = append(lintErrors, fmt.Errorf("error parsing %v: non-consecutive index %v for key '%v'", line, index, arrayPath)) + } + + lastIndex = index + lastArrayKey = arrayPath + } } if len(lintErrors) > 0 { - return errors.Join(lintErrors...) + return nil, errors.Join(lintErrors...) } - return nil + return out, nil } // Extracts and parses the given marker comments into a map of key -> value. @@ -298,52 +403,30 @@ func lintArrayIndices(markerComments []string, prefix string) error { // The prefix is removed from the key in the returned map. // Empty keys and invalid values will return errors, refs are currently unsupported and will be skipped. func parseMarkers(markerComments []string, prefix string) (map[string]any, error) { - if err := lintArrayIndices(markerComments, prefix); err != nil { + markers, err := extractCommentTags(prefix, markerComments) + if err != nil { return nil, err } - markers := types.ExtractCommentTags("+"+prefix, markerComments) - // Parse the values as JSON result := map[string]any{} for key, value := range markers { - // Skip ref markers - if len(value) == 1 { - _, ok := defaultergen.ParseSymbolReference(value[0], "") - if ok { - continue - } - } + var unmarshalled interface{} + if len(key) == 0 { return nil, fmt.Errorf("cannot have empty key for marker comment") - } else if len(value) == 0 || (len(value) == 1 && len(value[0]) == 0) { + } else if _, ok := defaultergen.ParseSymbolReference(value, ""); ok { + // Skip ref markers + continue + } else if len(value) == 0 { // Empty value means key is implicitly a bool result[key] = true - continue - } - - newVal := []any{} - for _, v := range value { - var unmarshalled interface{} - err := json.Unmarshal([]byte(v), &unmarshalled) - if err != nil { - // If not valid JSON, pass the raw string instead to allow - // implicit raw strings without quotes or escaping. This enables - // CEL rules to not have to worry about escaping their code for - // JSON strings. - // - // When interpreting the result of this function as a CommentTags, - // the value will be properly type checked. - newVal = append(newVal, v) - } else { - newVal = append(newVal, unmarshalled) - } - } - - if len(newVal) == 1 { - result[key] = newVal[0] + } else if err := json.Unmarshal([]byte(value), &unmarshalled); err != nil { + // Not valid JSON, throw error + return nil, fmt.Errorf("failed to parse value for key %v as JSON: %w", key, err) } else { - return nil, fmt.Errorf("cannot have multiple values for key '%v'", key) + // Is is valid JSON, use as a JSON value + result[key] = unmarshalled } } return result, nil @@ -376,8 +459,8 @@ func nestMarkers(markers map[string]any) (map[string]any, error) { for key, value := range markers { var err error keys := strings.Split(key, ":") - nested, err = putNestedValue(nested, keys, value) - if err != nil { + + if err = putNestedValue(nested, keys, value); err != nil { errs = append(errs, err) } } @@ -392,34 +475,26 @@ func nestMarkers(markers map[string]any) (map[string]any, error) { // Recursively puts a value into the given keypath, creating intermediate maps // and slices as needed. If a key is of the form `foo[bar]`, then bar will be // treated as an index into the array foo. If bar is not a valid integer, putNestedValue returns an error. -func putNestedValue(m map[string]any, k []string, v any) (map[string]any, error) { +func putNestedValue(m map[string]any, k []string, v any) error { if len(k) == 0 { - return m, nil + return nil } key := k[0] rest := k[1:] - if idxIdx := strings.Index(key, "["); idxIdx > -1 { - subscript := strings.Split(key[idxIdx+1:], "]")[0] - if len(subscript) == 0 { - return nil, fmt.Errorf("empty subscript not allowed") - } - - key := key[:idxIdx] - index, err := strconv.Atoi(subscript) - if err != nil { - // Ignore key - return nil, fmt.Errorf("expected integer index in key %v", key) - } - + // Array case + if arrayKeyWithoutSubscript, index, hasSubscript, err := extractArraySubscript(key); err != nil { + return fmt.Errorf("error parsing subscript for key %v: %w", key, err) + } else if hasSubscript { + key = arrayKeyWithoutSubscript var arrayDestination []any if existing, ok := m[key]; !ok { arrayDestination = make([]any, index+1) } else if existing, ok := existing.([]any); !ok { // Error case. Existing isn't of correct type. Can happen if // someone is subscripting a field that was previously not an array - return nil, fmt.Errorf("expected []any at key %v, got %T", key, existing) + return fmt.Errorf("expected []any at key %v, got %T", key, existing) } else if index >= len(existing) { // Ensure array is big enough arrayDestination = append(existing, make([]any, index-len(existing)+1)...) @@ -433,25 +508,26 @@ func putNestedValue(m map[string]any, k []string, v any) (map[string]any, error) // Assumes the destination is a map for now. Theoretically could be // extended to support arrays of arrays, but that's not needed yet. destination := make(map[string]any) - if arrayDestination[index], err = putNestedValue(destination, rest, v); err != nil { - return nil, err + arrayDestination[index] = destination + if err = putNestedValue(destination, rest, v); err != nil { + return err } } else if dst, ok := arrayDestination[index].(map[string]any); ok { // Already exists case, correct type - if arrayDestination[index], err = putNestedValue(dst, rest, v); err != nil { - return nil, err + if putNestedValue(dst, rest, v); err != nil { + return err } } else { // Already exists, incorrect type. Error // This shouldn't be possible. - return nil, fmt.Errorf("expected map at %v[%v], got %T", key, index, arrayDestination[index]) + return fmt.Errorf("expected map at %v[%v], got %T", key, index, arrayDestination[index]) } - return m, nil + return nil } else if len(rest) == 0 { // Base case. Single key. Just set into destination m[key] = v - return m, nil + return nil } if existing, ok := m[key]; !ok { @@ -463,6 +539,35 @@ func putNestedValue(m map[string]any, k []string, v any) (map[string]any, error) } else { // Error case. Existing isn't of correct type. Can happen if prior comment // referred to value as an error - return nil, fmt.Errorf("expected map[string]any at key %v, got %T", key, existing) + return fmt.Errorf("expected map[string]any at key %v, got %T", key, existing) } } + +// extractArraySubscript extracts the left array subscript from a key of +// the form `foo[bar][baz]` -> "bar". +// Returns the key without the subscript, the index, and a bool indicating if +// the key had a subscript. +// If the key has a subscript, but the subscript is not a valid integer, returns an error. +// +// This can be adapted to support multidimensional subscripts probably fairly +// easily by retuning a list of ints +func extractArraySubscript(str string) (string, int, bool, error) { + subscriptIdx := strings.Index(str, "[") + if subscriptIdx == -1 { + return "", -1, false, nil + } + + subscript := strings.Split(str[subscriptIdx+1:], "]")[0] + if len(subscript) == 0 { + return "", -1, false, fmt.Errorf("empty subscript not allowed") + } + + index, err := strconv.Atoi(subscript) + if err != nil { + return "", -1, false, fmt.Errorf("expected integer index in key %v", str) + } else if index < 0 { + return "", -1, false, fmt.Errorf("subscript '%v' is invalid. index must be positive", subscript) + } + + return str[:subscriptIdx], index, true, nil +} diff --git a/pkg/generators/markers_test.go b/pkg/generators/markers_test.go index de9ab979c..1b7260b39 100644 --- a/pkg/generators/markers_test.go +++ b/pkg/generators/markers_test.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, @@ -13,7 +13,6 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ - package generators_test import ( @@ -123,16 +122,24 @@ func TestParseCommentTags(t *testing.T) { }, { t: types.Float64, - name: "invalid: invalid value", + name: "invalid: non-JSON value", + comments: []string{ + `+k8s:validation:minimum=asdf`, + }, + expectedError: `failed to parse marker comments: failed to parse value for key minimum as JSON: invalid character 'a' looking for beginning of value`, + }, + { + t: types.Float64, + name: "invalid: invalid value type", comments: []string{ - "+k8s:validation:minimum=asdf", + `+k8s:validation:minimum="asdf"`, }, expectedError: `failed to unmarshal marker comments: json: cannot unmarshal string into Go struct field CommentTags.minimum of type float64`, }, { t: structKind, - name: "invalid: invalid value", + name: "invalid: empty key", comments: []string{ "+k8s:validation:", }, @@ -173,7 +180,7 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[2]:rule="self > 5"`, `+k8s:validation:cel[2]:message="must be greater than 5"`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[2]:rule="self > 5": non-consecutive index 2 for key '+k8s:validation:cel'`, + expectedError: `failed to parse marker comments: error parsing cel[2]:rule="self > 5": non-consecutive index 2 for key 'cel'`, }, { t: types.Float64, @@ -236,7 +243,7 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[2]:optionalOldSelf=true`, `+k8s:validation:cel[2]:message="must be greater than 5"`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[2]:rule="self > 5": non-consecutive index 2 for key '+k8s:validation:cel'`, + expectedError: `failed to parse marker comments: error parsing cel[2]:rule="self > 5": non-consecutive index 2 for key 'cel'`, }, { t: types.Float64, @@ -248,7 +255,7 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[0]:optionalOldSelf=true`, `+k8s:validation:cel[0]:message="must be greater than 5"`, }, - expectedError: "failed to parse marker comments: error parsing +k8s:validation:cel[0]:optionalOldSelf=true: non-consecutive index 0 for key '+k8s:validation:cel'", + expectedError: "failed to parse marker comments: error parsing cel[0]:optionalOldSelf=true: non-consecutive index 0 for key 'cel'", }, { t: types.Float64, @@ -262,7 +269,7 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[2]:rule="a rule"`, `+k8s:validation:cel[2]:message="message 2"`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[2]:rule="a rule": non-consecutive index 2 for key '+k8s:validation:cel'`, + expectedError: "failed to parse marker comments: error parsing cel[2]:rule=\"a rule\": non-consecutive index 2 for key 'cel'", }, { t: types.Float64, @@ -276,7 +283,58 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[2]:rule="a rule"`, `+k8s:validation:cel[2]:message="message 2"`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[2]:rule="a rule": non-consecutive index 2 for key '+k8s:validation:cel'`, + expectedError: "failed to parse marker comments: error parsing cel[2]:rule=\"a rule\": non-consecutive index 2 for key 'cel'", + }, + { + t: types.Float64, + name: "non-consecutive raw string indexing", + comments: []string{ + `+k8s:validation:cel[0]:rule> raw string rule`, + `+k8s:validation:cel[1]:rule="self > 5"`, + `+k8s:validation:cel[1]:message="must be greater than 5"`, + `+k8s:validation:cel[0]:message>another raw string message`, + }, + expectedError: "failed to parse marker comments: error parsing cel[0]:message>another raw string message: non-consecutive index 0 for key 'cel'", + }, + { + t: types.String, + name: "non-consecutive string indexing false positive", + comments: []string{ + `+k8s:validation:cel[0]:message>[3]string rule [1]`, + `+k8s:validation:cel[0]:rule="string rule [1]"`, + `+k8s:validation:pattern="self[3] == 'hi'"`, + }, + expected: generators.CommentTags{ + CEL: []generators.CELTag{ + { + Rule: "string rule [1]", + Message: "[3]string rule [1]", + }, + }, + SchemaProps: spec.SchemaProps{ + Pattern: "self[3] == 'hi'", + }, + }, + }, + { + t: types.String, + name: "non-consecutive raw string indexing false positive", + comments: []string{ + `+k8s:validation:cel[0]:message>[3]raw string message with subscirpt [3]"`, + `+k8s:validation:cel[0]:rule> raw string rule [1]`, + `+k8s:validation:pattern>"self[3] == 'hi'"`, + }, + expected: generators.CommentTags{ + CEL: []generators.CELTag{ + { + Rule: "raw string rule [1]", + Message: "[3]raw string message with subscirpt [3]\"", + }, + }, + SchemaProps: spec.SchemaProps{ + Pattern: `"self[3] == 'hi'"`, + }, + }, }, { t: types.Float64, @@ -286,7 +344,7 @@ func TestParseCommentTags(t *testing.T) { `+k8s:validation:cel[0]:message="cant change"`, `+k8s:validation:cel[2]:optionalOldSelf`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[2]:optionalOldSelf: non-consecutive index 2 for key '+k8s:validation:cel'`, + expectedError: `failed to parse marker comments: error parsing cel[2]:optionalOldSelf: non-consecutive index 2 for key 'cel'`, }, { t: types.Float64, @@ -299,7 +357,7 @@ func TestParseCommentTags(t *testing.T) { `+minimum=5`, `+k8s:validation:cel[1]:optionalOldSelf`, }, - expectedError: `failed to parse marker comments: error parsing +k8s:validation:cel[1]:optionalOldSelf: non-consecutive index 1 for key '+k8s:validation:cel'`, + expectedError: `failed to parse marker comments: error parsing cel[1]:optionalOldSelf: non-consecutive index 1 for key 'cel'`, }, { t: types.Float64, @@ -325,11 +383,83 @@ func TestParseCommentTags(t *testing.T) { }, }, }, + { + t: types.Float64, + name: "raw string rule", + comments: []string{ + `+k8s:validation:cel[0]:rule> raw string rule`, + `+k8s:validation:cel[0]:message="raw string message"`, + }, + expected: generators.CommentTags{ + CEL: []generators.CELTag{ + { + Rule: "raw string rule", + Message: "raw string message", + }, + }, + }, + }, + { + t: types.Float64, + name: "multiline string rule", + comments: []string{ + `+k8s:validation:cel[0]:rule> self.length() % 2 == 0`, + `+k8s:validation:cel[0]:rule> ? self.field == self.name + ' is even'`, + `+k8s:validation:cel[0]:rule> : self.field == self.name + ' is odd'`, + `+k8s:validation:cel[0]:message>raw string message`, + }, + expected: generators.CommentTags{ + CEL: []generators.CELTag{ + { + Rule: "self.length() % 2 == 0\n? self.field == self.name + ' is even'\n: self.field == self.name + ' is odd'", + Message: "raw string message", + }, + }, + }, + }, + { + t: types.Float64, + name: "mix raw and non-raw string marker", + comments: []string{ + `+k8s:validation:cel[0]:message>raw string message`, + `+k8s:validation:cel[0]:rule="self.length() % 2 == 0"`, + `+k8s:validation:cel[0]:rule> ? self.field == self.name + ' is even'`, + `+k8s:validation:cel[0]:rule> : self.field == self.name + ' is odd'`, + }, + expected: generators.CommentTags{ + CEL: []generators.CELTag{ + { + Rule: "self.length() % 2 == 0\n? self.field == self.name + ' is even'\n: self.field == self.name + ' is odd'", + Message: "raw string message", + }, + }, + }, + }, + { + name: "raw string with different key in between", + t: types.Float64, + comments: []string{ + `+k8s:validation:cel[0]:message>raw string message`, + `+k8s:validation:cel[0]:rule="self.length() % 2 == 0"`, + `+k8s:validation:cel[0]:message>raw string message 2`, + }, + expectedError: `failed to parse marker comments: concatenations to key 'cel[0]:message' must be consecutive with its assignment`, + }, + { + name: "raw string with different raw string key in between", + t: types.Float64, + comments: []string{ + `+k8s:validation:cel[0]:message>raw string message`, + `+k8s:validation:cel[0]:rule>self.length() % 2 == 0`, + `+k8s:validation:cel[0]:message>raw string message 2`, + }, + expectedError: `failed to parse marker comments: concatenations to key 'cel[0]:message' must be consecutive with its assignment`, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - actual, err := generators.ParseCommentTags(tc.t, tc.comments, "k8s:validation:") + actual, err := generators.ParseCommentTags(tc.t, tc.comments, "+k8s:validation:") if tc.expectedError != "" { require.Error(t, err) require.EqualError(t, err, tc.expectedError) @@ -588,7 +718,7 @@ func TestCommentTags_Validate(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - _, err := generators.ParseCommentTags(tc.t, tc.comments, "k8s:validation:") + _, err := generators.ParseCommentTags(tc.t, tc.comments, "+k8s:validation:") if tc.errorMessage != "" { require.Error(t, err) require.Equal(t, "invalid marker comments: "+tc.errorMessage, err.Error()) diff --git a/pkg/generators/openapi.go b/pkg/generators/openapi.go index 47b687248..259f21dbc 100644 --- a/pkg/generators/openapi.go +++ b/pkg/generators/openapi.go @@ -38,7 +38,7 @@ import ( // This is the comment tag that carries parameters for open API generation. const tagName = "k8s:openapi-gen" -const markerPrefix = "k8s:validation:" +const markerPrefix = "+k8s:validation:" const tagOptional = "optional" const tagDefault = "default" @@ -640,14 +640,14 @@ func (g openAPITypeWriter) emitExtensions(extensions []extension, unions []union for _, rule := range celRules { g.Do("map[string]interface{}{\n", nil) - g.Do("\"rule\": \"$.$\",\n", rule.Rule) + g.Do("\"rule\": $.$,\n", fmt.Sprintf("%#v", rule.Rule)) if len(rule.Message) > 0 { - g.Do("\"message\": \"$.$\",\n", rule.Message) + g.Do("\"message\": $.$,\n", fmt.Sprintf("%#v", rule.Message)) } if len(rule.MessageExpression) > 0 { - g.Do("\"messageExpression\": \"$.$\",\n", rule.MessageExpression) + g.Do("\"messageExpression\": $.$,\n", fmt.Sprintf("%#v", rule.MessageExpression)) } if rule.OptionalOldSelf != nil && *rule.OptionalOldSelf { diff --git a/pkg/generators/openapi_test.go b/pkg/generators/openapi_test.go index 9e0b92dd5..bb33b4dd8 100644 --- a/pkg/generators/openapi_test.go +++ b/pkg/generators/openapi_test.go @@ -2102,6 +2102,86 @@ func TestCELMarkerComments(t *testing.T) { } } +func TestMultilineCELMarkerComments(t *testing.T) { + + callErr, funcErr, assert, _, funcBuffer, imports := testOpenAPITypeWriter(t, ` + package foo + + // +k8s:openapi-gen=true + // +k8s:validation:cel[0]:rule="self == oldSelf" + // +k8s:validation:cel[0]:message="message1" + type Blah struct { + // +k8s:validation:cel[0]:rule="self.length() > 0" + // +k8s:validation:cel[0]:message="string message" + // +k8s:validation:cel[1]:rule> !oldSelf.hasValue() || self.length() % 2 == 0 + // +k8s:validation:cel[1]:rule> ? self.field == "even" + // +k8s:validation:cel[1]:rule> : self.field == "odd" + // +k8s:validation:cel[1]:messageExpression="field must be whether the length of the string is even or odd" + // +k8s:validation:cel[1]:optionalOldSelf + // +optional + Field string + } + `) + + assert.NoError(funcErr) + assert.NoError(callErr) + assert.ElementsMatch(imports, []string{`foo "base/foo"`, `common "k8s.io/kube-openapi/pkg/common"`, `spec "k8s.io/kube-openapi/pkg/validation/spec"`, `ptr "k8s.io/utils/ptr"`}) + + if formatted, err := format.Source(funcBuffer.Bytes()); err != nil { + t.Fatalf("%v\n%v", err, string(funcBuffer.Bytes())) + } else { + formatted_expected, ree := format.Source([]byte(`func schema_base_foo_Blah(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "Field": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": "self.length() > 0", + "message": "string message", + }, + map[string]interface{}{ + "rule": "!oldSelf.hasValue() || self.length() % 2 == 0\n? self.field == \"even\"\n: self.field == \"odd\"", + "messageExpression": "field must be whether the length of the string is even or odd", + "optionalOldSelf": ptr.To[bool](true), + }, + }, + }, + }, + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": "self == oldSelf", + "message": "message1", + }, + }, + }, + }, + }, + } + } + +`)) + if ree != nil { + t.Fatal(ree) + } + assert.Equal(string(formatted_expected), string(formatted)) + } +} + func TestMarkerCommentsCustomDefsV3(t *testing.T) { callErr, funcErr, assert, callBuffer, funcBuffer, _ := testOpenAPITypeWriter(t, ` package foo