diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go index 1ce53d0ebb..0c01ff3920 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter.go @@ -20,6 +20,7 @@ import ( "bytes" encodingjson "encoding/json" "fmt" + "math" "os" "reflect" "strconv" @@ -31,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/json" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "github.com/golang/glog" ) @@ -82,7 +84,7 @@ var ( func parseBool(key string) bool { value, err := strconv.ParseBool(key) if err != nil { - glog.Errorf("Couldn't parse %s as bool", key) + utilruntime.HandleError(fmt.Errorf("Couldn't parse '%s' as bool for unstructured mismatch detection", key)) } return value } @@ -173,8 +175,12 @@ func fromUnstructured(sv, dv reflect.Value) error { dv.Set(sv.Convert(dt)) return nil } + if sv.Float() == math.Trunc(sv.Float()) { + dv.Set(sv.Convert(dt)) + return nil + } } - return fmt.Errorf("cannot convert %s to %d", st.String(), dt.String()) + return fmt.Errorf("cannot convert %s to %s", st.String(), dt.String()) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go index 9a8b595c9d..cbab0d1dad 100644 --- a/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/conversion/unstructured/converter_test.go @@ -438,3 +438,25 @@ func TestUnrecognized(t *testing.T) { } } } + +func TestFloatIntConversion(t *testing.T) { + unstr := map[string]interface{}{"fd": float64(3)} + + var obj F + if err := DefaultConverter.FromUnstructured(unstr, &obj); err != nil { + t.Errorf("Unexpected error in FromUnstructured: %v", err) + } + + data, err := json.Marshal(unstr) + if err != nil { + t.Fatalf("Error when marshaling unstructured: %v", err) + } + var unmarshalled F + if err := json.Unmarshal(data, &unmarshalled); err != nil { + t.Fatalf("Error when unmarshaling to object: %v", err) + } + + if !reflect.DeepEqual(obj, unmarshalled) { + t.Errorf("Incorrect conversion, diff: %v", diff.ObjectReflectDiff(obj, unmarshalled)) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index 52636c3957..5265a86038 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -65,11 +65,12 @@ type APIGroupVersion struct { Serializer runtime.NegotiatedSerializer ParameterCodec runtime.ParameterCodec - Typer runtime.ObjectTyper - Creater runtime.ObjectCreater - Convertor runtime.ObjectConvertor - Copier runtime.ObjectCopier - Linker runtime.SelfLinker + Typer runtime.ObjectTyper + Creater runtime.ObjectCreater + Convertor runtime.ObjectConvertor + Copier runtime.ObjectCopier + Linker runtime.SelfLinker + UnsafeConvertor runtime.ObjectConvertor Admit admission.Interface Context request.RequestContextMapper diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 1153586057..a44651c47a 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/strategicpatch" @@ -77,6 +78,7 @@ func patchObjectJSON( // and stores the result in . // It additionally returns the map[string]interface{} representation of the // and . +// NOTE: Both and are supposed to be versioned. func strategicPatchObject( codec runtime.Codec, originalObject runtime.Object, @@ -84,14 +86,8 @@ func strategicPatchObject( objToUpdate runtime.Object, versionedObj runtime.Object, ) (originalObjMap map[string]interface{}, patchMap map[string]interface{}, retErr error) { - // TODO: This should be one-step conversion that doesn't require - // json marshaling and unmarshaling once #39017 is fixed. - data, err := runtime.Encode(codec, originalObject) - if err != nil { - return nil, nil, err - } originalObjMap = make(map[string]interface{}) - if err := json.Unmarshal(data, &originalObjMap); err != nil { + if err := unstructured.DefaultConverter.ToUnstructured(originalObject, &originalObjMap); err != nil { return nil, nil, err } @@ -121,11 +117,5 @@ func applyPatchToObject( return err } - // TODO: This should be one-step conversion that doesn't require - // json marshaling and unmarshaling once #39017 is fixed. - data, err := json.Marshal(patchedObjMap) - if err != nil { - return err - } - return runtime.DecodeInto(codec, data, objToUpdate) + return unstructured.DefaultConverter.FromUnstructured(patchedObjMap, objToUpdate) } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 280729c626..ac4db5ab99 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/conversion/unstructured" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -81,9 +82,11 @@ type RequestScope struct { Serializer runtime.NegotiatedSerializer runtime.ParameterCodec - Creater runtime.ObjectCreater - Convertor runtime.ObjectConvertor - Copier runtime.ObjectCopier + Creater runtime.ObjectCreater + Convertor runtime.ObjectConvertor + Copier runtime.ObjectCopier + Typer runtime.ObjectTyper + UnsafeConvertor runtime.ObjectConvertor Resource schema.GroupVersionResource Kind schema.GroupVersionKind @@ -520,7 +523,8 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface return nil } - result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS, scope.Namer, scope.Copier, scope.Resource, codec) + result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS, + scope.Namer, scope.Copier, scope.Creater, scope.UnsafeConvertor, scope.Kind, scope.Resource, codec) if err != nil { scope.err(err, res.ResponseWriter, req.Request) return @@ -550,6 +554,9 @@ func patchResource( patchJS []byte, namer ScopeNamer, copier runtime.ObjectCopier, + creater runtime.ObjectCreater, + unsafeConvertor runtime.ObjectConvertor, + kind schema.GroupVersionKind, resource schema.GroupVersionResource, codec runtime.Codec, ) (runtime.Object, error) { @@ -594,10 +601,28 @@ func patchResource( } originalObjJS, originalPatchedObjJS = originalJS, patchedJS case types.StrategicMergePatchType: - originalMap, patchMap, err := strategicPatchObject(codec, currentObject, patchJS, objToUpdate, versionedObj) + // Since the patch is applied on versioned objects, we need to convert the + // current object to versioned representation first. + currentVersionedObject, err := unsafeConvertor.ConvertToVersion(currentObject, kind.GroupVersion()) if err != nil { return nil, err } + versionedObjToUpdate, err := creater.New(kind) + if err != nil { + return nil, err + } + originalMap, patchMap, err := strategicPatchObject(codec, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj) + if err != nil { + return nil, err + } + // Convert the object back to unversioned. + gvk := kind.GroupKind().WithVersion(runtime.APIVersionInternal) + unversionedObjToUpdate, err := unsafeConvertor.ConvertToVersion(versionedObjToUpdate, gvk.GroupVersion()) + if err != nil { + return nil, err + } + objToUpdate = unversionedObjToUpdate + // Store unstructured representations for possible retries. originalObjMap, originalPatchMap = originalMap, patchMap } if err := checkName(objToUpdate, name, namespace, namer); err != nil { @@ -614,14 +639,15 @@ func patchResource( // 3. ensure no conflicts between the two patches // 4. apply the #1 patch to the currentJS object - // TODO: This should be one-step conversion that doesn't require - // json marshaling and unmarshaling once #39017 is fixed. - data, err := runtime.Encode(codec, currentObject) + currentObjMap := make(map[string]interface{}) + + // Since the patch is applied on versioned objects, we need to convert the + // current object to versioned representation first. + currentVersionedObject, err := unsafeConvertor.ConvertToVersion(currentObject, kind.GroupVersion()) if err != nil { return nil, err } - currentObjMap := make(map[string]interface{}) - if err := json.Unmarshal(data, ¤tObjMap); err != nil { + if err := unstructured.DefaultConverter.ToUnstructured(currentVersionedObject, ¤tObjMap); err != nil { return nil, err } @@ -677,8 +703,17 @@ func patchResource( return nil, errors.NewConflict(resource.GroupResource(), name, patchDiffErr) } - objToUpdate := patcher.New() - if err := applyPatchToObject(codec, currentObjMap, originalPatchMap, objToUpdate, versionedObj); err != nil { + versionedObjToUpdate, err := creater.New(kind) + if err != nil { + return nil, err + } + if err := applyPatchToObject(codec, currentObjMap, originalPatchMap, versionedObjToUpdate, versionedObj); err != nil { + return nil, err + } + // Convert the object back to unversioned. + gvk := kind.GroupKind().WithVersion(runtime.APIVersionInternal) + objToUpdate, err := unsafeConvertor.ConvertToVersion(versionedObjToUpdate, gvk.GroupVersion()) + if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go index 10354d7d38..0f4e6ac121 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go @@ -202,6 +202,9 @@ func (tc *patchTestCase) Run(t *testing.T) { namer := &testNamer{namespace, name} copier := runtime.ObjectCopier(api.Scheme) + creater := runtime.ObjectCreater(api.Scheme) + convertor := runtime.UnsafeObjectConvertor(api.Scheme) + kind := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"} resource := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"} versionedObj := &v1.Pod{} @@ -244,7 +247,7 @@ func (tc *patchTestCase) Run(t *testing.T) { } - resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, copier, resource, codec) + resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, copier, creater, convertor, kind, resource, codec) if len(tc.expectedError) != 0 { if err == nil || err.Error() != tc.expectedError { t.Errorf("%s: expected error %v, but got %v", tc.name, tc.expectedError, err) @@ -266,18 +269,12 @@ func (tc *patchTestCase) Run(t *testing.T) { resultPod := resultObj.(*api.Pod) - // roundtrip to get defaulting - expectedJS, err := runtime.Encode(codec, tc.expectedPod) - if err != nil { - t.Errorf("%s: unexpected error: %v", tc.name, err) - return - } - expectedObj, err := runtime.Decode(codec, expectedJS) - if err != nil { - t.Errorf("%s: unexpected error: %v", tc.name, err) - return - } - reallyExpectedPod := expectedObj.(*api.Pod) + // TODO: In the process of applying patches, we are calling + // conversions between versioned and unversioned objects. + // In case of Pod, conversion from versioned to unversioned + // is setting PodSecurityContext, so we need to set it here too. + reallyExpectedPod := tc.expectedPod + reallyExpectedPod.Spec.SecurityContext = &api.PodSecurityContext{} if !reflect.DeepEqual(*reallyExpectedPod, *resultPod) { t.Errorf("%s mismatch: %v\n", tc.name, diff.ObjectGoPrintDiff(reallyExpectedPod, resultPod)) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index d853873d99..f39f4619bb 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -516,12 +516,14 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag kubeVerbs := map[string]struct{}{} reqScope := handlers.RequestScope{ - ContextFunc: ctxFn, - Serializer: a.group.Serializer, - ParameterCodec: a.group.ParameterCodec, - Creater: a.group.Creater, - Convertor: a.group.Convertor, - Copier: a.group.Copier, + ContextFunc: ctxFn, + Serializer: a.group.Serializer, + ParameterCodec: a.group.ParameterCodec, + Creater: a.group.Creater, + Convertor: a.group.Convertor, + Copier: a.group.Copier, + Typer: a.group.Typer, + UnsafeConvertor: a.group.UnsafeConvertor, // TODO: This seems wrong for cross-group subresources. It makes an assumption that a subresource and its parent are in the same group version. Revisit this. Resource: a.group.GroupVersion.WithResource(resource), diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 2c19a0d282..4babef1bf1 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -335,12 +335,13 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV GroupVersion: groupVersion, MetaGroupVersion: apiGroupInfo.MetaGroupVersion, - ParameterCodec: apiGroupInfo.ParameterCodec, - Serializer: apiGroupInfo.NegotiatedSerializer, - Creater: apiGroupInfo.Scheme, - Convertor: apiGroupInfo.Scheme, - Copier: apiGroupInfo.Scheme, - Typer: apiGroupInfo.Scheme, + ParameterCodec: apiGroupInfo.ParameterCodec, + Serializer: apiGroupInfo.NegotiatedSerializer, + Creater: apiGroupInfo.Scheme, + Convertor: apiGroupInfo.Scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(apiGroupInfo.Scheme), + Copier: apiGroupInfo.Scheme, + Typer: apiGroupInfo.Scheme, SubresourceGroupVersionKind: apiGroupInfo.SubresourceGroupVersionKind, Linker: apiGroupInfo.GroupMeta.SelfLinker, Mapper: apiGroupInfo.GroupMeta.RESTMapper, diff --git a/vendor/BUILD b/vendor/BUILD index 58d884cdd4..a8d97dba98 100644 --- a/vendor/BUILD +++ b/vendor/BUILD @@ -8539,6 +8539,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/runtime", "//vendor:k8s.io/apimachinery/pkg/util/diff", "//vendor:k8s.io/apimachinery/pkg/util/json", + "//vendor:k8s.io/apimachinery/pkg/util/runtime", ], ) @@ -9900,6 +9901,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/api/meta", "//vendor:k8s.io/apimachinery/pkg/apis/meta/internalversion", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", + "//vendor:k8s.io/apimachinery/pkg/conversion/unstructured", "//vendor:k8s.io/apimachinery/pkg/fields", "//vendor:k8s.io/apimachinery/pkg/runtime", "//vendor:k8s.io/apimachinery/pkg/runtime/schema",