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

@@ -91,9 +91,17 @@ func (c commentTags) ValidationSchema() (*spec.Schema, error) {
 		SchemaProps: c.SchemaProps,
 	}
 
-	if len(c.CEL) > 0 {
+	if res.AllOf != nil {
+		res.AllOf = append([]spec.Schema{}, res.AllOf...)
+	}
+	ccel := append([]CELTag{}, c.CEL...)
+	if _, exists := NameFormats[c.Format]; exists {
+		ccel = append([]CELTag{{Rule: "!format." + c.Format + "().validate(self).hasValue()", MessageExpression: "format." + c.Format + "().validate(self).value()"}}, ccel...)
+	}
+
+	if len(ccel) > 0 {
 		// Convert the CELTag to a map[string]interface{} via JSON
-		celTagJSON, err := json.Marshal(c.CEL)
+		celTagJSON, err := json.Marshal(ccel)
 		if err != nil {
 			return nil, fmt.Errorf("failed to marshal CEL tag: %w", err)
 		}
@@ -108,6 +116,17 @@ func (c commentTags) ValidationSchema() (*spec.Schema, error) {
 	return &res, nil
 }
 
+var NameFormats = map[string]struct{}{
+	"dns1123Label":             struct{}{},
+	"dns1123Subdomain":         struct{}{},
+	"httpPath":                 struct{}{},
+	"qualifiedName":            struct{}{},
+	"wildcardDNS1123Subdomain": struct{}{},
+	"cIdentifier":              struct{}{},
+	"dns1035Label":             struct{}{},
+	"labelValue":               struct{}{},
+}
+
 // validates the parameters in a CommentTags instance. Returns any errors encountered.
 func (c commentTags) Validate() error {
 
@@ -164,6 +183,13 @@ func (c commentTags) Validate() error {
 		err = errors.Join(err, fmt.Errorf("invalid CEL tag at index %d: %w", i, celError))
 	}
 
+	if c.Format != "" {
+		_, ok := NameFormats[c.Format]
+		if !ok {
+			err = errors.Join(err, fmt.Errorf("invalid nameFormat: %v", c.Format))
+		}
+	}
+
 	return err
 }
 
@@ -226,6 +252,9 @@ func (c commentTags) ValidateType(t *types.Type) error {
 	if c.ExclusiveMaximum && !isInt && !isFloat {
 		err = errors.Join(err, fmt.Errorf("exclusiveMaximum can only be used on numeric types"))
 	}
+	if c.Format != "" && !isString {
+		err = errors.Join(err, fmt.Errorf("Format can only be used on string types"))
+	}
 
 	return err
 }
@@ -235,26 +264,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
@@ -278,6 +308,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"

pkg/generators/markers_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package generators_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -494,6 +495,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 {
@@ -512,6 +576,75 @@ func TestParseCommentTags(t *testing.T) {
 	}
 }
 
+func TestFormat(t *testing.T) {
+
+	formatNames := []string{
+		"dns1123Label",
+		"dns1123Subdomain",
+		"httpPath",
+		"qualifiedName",
+		"wildcardDNS1123Subdomain",
+		"cIdentifier",
+		"dns1035Label",
+		"labelValue",
+	}
+
+	cases := []struct {
+		t             *types.Type
+		name          string
+		comments      []string
+		expected      *spec.Schema
+		expectedError string
+	}{}
+
+	for _, formatName := range formatNames {
+
+		cases = append(cases, struct {
+			t             *types.Type
+			name          string
+			comments      []string
+			expected      *spec.Schema
+			expectedError string
+		}{
+			t:    types.String,
+			name: formatName,
+			comments: []string{
+				fmt.Sprintf("+k8s:validation:format=\"%s\"", formatName),
+			},
+			expected: &spec.Schema{
+				VendorExtensible: spec.VendorExtensible{
+					Extensions: spec.Extensions{
+						"x-kubernetes-validations": []interface{}{
+							map[string]interface{}{
+								"messageExpression": "format." + formatName + "().validate(self).value()",
+								"rule":              "!format." + formatName + "().validate(self).hasValue()",
+							},
+						},
+					},
+				},
+				SchemaProps: spec.SchemaProps{
+					Format: formatName,
+				},
+			},
+		})
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual, err := generators.ParseCommentTags(tc.t, tc.comments, "+k8s:validation:")
+			if tc.expectedError != "" {
+				require.Error(t, err)
+				require.Regexp(t, tc.expectedError, err.Error())
+				return
+			} else {
+				require.NoError(t, err)
+			}
+
+			require.Equal(t, tc.expected, actual)
+		})
+	}
+}
+
 // Test comment tag validation function
 func TestCommentTags_Validate(t *testing.T) {