From b2687c1a84e4b3b1be6700251e6ef7233427712a Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 25 Mar 2015 14:45:07 -0700 Subject: [PATCH 1/2] rename ResetBeforeCreate to PrepareForCreate --- pkg/api/rest/create.go | 11 ++++++----- pkg/api/rest/types.go | 4 ++-- pkg/registry/controller/rest.go | 4 ++-- pkg/registry/endpoint/rest.go | 4 ++-- pkg/registry/generic/etcd/etcd_test.go | 2 +- pkg/registry/minion/rest.go | 4 ++-- pkg/registry/namespace/rest.go | 4 ++-- pkg/registry/namespace/rest_test.go | 2 +- pkg/registry/pod/rest.go | 4 ++-- pkg/registry/resourcequota/rest.go | 4 ++-- pkg/registry/resourcequota/rest_test.go | 2 +- 11 files changed, 23 insertions(+), 22 deletions(-) diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go index 27f0d34c397..9089554a9d2 100644 --- a/pkg/api/rest/create.go +++ b/pkg/api/rest/create.go @@ -34,16 +34,17 @@ type RESTCreateStrategy interface { // NamespaceScoped returns true if the object must be within a namespace. NamespaceScoped() bool - // ResetBeforeCreate is invoked on create before validation to remove any fields - // that may not be persisted. - ResetBeforeCreate(obj runtime.Object) + // PrepareForCreate is invoked on create before validation to normalize + // the object. For example: remove fields that are not to be persisted, + // sort order-insensitive list fields, etc. + PrepareForCreate(obj runtime.Object) // Validate is invoked after default fields in the object have been filled in before // the object is persisted. Validate(obj runtime.Object) fielderrors.ValidationErrorList } // BeforeCreate ensures that common operations for all resources are performed on creation. It only returns -// errors that can be converted to api.Status. It invokes ResetBeforeCreate, then GenerateName, then Validate. +// errors that can be converted to api.Status. It invokes PrepareForCreate, then GenerateName, then Validate. // It returns nil if the object should be created. func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Object) error { objectMeta, kind, kerr := objectMetaAndKind(strategy, obj) @@ -58,7 +59,7 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Obje } else { objectMeta.Namespace = api.NamespaceNone } - strategy.ResetBeforeCreate(obj) + strategy.PrepareForCreate(obj) api.FillObjectMetaSystemFields(ctx, objectMeta) api.GenerateName(strategy, objectMeta) diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go index fe839781d71..6ed38e998f8 100644 --- a/pkg/api/rest/types.go +++ b/pkg/api/rest/types.go @@ -60,8 +60,8 @@ func (svcStrategy) NamespaceScoped() bool { return true } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (svcStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (svcStrategy) PrepareForCreate(obj runtime.Object) { service := obj.(*api.Service) service.Status = api.ServiceStatus{} } diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 4ca1e22707d..2a7890748b9 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -42,8 +42,8 @@ func (rcStrategy) NamespaceScoped() bool { return true } -// ResetBeforeCreate clears the status of a replication controller before creation. -func (rcStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears the status of a replication controller before creation. +func (rcStrategy) PrepareForCreate(obj runtime.Object) { controller := obj.(*api.ReplicationController) controller.Status = api.ReplicationControllerStatus{} } diff --git a/pkg/registry/endpoint/rest.go b/pkg/registry/endpoint/rest.go index b41b72709e1..9cad99e5d0e 100644 --- a/pkg/registry/endpoint/rest.go +++ b/pkg/registry/endpoint/rest.go @@ -43,8 +43,8 @@ func (endpointsStrategy) NamespaceScoped() bool { return true } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (endpointsStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (endpointsStrategy) PrepareForCreate(obj runtime.Object) { _ = obj.(*api.Endpoints) } diff --git a/pkg/registry/generic/etcd/etcd_test.go b/pkg/registry/generic/etcd/etcd_test.go index 72580ec691a..d5bb4c984e7 100644 --- a/pkg/registry/generic/etcd/etcd_test.go +++ b/pkg/registry/generic/etcd/etcd_test.go @@ -43,7 +43,7 @@ type testRESTStrategy struct { func (t *testRESTStrategy) NamespaceScoped() bool { return t.namespaceScoped } func (t *testRESTStrategy) AllowCreateOnUpdate() bool { return t.allowCreateOnUpdate } -func (t *testRESTStrategy) ResetBeforeCreate(obj runtime.Object) {} +func (t *testRESTStrategy) PrepareForCreate(obj runtime.Object) {} func (t *testRESTStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { return nil } diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index 418ae7b00a7..ed7e4ef8758 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -53,8 +53,8 @@ func (nodeStrategy) AllowCreateOnUpdate() bool { return false } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (nodeStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (nodeStrategy) PrepareForCreate(obj runtime.Object) { _ = obj.(*api.Node) // Nodes allow *all* fields, including status, to be set. } diff --git a/pkg/registry/namespace/rest.go b/pkg/registry/namespace/rest.go index 371524991ed..d9379ed3929 100644 --- a/pkg/registry/namespace/rest.go +++ b/pkg/registry/namespace/rest.go @@ -43,8 +43,8 @@ func (namespaceStrategy) NamespaceScoped() bool { return false } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (namespaceStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (namespaceStrategy) PrepareForCreate(obj runtime.Object) { // on create, status is active namespace := obj.(*api.Namespace) namespace.Status = api.NamespaceStatus{ diff --git a/pkg/registry/namespace/rest_test.go b/pkg/registry/namespace/rest_test.go index 0b36fa87843..9ac4d138484 100644 --- a/pkg/registry/namespace/rest_test.go +++ b/pkg/registry/namespace/rest_test.go @@ -33,7 +33,7 @@ func TestNamespaceStrategy(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "foo"}, Status: api.NamespaceStatus{Phase: api.NamespaceTerminating}, } - Strategy.ResetBeforeCreate(namespace) + Strategy.PrepareForCreate(namespace) if namespace.Status.Phase != api.NamespaceActive { t.Errorf("Namespaces do not allow setting phase on create") } diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index e92da88cc85..bc7f6fce73f 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -50,8 +50,8 @@ func (podStrategy) NamespaceScoped() bool { return true } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (podStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (podStrategy) PrepareForCreate(obj runtime.Object) { pod := obj.(*api.Pod) pod.Status = api.PodStatus{ Host: pod.Spec.Host, diff --git a/pkg/registry/resourcequota/rest.go b/pkg/registry/resourcequota/rest.go index d800afc56ec..dbc351d2462 100644 --- a/pkg/registry/resourcequota/rest.go +++ b/pkg/registry/resourcequota/rest.go @@ -43,8 +43,8 @@ func (resourcequotaStrategy) NamespaceScoped() bool { return true } -// ResetBeforeCreate clears fields that are not allowed to be set by end users on creation. -func (resourcequotaStrategy) ResetBeforeCreate(obj runtime.Object) { +// PrepareForCreate clears fields that are not allowed to be set by end users on creation. +func (resourcequotaStrategy) PrepareForCreate(obj runtime.Object) { resourcequota := obj.(*api.ResourceQuota) resourcequota.Status = api.ResourceQuotaStatus{} } diff --git a/pkg/registry/resourcequota/rest_test.go b/pkg/registry/resourcequota/rest_test.go index f54eb3a0031..cc36b91c0cf 100644 --- a/pkg/registry/resourcequota/rest_test.go +++ b/pkg/registry/resourcequota/rest_test.go @@ -51,7 +51,7 @@ func TestResourceQuotaStrategy(t *testing.T) { }, }, } - Strategy.ResetBeforeCreate(resourceQuota) + Strategy.PrepareForCreate(resourceQuota) if resourceQuota.Status.Used != nil { t.Errorf("ResourceQuota does not allow setting status on create") } From 0f36c6824454333cd510f360d8a52478557f5640 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 25 Mar 2015 17:03:30 -0700 Subject: [PATCH 2/2] Add REST PrepareForUpdate() hook As per discussion with @snmarterclayton. I implemented this for most types in the "obvious" way. I am not sure how to implement this for a couple types, though. --- pkg/api/rest/types.go | 8 ++++++++ pkg/api/rest/update.go | 5 +++++ pkg/registry/controller/rest.go | 8 ++++++++ pkg/registry/endpoint/rest.go | 6 ++++++ pkg/registry/generic/etcd/etcd_test.go | 3 ++- pkg/registry/minion/rest.go | 7 +++++++ pkg/registry/namespace/rest.go | 14 ++++++++++++++ pkg/registry/pod/rest.go | 13 +++++++++++++ pkg/registry/resourcequota/rest.go | 13 +++++++++++++ 9 files changed, 76 insertions(+), 1 deletion(-) diff --git a/pkg/api/rest/types.go b/pkg/api/rest/types.go index 6ed38e998f8..1442efe70a4 100644 --- a/pkg/api/rest/types.go +++ b/pkg/api/rest/types.go @@ -66,6 +66,14 @@ func (svcStrategy) PrepareForCreate(obj runtime.Object) { service.Status = api.ServiceStatus{} } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (svcStrategy) PrepareForUpdate(obj, old runtime.Object) { + // TODO: once service has a status sub-resource we can enable this. + //newService := obj.(*api.Service) + //oldService := old.(*api.Service) + //newService.Status = oldService.Status +} + // Validate validates a new service. func (svcStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { service := obj.(*api.Service) diff --git a/pkg/api/rest/update.go b/pkg/api/rest/update.go index f39f32da2e6..1eab5fd2b02 100644 --- a/pkg/api/rest/update.go +++ b/pkg/api/rest/update.go @@ -33,6 +33,10 @@ type RESTUpdateStrategy interface { NamespaceScoped() bool // AllowCreateOnUpdate returns true if the object can be created by a PUT. AllowCreateOnUpdate() bool + // PrepareForUpdate is invoked on update before validation to normalize + // the object. For example: remove fields that are not to be persisted, + // sort order-insensitive list fields, etc. + PrepareForUpdate(obj, old runtime.Object) // ValidateUpdate is invoked after default fields in the object have been filled in before // the object is persisted. ValidateUpdate(obj, old runtime.Object) fielderrors.ValidationErrorList @@ -53,6 +57,7 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx api.Context, obj, old runtime } else { objectMeta.Namespace = api.NamespaceNone } + strategy.PrepareForUpdate(obj, old) if errs := strategy.ValidateUpdate(obj, old); len(errs) > 0 { return errors.NewInvalid(kind, objectMeta.Name, errs) } diff --git a/pkg/registry/controller/rest.go b/pkg/registry/controller/rest.go index 2a7890748b9..d4fd4c1dffe 100644 --- a/pkg/registry/controller/rest.go +++ b/pkg/registry/controller/rest.go @@ -48,6 +48,14 @@ func (rcStrategy) PrepareForCreate(obj runtime.Object) { controller.Status = api.ReplicationControllerStatus{} } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (rcStrategy) PrepareForUpdate(obj, old runtime.Object) { + // TODO: once RC has a status sub-resource we can enable this. + //newController := obj.(*api.ReplicationController) + //oldController := old.(*api.ReplicationController) + //newController.Status = oldController.Status +} + // Validate validates a new replication controller. func (rcStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { controller := obj.(*api.ReplicationController) diff --git a/pkg/registry/endpoint/rest.go b/pkg/registry/endpoint/rest.go index 9cad99e5d0e..d551885241e 100644 --- a/pkg/registry/endpoint/rest.go +++ b/pkg/registry/endpoint/rest.go @@ -48,6 +48,12 @@ func (endpointsStrategy) PrepareForCreate(obj runtime.Object) { _ = obj.(*api.Endpoints) } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (endpointsStrategy) PrepareForUpdate(obj, old runtime.Object) { + _ = obj.(*api.Endpoints) + _ = old.(*api.Endpoints) +} + // Validate validates a new endpoints. func (endpointsStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { return validation.ValidateEndpoints(obj.(*api.Endpoints)) diff --git a/pkg/registry/generic/etcd/etcd_test.go b/pkg/registry/generic/etcd/etcd_test.go index d5bb4c984e7..acb25bb1c75 100644 --- a/pkg/registry/generic/etcd/etcd_test.go +++ b/pkg/registry/generic/etcd/etcd_test.go @@ -43,7 +43,8 @@ type testRESTStrategy struct { func (t *testRESTStrategy) NamespaceScoped() bool { return t.namespaceScoped } func (t *testRESTStrategy) AllowCreateOnUpdate() bool { return t.allowCreateOnUpdate } -func (t *testRESTStrategy) PrepareForCreate(obj runtime.Object) {} +func (t *testRESTStrategy) PrepareForCreate(obj runtime.Object) {} +func (t *testRESTStrategy) PrepareForUpdate(obj, old runtime.Object) {} func (t *testRESTStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { return nil } diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index ed7e4ef8758..63c200869b9 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -59,6 +59,13 @@ func (nodeStrategy) PrepareForCreate(obj runtime.Object) { // Nodes allow *all* fields, including status, to be set. } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (nodeStrategy) PrepareForUpdate(obj, old runtime.Object) { + _ = obj.(*api.Node) + _ = old.(*api.Node) + // Nodes allow *all* fields, including status, to be set. +} + // Validate validates a new node. func (nodeStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { node := obj.(*api.Node) diff --git a/pkg/registry/namespace/rest.go b/pkg/registry/namespace/rest.go index d9379ed3929..a9c522e6abe 100644 --- a/pkg/registry/namespace/rest.go +++ b/pkg/registry/namespace/rest.go @@ -68,6 +68,14 @@ func (namespaceStrategy) PrepareForCreate(obj runtime.Object) { } } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (namespaceStrategy) PrepareForUpdate(obj, old runtime.Object) { + newNamespace := obj.(*api.Namespace) + oldNamespace := old.(*api.Namespace) + newNamespace.Spec.Finalizers = oldNamespace.Spec.Finalizers + newNamespace.Status = oldNamespace.Status +} + // Validate validates a new namespace. func (namespaceStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { namespace := obj.(*api.Namespace) @@ -90,6 +98,12 @@ type namespaceStatusStrategy struct { var StatusStrategy = namespaceStatusStrategy{Strategy} +func (namespaceStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { + newNamespace := obj.(*api.Namespace) + oldNamespace := old.(*api.Namespace) + newNamespace.Spec = oldNamespace.Spec +} + func (namespaceStatusStrategy) ValidateUpdate(obj, old runtime.Object) fielderrors.ValidationErrorList { return validation.ValidateNamespaceStatusUpdate(obj.(*api.Namespace), old.(*api.Namespace)) } diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index bc7f6fce73f..e301a91963b 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -59,6 +59,13 @@ func (podStrategy) PrepareForCreate(obj runtime.Object) { } } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (podStrategy) PrepareForUpdate(obj, old runtime.Object) { + newPod := obj.(*api.Pod) + oldPod := old.(*api.Pod) + newPod.Status = oldPod.Status +} + // Validate validates a new pod. func (podStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { pod := obj.(*api.Pod) @@ -86,6 +93,12 @@ type podStatusStrategy struct { var StatusStrategy = podStatusStrategy{Strategy} +func (podStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { + newPod := obj.(*api.Pod) + oldPod := old.(*api.Pod) + newPod.Spec = oldPod.Spec +} + func (podStatusStrategy) ValidateUpdate(obj, old runtime.Object) fielderrors.ValidationErrorList { // TODO: merge valid fields after update return validation.ValidatePodStatusUpdate(obj.(*api.Pod), old.(*api.Pod)) diff --git a/pkg/registry/resourcequota/rest.go b/pkg/registry/resourcequota/rest.go index dbc351d2462..ca6125f21be 100644 --- a/pkg/registry/resourcequota/rest.go +++ b/pkg/registry/resourcequota/rest.go @@ -49,6 +49,13 @@ func (resourcequotaStrategy) PrepareForCreate(obj runtime.Object) { resourcequota.Status = api.ResourceQuotaStatus{} } +// PrepareForUpdate clears fields that are not allowed to be set by end users on update. +func (resourcequotaStrategy) PrepareForUpdate(obj, old runtime.Object) { + newResourcequota := obj.(*api.ResourceQuota) + oldResourcequota := old.(*api.ResourceQuota) + newResourcequota.Status = oldResourcequota.Status +} + // Validate validates a new resourcequota. func (resourcequotaStrategy) Validate(obj runtime.Object) fielderrors.ValidationErrorList { resourcequota := obj.(*api.ResourceQuota) @@ -71,6 +78,12 @@ type resourcequotaStatusStrategy struct { var StatusStrategy = resourcequotaStatusStrategy{Strategy} +func (resourcequotaStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { + newResourcequota := obj.(*api.ResourceQuota) + oldResourcequota := old.(*api.ResourceQuota) + newResourcequota.Spec = oldResourcequota.Spec +} + func (resourcequotaStatusStrategy) ValidateUpdate(obj, old runtime.Object) fielderrors.ValidationErrorList { return validation.ValidateResourceQuotaStatusUpdate(obj.(*api.ResourceQuota), old.(*api.ResourceQuota)) }