Validate deletion timestamp doesn't change on update

pull/6/head
Jordan Liggitt 2016-04-28 11:50:48 -04:00
parent 7a725418af
commit 1e5815872e
4 changed files with 165 additions and 10 deletions

View File

@ -360,9 +360,12 @@ func ValidateObjectMetaUpdate(newMeta, oldMeta *api.ObjectMeta, fldPath *field.P
}
// TODO: needs to check if newMeta==nil && oldMeta !=nil after the repair logic is removed.
if newMeta.DeletionGracePeriodSeconds != nil && oldMeta.DeletionGracePeriodSeconds != nil && *newMeta.DeletionGracePeriodSeconds != *oldMeta.DeletionGracePeriodSeconds {
if newMeta.DeletionGracePeriodSeconds != nil && (oldMeta.DeletionGracePeriodSeconds == nil || *newMeta.DeletionGracePeriodSeconds != *oldMeta.DeletionGracePeriodSeconds) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("deletionGracePeriodSeconds"), newMeta.DeletionGracePeriodSeconds, "field is immutable; may only be changed via deletion"))
}
if newMeta.DeletionTimestamp != nil && (oldMeta.DeletionTimestamp == nil || !newMeta.DeletionTimestamp.Equal(*oldMeta.DeletionTimestamp)) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("deletionTimestamp"), newMeta.DeletionTimestamp, "field is immutable; may only be changed via deletion"))
}
// Reject updates that don't specify a resource version
if len(newMeta.ResourceVersion) == 0 {

View File

@ -122,6 +122,89 @@ func TestValidateObjectMetaUpdateIgnoresCreationTimestamp(t *testing.T) {
}
}
func TestValidateObjectMetaUpdatePreventsDeletionFieldMutation(t *testing.T) {
now := unversioned.NewTime(time.Unix(1000, 0).UTC())
later := unversioned.NewTime(time.Unix(2000, 0).UTC())
gracePeriodShort := int64(30)
gracePeriodLong := int64(40)
testcases := map[string]struct {
Old api.ObjectMeta
New api.ObjectMeta
ExpectedNew api.ObjectMeta
ExpectedErrs []string
}{
"valid without deletion fields": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
ExpectedErrs: []string{},
},
"valid with deletion fields": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now, DeletionGracePeriodSeconds: &gracePeriodShort},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now, DeletionGracePeriodSeconds: &gracePeriodShort},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now, DeletionGracePeriodSeconds: &gracePeriodShort},
ExpectedErrs: []string{},
},
"invalid set deletionTimestamp": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
ExpectedErrs: []string{"field.deletionTimestamp: Invalid value: \"1970-01-01T00:16:40Z\": field is immutable; may only be changed via deletion"},
},
"invalid clear deletionTimestamp": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
ExpectedErrs: []string{}, // no errors, validation copies the old value
},
"invalid change deletionTimestamp": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &later},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionTimestamp: &now},
ExpectedErrs: []string{}, // no errors, validation copies the old value
},
"invalid set deletionGracePeriodSeconds": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodShort},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodShort},
ExpectedErrs: []string{"field.deletionGracePeriodSeconds: Invalid value: 30: field is immutable; may only be changed via deletion"},
},
"invalid clear deletionGracePeriodSeconds": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodShort},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1"},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodShort},
ExpectedErrs: []string{}, // no errors, validation copies the old value
},
"invalid change deletionGracePeriodSeconds": {
Old: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodShort},
New: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodLong},
ExpectedNew: api.ObjectMeta{Name: "test", ResourceVersion: "1", DeletionGracePeriodSeconds: &gracePeriodLong},
ExpectedErrs: []string{"field.deletionGracePeriodSeconds: Invalid value: 40: field is immutable; may only be changed via deletion"},
},
}
for k, tc := range testcases {
errs := ValidateObjectMetaUpdate(&tc.New, &tc.Old, field.NewPath("field"))
if len(errs) != len(tc.ExpectedErrs) {
t.Logf("%s: Expected: %#v", k, tc.ExpectedErrs)
t.Logf("%s: Got: %#v", k, errs)
t.Errorf("%s: expected %d errors, got %d", k, len(tc.ExpectedErrs), len(errs))
continue
}
for i := range errs {
if errs[i].Error() != tc.ExpectedErrs[i] {
t.Errorf("%s: error #%d: expected %q, got %q", k, i, tc.ExpectedErrs[i], errs[i].Error())
}
}
if !reflect.DeepEqual(tc.New, tc.ExpectedNew) {
t.Errorf("%s: Expected after validation:\n%#v\ngot\n%#v", k, tc.ExpectedNew, tc.New)
}
}
}
// Ensure trailing slash is allowed in generate name
func TestValidateObjectMetaTrimsTrailingSlash(t *testing.T) {
errs := ValidateObjectMeta(
@ -2124,11 +2207,11 @@ func TestValidatePodUpdate(t *testing.T) {
},
{
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo", DeletionTimestamp: &now},
ObjectMeta: api.ObjectMeta{Name: "foo"},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
api.Pod{
ObjectMeta: api.ObjectMeta{Name: "foo"},
ObjectMeta: api.ObjectMeta{Name: "foo", DeletionTimestamp: &now},
Spec: api.PodSpec{Containers: []api.Container{{Image: "foo:V1"}}},
},
true,
@ -4452,6 +4535,7 @@ func TestValidateNamespaceStatusUpdate(t *testing.T) {
Phase: api.NamespaceActive,
},
}, true},
// Cannot set deletionTimestamp via status update
{api.Namespace{
ObjectMeta: api.ObjectMeta{
Name: "foo"}},
@ -4462,6 +4546,19 @@ func TestValidateNamespaceStatusUpdate(t *testing.T) {
Status: api.NamespaceStatus{
Phase: api.NamespaceTerminating,
},
}, false},
// Can update phase via status update
{api.Namespace{
ObjectMeta: api.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now}},
api.Namespace{
ObjectMeta: api.ObjectMeta{
Name: "foo",
DeletionTimestamp: &now},
Status: api.NamespaceStatus{
Phase: api.NamespaceTerminating,
},
}, true},
{api.Namespace{
ObjectMeta: api.ObjectMeta{

View File

@ -21,6 +21,7 @@ import (
"k8s.io/kubernetes/pkg/api"
apierrors "k8s.io/kubernetes/pkg/api/errors"
storageerr "k8s.io/kubernetes/pkg/api/errors/storage"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/fields"
"k8s.io/kubernetes/pkg/labels"
@ -29,6 +30,7 @@ import (
"k8s.io/kubernetes/pkg/registry/generic/registry"
"k8s.io/kubernetes/pkg/registry/namespace"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/storage"
)
// rest implements a RESTStorage for namespaces against etcd
@ -99,13 +101,66 @@ func (r *REST) Delete(ctx api.Context, name string, options *api.DeleteOptions)
namespace := nsObj.(*api.Namespace)
// Ensure we have a UID precondition
if options == nil {
options = api.NewDeleteOptions(0)
}
if options.Preconditions == nil {
options.Preconditions = &api.Preconditions{}
}
if options.Preconditions.UID == nil {
options.Preconditions.UID = &namespace.UID
} else if *options.Preconditions.UID != namespace.UID {
err = apierrors.NewConflict(
api.Resource("namespaces"),
name,
fmt.Errorf("Precondition failed: UID in precondition: %v, UID in object meta: %v", *options.Preconditions.UID, namespace.UID),
)
return nil, err
}
// upon first request to delete, we switch the phase to start namespace termination
// TODO: enhance graceful deletion's calls to DeleteStrategy to allow phase change and finalizer patterns
if namespace.DeletionTimestamp.IsZero() {
key, err := r.Store.KeyFunc(ctx, name)
if err != nil {
return nil, err
}
preconditions := storage.Preconditions{UID: options.Preconditions.UID}
out := r.Store.NewFunc()
err = r.Store.Storage.GuaranteedUpdate(
ctx, key, out, false, &preconditions,
storage.SimpleUpdate(func(existing runtime.Object) (runtime.Object, error) {
existingNamespace, ok := existing.(*api.Namespace)
if !ok {
// wrong type
return nil, fmt.Errorf("expected *api.Namespace, got %v", existing)
}
// Set the deletion timestamp if needed
if existingNamespace.DeletionTimestamp.IsZero() {
now := unversioned.Now()
namespace.DeletionTimestamp = &now
namespace.Status.Phase = api.NamespaceTerminating
result, _, err := r.status.Update(ctx, namespace)
return result, err
existingNamespace.DeletionTimestamp = &now
}
// Set the namespace phase to terminating, if needed
if existingNamespace.Status.Phase != api.NamespaceTerminating {
existingNamespace.Status.Phase = api.NamespaceTerminating
}
return existingNamespace, nil
}),
)
if err != nil {
err = storageerr.InterpretGetError(err, api.Resource("namespaces"), name)
err = storageerr.InterpretUpdateError(err, api.Resource("namespaces"), name)
if _, ok := err.(*apierrors.StatusError); !ok {
err = apierrors.NewInternalError(err)
}
return nil, err
}
return out, nil
}
// prior to final deletion, we must ensure that finalizers is empty
@ -113,7 +168,7 @@ func (r *REST) Delete(ctx api.Context, name string, options *api.DeleteOptions)
err = apierrors.NewConflict(api.Resource("namespaces"), namespace.Name, fmt.Errorf("The system is ensuring all content is removed from this namespace. Upon completion, this namespace will automatically be purged by the system."))
return nil, err
}
return r.Store.Delete(ctx, name, nil)
return r.Store.Delete(ctx, name, options)
}
func (r *StatusREST) New() runtime.Object {

View File

@ -75,7 +75,7 @@ func TestNamespaceStatusStrategy(t *testing.T) {
}
now := unversioned.Now()
oldNamespace := &api.Namespace{
ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"},
ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10", DeletionTimestamp: &now},
Spec: api.NamespaceSpec{Finalizers: []api.FinalizerName{"kubernetes"}},
Status: api.NamespaceStatus{Phase: api.NamespaceActive},
}