mirror of https://github.com/k3s-io/k3s
Merge pull request #19481 from deads2k/fix-patch-admission
make patch call update admission chain after applying the patchpull/6/head
Alex Mohr
commit
21a5d57c75
|
|
@ -466,21 +466,9 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
|
|||
return
|
||||
}
|
||||
|
||||
obj := r.New()
|
||||
ctx := scope.ContextFunc(req)
|
||||
ctx = api.WithNamespace(ctx, namespace)
|
||||
|
||||
// PATCH requires same permission as UPDATE
|
||||
if admit.Handles(admission.Update) {
|
||||
userInfo, _ := api.UserFrom(ctx)
|
||||
|
||||
err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
|
||||
if err != nil {
|
||||
errorJSON(err, scope.Codec, w)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
versionedObj, err := converter.ConvertToVersion(r.New(), scope.Kind.GroupVersion().String())
|
||||
if err != nil {
|
||||
errorJSON(err, scope.Codec, w)
|
||||
|
|
@ -500,7 +488,16 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
|
|||
return
|
||||
}
|
||||
|
||||
result, err := patchResource(ctx, timeout, versionedObj, r, name, patchType, patchJS, scope.Namer, scope.Codec)
|
||||
updateAdmit := func(updatedObject runtime.Object) error {
|
||||
if admit != nil && admit.Handles(admission.Update) {
|
||||
userInfo, _ := api.UserFrom(ctx)
|
||||
return admit.Admit(admission.NewAttributesRecord(updatedObject, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
result, err := patchResource(ctx, updateAdmit, timeout, versionedObj, r, name, patchType, patchJS, scope.Namer, scope.Codec)
|
||||
if err != nil {
|
||||
errorJSON(err, scope.Codec, w)
|
||||
return
|
||||
|
|
@ -516,8 +513,10 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
|
|||
|
||||
}
|
||||
|
||||
type updateAdmissionFunc func(updatedObject runtime.Object) error
|
||||
|
||||
// patchResource divides PatchResource for easier unit testing
|
||||
func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.Object, patcher rest.Patcher, name string, patchType api.PatchType, patchJS []byte, namer ScopeNamer, codec runtime.Codec) (runtime.Object, error) {
|
||||
func patchResource(ctx api.Context, admit updateAdmissionFunc, timeout time.Duration, versionedObj runtime.Object, patcher rest.Patcher, name string, patchType api.PatchType, patchJS []byte, namer ScopeNamer, codec runtime.Codec) (runtime.Object, error) {
|
||||
namespace := api.NamespaceValue(ctx)
|
||||
|
||||
original, err := patcher.Get(ctx, name)
|
||||
|
|
@ -543,6 +542,10 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.
|
|||
}
|
||||
|
||||
return finishRequest(timeout, func() (runtime.Object, error) {
|
||||
if err := admit(objToUpdate); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// update should never create as previous get would fail
|
||||
updateObject, _, updateErr := patcher.Update(ctx, objToUpdate)
|
||||
for i := 0; i < MaxPatchConflicts && (errors.IsConflict(updateErr)); i++ {
|
||||
|
|
@ -597,6 +600,10 @@ func patchResource(ctx api.Context, timeout time.Duration, versionedObj runtime.
|
|||
return nil, err
|
||||
}
|
||||
|
||||
if err := admit(objToUpdate); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
updateObject, _, updateErr = patcher.Update(ctx, objToUpdate)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -133,6 +133,9 @@ func (p *testNamer) GenerateListLink(req *restful.Request) (path, query string,
|
|||
type patchTestCase struct {
|
||||
name string
|
||||
|
||||
// admission chain to use, nil is fine
|
||||
admit updateAdmissionFunc
|
||||
|
||||
// startingPod is used for the first Get
|
||||
startingPod *api.Pod
|
||||
// changedPod is the "destination" pod for the patch. The test will create a patch from the startingPod to the changedPod
|
||||
|
|
@ -153,6 +156,12 @@ func (tc *patchTestCase) Run(t *testing.T) {
|
|||
name := tc.startingPod.Name
|
||||
|
||||
codec := latest.GroupOrDie(api.GroupName).Codec
|
||||
admit := tc.admit
|
||||
if admit == nil {
|
||||
admit = func(updatedObject runtime.Object) error {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
testPatcher := &testPatcher{}
|
||||
testPatcher.startingPod = tc.startingPod
|
||||
|
|
@ -208,7 +217,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
|
|||
|
||||
}
|
||||
|
||||
resultObj, err := patchResource(ctx, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, codec)
|
||||
resultObj, err := patchResource(ctx, admit, 1*time.Second, versionedObj, testPatcher, name, patchType, patch, namer, 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)
|
||||
|
|
@ -329,3 +338,86 @@ func TestPatchResourceWithConflict(t *testing.T) {
|
|||
|
||||
tc.Run(t)
|
||||
}
|
||||
|
||||
func TestPatchWithAdmissionRejection(t *testing.T) {
|
||||
namespace := "bar"
|
||||
name := "foo"
|
||||
fifteen := int64(15)
|
||||
thirty := int64(30)
|
||||
|
||||
tc := &patchTestCase{
|
||||
name: "TestPatchWithAdmissionRejection",
|
||||
|
||||
admit: func(updatedObject runtime.Object) error {
|
||||
return errors.New("admission failure")
|
||||
},
|
||||
|
||||
startingPod: &api.Pod{},
|
||||
changedPod: &api.Pod{},
|
||||
updatePod: &api.Pod{},
|
||||
|
||||
expectedError: "admission failure",
|
||||
}
|
||||
|
||||
tc.startingPod.Name = name
|
||||
tc.startingPod.Namespace = namespace
|
||||
tc.startingPod.ResourceVersion = "1"
|
||||
tc.startingPod.APIVersion = "v1"
|
||||
tc.startingPod.Spec.ActiveDeadlineSeconds = &fifteen
|
||||
|
||||
tc.changedPod.Name = name
|
||||
tc.changedPod.Namespace = namespace
|
||||
tc.changedPod.ResourceVersion = "1"
|
||||
tc.changedPod.APIVersion = "v1"
|
||||
tc.changedPod.Spec.ActiveDeadlineSeconds = &thirty
|
||||
|
||||
tc.Run(t)
|
||||
}
|
||||
|
||||
func TestPatchWithVersionConflictThenAdmissionFailure(t *testing.T) {
|
||||
namespace := "bar"
|
||||
name := "foo"
|
||||
fifteen := int64(15)
|
||||
thirty := int64(30)
|
||||
seen := false
|
||||
|
||||
tc := &patchTestCase{
|
||||
name: "TestPatchWithVersionConflictThenAdmissionFailure",
|
||||
|
||||
admit: func(updatedObject runtime.Object) error {
|
||||
if seen {
|
||||
return errors.New("admission failure")
|
||||
}
|
||||
|
||||
seen = true
|
||||
return nil
|
||||
},
|
||||
|
||||
startingPod: &api.Pod{},
|
||||
changedPod: &api.Pod{},
|
||||
updatePod: &api.Pod{},
|
||||
|
||||
expectedError: "admission failure",
|
||||
}
|
||||
|
||||
tc.startingPod.Name = name
|
||||
tc.startingPod.Namespace = namespace
|
||||
tc.startingPod.ResourceVersion = "1"
|
||||
tc.startingPod.APIVersion = "v1"
|
||||
tc.startingPod.Spec.ActiveDeadlineSeconds = &fifteen
|
||||
|
||||
tc.changedPod.Name = name
|
||||
tc.changedPod.Namespace = namespace
|
||||
tc.changedPod.ResourceVersion = "1"
|
||||
tc.changedPod.APIVersion = "v1"
|
||||
tc.changedPod.Spec.ActiveDeadlineSeconds = &thirty
|
||||
|
||||
tc.updatePod.Name = name
|
||||
tc.updatePod.Namespace = namespace
|
||||
tc.updatePod.ResourceVersion = "2"
|
||||
tc.updatePod.APIVersion = "v1"
|
||||
tc.updatePod.Spec.ActiveDeadlineSeconds = &fifteen
|
||||
tc.updatePod.Spec.NodeName = "anywhere"
|
||||
|
||||
tc.Run(t)
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue