Fix validation and add many tests

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go

@@ -87,7 +87,7 @@ type JSONSchemaProps struct {
 	//    - ... zero or more
 	XIntOrString bool
 
-	// x-kubernetes-list-map-keys annotates lists with the x-kubernetes-list-type `map` by specifying the keys used
+	// x-kubernetes-list-map-keys annotates an array with the x-kubernetes-list-type `map` by specifying the keys used
 	// as the index of the map.
 	//
 	// This tag MUST only be used on lists that have the "x-kubernetes-list-type"
@@ -95,19 +95,19 @@ type JSONSchemaProps struct {
 	// be a scalar typed field of the child structure (no nesting is supported).
 	XListMapKeys []string
 
-	// x-kubernetes-list-type annotates a list to further describe its topology.
+	// x-kubernetes-list-type annotates an array to further describe its topology.
 	// This extension must only be used on lists and may have 3 possible values:
 	//
 	// 1) `atomic`: the list is treated as a single entity, like a scalar.
 	//      Atomic lists will be entirely replaced when updated. This extension
 	//      may be used on any type of list (struct, scalar, ...).
 	// 2) `set`:
-	//      Sets are lists that must not have multiple times the same value. Each
+	//      Sets are lists that must not have multiple items with the same value. Each
 	//      value must be a scalar (or another atomic type).
 	// 3) `map`:
 	//      These lists are like maps in that their elements have a non-index key
 	//      used to identify them. Order is preserved upon merge. The map tag
-	//      must only be used on a list with struct elements.
+	//      must only be used on a list with elements of type object.
 	XListType *string
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults.go

@@ -59,3 +59,47 @@ func SetDefaults_ServiceReference(obj *ServiceReference) {
 		obj.Port = utilpointer.Int32Ptr(443)
 	}
 }
+
+// SetDefaults_JSONSchemaProps sets the defaults for JSONSchemaProps
+func SetDefaults_JSONSchemaProps(obj *JSONSchemaProps) {
+	if obj == nil {
+		return
+	}
+	if obj.Type == "array" && obj.XListType == nil {
+		obj.XListType = utilpointer.StringPtr("atomic")
+	}
+	if obj.Items != nil {
+		SetDefaults_JSONSchemaProps(obj.Items.Schema)
+		defaultJSONSchemaPropsArray(obj.Items.JSONSchemas)
+	}
+	defaultJSONSchemaPropsArray(obj.AllOf)
+	defaultJSONSchemaPropsArray(obj.OneOf)
+	defaultJSONSchemaPropsArray(obj.AnyOf)
+	SetDefaults_JSONSchemaProps(obj.Not)
+	defaultJSONSchemaPropsMap(obj.Properties)
+	if obj.AdditionalProperties != nil {
+		SetDefaults_JSONSchemaProps(obj.AdditionalProperties.Schema)
+	}
+	defaultJSONSchemaPropsMap(obj.PatternProperties)
+	for i := range obj.Dependencies {
+		SetDefaults_JSONSchemaProps(obj.Dependencies[i].Schema)
+	}
+	if obj.AdditionalItems != nil {
+		SetDefaults_JSONSchemaProps(obj.AdditionalItems.Schema)
+	}
+	defaultJSONSchemaPropsMap(map[string]JSONSchemaProps(obj.Definitions))
+}
+
+func defaultJSONSchemaPropsArray(obj []JSONSchemaProps) {
+	for i := range obj {
+		SetDefaults_JSONSchemaProps(&obj[i])
+	}
+}
+
+func defaultJSONSchemaPropsMap(obj map[string]JSONSchemaProps) {
+	for i := range obj {
+		props := obj[i]
+		SetDefaults_JSONSchemaProps(&props)
+		obj[i] = props
+	}
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/defaults_test.go

@@ -124,6 +124,47 @@ func TestDefaults(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "list type extension defaults",
+			original: &CustomResourceDefinition{
+				Spec: CustomResourceDefinitionSpec{
+					Scope:      NamespaceScoped,
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
+					Versions: []CustomResourceDefinitionVersion{
+						{
+							Name:    "v1",
+							Storage: true,
+							Schema: &CustomResourceValidation{
+								OpenAPIV3Schema: &JSONSchemaProps{
+									Type: "array",
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: &CustomResourceDefinition{
+				Spec: CustomResourceDefinitionSpec{
+					Scope:      NamespaceScoped,
+					Conversion: &CustomResourceConversion{Strategy: NoneConverter},
+					Versions: []CustomResourceDefinitionVersion{
+						{
+							Name:    "v1",
+							Storage: true,
+							Schema: &CustomResourceValidation{
+								OpenAPIV3Schema: &JSONSchemaProps{
+									Type:      "array",
+									XListType: utilpointer.StringPtr("atomic"),
+								},
+							},
+						},
+					},
+				},
+				Status: CustomResourceDefinitionStatus{
+					StoredVersions: []string{"v1"},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go

@@ -91,27 +91,31 @@ type JSONSchemaProps struct {
 	//    - ... zero or more
 	XIntOrString bool `json:"x-kubernetes-int-or-string,omitempty" protobuf:"bytes,40,opt,name=xKubernetesIntOrString"`
 
-	// x-kubernetes-list-map-keys annotates lists with the x-kubernetes-list-type `map` by specifying the keys used
+	// x-kubernetes-list-map-keys annotates an array with the x-kubernetes-list-type `map` by specifying the keys used
 	// as the index of the map.
 	//
 	// This tag MUST only be used on lists that have the "x-kubernetes-list-type"
 	// extension set to "map". Also, the values specified for this attribute must
 	// be a scalar typed field of the child structure (no nesting is supported).
+	//
+	// +optional
 	XListMapKeys []string `json:"x-kubernetes-list-map-keys,omitempty" protobuf:"bytes,41,rep,name=xKubernetesListMapKeys"`
 
-	// x-kubernetes-list-type annotates a list to further describe its topology.
+	// x-kubernetes-list-type annotates an array to further describe its topology.
 	// This extension must only be used on lists and may have 3 possible values:
 	//
 	// 1) `atomic`: the list is treated as a single entity, like a scalar.
 	//      Atomic lists will be entirely replaced when updated. This extension
 	//      may be used on any type of list (struct, scalar, ...).
 	// 2) `set`:
-	//      Sets are lists that must not have multiple times the same value. Each
+	//      Sets are lists that must not have multiple items with the same value. Each
 	//      value must be a scalar (or another atomic type).
 	// 3) `map`:
 	//      These lists are like maps in that their elements have a non-index key
 	//      used to identify them. Order is preserved upon merge. The map tag
-	//      must only be used on a list with struct elements.
+	//      must only be used on a list with elements of type object.
+	// Defaults to atomic for arrays.
+	// +optional
 	XListType *string `json:"x-kubernetes-list-type,omitempty" protobuf:"bytes,42,opt,name=xKubernetesListType"`
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults.go

@@ -80,3 +80,47 @@ func SetDefaults_ServiceReference(obj *ServiceReference) {
 		obj.Port = utilpointer.Int32Ptr(443)
 	}
 }
+
+// SetDefaults_JSONSchemaProps sets the defaults for JSONSchemaProps
+func SetDefaults_JSONSchemaProps(obj *JSONSchemaProps) {
+	if obj == nil {
+		return
+	}
+	if obj.Type == "array" && obj.XListType == nil {
+		obj.XListType = utilpointer.StringPtr("atomic")
+	}
+	if obj.Items != nil {
+		SetDefaults_JSONSchemaProps(obj.Items.Schema)
+		defaultJSONSchemaPropsArray(obj.Items.JSONSchemas)
+	}
+	defaultJSONSchemaPropsArray(obj.AllOf)
+	defaultJSONSchemaPropsArray(obj.OneOf)
+	defaultJSONSchemaPropsArray(obj.AnyOf)
+	SetDefaults_JSONSchemaProps(obj.Not)
+	defaultJSONSchemaPropsMap(obj.Properties)
+	if obj.AdditionalProperties != nil {
+		SetDefaults_JSONSchemaProps(obj.AdditionalProperties.Schema)
+	}
+	defaultJSONSchemaPropsMap(obj.PatternProperties)
+	for i := range obj.Dependencies {
+		SetDefaults_JSONSchemaProps(obj.Dependencies[i].Schema)
+	}
+	if obj.AdditionalItems != nil {
+		SetDefaults_JSONSchemaProps(obj.AdditionalItems.Schema)
+	}
+	defaultJSONSchemaPropsMap(map[string]JSONSchemaProps(obj.Definitions))
+}
+
+func defaultJSONSchemaPropsArray(obj []JSONSchemaProps) {
+	for i := range obj {
+		SetDefaults_JSONSchemaProps(&obj[i])
+	}
+}
+
+func defaultJSONSchemaPropsMap(obj map[string]JSONSchemaProps) {
+	for i := range obj {
+		props := obj[i]
+		SetDefaults_JSONSchemaProps(&props)
+		obj[i] = props
+	}
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/defaults_test.go

@@ -129,6 +129,34 @@ func TestDefaults(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "list type extension defaults",
+			original: &CustomResourceDefinition{
+				Spec: CustomResourceDefinitionSpec{
+					Scope:                 NamespaceScoped,
+					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					PreserveUnknownFields: utilpointer.BoolPtr(true),
+					Validation: &CustomResourceValidation{
+						OpenAPIV3Schema: &JSONSchemaProps{
+							Type: "array",
+						},
+					},
+				},
+			},
+			expected: &CustomResourceDefinition{
+				Spec: CustomResourceDefinitionSpec{
+					Scope:                 NamespaceScoped,
+					Conversion:            &CustomResourceConversion{Strategy: NoneConverter},
+					PreserveUnknownFields: utilpointer.BoolPtr(true),
+					Validation: &CustomResourceValidation{
+						OpenAPIV3Schema: &JSONSchemaProps{
+							Type:      "array",
+							XListType: utilpointer.StringPtr("atomic"),
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go

@@ -91,27 +91,31 @@ type JSONSchemaProps struct {
 	//    - ... zero or more
 	XIntOrString bool `json:"x-kubernetes-int-or-string,omitempty" protobuf:"bytes,40,opt,name=xKubernetesIntOrString"`
 
-	// x-kubernetes-list-map-keys annotates lists with the x-kubernetes-list-type `map` by specifying the keys used
+	// x-kubernetes-list-map-keys annotates an array with the x-kubernetes-list-type `map` by specifying the keys used
 	// as the index of the map.
 	//
 	// This tag MUST only be used on lists that have the "x-kubernetes-list-type"
 	// extension set to "map". Also, the values specified for this attribute must
 	// be a scalar typed field of the child structure (no nesting is supported).
+	//
+	// +optional
 	XListMapKeys []string `json:"x-kubernetes-list-map-keys,omitempty" protobuf:"bytes,41,rep,name=xKubernetesListMapKeys"`
 
-	// x-kubernetes-list-type annotates a list to further describe its topology.
+	// x-kubernetes-list-type annotates an array to further describe its topology.
 	// This extension must only be used on lists and may have 3 possible values:
 	//
 	// 1) `atomic`: the list is treated as a single entity, like a scalar.
 	//      Atomic lists will be entirely replaced when updated. This extension
 	//      may be used on any type of list (struct, scalar, ...).
 	// 2) `set`:
-	//      Sets are lists that must not have multiple times the same value. Each
+	//      Sets are lists that must not have multiple items with the same value. Each
 	//      value must be a scalar (or another atomic type).
 	// 3) `map`:
 	//      These lists are like maps in that their elements have a non-index key
 	//      used to identify them. Order is preserved upon merge. The map tag
-	//      must only be used on a list with struct elements.
+	//      must only be used on a list with elements of type object.
+	// Defaults to atomic for arrays.
+	// +optional
 	XListType *string `json:"x-kubernetes-list-type,omitempty" protobuf:"bytes,42,opt,name=xKubernetesListType"`
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go

@@ -796,8 +796,12 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
 		}
 	}
 
+	if schema.XListType == nil && schema.Type == "array" {
+		allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-list-type"), "must be set if type is array"))
+	}
+
 	if schema.XListType != nil && *schema.XListType != "atomic" && *schema.XListType != "set" && *schema.XListType != "map" {
-		allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, "must be one of 'atomic', 'set', 'map', or unset"))
+		allErrs = append(allErrs, field.NotSupported(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, []string{"atomic", "set", "map"}))
 	}
 
 	if len(schema.XListMapKeys) > 0 && (schema.XListType == nil || *schema.XListType != "map") {
@@ -808,6 +812,35 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
 		}
 	}
 
+	if len(schema.XListMapKeys) == 0 && schema.XListType != nil && *schema.XListType == "map" {
+		allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-list-map-keys"), "must not be empty if x-kubernetes-list-type is map"))
+	}
+
+	if schema.Items != nil && schema.Items.Schema == nil && schema.XListType != nil && *schema.XListType == "map" {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("items"), schema.Items, "must only have a single schema if x-kubernetes-list-type is map"))
+	}
+
+	if schema.Items != nil && schema.Items.Schema != nil && schema.Items.Schema.Type != "object" && schema.XListType != nil && *schema.XListType == "map" {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("type"), schema.Items.Schema.Type, "must be object if parent array's x-kubernetes-list-type is map"))
+	}
+
+	if schema.Items != nil && schema.Items.Schema != nil && schema.Items.Schema.Type == "object" && schema.XListType != nil && *schema.XListType == "map" {
+		keys := map[string]struct{}{}
+		for _, k := range schema.XListMapKeys {
+			if s, ok := schema.Items.Schema.Properties[k]; ok {
+				if s.Type == "array" || s.Type == "object" {
+					allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("properties").Child(k).Child("type"), schema.Items.Schema.Type, "must be a scalar type if parent array's x-kubernetes-list-type is map"))
+				}
+			} else {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "entries must all be names of item properties"))
+			}
+			if _, ok := keys[k]; ok {
+				allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "must not contain duplicate entries"))
+			}
+			keys[k] = struct{}{}
+		}
+	}
+
 	return allErrs
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go

@@ -6510,6 +6510,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 						"array": {
 							Type:     "array",
 							Nullable: true,
+
+							// This value is defaulted
+							XListType: strPtr("atomic"),
 						},
 						"number": {
 							Type:     "number",
@@ -6577,7 +6580,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
 					Type:      "object",
-					XListType: strPtr("map"),
+					XListType: strPtr("set"),
 				},
 			},
 			wantError: true,
@@ -6586,13 +6589,32 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 			name: "unset type with list type extension set",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
-					XListType: strPtr("map"),
+					XListType: strPtr("set"),
 				},
 			},
 			wantError: true,
 		},
 		{
-			name: "invalid list type extension with list map keys extension set",
+			name: "unset list type extension with type array",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type: "array",
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "invalid list type extension",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:      "array",
+					XListType: strPtr("invalid"),
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "invalid list type extension with list map keys extension non-empty",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
 					Type:         "array",
@@ -6603,25 +6625,142 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 			wantError: true,
 		},
 		{
-			name: "unset list type extension with list map keys extension set",
+			name: "unset list type extension with list map keys extension non-empty",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
-					Type:         "array",
 					XListMapKeys: []string{"key"},
 				},
 			},
 			wantError: true,
 		},
 		{
-			name: "invalid list type",
+			name: "empty list map keys extension with list type extension map",
 			input: apiextensions.CustomResourceValidation{
 				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
 					Type:      "array",
-					XListType: strPtr("invalid"),
+					XListType: strPtr("map"),
 				},
 			},
 			wantError: true,
 		},
+		{
+			name: "multiple schema items with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"key"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						JSONSchemas: []apiextensions.JSONSchemaProps{
+							{
+								Type: "string",
+							}, {
+								Type: "integer",
+							},
+						},
+					},
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "non object item with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"key"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Type: "string",
+						},
+					},
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "items with key missing from properties with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"key"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+						},
+					},
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "items with non scalar key property type with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"key"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensions.JSONSchemaProps{
+								"key": {
+									Type: "object",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "duplicate map keys with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"key", "key"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensions.JSONSchemaProps{
+								"key": {
+									Type: "string",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantError: true,
+		},
+		{
+			name: "allowed schema with list type extension map",
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type:         "array",
+					XListType:    strPtr("map"),
+					XListMapKeys: []string{"keyA", "keyB"},
+					Items: &apiextensions.JSONSchemaPropsOrArray{
+						Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensions.JSONSchemaProps{
+								"keyA": {
+									Type: "string",
+								},
+								"keyB": {
+									Type: "integer",
+								},
+							},
+						},
+					},
+				},
+			},
+			wantError: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -655,7 +655,11 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd
 			}
 			specs = append(specs, s)
 		}
-		mergedOpenAPI := builder.MergeSpecs(r.staticOpenAPISpec, specs...)
+		mergedOpenAPI, err := builder.MergeSpecs(r.staticOpenAPISpec, specs...)
+		if err != nil {
+			utilruntime.HandleError(err)
+			return nil, fmt.Errorf("the server could not properly merge the CR schema")
+		}
 		openAPIModels, err = utilopenapi.ToProtoModels(mergedOpenAPI)
 		if err != nil {
 			utilruntime.HandleError(err)

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/goopenapi.go

@@ -82,7 +82,7 @@ func (x *Extensions) toGoOpenAPI(ret *spec.Schema) {
 		ret.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", x.XListMapKeys)
 	}
 	if x.XListType != nil {
-		ret.VendorExtensible.AddExtension("x-kubernetes-list-type", x.XListType)
+		ret.VendorExtensible.AddExtension("x-kubernetes-list-type", *x.XListType)
 	}
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go

@@ -108,12 +108,12 @@ type Extensions struct {
 	//      Atomic lists will be entirely replaced when updated. This extension
 	//      may be used on any type of list (struct, scalar, ...).
 	// 2) `set`:
-	//      Sets are lists that must not have multiple times the same value. Each
+	//      Sets are lists that must not have multiple items with the same value. Each
 	//      value must be a scalar (or another atomic type).
 	// 3) `map`:
 	//      These lists are like maps in that their elements have a non-index key
 	//      used to identify them. Order is preserved upon merge. The map tag
-	//      must only be used on a list with struct elements.
+	//      must only be used on a list with elements of type object.
 	XListType *string
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go

@@ -250,7 +250,7 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
 		out.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", in.XListMapKeys)
 	}
 	if in.XListType != nil {
-		out.VendorExtensible.AddExtension("x-kubernetes-list-type", in.XListType)
+		out.VendorExtensible.AddExtension("x-kubernetes-list-type", *in.XListType)
 	}
 
 	return nil

test/e2e/apimachinery/crd_publish_openapi.go

@@ -35,6 +35,7 @@ import (
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	utildiff "k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/wait"
 	k8sclientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -507,7 +508,8 @@ func waitForDefinition(c k8sclientset.Interface, name string, schema []byte) err
 			// drop properties and extension that we added
 			dropDefaults(&d)
 			if !apiequality.Semantic.DeepEqual(expect, d) {
-				return false, fmt.Sprintf("spec.SwaggerProps.Definitions[\"%s\"] not match; expect: %v, actual: %v", name, expect, d)
+				diff := utildiff.ObjectGoPrintSideBySide(expect, d)
+				return false, fmt.Sprintf("spec.SwaggerProps.Definitions[\"%s\"] not match; expect: %v, actual: %v\n%v", name, expect, d, diff)
 			}
 		}
 		return true, ""
@@ -627,6 +629,7 @@ properties:
       bars:
         description: List of Bars and their specs.
         type: array
+        x-kubernetes-list-type: atomic
         items:
           type: object
           required:
@@ -643,6 +646,7 @@ properties:
               items:
                 type: string
               type: array
+              x-kubernetes-list-type: atomic
   status:
     description: Status of Foo
     type: object
@@ -650,6 +654,7 @@ properties:
       bars:
         description: List of Bars and their statuses.
         type: array
+        x-kubernetes-list-type: atomic
         items:
           type: object
           properties:
@@ -681,6 +686,7 @@ properties:
       bars:
         description: List of Bars and their statuses.
         type: array
+        x-kubernetes-list-type: atomic
         items:
           type: object`)
 
@@ -702,6 +708,7 @@ properties:
       bars:
         description: List of Bars and their statuses.
         type: array
+        x-kubernetes-list-type: atomic
         items:
           type: object`)
 
@@ -723,5 +730,6 @@ properties:
       bars:
         description: List of Bars and their statuses.
         type: array
+        x-kubernetes-list-type: atomic
         items:
           type: object`)

test/integration/apiserver/apply/apply_crd_test.go

@@ -19,12 +19,14 @@ package apiserver
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"testing"
 
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	"k8s.io/apiextensions-apiserver/test/integration/fixtures"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	genericfeatures "k8s.io/apiserver/pkg/features"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -83,6 +85,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to create custom resource with apply: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 1)
 
 	// Patch object to change the number of replicas
 	result, err = rest.Patch(types.MergePatchType).
@@ -93,6 +96,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to update number of replicas with merge patch: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 5)
 
 	// Re-apply, we should get conflicts now, since the number of replicas was changed.
 	result, err = rest.Patch(types.ApplyPatchType).
@@ -108,8 +112,8 @@ spec:
 	if !ok {
 		t.Fatalf("Expecting to get conflicts as API error")
 	}
-	if len(status.Status().Details.Causes) < 1 {
-		t.Fatalf("Expecting to get at least one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
+	if len(status.Status().Details.Causes) != 1 {
+		t.Fatalf("Expecting to get one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
 	}
 
 	// Re-apply with force, should work fine.
@@ -123,6 +127,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 1)
 }
 
 // TestApplyCRDStructuralSchema tests that when a CRD has a structural schema in its validation field,
@@ -239,27 +244,38 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to create custom resource with apply: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 1)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyReplicas(t, result, 1)
+	verifyNumPorts(t, result, 1)
 
 	// Patch object to add another finalizer to the finalizers list
 	result, err = rest.Patch(types.MergePatchType).
 		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
 		Name(name).
-		Body([]byte(`{"metadata":{"finalizers":["another-one"]}}`)).
+		Body([]byte(`{"metadata":{"finalizers":["test-finalizer","another-one"]}}`)).
 		DoRaw()
 	if err != nil {
 		t.Fatalf("failed to add finalizer with merge patch: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 2)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyFinalizersIncludes(t, result, "another-one")
 
 	// Re-apply the same config, should work fine, since finalizers should have the list-type extension 'set'.
 	result, err = rest.Patch(types.ApplyPatchType).
 		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
 		Name(name).
 		Param("fieldManager", "apply_test").
+		SetHeader("Accept", "application/json").
 		Body(yamlBody).
 		DoRaw()
 	if err != nil {
 		t.Fatalf("failed to apply same config after adding a finalizer: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 2)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyFinalizersIncludes(t, result, "another-one")
 
 	// Patch object to change the number of replicas
 	result, err = rest.Patch(types.MergePatchType).
@@ -270,6 +286,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to update number of replicas with merge patch: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 5)
 
 	// Re-apply, we should get conflicts now, since the number of replicas was changed.
 	result, err = rest.Patch(types.ApplyPatchType).
@@ -285,8 +302,8 @@ spec:
 	if !ok {
 		t.Fatalf("Expecting to get conflicts as API error")
 	}
-	if len(status.Status().Details.Causes) < 1 {
-		t.Fatalf("Expecting to get at least one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
+	if len(status.Status().Details.Causes) != 1 {
+		t.Fatalf("Expecting to get one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
 	}
 
 	// Re-apply with force, should work fine.
@@ -300,6 +317,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 1)
 
 	// New applier tries to edit an existing list item, we should get conflicts.
 	result, err = rest.Patch(types.ApplyPatchType).
@@ -324,8 +342,8 @@ spec:
 	if !ok {
 		t.Fatalf("Expecting to get conflicts as API error")
 	}
-	if len(status.Status().Details.Causes) < 1 {
-		t.Fatalf("Expecting to get at least one conflict when a different applier updates existing list item, got: %v", status.Status().Details.Causes)
+	if len(status.Status().Details.Causes) != 1 {
+		t.Fatalf("Expecting to get one conflict when a different applier updates existing list item, got: %v", status.Status().Details.Causes)
 	}
 
 	// New applier tries to add a new list item, should work fine.
@@ -343,10 +361,12 @@ spec:
   - name: "y"
     containerPort: 8080
     protocol: TCP`, apiVersion, kind, name))).
+		SetHeader("Accept", "application/json").
 		DoRaw()
 	if err != nil {
 		t.Fatalf("failed to add a new list item to the object as a different applier: %v:\n%v", err, string(result))
 	}
+	verifyNumPorts(t, result, 2)
 }
 
 // TestApplyCRDNonStructuralSchema tests that when a CRD has a non-structural schema in its validation field,
@@ -430,27 +450,37 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to create custom resource with apply: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 1)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyReplicas(t, result, 1.0)
 
 	// Patch object to add another finalizer to the finalizers list
 	result, err = rest.Patch(types.MergePatchType).
 		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
 		Name(name).
-		Body([]byte(`{"metadata":{"finalizers":["another-one"]}}`)).
+		Body([]byte(`{"metadata":{"finalizers":["test-finalizer","another-one"]}}`)).
 		DoRaw()
 	if err != nil {
 		t.Fatalf("failed to add finalizer with merge patch: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 2)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyFinalizersIncludes(t, result, "another-one")
 
 	// Re-apply the same config, should work fine, since finalizers should have the list-type extension 'set'.
 	result, err = rest.Patch(types.ApplyPatchType).
 		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
 		Name(name).
 		Param("fieldManager", "apply_test").
+		SetHeader("Accept", "application/json").
 		Body(yamlBody).
 		DoRaw()
 	if err != nil {
 		t.Fatalf("failed to apply same config after adding a finalizer: %v:\n%v", err, string(result))
 	}
+	verifyNumFinalizers(t, result, 2)
+	verifyFinalizersIncludes(t, result, "test-finalizer")
+	verifyFinalizersIncludes(t, result, "another-one")
 
 	// Patch object to change the number of replicas
 	result, err = rest.Patch(types.MergePatchType).
@@ -461,6 +491,7 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to update number of replicas with merge patch: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 5.0)
 
 	// Re-apply, we should get conflicts now, since the number of replicas was changed.
 	result, err = rest.Patch(types.ApplyPatchType).
@@ -476,8 +507,8 @@ spec:
 	if !ok {
 		t.Fatalf("Expecting to get conflicts as API error")
 	}
-	if len(status.Status().Details.Causes) < 1 {
-		t.Fatalf("Expecting to get at least one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
+	if len(status.Status().Details.Causes) != 1 {
+		t.Fatalf("Expecting to get one conflict when applying object after updating replicas, got: %v", status.Status().Details.Causes)
 	}
 
 	// Re-apply with force, should work fine.
@@ -491,4 +522,88 @@ spec:
 	if err != nil {
 		t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result))
 	}
+	verifyReplicas(t, result, 1.0)
+}
+
+// verifyNumFinalizers checks that len(.metadata.finalizers) == n
+func verifyNumFinalizers(t *testing.T, b []byte, n int) {
+	obj := unstructured.Unstructured{}
+	err := obj.UnmarshalJSON(b)
+	if err != nil {
+		t.Fatalf("failed to unmarshal response: %v", err)
+	}
+	if actual, expected := len(obj.GetFinalizers()), n; actual != expected {
+		t.Fatalf("expected %v finalizers but got %v:\n%v", expected, actual, string(b))
+	}
+}
+
+// verifyFinalizersIncludes checks that .metadata.finalizers includes e
+func verifyFinalizersIncludes(t *testing.T, b []byte, e string) {
+	obj := unstructured.Unstructured{}
+	err := obj.UnmarshalJSON(b)
+	if err != nil {
+		t.Fatalf("failed to unmarshal response: %v", err)
+	}
+	for _, a := range obj.GetFinalizers() {
+		if a == e {
+			return
+		}
+	}
+	t.Fatalf("expected finalizers to include %q but got: %v", e, obj.GetFinalizers())
+}
+
+// verifyReplicas checks that .spec.replicas == r
+func verifyReplicas(t *testing.T, b []byte, r int) {
+	obj := unstructured.Unstructured{}
+	err := obj.UnmarshalJSON(b)
+	if err != nil {
+		t.Fatalf("failed to find replicas number in response: %v:\n%v", err, string(b))
+	}
+	spec, ok := obj.Object["spec"]
+	if !ok {
+		t.Fatalf("failed to find replicas number in response:\n%v", string(b))
+	}
+	specMap, ok := spec.(map[string]interface{})
+	if !ok {
+		t.Fatalf("failed to find replicas number in response:\n%v", string(b))
+	}
+	replicas, ok := specMap["replicas"]
+	if !ok {
+		t.Fatalf("failed to find replicas number in response:\n%v", string(b))
+	}
+	replicasNumber, ok := replicas.(int64)
+	if !ok {
+		t.Fatalf("failed to find replicas number in response: expected int64 but got: %v", reflect.TypeOf(replicas))
+	}
+	if actual, expected := replicasNumber, int64(r); actual != expected {
+		t.Fatalf("expected %v ports but got %v:\n%v", expected, actual, string(b))
+	}
+}
+
+// verifyNumPorts checks that len(.spec.ports) == n
+func verifyNumPorts(t *testing.T, b []byte, n int) {
+	obj := unstructured.Unstructured{}
+	err := obj.UnmarshalJSON(b)
+	if err != nil {
+		t.Fatalf("failed to find ports list in response: %v:\n%v", err, string(b))
+	}
+	spec, ok := obj.Object["spec"]
+	if !ok {
+		t.Fatalf("failed to find ports list in response:\n%v", string(b))
+	}
+	specMap, ok := spec.(map[string]interface{})
+	if !ok {
+		t.Fatalf("failed to find ports list in response:\n%v", string(b))
+	}
+	ports, ok := specMap["ports"]
+	if !ok {
+		t.Fatalf("failed to find ports list in response:\n%v", string(b))
+	}
+	portsList, ok := ports.([]interface{})
+	if !ok {
+		t.Fatalf("failed to find ports list in response: expected array but got: %v", reflect.TypeOf(ports))
+	}
+	if actual, expected := len(portsList), n; actual != expected {
+		t.Fatalf("expected %v ports but got %v:\n%v", expected, actual, string(b))
+	}
 }