Merge pull request #97172 from Jefftree/opentest

Fix openapi model generation for apply when x-int-or-string is set
This commit is contained in:
Kubernetes Prow Robot 2021-01-12 16:02:35 -08:00 committed by GitHub
commit 70435a298c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 107 additions and 159 deletions

View File

@ -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",
],
)

View File

@ -1321,7 +1321,7 @@ func buildOpenAPIModelsForApply(staticOpenAPISpec *goopenapispec.Swagger, crd *a
specs := []*goopenapispec.Swagger{}
for _, v := range crd.Spec.Versions {
// 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: true})
s, err := builder.BuildSwagger(crd, v.Name, builder.Options{V2: false, StripValueValidation: true, StripNullable: true, AllowNonStructural: false})
if err != nil {
return nil, err
}

View File

@ -25,10 +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"
@ -755,3 +759,98 @@ 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{
{
OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensionsv1.JSONSchemaProps{"num": {Type: "integer", Description: "v1beta1 num field"}},
},
},
{
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.Spec.Versions[0].Schema = &test
if _, err := buildOpenAPIModelsForApply(staticSpec, &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
}

View File

@ -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()
}
}
}

View File

@ -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{}