diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 9c456937612..bff4962b924 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -644,27 +644,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd structuralSchemas[v.Name] = s } - var openAPIModels proto.Models - if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) && r.staticOpenAPISpec != nil { - specs := []*spec.Swagger{} - for _, v := range crd.Spec.Versions { - s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true}) - if err != nil { - utilruntime.HandleError(err) - return nil, fmt.Errorf("the server could not properly serve the CR schema") - } - specs = append(specs, s) - } - 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) - return nil, fmt.Errorf("the server could not properly serve the CR schema") - } + openAPIModels, err := buildOpenAPIModelsForApply(r.staticOpenAPISpec, crd) + if err != nil { + utilruntime.HandleError(fmt.Errorf("error building openapi models for %s: %v", crd.Name, err)) + openAPIModels = nil } for _, v := range crd.Spec.Versions { @@ -1239,3 +1222,35 @@ func serverStartingError() error { } return err } + +// buildOpenAPIModelsForApply constructs openapi models from any validation schemas specified in the custom resource, +// and merges it with the models defined in the static OpenAPI spec. +// Returns nil models if the ServerSideApply feature is disabled, or the static spec is nil, or an error is encountered. +func buildOpenAPIModelsForApply(staticOpenAPISpec *spec.Swagger, crd *apiextensions.CustomResourceDefinition) (proto.Models, error) { + if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { + return nil, nil + } + if staticOpenAPISpec == nil { + return nil, nil + } + + specs := []*spec.Swagger{} + for _, v := range crd.Spec.Versions { + s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripDefaults: true, StripValueValidation: true}) + if err != nil { + return nil, err + } + specs = append(specs, s) + } + + mergedOpenAPI, err := builder.MergeSpecs(staticOpenAPISpec, specs...) + if err != nil { + return nil, err + } + + models, err := utilopenapi.ToProtoModels(mergedOpenAPI) + if err != nil { + return nil, err + } + return models, nil +} diff --git a/test/integration/apiserver/apply/apply_crd_test.go b/test/integration/apiserver/apply/apply_crd_test.go index 83796feaa51..2e1bc1a91bb 100644 --- a/test/integration/apiserver/apply/apply_crd_test.go +++ b/test/integration/apiserver/apply/apply_crd_test.go @@ -607,3 +607,114 @@ func verifyNumPorts(t *testing.T, b []byte, n int) { t.Fatalf("expected %v ports but got %v:\n%v", expected, actual, string(b)) } } + +// TestApplyCRDUnhandledSchema tests that when a CRD has a schema that kube-openapi ToProtoModels cannot handle correctly, +// apply falls back to non-schema behavior +func TestApplyCRDUnhandledSchema(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) + + // This is a schema that kube-openapi ToProtoModels does not handle correctly. + // https://github.com/kubernetes/kubernetes/blob/38752f7f99869ed65fb44378360a517649dc2f83/vendor/k8s.io/kube-openapi/pkg/util/proto/document.go#L184 + var c apiextensionsv1beta1.CustomResourceValidation + err = json.Unmarshal([]byte(`{ + "openAPIV3Schema": { + "properties": { + "TypeFooBar": { + "type": "array" + } + } + } + }`), &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 +spec: + 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() + 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). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Name(name). + Body([]byte(`{"spec":{"replicas": 5}}`)). + DoRaw() + 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). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Name(name). + Param("fieldManager", "apply_test"). + Body(yamlBody). + DoRaw() + if err == nil { + t.Fatalf("Expecting to get conflicts when applying object after updating replicas, got no error: %s", result) + } + status, ok := err.(*errors.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() + if err != nil { + t.Fatalf("failed to apply object with force after updating replicas: %v:\n%v", err, string(result)) + } + verifyReplicas(t, result, 1) +}