tests for buildOpenAPIModelsForApply

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD

@@ -100,6 +100,9 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = ["customresource_handler_test.go"],
+    data = [
+        "//api/openapi-spec",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library",
@@ -126,6 +129,7 @@ go_test(
         "//staging/src/k8s.io/apiserver/pkg/storage/etcd3/testing:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library",
         "//staging/src/k8s.io/client-go/tools/cache:go_default_library",
+        "//vendor/github.com/go-openapi/spec:go_default_library",
         "//vendor/sigs.k8s.io/yaml:go_default_library",
     ],
 )

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

@@ -1320,7 +1320,8 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *goopenapispec.Swagger, crd *a
 
 	specs := []*goopenapispec.Swagger{}
 	for _, v := range crd.Spec.Versions {
-		s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: true, StripValueValidation: true, StripNullable: true, AllowNonStructural: false})
+		// Defaults are not pruned here, but before being served.
+		s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripValueValidation: true, StripNullable: true, AllowNonStructural: false})
 		if err != nil {
 			return nil, err
 		}

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

@@ -25,12 +25,14 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
+	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/go-openapi/spec"
-
 	"sigs.k8s.io/yaml"
 
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -761,52 +763,94 @@ func Test_defaultDeprecationWarning(t *testing.T) {
 func TestBuildOpenAPIModelsForApply(t *testing.T) {
 	// This is a list of validation that we expect to work.
 	tests := []apiextensionsv1.CustomResourceValidation{
-		apiextensionsv1.CustomResourceValidation{
+		{
 			OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
 				Type:       "object",
 				Properties: map[string]apiextensionsv1.JSONSchemaProps{"num": {Type: "integer", Description: "v1beta1 num field"}},
 			},
 		},
-		apiextensionsv1.CustomResourceValidation{
+		{
 			OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
 				Type:         "",
 				XIntOrString: true,
 			},
 		},
+		{
+			OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
+				Type: "object",
+				Properties: map[string]apiextensionsv1.JSONSchemaProps{
+					"oneOf": {
+						OneOf: []apiextensionsv1.JSONSchemaProps{
+							{Type: "boolean"},
+							{Type: "string"},
+						},
+					},
+				},
+			},
+		},
+		{
+			OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
+				Type: "object",
+				Properties: map[string]apiextensionsv1.JSONSchemaProps{
+					"nullable": {
+						Type:     "integer",
+						Nullable: true,
+					},
+				},
+			},
+		},
+	}
+
+	staticSpec, err := getOpenAPISpecFromFile()
+	if err != nil {
+		t.Fatalf("Failed to load openapi spec: %v", err)
+	}
+
+	crd := apiextensionsv1.CustomResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "example.stable.example.com", UID: types.UID("12345")},
+		Spec: apiextensionsv1.CustomResourceDefinitionSpec{
+			Group: "stable.example.com",
+			Names: apiextensionsv1.CustomResourceDefinitionNames{
+				Plural: "examples", Singular: "example", Kind: "Example", ShortNames: []string{"ex"}, ListKind: "ExampleList", Categories: []string{"all"},
+			},
+			Conversion:            &apiextensionsv1.CustomResourceConversion{Strategy: apiextensionsv1.NoneConverter},
+			Scope:                 apiextensionsv1.ClusterScoped,
+			PreserveUnknownFields: false,
+			Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
+				{
+					Name: "v1beta1", Served: true, Storage: true,
+					Subresources: &apiextensionsv1.CustomResourceSubresources{Status: &apiextensionsv1.CustomResourceSubresourceStatus{}},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
-		crd := apiextensionsv1.CustomResourceDefinition{
+		crd.Spec.Versions[0].Schema = &test
-			ObjectMeta: metav1.ObjectMeta{Name: "example.stable.example.com", UID: types.UID("12345")},
+		if _, err := buildOpenAPIModelsForApply(staticSpec, &crd); err != nil {
-			Spec: apiextensionsv1.CustomResourceDefinitionSpec{
-				Group: "stable.example.com",
-				Names: apiextensionsv1.CustomResourceDefinitionNames{
-					Plural: "examples", Singular: "example", Kind: "Example", ShortNames: []string{"ex"}, ListKind: "ExampleList", Categories: []string{"all"},
-				},
-				Conversion:            &apiextensionsv1.CustomResourceConversion{Strategy: apiextensionsv1.NoneConverter},
-				Scope:                 apiextensionsv1.ClusterScoped,
-				PreserveUnknownFields: false,
-				Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
-					{
-						Name: "v1beta1", Served: true, Storage: true,
-						Subresources: &apiextensionsv1.CustomResourceSubresources{Status: &apiextensionsv1.CustomResourceSubresourceStatus{}},
-						Schema:       &test,
-					},
-				},
-			},
-		}
-		if _, err := buildOpenAPIModelsForApply(&spec.Swagger{
-			SwaggerProps: spec.SwaggerProps{
-				Info: &spec.Info{
-					InfoProps: spec.InfoProps{
-						Title:   "title",
-						Version: "v1",
-					},
-				},
-				Swagger: "2.0",
-			},
-		}, &crd); err != nil {
 			t.Fatalf("failed to convert to apply model: %v", err)
 		}
 	}
 }
+
+func getOpenAPISpecFromFile() (*spec.Swagger, error) {
+	path := filepath.Join(
+		strings.Repeat(".."+string(filepath.Separator), 6),
+		"api", "openapi-spec", "swagger.json")
+	_, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+	byteSpec, err := ioutil.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	staticSpec := &spec.Swagger{}
+
+	err = yaml.Unmarshal(byteSpec, staticSpec)
+	if err != nil {
+		return nil, err
+	}
+
+	return staticSpec, nil
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/openapi/builder/builder.go

@@ -103,14 +103,15 @@ func BuildSwagger(crd *apiextensionsv1.CustomResourceDefinition, version string,
 				if opts.AllowNonStructural || len(structuralschema.ValidateStructural(nil, ss)) == 0 {
 					schema = ss
 
+					// This adds ValueValidation fields (anyOf, allOf) which may be stripped below if opts.StripValueValidation is true
+					schema = schema.Unfold()
+
 					if opts.StripValueValidation {
 						schema = schema.StripValueValidations()
 					}
 					if opts.StripNullable {
 						schema = schema.StripNullable()
 					}
-
-					schema = schema.Unfold()
 				}
 			}
 		}

test/integration/apiserver/apply/apply_crd_test.go

@@ -469,162 +469,6 @@ spec:
 	}
 }
 
-// TestApplyCRDNonStructuralSchema tests that when a CRD has a non-structural schema in its validation field,
-// it will be used to construct the CR schema used by apply, but any non-structural parts of the schema will be treated as
-// nested maps (same as a CRD without a schema)
-func TestApplyCRDNonStructuralSchema(t *testing.T) {
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ServerSideApply, true)()
-
-	server, err := apiservertesting.StartTestServer(t, apiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd())
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer server.TearDownFn()
-	config := server.ClientConfig
-
-	apiExtensionClient, err := clientset.NewForConfig(config)
-	if err != nil {
-		t.Fatal(err)
-	}
-	dynamicClient, err := dynamic.NewForConfig(config)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	noxuDefinition := fixtures.NewNoxuCustomResourceDefinition(apiextensionsv1beta1.ClusterScoped)
-
-	var c apiextensionsv1beta1.CustomResourceValidation
-	err = json.Unmarshal([]byte(`{
-		"openAPIV3Schema": {
-			"type": "object",
-			"properties": {
-				"spec": {
-					"anyOf": [
-						{
-							"type": "object",
-							"properties": {
-								"cronSpec": {
-									"type": "string",
-									"pattern": "^(\\d+|\\*)(/\\d+)?(\\s+(\\d+|\\*)(/\\d+)?){4}$"
-								}
-							}
-						}, {
-							"type": "string"
-						}
-					]
-				}
-			}
-		}
-	}`), &c)
-	if err != nil {
-		t.Fatal(err)
-	}
-	noxuDefinition.Spec.Validation = &c
-
-	noxuDefinition, err = fixtures.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	kind := noxuDefinition.Spec.Names.Kind
-	apiVersion := noxuDefinition.Spec.Group + "/" + noxuDefinition.Spec.Version
-	name := "mytest"
-
-	rest := apiExtensionClient.Discovery().RESTClient()
-	yamlBody := []byte(fmt.Sprintf(`
-apiVersion: %s
-kind: %s
-metadata:
-  name: %s
-  finalizers:
-  - test-finalizer
-spec:
-  cronSpec: "* * * * */5"
-  replicas: 1`, apiVersion, kind, name))
-	result, err := rest.Patch(types.ApplyPatchType).
-		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
-		Name(name).
-		Param("fieldManager", "apply_test").
-		Body(yamlBody).
-		DoRaw(context.TODO())
-	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":["test-finalizer","another-one"]}}`)).
-		DoRaw(context.TODO())
-	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(context.TODO())
-	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).
-		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
-		Name(name).
-		Body([]byte(`{"spec":{"replicas": 5}}`)).
-		DoRaw(context.TODO())
-	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).
-		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
-		Name(name).
-		Param("fieldManager", "apply_test").
-		Body(yamlBody).
-		DoRaw(context.TODO())
-	if err == nil {
-		t.Fatalf("Expecting to get conflicts when applying object after updating replicas, got no error: %s", result)
-	}
-	status, ok := err.(*apierrors.StatusError)
-	if !ok {
-		t.Fatalf("Expecting to get conflicts as API error")
-	}
-	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.
-	result, err = rest.Patch(types.ApplyPatchType).
-		AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).
-		Name(name).
-		Param("force", "true").
-		Param("fieldManager", "apply_test").
-		Body(yamlBody).
-		DoRaw(context.TODO())
-	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{}