From ce769a5edeaf9c1cdfe2cc3b214e741fda47ca34 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 15 Aug 2019 12:19:43 -0400 Subject: [PATCH] ConversionReview v1 --- .../apiextensions/validation/validation.go | 15 +- .../pkg/apiserver/conversion/BUILD | 6 + .../apiserver/conversion/webhook_converter.go | 178 ++++++--- .../conversion/webhook_converter_test.go | 355 ++++++++++++++++++ .../test/integration/conversion/BUILD | 1 + .../integration/conversion/conversion_test.go | 131 +++++-- .../test/integration/conversion/webhook.go | 90 ++++- 7 files changed, 681 insertions(+), 95 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index 370319474dd..1591ad8a6dd 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -311,15 +311,14 @@ func validateEnumStrings(fldPath *field.Path, value string, accepted []string, r return field.ErrorList{field.NotSupported(fldPath, value, accepted)} } -var acceptedConversionReviewVersion = []string{v1beta1.SchemeGroupVersion.Version} +// AcceptedConversionReviewVersions contains the list of ConversionReview versions the *prior* version of the API server understands. +// 1.15: server understands v1beta1; accepted versions are ["v1beta1"] +// 1.16: server understands v1, v1beta1; accepted versions are ["v1beta1"] +// TODO(liggitt): 1.17: server understands v1, v1beta1; accepted versions are ["v1","v1beta1"] +var acceptedConversionReviewVersions = sets.NewString(v1beta1.SchemeGroupVersion.Version) func isAcceptedConversionReviewVersion(v string) bool { - for _, version := range acceptedConversionReviewVersion { - if v == version { - return true - } - } - return false + return acceptedConversionReviewVersions.Has(v) } func validateConversionReviewVersions(versions []string, requireRecognizedVersion bool, fldPath *field.Path) field.ErrorList { @@ -346,7 +345,7 @@ func validateConversionReviewVersions(versions []string, requireRecognizedVersio allErrs = append(allErrs, field.Invalid( fldPath, versions, fmt.Sprintf("must include at least one of %v", - strings.Join(acceptedConversionReviewVersion, ", ")))) + strings.Join(acceptedConversionReviewVersions.List(), ", ")))) } } return allErrs diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD index 710340a6c0a..e06289dd6fb 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/BUILD @@ -14,6 +14,7 @@ go_library( deps = [ "//staging/src/k8s.io/api/autoscaling/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library", "//staging/src/k8s.io/apiextensions-apiserver/pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/validation:go_default_library", @@ -22,6 +23,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation: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/uuid:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", @@ -56,11 +58,15 @@ go_test( embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library", + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library", + "//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/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/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/webhook:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go index 602659b0093..fabc8b3f6f6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter.go @@ -18,22 +18,24 @@ package conversion import ( "context" + "errors" "fmt" "time" + internal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apivalidation "k8s.io/apimachinery/pkg/api/validation" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/util/webhook" "k8s.io/client-go/rest" - - internal "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" utiltrace "k8s.io/utils/trace" ) @@ -42,7 +44,11 @@ type webhookConverterFactory struct { } func newWebhookConverterFactory(serviceResolver webhook.ServiceResolver, authResolverWrapper webhook.AuthenticationInfoResolverWrapper) (*webhookConverterFactory, error) { - clientManager, err := webhook.NewClientManager([]schema.GroupVersion{v1beta1.SchemeGroupVersion}, v1beta1.AddToScheme) + clientManager, err := webhook.NewClientManager( + []schema.GroupVersion{v1.SchemeGroupVersion, v1beta1.SchemeGroupVersion}, + v1beta1.AddToScheme, + v1.AddToScheme, + ) if err != nil { return nil, err } @@ -116,7 +122,11 @@ func (c *webhookConverter) hasConversionReviewVersion(v string) bool { return false } -func createConversionReview(obj runtime.Object, apiVersion string) *v1beta1.ConversionReview { +// getObjectsToConvert returns a list of objects requiring conversion. +// if obj is a list, getObjectsToConvert returns a (potentially empty) list of the items that are not already in the desired version. +// if obj is not a list, and is already in the desired version, getObjectsToConvert returns an empty list. +// if obj is not a list, and is not already in the desired version, getObjectsToConvert returns a list containing only obj. +func getObjectsToConvert(obj runtime.Object, apiVersion string) []runtime.RawExtension { listObj, isList := obj.(*unstructured.UnstructuredList) var objects []runtime.RawExtension if isList { @@ -131,14 +141,34 @@ func createConversionReview(obj runtime.Object, apiVersion string) *v1beta1.Conv objects = []runtime.RawExtension{{Object: obj}} } } - return &v1beta1.ConversionReview{ - Request: &v1beta1.ConversionRequest{ - Objects: objects, - DesiredAPIVersion: apiVersion, - UID: uuid.NewUUID(), - }, - Response: &v1beta1.ConversionResponse{}, + return objects +} + +// createConversionReviewObjects returns ConversionReview request and response objects for the first supported version found in conversionReviewVersions. +func createConversionReviewObjects(conversionReviewVersions []string, objects []runtime.RawExtension, apiVersion string, requestUID types.UID) (request, response runtime.Object, err error) { + for _, version := range conversionReviewVersions { + switch version { + case v1beta1.SchemeGroupVersion.Version: + return &v1beta1.ConversionReview{ + Request: &v1beta1.ConversionRequest{ + Objects: objects, + DesiredAPIVersion: apiVersion, + UID: requestUID, + }, + Response: &v1beta1.ConversionResponse{}, + }, &v1beta1.ConversionReview{}, nil + case v1.SchemeGroupVersion.Version: + return &v1.ConversionReview{ + Request: &v1.ConversionRequest{ + Objects: objects, + DesiredAPIVersion: apiVersion, + UID: requestUID, + }, + Response: &v1.ConversionResponse{}, + }, &v1.ConversionReview{}, nil + } } + return nil, nil, fmt.Errorf("no supported conversion review versions") } func getRawExtensionObject(rx runtime.RawExtension) (runtime.Object, error) { @@ -153,6 +183,59 @@ func getRawExtensionObject(rx runtime.RawExtension) (runtime.Object, error) { return &u, nil } +// getConvertedObjectsFromResponse validates the response, and returns the converted objects. +// if the response is malformed, an error is returned instead. +// if the response does not indicate success, the error message is returned instead. +func getConvertedObjectsFromResponse(expectedUID types.UID, response runtime.Object) (convertedObjects []runtime.RawExtension, err error) { + switch response := response.(type) { + case *v1.ConversionReview: + // Verify GVK to make sure we decoded what we intended to + v1GVK := v1.SchemeGroupVersion.WithKind("ConversionReview") + if response.GroupVersionKind() != v1GVK { + return nil, fmt.Errorf("expected webhook response of %v, got %v", v1GVK.String(), response.GroupVersionKind().String()) + } + + if response.Response == nil { + return nil, fmt.Errorf("no response provided") + } + + // Verify UID to make sure this response was actually meant for the request we sent + if response.Response.UID != expectedUID { + return nil, fmt.Errorf("expected response.uid=%q, got %q", expectedUID, response.Response.UID) + } + + if response.Response.Result.Status != metav1.StatusSuccess { + // TODO: Return a webhook specific error to be able to convert it to meta.Status + if len(response.Response.Result.Message) > 0 { + return nil, errors.New(response.Response.Result.Message) + } + return nil, fmt.Errorf("response.result.status was '%s', not 'Success'", response.Response.Result.Status) + } + + return response.Response.ConvertedObjects, nil + + case *v1beta1.ConversionReview: + // v1beta1 processing did not verify GVK or UID, so skip those for compatibility + + if response.Response == nil { + return nil, fmt.Errorf("no response provided") + } + + if response.Response.Result.Status != metav1.StatusSuccess { + // TODO: Return a webhook specific error to be able to convert it to meta.Status + if len(response.Response.Result.Message) > 0 { + return nil, errors.New(response.Response.Result.Message) + } + return nil, fmt.Errorf("response.result.status was '%s', not 'Success'", response.Response.Result.Status) + } + + return response.Response.ConvertedObjects, nil + + default: + return nil, fmt.Errorf("unrecognized response type: %T", response) + } +} + func (c *webhookConverter) Convert(in runtime.Object, toGV schema.GroupVersion) (runtime.Object, error) { // In general, the webhook should not do any defaulting or validation. A special case of that is an empty object // conversion that must result an empty object and practically is the same as nopConverter. @@ -165,18 +248,22 @@ func (c *webhookConverter) Convert(in runtime.Object, toGV schema.GroupVersion) listObj, isList := in.(*unstructured.UnstructuredList) - // Currently converter only supports `v1beta1` ConversionReview - // TODO: Make CRD webhooks caller capable of sending/receiving multiple ConversionReview versions - if !c.hasConversionReviewVersion(v1beta1.SchemeGroupVersion.Version) { - return nil, fmt.Errorf("webhook does not accept v1beta1 ConversionReview") + requestUID := uuid.NewUUID() + desiredAPIVersion := toGV.String() + objectsToConvert := getObjectsToConvert(in, desiredAPIVersion) + request, response, err := createConversionReviewObjects(c.conversionReviewVersions, objectsToConvert, desiredAPIVersion, requestUID) + if err != nil { + return nil, err } - request := createConversionReview(in, toGV.String()) - objCount := len(request.Request.Objects) + objCount := len(objectsToConvert) if objCount == 0 { + // no objects needed conversion if !isList { + // for a single item, return as-is return in, nil } + // for a list, set the version of the top-level list object (all individual objects are already in the correct version) out := listObj.DeepCopy() out.SetAPIVersion(toGV.String()) return out, nil @@ -184,35 +271,30 @@ func (c *webhookConverter) Convert(in runtime.Object, toGV schema.GroupVersion) trace := utiltrace.New("Call conversion webhook", utiltrace.Field{"custom-resource-definition", c.name}, - utiltrace.Field{"desired-api-version", request.Request.DesiredAPIVersion}, + utiltrace.Field{"desired-api-version", desiredAPIVersion}, utiltrace.Field{"object-count", objCount}, - utiltrace.Field{"UID", request.Request.UID}) + utiltrace.Field{"UID", requestUID}) // Only log conversion webhook traces that exceed a 8ms per object limit plus a 50ms request overhead allowance. // The per object limit uses the SLO for conversion webhooks (~4ms per object) plus time to serialize/deserialize // the conversion request on the apiserver side (~4ms per object). defer trace.LogIfLong(time.Duration(50+8*objCount) * time.Millisecond) - response := &v1beta1.ConversionReview{} // TODO: Figure out if adding one second timeout make sense here. ctx := context.TODO() r := c.restClient.Post().Context(ctx).Body(request).Do() if err := r.Into(response); err != nil { // TODO: Return a webhook specific error to be able to convert it to meta.Status - return nil, fmt.Errorf("conversion webhook for %v failed: %v", in.GetObjectKind(), err) + return nil, fmt.Errorf("conversion webhook for %v failed: %v", in.GetObjectKind().GroupVersionKind(), err) } trace.Step("Request completed") - if response.Response == nil { - // TODO: Return a webhook specific error to be able to convert it to meta.Status - return nil, fmt.Errorf("conversion webhook for %v lacked response", in.GetObjectKind()) + convertedObjects, err := getConvertedObjectsFromResponse(requestUID, response) + if err != nil { + return nil, fmt.Errorf("conversion webhook for %v failed: %v", in.GetObjectKind().GroupVersionKind(), err) } - if response.Response.Result.Status != v1.StatusSuccess { - return nil, fmt.Errorf("conversion webhook for %v failed: %v", in.GetObjectKind(), response.Response.Result.Message) - } - - if len(response.Response.ConvertedObjects) != len(request.Request.Objects) { - return nil, fmt.Errorf("conversion webhook for %v returned %d objects, expected %d", in.GetObjectKind(), len(response.Response.ConvertedObjects), len(request.Request.Objects)) + if len(convertedObjects) != len(objectsToConvert) { + return nil, fmt.Errorf("conversion webhook for %v returned %d objects, expected %d", in.GetObjectKind().GroupVersionKind(), len(convertedObjects), len(objectsToConvert)) } if isList { @@ -227,27 +309,27 @@ func (c *webhookConverter) Convert(in runtime.Object, toGV schema.GroupVersion) // convertedList has the right item already. continue } - converted, err := getRawExtensionObject(response.Response.ConvertedObjects[convertedIndex]) + converted, err := getRawExtensionObject(convertedObjects[convertedIndex]) if err != nil { - return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: %v", in.GetObjectKind(), convertedIndex, err) + return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: %v", in.GetObjectKind().GroupVersionKind(), convertedIndex, err) } convertedIndex++ if expected, got := toGV, converted.GetObjectKind().GroupVersionKind().GroupVersion(); expected != got { - return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid groupVersion, expected=%v, got=%v", in.GetObjectKind(), convertedIndex, expected, got) + return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid groupVersion (expected %v, received %v)", in.GetObjectKind().GroupVersionKind(), convertedIndex, expected, got) } if expected, got := original.GetObjectKind().GroupVersionKind().Kind, converted.GetObjectKind().GroupVersionKind().Kind; expected != got { - return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid kind, expected=%v, got=%v", in.GetObjectKind(), convertedIndex, expected, got) + return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid kind (expected %v, received %v)", in.GetObjectKind().GroupVersionKind(), convertedIndex, expected, got) } unstructConverted, ok := converted.(*unstructured.Unstructured) if !ok { // this should not happened - return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid type, expected=Unstructured, got=%T", in.GetObjectKind(), convertedIndex, converted) + return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: invalid type, expected=Unstructured, got=%T", in.GetObjectKind().GroupVersionKind(), convertedIndex, converted) } if err := validateConvertedObject(original, unstructConverted); err != nil { - return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: %v", in.GetObjectKind(), convertedIndex, err) + return nil, fmt.Errorf("conversion webhook for %v returned invalid converted object at index %v: %v", in.GetObjectKind().GroupVersionKind(), convertedIndex, err) } if err := restoreObjectMeta(original, unstructConverted); err != nil { - return nil, fmt.Errorf("conversion webhook for %v returned invalid metadata in object at index %v: %v", in.GetObjectKind(), convertedIndex, err) + return nil, fmt.Errorf("conversion webhook for %v returned invalid metadata in object at index %v: %v", in.GetObjectKind().GroupVersionKind(), convertedIndex, err) } convertedList.Items[i] = *unstructConverted } @@ -255,35 +337,35 @@ func (c *webhookConverter) Convert(in runtime.Object, toGV schema.GroupVersion) return convertedList, nil } - if len(response.Response.ConvertedObjects) != 1 { + if len(convertedObjects) != 1 { // This should not happened - return nil, fmt.Errorf("conversion webhook for %v failed", in.GetObjectKind()) + return nil, fmt.Errorf("conversion webhook for %v failed, no objects returned", in.GetObjectKind()) } - converted, err := getRawExtensionObject(response.Response.ConvertedObjects[0]) + converted, err := getRawExtensionObject(convertedObjects[0]) if err != nil { return nil, err } if e, a := toGV, converted.GetObjectKind().GroupVersionKind().GroupVersion(); e != a { - return nil, fmt.Errorf("conversion webhook for %v returned invalid object: invalid groupVersion, e=%v, a=%v", in.GetObjectKind(), e, a) + return nil, fmt.Errorf("conversion webhook for %v returned invalid object at index 0: invalid groupVersion (expected %v, received %v)", in.GetObjectKind().GroupVersionKind(), e, a) } if e, a := in.GetObjectKind().GroupVersionKind().Kind, converted.GetObjectKind().GroupVersionKind().Kind; e != a { - return nil, fmt.Errorf("conversion webhook for %v returned invalid object: invalid kind, e=%v, a=%v", in.GetObjectKind(), e, a) + return nil, fmt.Errorf("conversion webhook for %v returned invalid object at index 0: invalid kind (expected %v, received %v)", in.GetObjectKind().GroupVersionKind(), e, a) } unstructConverted, ok := converted.(*unstructured.Unstructured) if !ok { // this should not happened - return nil, fmt.Errorf("conversion webhook for %v failed", in.GetObjectKind()) + return nil, fmt.Errorf("conversion webhook for %v failed, unexpected type %T at index 0", in.GetObjectKind().GroupVersionKind(), converted) } unstructIn, ok := in.(*unstructured.Unstructured) if !ok { // this should not happened - return nil, fmt.Errorf("conversion webhook for %v failed", in.GetObjectKind()) + return nil, fmt.Errorf("conversion webhook for %v failed unexpected input type %T", in.GetObjectKind().GroupVersionKind(), in) } if err := validateConvertedObject(unstructIn, unstructConverted); err != nil { - return nil, fmt.Errorf("conversion webhook for %v returned invalid object: %v", in.GetObjectKind(), err) + return nil, fmt.Errorf("conversion webhook for %v returned invalid object: %v", in.GetObjectKind().GroupVersionKind(), err) } if err := restoreObjectMeta(unstructIn, unstructConverted); err != nil { - return nil, fmt.Errorf("conversion webhook for %v returned invalid metadata: %v", in.GetObjectKind(), err) + return nil, fmt.Errorf("conversion webhook for %v returned invalid metadata: %v", in.GetObjectKind().GroupVersionKind(), err) } return converted, nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go index 50e337dc352..a1ca6a2a60a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/webhook_converter_test.go @@ -21,7 +21,13 @@ import ( "strings" "testing" + "github.com/google/go-cmp/cmp" + + v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + 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/util/diff" "k8s.io/apimachinery/pkg/util/validation" ) @@ -199,3 +205,352 @@ func TestRestoreObjectMeta(t *testing.T) { }) } } + +func TestGetObjectsToConvert(t *testing.T) { + v1Object := &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "foo/v1", "kind": "Widget", "metadata": map[string]interface{}{"name": "myv1"}}} + v2Object := &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "foo/v2", "kind": "Widget", "metadata": map[string]interface{}{"name": "myv2"}}} + v3Object := &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "foo/v3", "kind": "Widget", "metadata": map[string]interface{}{"name": "myv3"}}} + + testcases := []struct { + Name string + Object runtime.Object + APIVersion string + + ExpectObjects []runtime.RawExtension + }{ + { + Name: "empty list", + Object: &unstructured.UnstructuredList{}, + APIVersion: "foo/v1", + ExpectObjects: nil, + }, + { + Name: "one-item list, in desired version", + Object: &unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*v1Object}, + }, + APIVersion: "foo/v1", + ExpectObjects: nil, + }, + { + Name: "one-item list, not in desired version", + Object: &unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*v2Object}, + }, + APIVersion: "foo/v1", + ExpectObjects: []runtime.RawExtension{{Object: v2Object}}, + }, + { + Name: "multi-item list, in desired version", + Object: &unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*v1Object, *v1Object, *v1Object}, + }, + APIVersion: "foo/v1", + ExpectObjects: nil, + }, + { + Name: "multi-item list, mixed versions", + Object: &unstructured.UnstructuredList{ + Items: []unstructured.Unstructured{*v1Object, *v2Object, *v3Object}, + }, + APIVersion: "foo/v1", + ExpectObjects: []runtime.RawExtension{{Object: v2Object}, {Object: v3Object}}, + }, + { + Name: "single item, in desired version", + Object: v1Object, + APIVersion: "foo/v1", + ExpectObjects: nil, + }, + { + Name: "single item, not in desired version", + Object: v2Object, + APIVersion: "foo/v1", + ExpectObjects: []runtime.RawExtension{{Object: v2Object}}, + }, + } + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + if objects := getObjectsToConvert(tc.Object, tc.APIVersion); !reflect.DeepEqual(objects, tc.ExpectObjects) { + t.Errorf("unexpected diff: %s", cmp.Diff(tc.ExpectObjects, objects)) + } + }) + } +} + +func TestCreateConversionReviewObjects(t *testing.T) { + objects := []runtime.RawExtension{ + {Object: &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "foo/v2", "Kind": "Widget"}}}, + } + + testcases := []struct { + Name string + Versions []string + + ExpectRequest runtime.Object + ExpectResponse runtime.Object + ExpectErr string + }{ + { + Name: "no supported versions", + Versions: []string{"vx"}, + ExpectErr: "no supported conversion review versions", + }, + { + Name: "v1", + Versions: []string{"v1", "v1beta1", "v2"}, + ExpectRequest: &v1.ConversionReview{ + Request: &v1.ConversionRequest{UID: "uid", DesiredAPIVersion: "foo/v1", Objects: objects}, + Response: &v1.ConversionResponse{}, + }, + ExpectResponse: &v1.ConversionReview{}, + }, + { + Name: "v1beta1", + Versions: []string{"v1beta1", "v1", "v2"}, + ExpectRequest: &v1beta1.ConversionReview{ + Request: &v1beta1.ConversionRequest{UID: "uid", DesiredAPIVersion: "foo/v1", Objects: objects}, + Response: &v1beta1.ConversionResponse{}, + }, + ExpectResponse: &v1beta1.ConversionReview{}, + }, + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + request, response, err := createConversionReviewObjects(tc.Versions, objects, "foo/v1", "uid") + + if err == nil && len(tc.ExpectErr) > 0 { + t.Errorf("expected error, got none") + } else if err != nil && len(tc.ExpectErr) == 0 { + t.Errorf("unexpected error %v", err) + } else if err != nil && !strings.Contains(err.Error(), tc.ExpectErr) { + t.Errorf("expected error containing %q, got %v", tc.ExpectErr, err) + } + + if e, a := tc.ExpectRequest, request; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff: %s", cmp.Diff(e, a)) + } + if e, a := tc.ExpectResponse, response; !reflect.DeepEqual(e, a) { + t.Errorf("unexpected diff: %s", cmp.Diff(e, a)) + } + }) + } +} + +func TestGetConvertedObjectsFromResponse(t *testing.T) { + v1Object := &unstructured.Unstructured{Object: map[string]interface{}{"apiVersion": "foo/v1", "kind": "Widget", "metadata": map[string]interface{}{"name": "myv1"}}} + + testcases := []struct { + Name string + Response runtime.Object + + ExpectObjects []runtime.RawExtension + ExpectErr string + }{ + { + Name: "nil response", + Response: nil, + ExpectErr: "unrecognized response type", + }, + { + Name: "unknown type", + Response: &unstructured.Unstructured{}, + ExpectErr: "unrecognized response type", + }, + + { + Name: "minimal valid v1beta1", + Response: &v1beta1.ConversionReview{ + // apiVersion/kind were not validated originally, preserve backward compatibility + Response: &v1beta1.ConversionResponse{ + // uid was not validated originally, preserve backward compatibility + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectObjects: nil, + }, + { + Name: "valid v1beta1 with objects", + Response: &v1beta1.ConversionReview{ + // apiVersion/kind were not validated originally, preserve backward compatibility + Response: &v1beta1.ConversionResponse{ + // uid was not validated originally, preserve backward compatibility + Result: metav1.Status{Status: metav1.StatusSuccess}, + ConvertedObjects: []runtime.RawExtension{{Object: v1Object}}, + }, + }, + ExpectObjects: []runtime.RawExtension{{Object: v1Object}}, + }, + { + Name: "error v1beta1, empty status", + Response: &v1beta1.ConversionReview{ + Response: &v1beta1.ConversionResponse{ + Result: metav1.Status{Status: ""}, + }, + }, + ExpectErr: `response.result.status was '', not 'Success'`, + }, + { + Name: "error v1beta1, failure status", + Response: &v1beta1.ConversionReview{ + Response: &v1beta1.ConversionResponse{ + Result: metav1.Status{Status: metav1.StatusFailure}, + }, + }, + ExpectErr: `response.result.status was 'Failure', not 'Success'`, + }, + { + Name: "error v1beta1, custom status", + Response: &v1beta1.ConversionReview{ + Response: &v1beta1.ConversionResponse{ + Result: metav1.Status{Status: metav1.StatusFailure, Message: "some failure message"}, + }, + }, + ExpectErr: `some failure message`, + }, + { + Name: "invalid v1beta1, no response", + Response: &v1beta1.ConversionReview{}, + ExpectErr: "no response provided", + }, + + { + Name: "minimal valid v1", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectObjects: nil, + }, + { + Name: "valid v1 with objects", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + ConvertedObjects: []runtime.RawExtension{{Object: v1Object}}, + }, + }, + ExpectObjects: []runtime.RawExtension{{Object: v1Object}}, + }, + { + Name: "invalid v1, no uid", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectErr: `expected response.uid="uid"`, + }, + { + Name: "invalid v1, no apiVersion", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectErr: `expected webhook response of apiextensions.k8s.io/v1, Kind=ConversionReview`, + }, + { + Name: "invalid v1, no kind", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectErr: `expected webhook response of apiextensions.k8s.io/v1, Kind=ConversionReview`, + }, + { + Name: "invalid v1, mismatched apiVersion", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v2", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectErr: `expected webhook response of apiextensions.k8s.io/v1, Kind=ConversionReview`, + }, + { + Name: "invalid v1, mismatched kind", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview2"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusSuccess}, + }, + }, + ExpectErr: `expected webhook response of apiextensions.k8s.io/v1, Kind=ConversionReview`, + }, + { + Name: "error v1, empty status", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: ""}, + }, + }, + ExpectErr: `response.result.status was '', not 'Success'`, + }, + { + Name: "error v1, failure status", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusFailure}, + }, + }, + ExpectErr: `response.result.status was 'Failure', not 'Success'`, + }, + { + Name: "error v1, custom status", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + Response: &v1.ConversionResponse{ + UID: "uid", + Result: metav1.Status{Status: metav1.StatusFailure, Message: "some failure message"}, + }, + }, + ExpectErr: `some failure message`, + }, + { + Name: "invalid v1, no response", + Response: &v1.ConversionReview{ + TypeMeta: metav1.TypeMeta{APIVersion: "apiextensions.k8s.io/v1", Kind: "ConversionReview"}, + }, + ExpectErr: "no response provided", + }, + } + + for _, tc := range testcases { + t.Run(tc.Name, func(t *testing.T) { + + objects, err := getConvertedObjectsFromResponse("uid", tc.Response) + + if err == nil && len(tc.ExpectErr) > 0 { + t.Errorf("expected error, got none") + } else if err != nil && len(tc.ExpectErr) == 0 { + t.Errorf("unexpected error %v", err) + } else if err != nil && !strings.Contains(err.Error(), tc.ExpectErr) { + t.Errorf("expected error containing %q, got %v", tc.ExpectErr, err) + } + + if !reflect.DeepEqual(objects, tc.ExpectObjects) { + t.Errorf("unexpected diff: %s", cmp.Diff(tc.ExpectObjects, objects)) + } + + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD index 07acdaf91f4..75dd7342153 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/BUILD @@ -39,6 +39,7 @@ go_library( importpath = "k8s.io/apiextensions-apiserver/test/integration/conversion", visibility = ["//visibility:public"], deps = [ + "//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1:go_default_library", "//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", 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 index eb61e9f6965..107d0e67cdf 100644 --- 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 @@ -71,39 +71,94 @@ func TestWebhookConverterWithDefaulting(t *testing.T) { func testWebhookConverter(t *testing.T, defaulting, watchCache bool) { tests := []struct { - group string - handler http.Handler - checks []Checker + group string + handler http.Handler + reviewVersions []string + checks []Checker }{ { - group: "noop-converter", - handler: NewObjectConverterWebhookHandler(t, noopConverter), - checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1")), // no v1beta2 as the schema differs + group: "noop-converter-v1", + handler: NewObjectConverterWebhookHandler(t, noopConverter), + reviewVersions: []string{"v1", "v1beta1"}, + 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, validateDefaulting), + group: "noop-converter-v1beta1", + handler: NewObjectConverterWebhookHandler(t, noopConverter), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1")), // no v1beta2 as the schema differs }, { - group: "metadata-mutating-converter", - handler: NewObjectConverterWebhookHandler(t, metadataMutatingConverter), - checks: checks(validateObjectMetaMutation), + group: "nontrivial-converter-v1", + handler: NewObjectConverterWebhookHandler(t, nontrivialConverter), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1", "v1beta2"), validateNonTrivialConverted, validateNonTrivialConvertedList, validateStoragePruning, validateDefaulting), }, { - group: "metadata-uid-mutating-converter", - handler: NewObjectConverterWebhookHandler(t, uidMutatingConverter), - checks: checks(validateUIDMutation), + group: "nontrivial-converter-v1beta1", + handler: NewObjectConverterWebhookHandler(t, nontrivialConverter), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(validateStorageVersion, validateServed, validateMixedStorageVersions("v1alpha1", "v1beta1", "v1beta2"), validateNonTrivialConverted, validateNonTrivialConvertedList, validateStoragePruning, validateDefaulting), }, { - group: "empty-response", - handler: NewReviewWebhookHandler(t, emptyResponseConverter), - checks: checks(expectConversionFailureMessage("empty-response", "returned 0 objects, expected 1")), + group: "metadata-mutating-v1", + handler: NewObjectConverterWebhookHandler(t, metadataMutatingConverter), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(validateObjectMetaMutation), }, { - group: "failure-message", - handler: NewReviewWebhookHandler(t, failureResponseConverter("custom webhook conversion error")), - checks: checks(expectConversionFailureMessage("failure-message", "custom webhook conversion error")), + group: "metadata-mutating-v1beta1", + handler: NewObjectConverterWebhookHandler(t, metadataMutatingConverter), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(validateObjectMetaMutation), + }, + { + group: "metadata-uid-mutating-v1", + handler: NewObjectConverterWebhookHandler(t, uidMutatingConverter), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(validateUIDMutation), + }, + { + group: "metadata-uid-mutating-v1beta1", + handler: NewObjectConverterWebhookHandler(t, uidMutatingConverter), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(validateUIDMutation), + }, + { + group: "empty-response-v1", + handler: NewReviewWebhookHandler(t, nil, emptyV1ResponseConverter), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(expectConversionFailureMessage("empty-response", "returned 0 objects, expected 1")), + }, + { + group: "empty-response-v1beta1", + handler: NewReviewWebhookHandler(t, emptyV1Beta1ResponseConverter, nil), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(expectConversionFailureMessage("empty-response", "returned 0 objects, expected 1")), + }, + { + group: "failure-message-v1", + handler: NewReviewWebhookHandler(t, nil, failureV1ResponseConverter("custom webhook conversion error")), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(expectConversionFailureMessage("failure-message", "custom webhook conversion error")), + }, + { + group: "failure-message-v1beta1", + handler: NewReviewWebhookHandler(t, failureV1Beta1ResponseConverter("custom webhook conversion error"), nil), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(expectConversionFailureMessage("failure-message", "custom webhook conversion error")), + }, + { + group: "unhandled-v1", + handler: NewReviewWebhookHandler(t, nil, nil), + reviewVersions: []string{"v1", "v1beta1"}, + checks: checks(expectConversionFailureMessage("server-error", "the server rejected our request")), + }, + { + group: "unhandled-v1beta1", + handler: NewReviewWebhookHandler(t, nil, nil), + reviewVersions: []string{"v1beta1", "v1"}, + checks: checks(expectConversionFailureMessage("server-error", "the server rejected our request")), }, } @@ -174,7 +229,7 @@ func testWebhookConverter(t *testing.T, defaulting, watchCache bool) { } defer tearDown() - ctc.setConversionWebhook(t, webhookClientConfig) + ctc.setConversionWebhook(t, webhookClientConfig, test.reviewVersions) defer ctc.removeConversionWebhook(t) // wait until new webhook is called the first time @@ -711,7 +766,15 @@ func noopConverter(desiredAPIVersion string, obj runtime.RawExtension) (runtime. return runtime.RawExtension{Raw: raw}, nil } -func emptyResponseConverter(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { +func emptyV1ResponseConverter(review *apiextensionsv1.ConversionReview) (*apiextensionsv1.ConversionReview, error) { + review.Response = &apiextensionsv1.ConversionResponse{ + UID: review.Request.UID, + ConvertedObjects: []runtime.RawExtension{}, + Result: metav1.Status{Status: "Success"}, + } + return review, nil +} +func emptyV1Beta1ResponseConverter(review *apiextensionsv1beta1.ConversionReview) (*apiextensionsv1beta1.ConversionReview, error) { review.Response = &apiextensionsv1beta1.ConversionResponse{ UID: review.Request.UID, ConvertedObjects: []runtime.RawExtension{}, @@ -720,8 +783,19 @@ func emptyResponseConverter(review apiextensionsv1beta1.ConversionReview) (apiex return review, nil } -func failureResponseConverter(message string) func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { - return func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { +func failureV1ResponseConverter(message string) func(review *apiextensionsv1.ConversionReview) (*apiextensionsv1.ConversionReview, error) { + return func(review *apiextensionsv1.ConversionReview) (*apiextensionsv1.ConversionReview, error) { + review.Response = &apiextensionsv1.ConversionResponse{ + UID: review.Request.UID, + ConvertedObjects: []runtime.RawExtension{}, + Result: metav1.Status{Message: message, Status: "Failure"}, + } + return review, nil + } +} + +func failureV1Beta1ResponseConverter(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{}, @@ -889,14 +963,15 @@ func (c *conversionTestContext) versionedClients(ns string) map[string]dynamic.R return ret } -func (c *conversionTestContext) setConversionWebhook(t *testing.T, webhookClientConfig *apiextensionsv1beta1.WebhookClientConfig) { +func (c *conversionTestContext) setConversionWebhook(t *testing.T, webhookClientConfig *apiextensionsv1beta1.WebhookClientConfig, reviewVersions []string) { 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, + Strategy: apiextensionsv1beta1.WebhookConverter, + WebhookClientConfig: webhookClientConfig, + ConversionReviewVersions: reviewVersions, } crd, err = c.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(crd) if err != nil { diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go index 6f82c999808..41adf59ab25 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/webhook.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -73,11 +74,14 @@ func StartConversionWebhookServer(handler http.Handler) (func(), *apiextensionsv return webhookServer.Close, webhookConfig, nil } -// ReviewConverterFunc converts an entire ConversionReview. -type ReviewConverterFunc func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) +// V1Beta1ReviewConverterFunc converts an entire ConversionReview. +type V1Beta1ReviewConverterFunc func(review *apiextensionsv1beta1.ConversionReview) (*apiextensionsv1beta1.ConversionReview, error) + +// V1ReviewConverterFunc converts an entire ConversionReview. +type V1ReviewConverterFunc func(review *apiextensionsv1.ConversionReview) (*apiextensionsv1.ConversionReview, error) // NewReviewWebhookHandler creates a handler that delegates the review conversion to the provided ReviewConverterFunc. -func NewReviewWebhookHandler(t *testing.T, converterFunc ReviewConverterFunc) http.Handler { +func NewReviewWebhookHandler(t *testing.T, v1beta1ConverterFunc V1Beta1ReviewConverterFunc, v1ConverterFunc V1ReviewConverterFunc) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() data, err := ioutil.ReadAll(r.Body) @@ -90,21 +94,63 @@ func NewReviewWebhookHandler(t *testing.T, converterFunc ReviewConverterFunc) ht return } - review := apiextensionsv1beta1.ConversionReview{} - if err := json.Unmarshal(data, &review); err != nil { + typeMeta := &metav1.TypeMeta{} + if err := json.Unmarshal(data, typeMeta); err != nil { t.Errorf("Fail to deserialize object: %s with error: %v", string(data), err) http.Error(w, err.Error(), 400) return } - review, err = converterFunc(review) - if err != nil { - t.Errorf("Error converting review: %v", err) - http.Error(w, err.Error(), 500) + var response runtime.Object + + switch typeMeta.GroupVersionKind() { + case apiextensionsv1.SchemeGroupVersion.WithKind("ConversionReview"): + review := &apiextensionsv1.ConversionReview{} + if err := json.Unmarshal(data, review); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(data), err) + http.Error(w, err.Error(), 400) + return + } + + if v1ConverterFunc == nil { + http.Error(w, "Cannot handle v1 ConversionReview", 422) + return + } + response, err = v1ConverterFunc(review) + if err != nil { + t.Errorf("Error converting review: %v", err) + http.Error(w, err.Error(), 500) + return + } + + case apiextensionsv1beta1.SchemeGroupVersion.WithKind("ConversionReview"): + review := &apiextensionsv1beta1.ConversionReview{} + if err := json.Unmarshal(data, review); err != nil { + t.Errorf("Fail to deserialize object: %s with error: %v", string(data), err) + http.Error(w, err.Error(), 400) + return + } + + if v1beta1ConverterFunc == nil { + http.Error(w, "Cannot handle v1beta1 ConversionReview", 422) + return + } + response, err = v1beta1ConverterFunc(review) + if err != nil { + t.Errorf("Error converting review: %v", err) + http.Error(w, err.Error(), 500) + return + } + + default: + err := fmt.Errorf("unrecognized request kind: %v", typeMeta.GroupVersionKind()) + t.Error(err) + http.Error(w, err.Error(), 400) + return } w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(review); err != nil { + if err := json.NewEncoder(w).Encode(response); err != nil { t.Errorf("Marshal of response failed with error: %v", err) } }) @@ -115,7 +161,7 @@ type ObjectConverterFunc func(desiredAPIVersion string, customResource runtime.R // NewObjectConverterWebhookHandler creates a handler that delegates custom resource conversion to the provided ConverterFunc. func NewObjectConverterWebhookHandler(t *testing.T, converterFunc ObjectConverterFunc) http.Handler { - return NewReviewWebhookHandler(t, func(review apiextensionsv1beta1.ConversionReview) (apiextensionsv1beta1.ConversionReview, error) { + return NewReviewWebhookHandler(t, func(review *apiextensionsv1beta1.ConversionReview) (*apiextensionsv1beta1.ConversionReview, error) { converted := []runtime.RawExtension{} errMsgs := []string{} for _, obj := range review.Request.Objects { @@ -137,6 +183,28 @@ func NewObjectConverterWebhookHandler(t *testing.T, converterFunc ObjectConverte review.Response.Result = metav1.Status{Status: "Failure", Message: strings.Join(errMsgs, ", ")} } return review, nil + }, func(review *apiextensionsv1.ConversionReview) (*apiextensionsv1.ConversionReview, error) { + converted := []runtime.RawExtension{} + errMsgs := []string{} + for _, obj := range review.Request.Objects { + convertedObj, err := converterFunc(review.Request.DesiredAPIVersion, obj) + if err != nil { + errMsgs = append(errMsgs, err.Error()) + } + + converted = append(converted, convertedObj) + } + + review.Response = &apiextensionsv1.ConversionResponse{ + UID: review.Request.UID, + ConvertedObjects: converted, + } + if len(errMsgs) == 0 { + review.Response.Result = metav1.Status{Status: "Success"} + } else { + review.Response.Result = metav1.Status{Status: "Failure", Message: strings.Join(errMsgs, ", ")} + } + return review, nil }) }