Merge pull request #863 from Danil-Grigorev/support-empty-maps-lists

🐛 Add support for empty maps or lists
kubernetes-sigs · Dec 6, 2023 · 72d3440 · 72d3440
2 parents 498f1db + 2fac91e
commit 72d3440
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 1 deletion.
diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go
@@ -477,6 +477,9 @@ func (m Default) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	if err != nil {
 		return err
 	}
+	if schema.Type == "array" && string(marshalledDefault) == "{}" {
+		marshalledDefault = []byte("[]")
+	}
 	schema.Default = &apiext.JSON{Raw: marshalledDefault}
 	return nil
 }

diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go
@@ -120,6 +120,18 @@ type CronJobSpec struct {
 	// +kubebuilder:example={{nested: {foo: "baz", bar: true}},{nested: {bar: false}}}
 	DefaultedObject []RootObject `json:"defaultedObject"`
 
+	// This tests that empty slice defaulting can be performed.
+	// +kubebuilder:default={}
+	DefaultedEmptySlice []string `json:"defaultedEmptySlice"`
+
+	// This tests that an empty object defaulting can be performed on a map.
+	// +kubebuilder:default={}
+	DefaultedEmptyMap map[string]string `json:"defaultedEmptyMap"`
+
+	// This tests that an empty object defaulting can be performed on an object.
+	// +kubebuilder:default={}
+	DefaultedEmptyObject EmpiableObject `json:"defaultedEmptyObject"`
+
 	// This tests that pattern validator is properly applied.
 	// +kubebuilder:validation:Pattern=`^$|^((https):\/\/?)[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|\/?))$`
 	PatternObject string `json:"patternObject"`
@@ -297,6 +309,13 @@ type MinMaxObject struct {
 	Baz string `json:"baz,omitempty"`
 }
 
+type EmpiableObject struct {
+
+	// +kubebuilder:default=forty-two
+	Foo string `json:"foo,omitempty"`
+	Bar string `json:"bar,omitempty"`
+}
+
 type unexportedStruct struct {
 	// This tests that exported fields are not skipped in the schema generation
 	Foo string `json:"foo"`

diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
@@ -96,6 +96,28 @@ spec:
                 - Forbid
                 - Replace
                 type: string
+              defaultedEmptyMap:
+                default: {}
+                description: This tests that an empty object defaulting can be performed on a map.
+                type: object
+                additionalProperties:
+                  type: string
+              defaultedEmptyObject:
+                default: {}
+                description: This tests that an empty object defaulting can be performed on an object.
+                properties:
+                  bar:
+                    type: string
+                  foo:
+                    type: string
+                    default: forty-two
+                type: object
+              defaultedEmptySlice:
+                default: []
+                description: This tests that empty slice defaulting can be performed.
+                items:
+                  type: string
+                type: array
               defaultedObject:
                 default:
                 - nested:
@@ -7434,6 +7456,9 @@ spec:
             - baz
             - binaryName
             - canBeNull
+            - defaultedEmptyMap
+            - defaultedEmptyObject
+            - defaultedEmptySlice
             - defaultedObject
             - defaultedSlice
             - defaultedString

diff --git a/pkg/markers/parse.go b/pkg/markers/parse.go
@@ -310,15 +310,17 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 		// We'll cross that bridge when we get there.
 
 		// look ahead till we can figure out if this is a map or a slice
+		hint = peekNoSpace(subScanner)
 		firstElemType := guessType(subScanner, subRaw, false)
 		if firstElemType.Type == StringType {
 			// might be a map or slice, parse the string and check for colon
 			// (blech, basically arbitrary look-ahead due to raw strings).
 			var keyVal string // just ignore this
 			(&Argument{Type: StringType}).parseString(subScanner, raw, reflect.Indirect(reflect.ValueOf(&keyVal)))
 
-			if subScanner.Scan() == ':' {
+			if token := subScanner.Scan(); token == ':' || hint == '}' {
 				// it's got a string followed by a colon -- it's a map
+				// or an empty map in case of {}
 				return &Argument{
 					Type:     MapType,
 					ItemType: &Argument{Type: AnyType},

diff --git a/pkg/markers/parse_test.go b/pkg/markers/parse_test.go
@@ -190,6 +190,7 @@ var _ = Describe("Parsing", func() {
 		It("should support delimitted slices of delimitted slices", argParseTestCase{arg: sliceOSlice, raw: "{{1,1},{2,3},{5,8}}", output: sliceOSliceOut}.Run)
 
 		It("should support maps", argParseTestCase{arg: Argument{Type: MapType, ItemType: &Argument{Type: StringType}}, raw: "{formal: hello, `informal`: `hi!`}", output: map[string]string{"formal": "hello", "informal": "hi!"}}.Run)
+		It("should work with empty maps (which are equal to empty lists in the output)", argParseTestCase{arg: Argument{Type: MapType, ItemType: &Argument{Type: StringType}}, raw: "{}", output: map[string]string{}}.Run)
 
 		Context("with any value", func() {
 			anyArg := Argument{Type: AnyType}