diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go index 3ad56720340..d9827ca4cb8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go @@ -35,14 +35,15 @@ type CRConverterFactory struct { // webhookConverterFactory is the factory for webhook converters. // This field should not be used if CustomResourceWebhookConversion feature is disabled. webhookConverterFactory *webhookConverterFactory - converterMetricFactory *converterMetricFactory } +// converterMetricFactorySingleton protects us from reregistration of metrics on repeated +// apiextensions-apiserver runs. +var converterMetricFactorySingleton = newConverterMertricFactory() + // NewCRConverterFactory creates a new CRConverterFactory func NewCRConverterFactory(serviceResolver webhook.ServiceResolver, authResolverWrapper webhook.AuthenticationInfoResolverWrapper) (*CRConverterFactory, error) { - converterFactory := &CRConverterFactory{ - converterMetricFactory: newConverterMertricFactory(), - } + converterFactory := &CRConverterFactory{} if utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CustomResourceWebhookConversion) { webhookConverterFactory, err := newWebhookConverterFactory(serviceResolver, authResolverWrapper) if err != nil { @@ -72,7 +73,7 @@ func (m *CRConverterFactory) NewConverter(crd *apiextensions.CustomResourceDefin if err != nil { return nil, nil, err } - converter, err = m.converterMetricFactory.addMetrics("webhook", crd.Name, converter) + converter, err = converterMetricFactorySingleton.addMetrics("webhook", crd.Name, converter) if err != nil { return nil, nil, err } 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 c9088ad4962..b977de65210 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 @@ -493,6 +493,23 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource statusScopes := map[string]*handlers.RequestScope{} scaleScopes := map[string]*handlers.RequestScope{} + structuralSchemas := map[string]*structuralschema.Structural{} + for _, v := range crd.Spec.Versions { + val, err := apiextensions.GetSchemaForVersion(crd, v.Name) + if err != nil { + utilruntime.HandleError(err) + return nil, fmt.Errorf("the server could not properly serve the CR schema") + } + if val == nil { + continue + } + structuralSchemas[v.Name], err = structuralschema.NewStructural(val.OpenAPIV3Schema) + if *crd.Spec.PreserveUnknownFields == false && err != nil { + utilruntime.HandleError(err) + return nil, fmt.Errorf("the server could not properly serve the CR schema") // validation should avoid this + } + } + for _, v := range crd.Spec.Versions { safeConverter, unsafeConverter, err := r.converterFactory.NewConverter(crd) if err != nil { @@ -529,14 +546,6 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource return nil, fmt.Errorf("unexpected nil spec.preserveUnknownFields in the CustomResourceDefinition") } - var structuralSchema *structuralschema.Structural - if validationSchema != nil { - structuralSchema, err = structuralschema.NewStructural(validationSchema.OpenAPIV3Schema) - if *crd.Spec.PreserveUnknownFields == false && err != nil { - return nil, err // validation should avoid this - } - } - var statusSpec *apiextensions.CustomResourceSubresourceStatus var statusValidator *validate.SchemaValidator subresources, err := apiextensions.GetSubresourcesForVersion(crd, v.Name) @@ -591,7 +600,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource converter: safeConverter, decoderVersion: schema.GroupVersion{Group: crd.Spec.Group, Version: v.Name}, encoderVersion: schema.GroupVersion{Group: crd.Spec.Group, Version: storageVersion}, - structuralSchema: structuralSchema, + structuralSchemas: structuralSchemas, structuralSchemaGK: kind.GroupKind(), preserveUnknownFields: *crd.Spec.PreserveUnknownFields, }, @@ -619,7 +628,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource typer: typer, creator: creator, converter: safeConverter, - structuralSchema: structuralSchema, + structuralSchemas: structuralSchemas, structuralSchemaGK: kind.GroupKind(), preserveUnknownFields: *crd.Spec.PreserveUnknownFields, }, @@ -676,7 +685,7 @@ func (r *crdHandler) getOrCreateServingInfoFor(crd *apiextensions.CustomResource statusScope.Serializer = unstructuredNegotiatedSerializer{ typer: typer, creator: creator, converter: safeConverter, - structuralSchema: structuralSchema, + structuralSchemas: structuralSchemas, structuralSchemaGK: kind.GroupKind(), preserveUnknownFields: *crd.Spec.PreserveUnknownFields, } @@ -715,7 +724,7 @@ type unstructuredNegotiatedSerializer struct { creator runtime.ObjectCreater converter runtime.ObjectConvertor - structuralSchema *structuralschema.Structural + structuralSchemas map[string]*structuralschema.Structural // by version structuralSchemaGK schema.GroupKind preserveUnknownFields bool } @@ -750,7 +759,7 @@ func (s unstructuredNegotiatedSerializer) EncoderForVersion(encoder runtime.Enco } func (s unstructuredNegotiatedSerializer) DecoderToVersion(decoder runtime.Decoder, gv runtime.GroupVersioner) runtime.Decoder { - d := schemaCoercingDecoder{delegate: decoder, validator: unstructuredSchemaCoercer{structuralSchema: s.structuralSchema, structuralSchemaGK: s.structuralSchemaGK, preserveUnknownFields: s.preserveUnknownFields}} + d := schemaCoercingDecoder{delegate: decoder, validator: unstructuredSchemaCoercer{structuralSchemas: s.structuralSchemas, structuralSchemaGK: s.structuralSchemaGK, preserveUnknownFields: s.preserveUnknownFields}} return versioning.NewDefaultingCodecForScheme(Scheme, nil, d, nil, gv) } @@ -842,7 +851,7 @@ type crdConversionRESTOptionsGetter struct { converter runtime.ObjectConvertor encoderVersion schema.GroupVersion decoderVersion schema.GroupVersion - structuralSchema *structuralschema.Structural + structuralSchemas map[string]*structuralschema.Structural // by version structuralSchemaGK schema.GroupKind preserveUnknownFields bool } @@ -853,12 +862,12 @@ func (t crdConversionRESTOptionsGetter) GetRESTOptions(resource schema.GroupReso d := schemaCoercingDecoder{delegate: ret.StorageConfig.Codec, validator: unstructuredSchemaCoercer{ // drop invalid fields while decoding old CRs (before we haven't had any ObjectMeta validation) dropInvalidMetadata: true, - structuralSchema: t.structuralSchema, + structuralSchemas: t.structuralSchemas, structuralSchemaGK: t.structuralSchemaGK, preserveUnknownFields: t.preserveUnknownFields, }} c := schemaCoercingConverter{delegate: t.converter, validator: unstructuredSchemaCoercer{ - structuralSchema: t.structuralSchema, + structuralSchemas: t.structuralSchemas, structuralSchemaGK: t.structuralSchemaGK, preserveUnknownFields: t.preserveUnknownFields, }} @@ -950,7 +959,7 @@ func (v schemaCoercingConverter) ConvertFieldLabel(gvk schema.GroupVersionKind, type unstructuredSchemaCoercer struct { dropInvalidMetadata bool - structuralSchema *structuralschema.Structural + structuralSchemas map[string]*structuralschema.Structural structuralSchemaGK schema.GroupKind preserveUnknownFields bool } @@ -976,7 +985,7 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) error { return err } if !v.preserveUnknownFields && gv.Group == v.structuralSchemaGK.Group && kind == v.structuralSchemaGK.Kind { - structuralpruning.Prune(u.Object, v.structuralSchema) + structuralpruning.Prune(u.Object, v.structuralSchemas[gv.Version]) } // restore meta fields, starting clean diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD index c8e993dd5fc..252307c8e06 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD @@ -12,7 +12,6 @@ go_test( "apply_test.go", "basic_test.go", "change_test.go", - "conversion_test.go", "finalization_test.go", "objectmeta_test.go", "pruning_test.go", @@ -32,9 +31,7 @@ go_test( "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/test/integration/convert:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures:go_default_library", - "//staging/src/k8s.io/apiextensions-apiserver/test/integration/storage:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", @@ -45,13 +42,11 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/yaml:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/storage/etcd3:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", @@ -76,7 +71,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", - "//staging/src/k8s.io/apiextensions-apiserver/test/integration/convert:all-srcs", + "//staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures:all-srcs", "//staging/src/k8s.io/apiextensions-apiserver/test/integration/storage:all-srcs", ], diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD new file mode 100644 index 00000000000..4468e4bff43 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD @@ -0,0 +1,61 @@ +package(default_visibility = ["//visibility:public"]) + +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_test( + name = "go_default_test", + srcs = ["conversion_test.go"], + embed = [":go_default_library"], + tags = ["integration"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/cmd/server/options:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/test/integration/storage:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/storage/etcd3:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/client-go/dynamic:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", + ], +) + +go_library( + name = "go_default_library", + srcs = ["webhook.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/test/integration/conversion", + importpath = "k8s.io/apiextensions-apiserver/test/integration/conversion", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go new file mode 100644 index 00000000000..192f0b9aada --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go @@ -0,0 +1,898 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package conversion + +import ( + "encoding/json" + "fmt" + "net/http" + "reflect" + "strings" + "sync" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" + serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" + apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apimachinery/pkg/util/wait" + etcd3watcher "k8s.io/apiserver/pkg/storage/etcd3" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/dynamic" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/utils/pointer" + + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + "k8s.io/apiextensions-apiserver/test/integration/storage" +) + +type Checker func(t *testing.T, ctc *conversionTestContext) + +func checks(checkers ...Checker) []Checker { + return checkers +} + +func TestWebhookConverter(t *testing.T) { + testWebhookConverter(t, false) +} + +func TestWebhookConverterWithPruning(t *testing.T) { + testWebhookConverter(t, true) +} + +func testWebhookConverter(t *testing.T, pruning bool) { + tests := []struct { + group string + handler http.Handler + checks []Checker + }{ + { + group: "noop-converter", + handler: NewObjectConverterWebhookHandler(t, noopConverter), + checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1")), // no v1beta2 as the schema differs + }, + { + group: "nontrivial-converter", + handler: NewObjectConverterWebhookHandler(t, nontrivialConverter), + checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1", "v1beta2"), validateNonTrivialConverted, validateNonTrivialConvertedList, validateStoragePruning), + }, + { + group: "empty-response", + handler: NewReviewWebhookHandler(t, emptyResponseConverter), + checks: checks(expectConversionFailureMessage("empty-response", "expected 1 converted objects")), + }, + { + group: "failure-message", + handler: NewReviewWebhookHandler(t, failureResponseConverter("custom webhook conversion error")), + checks: checks(expectConversionFailureMessage("failure-message", "custom webhook conversion error")), + }, + } + + // TODO: Added for integration testing of conversion webhooks, where decode errors due to conversion webhook failures need to be tested. + // Maybe we should identify conversion webhook related errors in decoding to avoid triggering this? Or maybe having this special casing + // of test cases in production code should be removed? + etcd3watcher.TestOnlySetFatalOnDecodeError(false) + defer etcd3watcher.TestOnlySetFatalOnDecodeError(true) + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, true)() + tearDown, config, options, err := fixtures.StartDefaultServer(t) + if err != nil { + t.Fatal(err) + } + + apiExtensionsClient, err := clientset.NewForConfig(config) + if err != nil { + tearDown() + t.Fatal(err) + } + + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + tearDown() + t.Fatal(err) + } + defer tearDown() + + crd := multiVersionFixture.DeepCopy() + crd.Spec.PreserveUnknownFields = pointer.BoolPtr(!pruning) + + RESTOptionsGetter := serveroptions.NewCRDRESTOptionsGetter(*options.RecommendedOptions.Etcd) + restOptions, err := RESTOptionsGetter.GetRESTOptions(schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Spec.Names.Plural}) + if err != nil { + t.Fatal(err) + } + etcdClient, _, err := storage.GetEtcdClients(restOptions.StorageConfig.Transport) + if err != nil { + t.Fatal(err) + } + defer etcdClient.Close() + + etcdObjectReader := storage.NewEtcdObjectReader(etcdClient, &restOptions, crd) + ctcTearDown, ctc := newConversionTestContext(t, apiExtensionsClient, dynamicClient, etcdObjectReader, crd) + defer ctcTearDown() + + // read only object to read at a different version than stored when we need to force conversion + marker, err := ctc.versionedClient("marker", "v1beta1").Create(newConversionMultiVersionFixture("marker", "marker", "v1beta1"), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + for _, test := range tests { + t.Run(test.group, func(t *testing.T) { + upCh, handler := closeOnCall(test.handler) + tearDown, webhookClientConfig, err := StartConversionWebhookServer(handler) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + ctc.setConversionWebhook(t, webhookClientConfig) + defer ctc.removeConversionWebhook(t) + + // wait until new webhook is called the first time + if err := wait.PollImmediate(time.Millisecond*100, wait.ForeverTestTimeout, func() (bool, error) { + _, err := ctc.versionedClient(marker.GetNamespace(), "v1alpha1").Get(marker.GetName(), metav1.GetOptions{}) + select { + case <-upCh: + return true, nil + default: + t.Logf("Waiting for webhook to become effective, getting marker object: %v", err) + return false, nil + } + }); err != nil { + t.Fatal(err) + } + + for i, checkFn := range test.checks { + name := fmt.Sprintf("check-%d", i) + t.Run(name, func(t *testing.T) { + defer ctc.setAndWaitStorageVersion(t, "v1beta1") + ctc.namespace = fmt.Sprintf("webhook-conversion-%s-%s", test.group, name) + checkFn(t, ctc) + }) + } + }) + } +} + +func validateStorageVersion(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + + for _, version := range ctc.crd.Spec.Versions { + t.Run(version.Name, func(t *testing.T) { + name := "storageversion-" + version.Name + client := ctc.versionedClient(ns, version.Name) + obj, err := client.Create(newConversionMultiVersionFixture(ns, name, version.Name), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + ctc.setAndWaitStorageVersion(t, "v1beta2") + + obj, err = client.Get(obj.GetName(), metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + ctc.setAndWaitStorageVersion(t, "v1beta1") + }) + } +} + +// validateMixedStorageVersions ensures that identical custom resources written at different storage versions +// are readable and remain the same. +func validateMixedStorageVersions(versions ...string) func(t *testing.T, ctc *conversionTestContext) { + return func(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + clients := ctc.versionedClients(ns) + + // Create CRs at all storage versions + objNames := []string{} + for _, version := range versions { + ctc.setAndWaitStorageVersion(t, version) + + name := "mixedstorage-stored-as-" + version + obj, err := clients[version].Create(newConversionMultiVersionFixture(ns, name, version), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + objNames = append(objNames, obj.GetName()) + } + + // Ensure copies of an object have the same fields and values at each custom resource definition version regardless of storage version + for clientVersion, client := range clients { + t.Run(clientVersion, func(t *testing.T) { + o1, err := client.Get(objNames[0], metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for _, objName := range objNames[1:] { + o2, err := client.Get(objName, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + // ignore metadata for comparison purposes + delete(o1.Object, "metadata") + delete(o2.Object, "metadata") + if !reflect.DeepEqual(o1.Object, o2.Object) { + t.Errorf("Expected custom resource to be same regardless of which storage version is used to create, but got: %s", cmp.Diff(o1, o2)) + } + } + }) + } + } +} + +func validateServed(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + + for _, version := range ctc.crd.Spec.Versions { + t.Run(version.Name, func(t *testing.T) { + name := "served-" + version.Name + client := ctc.versionedClient(ns, version.Name) + obj, err := client.Create(newConversionMultiVersionFixture(ns, name, version.Name), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + ctc.setServed(t, version.Name, false) + ctc.waitForServed(t, version.Name, false, client, obj) + ctc.setServed(t, version.Name, true) + ctc.waitForServed(t, version.Name, true, client, obj) + }) + } +} + +func validateNonTrivialConverted(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + + for _, createVersion := range ctc.crd.Spec.Versions { + t.Run(fmt.Sprintf("getting objects created as %s", createVersion.Name), func(t *testing.T) { + name := "converted-" + createVersion.Name + client := ctc.versionedClient(ns, createVersion.Name) + + fixture := newConversionMultiVersionFixture(ns, name, createVersion.Name) + if !*ctc.crd.Spec.PreserveUnknownFields { + if err := unstructured.SetNestedField(fixture.Object, "foo", "garbage"); err != nil { + t.Fatal(err) + } + } + if _, err := client.Create(fixture, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + + // verify that the right, pruned version is in storage + obj, err := ctc.etcdObjectReader.GetStoredCustomResource(ns, name) + if err != nil { + t.Fatal(err) + } + verifyMultiVersionObject(t, "v1beta1", obj) + + for _, getVersion := range ctc.crd.Spec.Versions { + client := ctc.versionedClient(ns, getVersion.Name) + obj, err := client.Get(name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + verifyMultiVersionObject(t, getVersion.Name, obj) + } + }) + } +} + +func validateNonTrivialConvertedList(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + "-list" + + names := sets.String{} + for _, createVersion := range ctc.crd.Spec.Versions { + name := "converted-" + createVersion.Name + client := ctc.versionedClient(ns, createVersion.Name) + fixture := newConversionMultiVersionFixture(ns, name, createVersion.Name) + if !*ctc.crd.Spec.PreserveUnknownFields { + if err := unstructured.SetNestedField(fixture.Object, "foo", "garbage"); err != nil { + t.Fatal(err) + } + } + _, err := client.Create(fixture, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + names.Insert(name) + } + + for _, listVersion := range ctc.crd.Spec.Versions { + t.Run(fmt.Sprintf("listing objects as %s", listVersion.Name), func(t *testing.T) { + client := ctc.versionedClient(ns, listVersion.Name) + obj, err := client.List(metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + if len(obj.Items) != len(ctc.crd.Spec.Versions) { + t.Fatal("unexpected number of items") + } + foundNames := sets.String{} + for _, u := range obj.Items { + foundNames.Insert(u.GetName()) + verifyMultiVersionObject(t, listVersion.Name, &u) + } + if !foundNames.Equal(names) { + t.Errorf("unexpected set of returned items: %s", foundNames.Difference(names)) + } + }) + } +} + +func validateStoragePruning(t *testing.T, ctc *conversionTestContext) { + if *ctc.crd.Spec.PreserveUnknownFields { + return + } + + ns := ctc.namespace + + for _, createVersion := range ctc.crd.Spec.Versions { + t.Run(fmt.Sprintf("getting objects created as %s", createVersion.Name), func(t *testing.T) { + name := "storagepruning-" + createVersion.Name + client := ctc.versionedClient(ns, createVersion.Name) + + fixture := newConversionMultiVersionFixture(ns, name, createVersion.Name) + if err := unstructured.SetNestedField(fixture.Object, "foo", "garbage"); err != nil { + t.Fatal(err) + } + _, err := client.Create(fixture, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + // verify that the right, pruned version is in storage + obj, err := ctc.etcdObjectReader.GetStoredCustomResource(ns, name) + if err != nil { + t.Fatal(err) + } + verifyMultiVersionObject(t, "v1beta1", obj) + + // add garbage and set a label + if err := unstructured.SetNestedField(obj.Object, "foo", "garbage"); err != nil { + t.Fatal(err) + } + labels := obj.GetLabels() + if labels == nil { + labels = map[string]string{} + } + labels["mutated"] = "true" + obj.SetLabels(labels) + if err := ctc.etcdObjectReader.SetStoredCustomResource(ns, name, obj); err != nil { + t.Fatal(err) + } + + for _, getVersion := range ctc.crd.Spec.Versions { + client := ctc.versionedClient(ns, getVersion.Name) + obj, err := client.Get(name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + // check that the direct mutation in etcd worked + labels := obj.GetLabels() + if labels["mutated"] != "true" { + t.Errorf("expected object %s in version %s to have label 'mutated=true'", name, getVersion.Name) + } + + verifyMultiVersionObject(t, getVersion.Name, obj) + } + }) + } +} + +func expectConversionFailureMessage(id, message string) func(t *testing.T, ctc *conversionTestContext) { + return func(t *testing.T, ctc *conversionTestContext) { + ns := ctc.namespace + clients := ctc.versionedClients(ns) + var err error + // storage version is v1beta1, so this skips conversion + obj, err := clients["v1beta1"].Create(newConversionMultiVersionFixture(ns, id, "v1beta1"), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + for _, verb := range []string{"get", "list", "create", "udpate", "patch", "delete", "deletecollection"} { + t.Run(verb, func(t *testing.T) { + switch verb { + case "get": + _, err = clients["v1beta2"].Get(obj.GetName(), metav1.GetOptions{}) + case "list": + _, err = clients["v1beta2"].List(metav1.ListOptions{}) + case "create": + _, err = clients["v1beta2"].Create(newConversionMultiVersionFixture(ns, id, "v1beta2"), metav1.CreateOptions{}) + case "update": + _, err = clients["v1beta2"].Update(obj, metav1.UpdateOptions{}) + case "patch": + _, err = clients["v1beta2"].Patch(obj.GetName(), types.MergePatchType, []byte(`{"metadata":{"annotations":{"patch":"true"}}}`), metav1.PatchOptions{}) + case "delete": + err = clients["v1beta2"].Delete(obj.GetName(), &metav1.DeleteOptions{}) + case "deletecollection": + err = clients["v1beta2"].DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{}) + } + + if err == nil { + t.Errorf("expected error with message %s, but got no error", message) + } else if !strings.Contains(err.Error(), message) { + t.Errorf("expected error with message %s, but got %v", message, err) + } + }) + } + for _, subresource := range []string{"status", "scale"} { + for _, verb := range []string{"get", "udpate", "patch"} { + t.Run(fmt.Sprintf("%s-%s", subresource, verb), func(t *testing.T) { + switch verb { + case "create": + _, err = clients["v1beta2"].Create(newConversionMultiVersionFixture(ns, id, "v1beta2"), metav1.CreateOptions{}, subresource) + case "update": + _, err = clients["v1beta2"].Update(obj, metav1.UpdateOptions{}, subresource) + case "patch": + _, err = clients["v1beta2"].Patch(obj.GetName(), types.MergePatchType, []byte(`{"metadata":{"annotations":{"patch":"true"}}}`), metav1.PatchOptions{}, subresource) + } + + if err == nil { + t.Errorf("expected error with message %s, but got no error", message) + } else if !strings.Contains(err.Error(), message) { + t.Errorf("expected error with message %s, but got %v", message, err) + } + }) + } + } + } +} + +func noopConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(obj.Raw, u); err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to deserialize object: %s with error: %v", string(obj.Raw), err) + } + u.Object["apiVersion"] = desiredAPIVersion + raw, err := json.Marshal(u) + if err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to serialize object: %v with error: %v", u, err) + } + return runtime.RawExtension{Raw: raw}, nil +} + +func emptyResponseConverter(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { + review.Response = &apiextensionsv1beta1.ConversionResponse{ + UID: review.Request.UID, + ConvertedObjects: []runtime.RawExtension{}, + Result: metav1.Status{Status: "Success"}, + } + return review, nil +} + +func failureResponseConverter(message string) func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { + return func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { + review.Response = &apiextensionsv1beta1.ConversionResponse{ + UID: review.Request.UID, + ConvertedObjects: []runtime.RawExtension{}, + Result: metav1.Status{Message: message, Status: "Failure"}, + } + return review, nil + } +} + +func nontrivialConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { + u := &unstructured.Unstructured{Object: map[string]interface{}{}} + if err := json.Unmarshal(obj.Raw, u); err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to deserialize object: %s with error: %v", string(obj.Raw), err) + } + + currentAPIVersion := u.GetAPIVersion() + + if currentAPIVersion == "stable.example.com/v1beta2" && (desiredAPIVersion == "stable.example.com/v1alpha1" || desiredAPIVersion == "stable.example.com/v1beta1") { + u.Object["num"] = u.Object["numv2"] + u.Object["content"] = u.Object["contentv2"] + delete(u.Object, "numv2") + delete(u.Object, "contentv2") + } else if (currentAPIVersion == "stable.example.com/v1alpha1" || currentAPIVersion == "stable.example.com/v1beta1") && desiredAPIVersion == "stable.example.com/v1beta2" { + u.Object["numv2"] = u.Object["num"] + u.Object["contentv2"] = u.Object["content"] + delete(u.Object, "num") + delete(u.Object, "content") + } else if currentAPIVersion == "stable.example.com/v1alpha1" && desiredAPIVersion == "stable.example.com/v1beta1" { + // same schema + } else if currentAPIVersion == "stable.example.com/v1beta1" && desiredAPIVersion == "stable.example.com/v1alpha1" { + // same schema + } else if currentAPIVersion != desiredAPIVersion { + return runtime.RawExtension{}, fmt.Errorf("cannot convert from %s to %s", currentAPIVersion, desiredAPIVersion) + } + u.Object["apiVersion"] = desiredAPIVersion + raw, err := json.Marshal(u) + if err != nil { + return runtime.RawExtension{}, fmt.Errorf("failed to serialize object: %v with error: %v", u, err) + } + return runtime.RawExtension{Raw: raw}, nil +} + +func newConversionTestContext(t *testing.T, apiExtensionsClient clientset.Interface, dynamicClient dynamic.Interface, etcdObjectReader *storage.EtcdObjectReader, crd *apiextensionsv1beta1.CustomResourceDefinition) (func(), *conversionTestContext) { + crd, err := fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionsClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + tearDown := func() { + if err := fixtures.DeleteCustomResourceDefinition(crd, apiExtensionsClient); err != nil { + t.Fatal(err) + } + } + + return tearDown, &conversionTestContext{apiExtensionsClient: apiExtensionsClient, dynamicClient: dynamicClient, crd: crd, etcdObjectReader: etcdObjectReader} +} + +type conversionTestContext struct { + namespace string + apiExtensionsClient clientset.Interface + dynamicClient dynamic.Interface + options *options.CustomResourceDefinitionsServerOptions + crd *apiextensionsv1beta1.CustomResourceDefinition + etcdObjectReader *storage.EtcdObjectReader +} + +func (c *conversionTestContext) versionedClient(ns string, version string) dynamic.ResourceInterface { + gvr := schema.GroupVersionResource{Group: c.crd.Spec.Group, Version: version, Resource: c.crd.Spec.Names.Plural} + if c.crd.Spec.Scope != apiextensionsv1beta1.ClusterScoped { + return c.dynamicClient.Resource(gvr).Namespace(ns) + } + return c.dynamicClient.Resource(gvr) +} + +func (c *conversionTestContext) versionedClients(ns string) map[string]dynamic.ResourceInterface { + ret := map[string]dynamic.ResourceInterface{} + for _, v := range c.crd.Spec.Versions { + ret[v.Name] = c.versionedClient(ns, v.Name) + } + return ret +} + +func (c *conversionTestContext) setConversionWebhook(t *testing.T, webhookClientConfig *apiextensionsv1beta1.WebhookClientConfig) { + crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + crd.Spec.Conversion = &apiextensionsv1beta1.CustomResourceConversion{ + Strategy: apiextensionsv1beta1.WebhookConverter, + WebhookClientConfig: webhookClientConfig, + } + crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) + if err != nil { + t.Fatal(err) + } + c.crd = crd + +} + +func (c *conversionTestContext) removeConversionWebhook(t *testing.T) { + crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + crd.Spec.Conversion = &apiextensionsv1beta1.CustomResourceConversion{ + Strategy: apiextensionsv1beta1.NoneConverter, + } + + crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) + if err != nil { + t.Fatal(err) + } + c.crd = crd +} + +func (c *conversionTestContext) setAndWaitStorageVersion(t *testing.T, version string) { + c.setStorageVersion(t, version) + + // create probe object. Version should be the default one to avoid webhook calls during test setup. + client := c.versionedClient("probe", "v1beta1") + name := fmt.Sprintf("probe-%v", uuid.NewUUID()) + storageProbe, err := client.Create(newConversionMultiVersionFixture("probe", name, "v1beta1"), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + // update object continuously and wait for etcd to have the target storage version. + c.waitForStorageVersion(t, version, c.versionedClient(storageProbe.GetNamespace(), "v1beta1"), storageProbe) + + err = client.Delete(name, &metav1.DeleteOptions{}) + if err != nil { + t.Fatal(err) + } +} + +func (c *conversionTestContext) setStorageVersion(t *testing.T, version string) { + crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for i, v := range crd.Spec.Versions { + crd.Spec.Versions[i].Storage = v.Name == version + } + crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) + if err != nil { + t.Fatal(err) + } + c.crd = crd +} + +func (c *conversionTestContext) waitForStorageVersion(t *testing.T, version string, versionedClient dynamic.ResourceInterface, obj *unstructured.Unstructured) *unstructured.Unstructured { + if err := c.etcdObjectReader.WaitForStorageVersion(version, obj.GetNamespace(), obj.GetName(), 30*time.Second, func() { + if _, err := versionedClient.Patch(obj.GetName(), types.MergePatchType, []byte(`{}`), metav1.PatchOptions{}); err != nil { + t.Fatalf("failed to update object: %v", err) + } + }); err != nil { + t.Fatalf("failed waiting for storage version %s: %v", version, err) + } + + t.Logf("Effective storage version: %s", version) + + return obj +} + +func (c *conversionTestContext) setServed(t *testing.T, version string, served bool) { + crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for i, v := range crd.Spec.Versions { + if v.Name == version { + crd.Spec.Versions[i].Served = served + } + } + crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) + if err != nil { + t.Fatal(err) + } + c.crd = crd +} + +func (c *conversionTestContext) waitForServed(t *testing.T, version string, served bool, versionedClient dynamic.ResourceInterface, obj *unstructured.Unstructured) { + timeout := 30 * time.Second + waitCh := time.After(timeout) + for { + obj, err := versionedClient.Get(obj.GetName(), metav1.GetOptions{}) + if (err == nil && served) || (errors.IsNotFound(err) && served == false) { + return + } + select { + case <-waitCh: + t.Fatalf("Timed out after %v waiting for CRD served=%t for version %s for %v. Last error: %v", timeout, served, version, obj, err) + case <-time.After(10 * time.Millisecond): + } + } +} + +var multiVersionFixture = &apiextensionsv1beta1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "multiversion.stable.example.com"}, + Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ + Group: "stable.example.com", + Version: "v1beta1", + Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ + Plural: "multiversion", + Singular: "multiversion", + Kind: "MultiVersion", + ShortNames: []string{"mv"}, + ListKind: "MultiVersionList", + Categories: []string{"all"}, + }, + Scope: apiextensionsv1beta1.NamespaceScoped, + Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{ + { + // storage version, same schema as v1alpha1 + Name: "v1beta1", + Served: true, + Storage: true, + Schema: &apiextensionsv1beta1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "content": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + "num": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "num1": {Type: "integer"}, + "num2": {Type: "integer"}, + }, + }, + }, + }, + }, + }, + { + // same schema as v1beta1 + Name: "v1alpha1", + Served: true, + Storage: false, + Schema: &apiextensionsv1beta1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "content": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + "num": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "num1": {Type: "integer"}, + "num2": {Type: "integer"}, + }, + }, + }, + }, + }, + }, + { + // different schema than v1beta1 and v1alpha1 + Name: "v1beta2", + Served: true, + Storage: false, + Schema: &apiextensionsv1beta1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1beta1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "contentv2": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "key": {Type: "string"}, + }, + }, + "numv2": { + Type: "object", + Properties: map[string]apiextensionsv1beta1.JSONSchemaProps{ + "num1": {Type: "integer"}, + "num2": {Type: "integer"}, + }, + }, + }, + }, + }, + }, + }, + Subresources: &apiextensionsv1beta1.CustomResourceSubresources{ + Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{}, + Scale: &apiextensionsv1beta1.CustomResourceSubresourceScale{ + SpecReplicasPath: ".spec.num.num1", + StatusReplicasPath: ".status.num.num2", + }, + }, + }, +} + +func newConversionMultiVersionFixture(namespace, name, version string) *unstructured.Unstructured { + u := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "stable.example.com/" + version, + "kind": "MultiVersion", + "metadata": map[string]interface{}{ + "namespace": namespace, + "name": name, + }, + }, + } + + switch version { + case "v1alpha1": + u.Object["content"] = map[string]interface{}{ + "key": "value", + } + u.Object["num"] = map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + } + case "v1beta1": + u.Object["content"] = map[string]interface{}{ + "key": "value", + } + u.Object["num"] = map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + } + case "v1beta2": + u.Object["contentv2"] = map[string]interface{}{ + "key": "value", + } + u.Object["numv2"] = map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + } + default: + panic(fmt.Sprintf("unknown version %s", version)) + } + + return u +} + +func verifyMultiVersionObject(t *testing.T, v string, obj *unstructured.Unstructured) { + j := runtime.DeepCopyJSON(obj.Object) + + if expected := "stable.example.com/" + v; obj.GetAPIVersion() != expected { + t.Errorf("unexpected apiVersion %q, expected %q", obj.GetAPIVersion(), expected) + return + } + + delete(j, "metadata") + + var expected = map[string]map[string]interface{}{ + "v1alpha1": { + "apiVersion": "stable.example.com/v1alpha1", + "kind": "MultiVersion", + "content": map[string]interface{}{ + "key": "value", + }, + "num": map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + }, + }, + "v1beta1": { + "apiVersion": "stable.example.com/v1beta1", + "kind": "MultiVersion", + "content": map[string]interface{}{ + "key": "value", + }, + "num": map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + }, + }, + "v1beta2": { + "apiVersion": "stable.example.com/v1beta2", + "kind": "MultiVersion", + "contentv2": map[string]interface{}{ + "key": "value", + }, + "numv2": map[string]interface{}{ + "num1": int64(1), + "num2": int64(1000000), + }, + }, + } + if !reflect.DeepEqual(expected[v], j) { + t.Errorf("unexpected %s object: %s", v, cmp.Diff(expected[v], j)) + } +} + +func closeOnCall(h http.Handler) (chan struct{}, http.Handler) { + ch := make(chan struct{}) + once := sync.Once{} + return ch, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + once.Do(func() { + close(ch) + }) + h.ServeHTTP(w, r) + }) +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/webhook.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go similarity index 65% rename from staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/webhook.go rename to staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go index d829ac2bb5a..e1dcfb84a33 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/webhook.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package convert +package conversion import ( "crypto/tls" @@ -25,91 +25,51 @@ import ( "net/http" "net/http/httptest" "strings" - "sync" "testing" "time" + "k8s.io/apimachinery/pkg/util/wait" + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/uuid" ) -// WaitReadyFunc calls triggerConversionFn periodically and waits until it detects that the webhook -// conversion server has handled at least 1 conversion request or the timeout is exceeded, in which -// case an error is returned. -type WaitReadyFunc func(timeout time.Duration, triggerConversionFn func() error) error - -// StartConversionWebhookServerWithWaitReady starts an http server with the provided handler and returns the WebhookClientConfig -// needed to configure a CRD to use this conversion webhook as its converter. -// It also returns a WaitReadyFunc to be called after the CRD is configured to wait until the conversion webhook handler -// accepts at least one conversion request. If the server fails to start, an error is returned. -// WaitReady is useful when changing the conversion webhook config of an existing CRD because the update does not take effect immediately. -func StartConversionWebhookServerWithWaitReady(handler http.Handler) (func(), *apiextensionsv1beta1.WebhookClientConfig, WaitReadyFunc, error) { - var once sync.Once - handlerReadyC := make(chan struct{}) - readyNotifyHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - once.Do(func() { - close(handlerReadyC) - }) - handler.ServeHTTP(w, r) - }) - - tearDown, webhookConfig, err := StartConversionWebhookServer(readyNotifyHandler) - if err != nil { - return nil, nil, nil, fmt.Errorf("error starting webhook server: %v", err) - } - - waitReady := func(timeout time.Duration, triggerConversionFn func() error) error { - var err error - for { - select { - case <-handlerReadyC: - return nil - case <-time.After(timeout): - return fmt.Errorf("Timed out waiting for CRD webhook converter update, last trigger conversion error: %v", err) - case <-time.After(100 * time.Millisecond): - err = triggerConversionFn() - } - } - } - return tearDown, webhookConfig, waitReady, err -} - // StartConversionWebhookServer starts an http server with the provided handler and returns the WebhookClientConfig // needed to configure a CRD to use this conversion webhook as its converter. func StartConversionWebhookServer(handler http.Handler) (func(), *apiextensionsv1beta1.WebhookClientConfig, error) { - // Use a unique path for each webhook server. This ensures that all conversion requests - // received by the handler are intended for it; if a WebhookClientConfig other than this one - // is applied in the api server, conversion requests will not reach the handler (if they - // reach the server they will be returned at 404). This helps prevent tests that require a - // specific conversion webhook from accidentally using a different one, which could otherwise - // cause a test to flake or pass when it should fail. Since updating the conversion client - // config of a custom resource definition does not take effect immediately, this is needed - // by the WaitReady returned StartConversionWebhookServerWithWaitReady to detect when a - // conversion client config change in the api server has taken effect. - path := fmt.Sprintf("/conversionwebhook-%s", uuid.NewUUID()) roots := x509.NewCertPool() if !roots.AppendCertsFromPEM(localhostCert) { - return nil, nil, fmt.Errorf("Failed to append Cert from PEM") + return nil, nil, fmt.Errorf("failed to append Cert from PEM") } cert, err := tls.X509KeyPair(localhostCert, localhostKey) if err != nil { - return nil, nil, fmt.Errorf("Failed to build cert with error: %+v", err) + return nil, nil, fmt.Errorf("failed to build cert with error: %+v", err) } + webhookMux := http.NewServeMux() - webhookMux.Handle(path, handler) + webhookMux.Handle("/convert", handler) webhookServer := httptest.NewUnstartedServer(webhookMux) webhookServer.TLS = &tls.Config{ RootCAs: roots, Certificates: []tls.Certificate{cert}, } webhookServer.StartTLS() - endpoint := webhookServer.URL + path + endpoint := webhookServer.URL + "/convert" webhookConfig := &apiextensionsv1beta1.WebhookClientConfig{ CABundle: localhostCert, URL: &endpoint, } + + // StartTLS returns immediately, there is a small chance of a race to avoid. + if err := wait.PollImmediate(time.Millisecond*100, wait.ForeverTestTimeout, func() (bool, error) { + _, err := webhookServer.Client().Get(webhookServer.URL) // even a 404 is fine + return err == nil, nil + }); err != nil { + webhookServer.Close() + return nil, nil, err + } + return webhookServer.Close, webhookConfig, nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion_test.go deleted file mode 100644 index 20068952770..00000000000 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion_test.go +++ /dev/null @@ -1,564 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package integration - -import ( - "encoding/json" - "fmt" - "net/http" - "reflect" - "strings" - "testing" - "time" - - "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" - "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" - serveroptions "k8s.io/apiextensions-apiserver/pkg/cmd/server/options" - apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/uuid" - etcd3watcher "k8s.io/apiserver/pkg/storage/etcd3" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/client-go/dynamic" - featuregatetesting "k8s.io/component-base/featuregate/testing" - - apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" - "k8s.io/apiextensions-apiserver/test/integration/convert" - "k8s.io/apiextensions-apiserver/test/integration/fixtures" - "k8s.io/apiextensions-apiserver/test/integration/storage" -) - -type Checker func(t *testing.T, ctc *conversionTestContext) - -func checks(checkers ...Checker) []Checker { - return checkers -} - -func TestWebhookConverter(t *testing.T) { - tests := []struct { - group string - handler http.Handler - checks []Checker - }{ - { - group: "noop-converter", - handler: convert.NewObjectConverterWebhookHandler(t, noopConverter), - checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions), - }, - { - group: "nontrivial-converter", - handler: convert.NewObjectConverterWebhookHandler(t, nontrivialConverter), - checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions), - }, - { - group: "empty-response", - handler: convert.NewReviewWebhookHandler(t, emptyResponseConverter), - checks: checks(expectConversionFailureMessage("empty-response", "expected 1 converted objects")), - }, - { - group: "failure-message", - handler: convert.NewReviewWebhookHandler(t, failureResponseConverter("custom webhook conversion error")), - checks: checks(expectConversionFailureMessage("failure-message", "custom webhook conversion error")), - }, - } - - // TODO: Added for integration testing of conversion webhooks, where decode errors due to conversion webhook failures need to be tested. - // Maybe we should identify conversion webhook related errors in decoding to avoid triggering this? Or maybe having this special casing - // of test cases in production code should be removed? - etcd3watcher.TestOnlySetFatalOnDecodeError(false) - defer etcd3watcher.TestOnlySetFatalOnDecodeError(true) - - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CustomResourceWebhookConversion, true)() - tearDown, config, options, err := fixtures.StartDefaultServer(t) - if err != nil { - t.Fatal(err) - } - - apiExtensionsClient, err := clientset.NewForConfig(config) - if err != nil { - tearDown() - t.Fatal(err) - } - - dynamicClient, err := dynamic.NewForConfig(config) - if err != nil { - tearDown() - t.Fatal(err) - } - defer tearDown() - - crd := multiVersionFixture.DeepCopy() - - RESTOptionsGetter := serveroptions.NewCRDRESTOptionsGetter(*options.RecommendedOptions.Etcd) - restOptions, err := RESTOptionsGetter.GetRESTOptions(schema.GroupResource{Group: crd.Spec.Group, Resource: crd.Spec.Names.Plural}) - if err != nil { - t.Fatal(err) - } - etcdClient, _, err := storage.GetEtcdClients(restOptions.StorageConfig.Transport) - if err != nil { - t.Fatal(err) - } - defer etcdClient.Close() - - etcdObjectReader := storage.NewEtcdObjectReader(etcdClient, &restOptions, crd) - ctcTearDown, ctc := newConversionTestContext(t, apiExtensionsClient, dynamicClient, etcdObjectReader, crd) - defer ctcTearDown() - - // read only object to read at a different version than stored when we need to force conversion - marker, err := ctc.versionedClient("marker", "v1beta2").Create(newConversionMultiVersionFixture("marker", "marker", "v1beta2"), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - - for _, test := range tests { - t.Run(test.group, func(t *testing.T) { - tearDown, webhookClientConfig, webhookWaitReady, err := convert.StartConversionWebhookServerWithWaitReady(test.handler) - if err != nil { - t.Fatal(err) - } - defer tearDown() - - ctc.setConversionWebhook(t, webhookClientConfig) - defer ctc.removeConversionWebhook(t) - - err = webhookWaitReady(30*time.Second, func() error { - // the marker's storage version is v1beta2, so a v1beta1 read always triggers conversion - _, err := ctc.versionedClient(marker.GetNamespace(), "v1beta1").Get(marker.GetName(), metav1.GetOptions{}) - return err - }) - if err != nil { - t.Fatal(err) - } - - for i, checkFn := range test.checks { - name := fmt.Sprintf("check-%d", i) - t.Run(name, func(t *testing.T) { - ctc.setAndWaitStorageVersion(t, "v1beta2") - ctc.namespace = fmt.Sprintf("webhook-conversion-%s-%s", test.group, name) - checkFn(t, ctc) - }) - } - }) - } -} - -func validateStorageVersion(t *testing.T, ctc *conversionTestContext) { - ns := ctc.namespace - - for _, version := range []string{"v1beta1", "v1beta2"} { - t.Run(version, func(t *testing.T) { - name := "storageversion-" + version - client := ctc.versionedClient(ns, version) - obj, err := client.Create(newConversionMultiVersionFixture(ns, name, version), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - ctc.setAndWaitStorageVersion(t, "v1beta2") - - obj, err = client.Get(obj.GetName(), metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - - ctc.setAndWaitStorageVersion(t, "v1beta1") - }) - } -} - -// validateMixedStorageVersions ensures that identical custom resources written at different storage versions -// are readable and remain the same. -func validateMixedStorageVersions(t *testing.T, ctc *conversionTestContext) { - ns := ctc.namespace - - v1client := ctc.versionedClient(ns, "v1beta1") - v2client := ctc.versionedClient(ns, "v1beta2") - clients := map[string]dynamic.ResourceInterface{"v1beta1": v1client, "v1beta2": v2client} - versions := []string{"v1beta1", "v1beta2"} - - // Create CRs at all storage versions - objNames := []string{} - for _, version := range versions { - ctc.setAndWaitStorageVersion(t, version) - - name := "stored-at-" + version - obj, err := clients[version].Create(newConversionMultiVersionFixture(ns, name, version), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - objNames = append(objNames, obj.GetName()) - } - - // Ensure copies of an object have the same fields and values at each custom resource definition version regardless of storage version - for clientVersion, client := range clients { - t.Run(clientVersion, func(t *testing.T) { - o1, err := client.Get(objNames[0], metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - for _, objName := range objNames[1:] { - o2, err := client.Get(objName, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - - // ignore metadata for comparison purposes - delete(o1.Object, "metadata") - delete(o2.Object, "metadata") - if !reflect.DeepEqual(o1.Object, o2.Object) { - t.Errorf("Expected custom resource to be same regardless of which storage version is used but got %+v != %+v", o1, o2) - } - } - }) - } -} - -func validateServed(t *testing.T, ctc *conversionTestContext) { - ns := ctc.namespace - - for _, version := range []string{"v1beta1", "v1beta2"} { - t.Run(version, func(t *testing.T) { - name := "served-" + version - client := ctc.versionedClient(ns, version) - obj, err := client.Create(newConversionMultiVersionFixture(ns, name, version), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - ctc.setServed(t, version, false) - ctc.waitForServed(t, version, false, client, obj) - ctc.setServed(t, version, true) - ctc.waitForServed(t, version, true, client, obj) - }) - } -} - -func expectConversionFailureMessage(id, message string) func(t *testing.T, ctc *conversionTestContext) { - return func(t *testing.T, ctc *conversionTestContext) { - ns := ctc.namespace - v1client := ctc.versionedClient(ns, "v1beta1") - v2client := ctc.versionedClient(ns, "v1beta2") - var err error - // storage version is v1beta2, so this skips conversion - obj, err := v2client.Create(newConversionMultiVersionFixture(ns, id, "v1beta2"), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - for _, verb := range []string{"get", "list", "create", "udpate", "patch", "delete", "deletecollection"} { - t.Run(verb, func(t *testing.T) { - switch verb { - case "get": - _, err = v1client.Get(obj.GetName(), metav1.GetOptions{}) - case "list": - _, err = v1client.List(metav1.ListOptions{}) - case "create": - _, err = v1client.Create(newConversionMultiVersionFixture(ns, id, "v1beta1"), metav1.CreateOptions{}) - case "update": - _, err = v1client.Update(obj, metav1.UpdateOptions{}) - case "patch": - _, err = v1client.Patch(obj.GetName(), types.MergePatchType, []byte(`{"metadata":{"annotations":{"patch":"true"}}}`), metav1.PatchOptions{}) - case "delete": - err = v1client.Delete(obj.GetName(), &metav1.DeleteOptions{}) - case "deletecollection": - err = v1client.DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{}) - } - - if err == nil { - t.Errorf("expected error with message %s, but got no error", message) - } else if !strings.Contains(err.Error(), message) { - t.Errorf("expected error with message %s, but got %v", message, err) - } - }) - } - for _, subresource := range []string{"status", "scale"} { - for _, verb := range []string{"get", "udpate", "patch"} { - t.Run(fmt.Sprintf("%s-%s", subresource, verb), func(t *testing.T) { - switch verb { - case "create": - _, err = v1client.Create(newConversionMultiVersionFixture(ns, id, "v1beta1"), metav1.CreateOptions{}, subresource) - case "update": - _, err = v1client.Update(obj, metav1.UpdateOptions{}, subresource) - case "patch": - _, err = v1client.Patch(obj.GetName(), types.MergePatchType, []byte(`{"metadata":{"annotations":{"patch":"true"}}}`), metav1.PatchOptions{}, subresource) - } - - if err == nil { - t.Errorf("expected error with message %s, but got no error", message) - } else if !strings.Contains(err.Error(), message) { - t.Errorf("expected error with message %s, but got %v", message, err) - } - }) - } - } - } -} - -func noopConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { - u := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := json.Unmarshal(obj.Raw, u); err != nil { - return runtime.RawExtension{}, fmt.Errorf("Fail to deserialize object: %s with error: %v", string(obj.Raw), err) - } - u.Object["apiVersion"] = desiredAPIVersion - raw, err := json.Marshal(u) - if err != nil { - return runtime.RawExtension{}, fmt.Errorf("Fail to serialize object: %v with error: %v", u, err) - } - return runtime.RawExtension{Raw: raw}, nil -} - -func emptyResponseConverter(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { - review.Response = &apiextensionsv1beta1.ConversionResponse{ - UID: review.Request.UID, - ConvertedObjects: []runtime.RawExtension{}, - Result: metav1.Status{Status: "Success"}, - } - return review, nil -} - -func failureResponseConverter(message string) func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { - return func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { - review.Response = &apiextensionsv1beta1.ConversionResponse{ - UID: review.Request.UID, - ConvertedObjects: []runtime.RawExtension{}, - Result: metav1.Status{Message: message, Status: "Failure"}, - } - return review, nil - } -} - -func nontrivialConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime.RawExtension, error) { - u := &unstructured.Unstructured{Object: map[string]interface{}{}} - if err := json.Unmarshal(obj.Raw, u); err != nil { - return runtime.RawExtension{}, fmt.Errorf("Fail to deserialize object: %s with error: %v", string(obj.Raw), err) - } - - currentAPIVersion := u.Object["apiVersion"] - if currentAPIVersion == "v1beta2" && desiredAPIVersion == "v1beta1" { - u.Object["num"] = u.Object["numv2"] - u.Object["content"] = u.Object["contentv2"] - delete(u.Object, "numv2") - delete(u.Object, "contentv2") - } - if currentAPIVersion == "v1beta1" && desiredAPIVersion == "v1beta2" { - u.Object["numv2"] = u.Object["num"] - u.Object["contentv2"] = u.Object["content"] - delete(u.Object, "num") - delete(u.Object, "content") - } - u.Object["apiVersion"] = desiredAPIVersion - raw, err := json.Marshal(u) - if err != nil { - return runtime.RawExtension{}, fmt.Errorf("Fail to serialize object: %v with error: %v", u, err) - } - return runtime.RawExtension{Raw: raw}, nil -} - -func newConversionTestContext(t *testing.T, apiExtensionsClient clientset.Interface, dynamicClient dynamic.Interface, etcdObjectReader *storage.EtcdObjectReader, crd *apiextensionsv1beta1.CustomResourceDefinition) (func(), *conversionTestContext) { - crd, err := fixtures.CreateNewCustomResourceDefinition(crd, apiExtensionsClient, dynamicClient) - if err != nil { - t.Fatal(err) - } - - tearDown := func() { - if err := fixtures.DeleteCustomResourceDefinition(crd, apiExtensionsClient); err != nil { - t.Fatal(err) - } - } - - return tearDown, &conversionTestContext{apiExtensionsClient: apiExtensionsClient, dynamicClient: dynamicClient, crd: crd, etcdObjectReader: etcdObjectReader} -} - -type conversionTestContext struct { - namespace string - apiExtensionsClient clientset.Interface - dynamicClient dynamic.Interface - options *options.CustomResourceDefinitionsServerOptions - crd *apiextensionsv1beta1.CustomResourceDefinition - etcdObjectReader *storage.EtcdObjectReader -} - -func (c *conversionTestContext) versionedClient(ns string, version string) dynamic.ResourceInterface { - return newNamespacedCustomResourceVersionedClient(ns, c.dynamicClient, c.crd, version) -} - -func (c *conversionTestContext) setConversionWebhook(t *testing.T, webhookClientConfig *apiextensionsv1beta1.WebhookClientConfig) { - crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - crd.Spec.Conversion = &apiextensionsv1beta1.CustomResourceConversion{ - Strategy: apiextensionsv1beta1.WebhookConverter, - WebhookClientConfig: webhookClientConfig, - } - crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) - if err != nil { - t.Fatal(err) - } - c.crd = crd - -} - -func (c *conversionTestContext) removeConversionWebhook(t *testing.T) { - crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - crd.Spec.Conversion = &apiextensionsv1beta1.CustomResourceConversion{ - Strategy: apiextensionsv1beta1.NoneConverter, - } - - crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) - if err != nil { - t.Fatal(err) - } - c.crd = crd -} - -func (c *conversionTestContext) setAndWaitStorageVersion(t *testing.T, version string) { - c.setStorageVersion(t, "v1beta2") - - client := c.versionedClient("probe", "v1beta2") - name := fmt.Sprintf("probe-%v", uuid.NewUUID()) - storageProbe, err := client.Create(newConversionMultiVersionFixture("probe", name, "v1beta2"), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - c.waitForStorageVersion(t, "v1beta2", c.versionedClient(storageProbe.GetNamespace(), "v1beta2"), storageProbe) - - err = client.Delete(name, &metav1.DeleteOptions{}) - if err != nil { - t.Fatal(err) - } -} - -func (c *conversionTestContext) setStorageVersion(t *testing.T, version string) { - crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - for i, v := range crd.Spec.Versions { - crd.Spec.Versions[i].Storage = (v.Name == version) - } - crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) - if err != nil { - t.Fatal(err) - } - c.crd = crd -} - -func (c *conversionTestContext) waitForStorageVersion(t *testing.T, version string, versionedClient dynamic.ResourceInterface, obj *unstructured.Unstructured) *unstructured.Unstructured { - c.etcdObjectReader.WaitForStorageVersion(version, obj.GetNamespace(), obj.GetName(), 30*time.Second, func() { - var err error - obj, err = versionedClient.Update(obj, metav1.UpdateOptions{}) - if err != nil { - t.Fatalf("failed to update object: %v", err) - } - }) - return obj -} - -func (c *conversionTestContext) setServed(t *testing.T, version string, served bool) { - crd, err := c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Get(c.crd.Name, metav1.GetOptions{}) - if err != nil { - t.Fatal(err) - } - for i, v := range crd.Spec.Versions { - if v.Name == version { - crd.Spec.Versions[i].Served = served - } - } - crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) - if err != nil { - t.Fatal(err) - } - c.crd = crd -} - -func (c *conversionTestContext) waitForServed(t *testing.T, version string, served bool, versionedClient dynamic.ResourceInterface, obj *unstructured.Unstructured) { - timeout := 30 * time.Second - waitCh := time.After(timeout) - for { - obj, err := versionedClient.Get(obj.GetName(), metav1.GetOptions{}) - if (err == nil && served) || (errors.IsNotFound(err) && served == false) { - return - } - select { - case <-waitCh: - t.Fatalf("Timed out after %v waiting for CRD served=%t for version %s for %v. Last error: %v", timeout, served, version, obj, err) - case <-time.After(10 * time.Millisecond): - } - } -} - -var multiVersionFixture = &apiextensionsv1beta1.CustomResourceDefinition{ - ObjectMeta: metav1.ObjectMeta{Name: "multiversion.stable.example.com"}, - Spec: apiextensionsv1beta1.CustomResourceDefinitionSpec{ - Group: "stable.example.com", - Version: "v1beta1", - Names: apiextensionsv1beta1.CustomResourceDefinitionNames{ - Plural: "multiversion", - Singular: "multiversion", - Kind: "MultiVersion", - ShortNames: []string{"mv"}, - ListKind: "MultiVersionList", - Categories: []string{"all"}, - }, - Scope: apiextensionsv1beta1.NamespaceScoped, - Versions: []apiextensionsv1beta1.CustomResourceDefinitionVersion{ - { - Name: "v1beta1", - Served: true, - Storage: false, - }, - { - Name: "v1beta2", - Served: true, - Storage: true, - }, - }, - Subresources: &apiextensionsv1beta1.CustomResourceSubresources{ - Status: &apiextensionsv1beta1.CustomResourceSubresourceStatus{}, - Scale: &apiextensionsv1beta1.CustomResourceSubresourceScale{ - SpecReplicasPath: ".spec.num.num1", - StatusReplicasPath: ".status.num.num2", - }, - }, - }, -} - -func newConversionMultiVersionFixture(namespace, name, version string) *unstructured.Unstructured { - return &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "stable.example.com/" + version, - "kind": "MultiVersion", - "metadata": map[string]interface{}{ - "namespace": namespace, - "name": name, - }, - "content": map[string]interface{}{ - "key": "value", - }, - "num": map[string]interface{}{ - "num1": 1, - "num2": 1000000, - }, - }, - } -} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/BUILD deleted file mode 100644 index eafa4b428f0..00000000000 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/convert/BUILD +++ /dev/null @@ -1,29 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") - -go_library( - name = "go_default_library", - srcs = ["webhook.go"], - importmap = "k8s.io/kubernetes/vendor/k8s.io/apiextensions-apiserver/test/integration/convert", - importpath = "k8s.io/apiextensions-apiserver/test/integration/convert", - visibility = ["//visibility:public"], - deps = [ - "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/server.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/server.go index 0a3d6444e9d..d2f487051fc 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/server.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/fixtures/server.go @@ -31,7 +31,7 @@ import ( ) // StartDefaultServer starts a test server. -func StartDefaultServer(t servertesting.Logger) (func(), *rest.Config, *options.CustomResourceDefinitionsServerOptions, error) { +func StartDefaultServer(t servertesting.Logger, flags ...string) (func(), *rest.Config, *options.CustomResourceDefinitionsServerOptions, error) { // create kubeconfig which will not actually be used. But authz/authn needs it to startup. fakeKubeConfig, err := ioutil.TempFile("", "kubeconfig") fakeKubeConfig.WriteString(` @@ -55,15 +55,16 @@ users: `) fakeKubeConfig.Close() - s, err := servertesting.StartTestServer(t, nil, []string{ + s, err := servertesting.StartTestServer(t, nil, append([]string{ "--etcd-prefix", uuid.New(), "--etcd-servers", strings.Join(IntegrationEtcdServers(), ","), "--authentication-skip-lookup", "--authentication-kubeconfig", fakeKubeConfig.Name(), "--authorization-kubeconfig", fakeKubeConfig.Name(), "--kubeconfig", fakeKubeConfig.Name(), - "--disable-admission-plugins", "NamespaceLifecycle,MutatingAdmissionWebhook,ValidatingAdmissionWebhook", - }, nil) + "--disable-admission-plugins", "NamespaceLifecycle,MutatingAdmissionWebhook,ValidatingAdmissionWebhook"}, + flags..., + ), nil) if err != nil { os.Remove(fakeKubeConfig.Name()) return nil, nil, nil, err @@ -78,8 +79,8 @@ users: } // StartDefaultServerWithClients starts a test server and returns clients for it. -func StartDefaultServerWithClients(t servertesting.Logger) (func(), clientset.Interface, dynamic.Interface, error) { - tearDown, config, _, err := StartDefaultServer(t) +func StartDefaultServerWithClients(t servertesting.Logger, extraFlags ...string) (func(), clientset.Interface, dynamic.Interface, error) { + tearDown, config, _, err := StartDefaultServer(t, extraFlags...) if err != nil { return nil, nil, nil, err } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/storage/objectreader.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/storage/objectreader.go index a6e21fd6242..d784458ce94 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/storage/objectreader.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/storage/objectreader.go @@ -84,6 +84,20 @@ func (s *EtcdObjectReader) GetStoredCustomResource(ns, name string) (*unstructur return u, nil } +// SetStoredCustomResource writes the storage representation of a custom resource to etcd. +func (s *EtcdObjectReader) SetStoredCustomResource(ns, name string, obj *unstructured.Unstructured) error { + bs, err := obj.MarshalJSON() + if err != nil { + return err + } + + key := path.Join("/", s.storagePrefix, s.crd.Spec.Group, s.crd.Spec.Names.Plural, ns, name) + if _, err := s.etcdClient.KV.Put(context.Background(), key, string(bs)); err != nil { + return fmt.Errorf("error setting storage object %s, %s from etcd at key %s: %v", ns, name, key, err) + } + return nil +} + // GetEtcdClients returns an initialized clientv3.Client and clientv3.KV. func GetEtcdClients(config storagebackend.TransportConfig) (*clientv3.Client, clientv3.KV, error) { tlsInfo := transport.TLSInfo{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go index d770f425911..6f3cf72b49b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go @@ -534,7 +534,7 @@ func TestValidateOnlyStatus(t *testing.T) { } createdNoxuInstance, err = noxuResourceClient.UpdateStatus(createdNoxuInstance, metav1.UpdateOptions{}) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } // update with .status.num = 15, expecting an error diff --git a/test/e2e/apimachinery/crd_conversion_webhook.go b/test/e2e/apimachinery/crd_conversion_webhook.go index 485b19b0023..07f2d3dbc35 100644 --- a/test/e2e/apimachinery/crd_conversion_webhook.go +++ b/test/e2e/apimachinery/crd_conversion_webhook.go @@ -40,6 +40,7 @@ import ( "github.com/onsi/ginkgo" "github.com/onsi/gomega" + // ensure libs have a chance to initialize _ "github.com/stretchr/testify/assert" ) @@ -386,6 +387,8 @@ func testCRListConversion(f *framework.Framework, testCrd *crd.TestCrd) { // After changing a CRD, the resources for versions will be re-created that can be result in // cancelled connection (e.g. "grpc connection closed" or "context canceled"). // Just retrying fixes that. + // + // TODO: we have to wait for the storage version to become effective. Storage version changes are not instant. for i := 0; i < 5; i++ { _, err = customResourceClients["v1"].Create(crInstance, metav1.CreateOptions{}) if err == nil {