Ensure patched objects are defaulted correctly

Kubernetes-commit: 464db160b410b22dba53af6d11fa097f0fa4fd6b
This commit is contained in:
Jordan Liggitt 2017-03-10 22:07:01 -05:00 committed by Kubernetes Publisher
parent f0174d3be4
commit 8543f589d2
7 changed files with 44 additions and 16 deletions

View File

@ -284,6 +284,7 @@ func handleInternal(storage map[string]rest.Storage, admissionControl admission.
Creater: scheme,
Convertor: scheme,
Copier: scheme,
Defaulter: scheme,
Typer: scheme,
Linker: selfLinker,
Mapper: namespaceMapper,
@ -2579,6 +2580,7 @@ func TestUpdateREST(t *testing.T) {
Creater: scheme,
Convertor: scheme,
Copier: scheme,
Defaulter: scheme,
Typer: scheme,
Linker: selfLinker,
@ -2663,6 +2665,7 @@ func TestParentResourceIsRequired(t *testing.T) {
Creater: scheme,
Convertor: scheme,
Copier: scheme,
Defaulter: scheme,
Typer: scheme,
Linker: selfLinker,
@ -2694,6 +2697,7 @@ func TestParentResourceIsRequired(t *testing.T) {
Creater: scheme,
Convertor: scheme,
Copier: scheme,
Defaulter: scheme,
Typer: scheme,
Linker: selfLinker,
@ -3307,6 +3311,7 @@ func TestXGSubresource(t *testing.T) {
Creater: scheme,
Convertor: scheme,
Copier: scheme,
Defaulter: scheme,
Typer: scheme,
Linker: selfLinker,
Mapper: namespaceMapper,

View File

@ -69,6 +69,7 @@ type APIGroupVersion struct {
Creater runtime.ObjectCreater
Convertor runtime.ObjectConvertor
Copier runtime.ObjectCopier
Defaulter runtime.ObjectDefaulter
Linker runtime.SelfLinker
UnsafeConvertor runtime.ObjectConvertor

View File

@ -81,6 +81,7 @@ func patchObjectJSON(
// NOTE: Both <originalObject> and <objToUpdate> are supposed to be versioned.
func strategicPatchObject(
codec runtime.Codec,
defaulter runtime.ObjectDefaulter,
originalObject runtime.Object,
patchJS []byte,
objToUpdate runtime.Object,
@ -96,17 +97,18 @@ func strategicPatchObject(
return nil, nil, err
}
if err := applyPatchToObject(codec, originalObjMap, patchMap, objToUpdate, versionedObj); err != nil {
if err := applyPatchToObject(codec, defaulter, originalObjMap, patchMap, objToUpdate, versionedObj); err != nil {
return nil, nil, err
}
return
}
// applyPatchToObject applies a strategic merge patch of <patchMap> to
// <originalMap> and stores the result in <objToUpdate>, though it operates
// on versioned map[string]interface{} representations.
// <originalMap> and stores the result in <objToUpdate>.
// NOTE: <objToUpdate> must be a versioned object.
func applyPatchToObject(
codec runtime.Codec,
defaulter runtime.ObjectDefaulter,
originalMap map[string]interface{},
patchMap map[string]interface{},
objToUpdate runtime.Object,
@ -117,5 +119,12 @@ func applyPatchToObject(
return err
}
return unstructured.DefaultConverter.FromUnstructured(patchedObjMap, objToUpdate)
// Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object
if err := unstructured.DefaultConverter.FromUnstructured(patchedObjMap, objToUpdate); err != nil {
return err
}
// Decoding from JSON to a versioned object would apply defaults, so we do the same here
defaulter.Default(objToUpdate)
return nil
}

View File

@ -84,6 +84,7 @@ type RequestScope struct {
Creater runtime.ObjectCreater
Convertor runtime.ObjectConvertor
Defaulter runtime.ObjectDefaulter
Copier runtime.ObjectCopier
Typer runtime.ObjectTyper
UnsafeConvertor runtime.ObjectConvertor
@ -525,7 +526,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface
}
result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS,
scope.Namer, scope.Copier, scope.Creater, scope.UnsafeConvertor, scope.Kind, scope.Resource, codec)
scope.Namer, scope.Copier, scope.Creater, scope.Defaulter, scope.UnsafeConvertor, scope.Kind, scope.Resource, codec)
if err != nil {
scope.err(err, res.ResponseWriter, req.Request)
return
@ -556,6 +557,7 @@ func patchResource(
namer ScopeNamer,
copier runtime.ObjectCopier,
creater runtime.ObjectCreater,
defaulter runtime.ObjectDefaulter,
unsafeConvertor runtime.ObjectConvertor,
kind schema.GroupVersionKind,
resource schema.GroupVersionResource,
@ -619,7 +621,7 @@ func patchResource(
if err != nil {
return nil, err
}
originalMap, patchMap, err := strategicPatchObject(codec, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj)
originalMap, patchMap, err := strategicPatchObject(codec, defaulter, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj)
if err != nil {
return nil, err
}
@ -716,7 +718,7 @@ func patchResource(
if err != nil {
return nil, err
}
if err := applyPatchToObject(codec, currentObjMap, originalPatchMap, versionedObjToUpdate, versionedObj); err != nil {
if err := applyPatchToObject(codec, defaulter, currentObjMap, originalPatchMap, versionedObjToUpdate, versionedObj); err != nil {
return nil, err
}
// Convert the object back to unversioned.

View File

@ -61,6 +61,7 @@ func TestPatchAnonymousField(t *testing.T) {
testGV := schema.GroupVersion{Group: "", Version: "v"}
api.Scheme.AddKnownTypes(testGV, &testPatchType{})
codec := api.Codecs.LegacyCodec(testGV)
defaulter := runtime.ObjectDefaulter(api.Scheme)
original := &testPatchType{
TypeMeta: metav1.TypeMeta{Kind: "testPatchType", APIVersion: "v"},
@ -73,7 +74,7 @@ func TestPatchAnonymousField(t *testing.T) {
}
actual := &testPatchType{}
_, _, err := strategicPatchObject(codec, original, []byte(patch), actual, &testPatchType{})
_, _, err := strategicPatchObject(codec, defaulter, original, []byte(patch), actual, &testPatchType{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@ -203,6 +204,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
namer := &testNamer{namespace, name}
copier := runtime.ObjectCopier(api.Scheme)
creater := runtime.ObjectCreater(api.Scheme)
defaulter := runtime.ObjectDefaulter(api.Scheme)
convertor := runtime.UnsafeObjectConvertor(api.Scheme)
kind := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}
resource := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}
@ -247,7 +249,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
}
resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, copier, creater, convertor, kind, resource, codec)
resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, copier, creater, defaulter, 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)
@ -269,12 +271,18 @@ func (tc *patchTestCase) Run(t *testing.T) {
resultPod := resultObj.(*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{}
// 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)
if !reflect.DeepEqual(*reallyExpectedPod, *resultPod) {
t.Errorf("%s mismatch: %v\n", tc.name, diff.ObjectGoPrintDiff(reallyExpectedPod, resultPod))
@ -286,6 +294,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
func TestNumberConversion(t *testing.T) {
codec := api.Codecs.LegacyCodec(schema.GroupVersion{Version: "v1"})
defaulter := runtime.ObjectDefaulter(api.Scheme)
currentVersionedObject := &v1.Service{
TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"},
@ -305,7 +314,7 @@ func TestNumberConversion(t *testing.T) {
patchJS := []byte(`{"spec":{"ports":[{"port":80,"nodePort":31789}]}}`)
_, _, err := strategicPatchObject(codec, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj)
_, _, err := strategicPatchObject(codec, defaulter, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj)
if err != nil {
t.Fatal(err)
}

View File

@ -522,6 +522,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
Creater: a.group.Creater,
Convertor: a.group.Convertor,
Copier: a.group.Copier,
Defaulter: a.group.Defaulter,
Typer: a.group.Typer,
UnsafeConvertor: a.group.UnsafeConvertor,

View File

@ -366,6 +366,7 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV
Convertor: apiGroupInfo.Scheme,
UnsafeConvertor: runtime.UnsafeObjectConvertor(apiGroupInfo.Scheme),
Copier: apiGroupInfo.Scheme,
Defaulter: apiGroupInfo.Scheme,
Typer: apiGroupInfo.Scheme,
SubresourceGroupVersionKind: apiGroupInfo.SubresourceGroupVersionKind,
Linker: apiGroupInfo.GroupMeta.SelfLinker,