From 35ab5c6234ad4c85bb40f6a532fcd50abc6d65e6 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 12 Nov 2015 21:13:16 -0800 Subject: [PATCH] Add a REST hook for post-validation canonicalize This is a simple hook that only Endpoints uses for now. --- pkg/api/rest/create.go | 10 ++++++++-- pkg/api/rest/update.go | 18 ++++++++++++++---- pkg/registry/controller/strategy.go | 4 ++++ pkg/registry/daemonset/strategy.go | 4 ++++ pkg/registry/deployment/strategy.go | 4 ++++ pkg/registry/endpoint/strategy.go | 11 ++++++----- pkg/registry/event/strategy.go | 4 ++++ pkg/registry/generic/etcd/etcd_test.go | 1 + .../horizontalpodautoscaler/strategy.go | 4 ++++ pkg/registry/ingress/strategy.go | 4 ++++ pkg/registry/job/strategy.go | 4 ++++ pkg/registry/limitrange/strategy.go | 4 ++++ pkg/registry/namespace/strategy.go | 4 ++++ pkg/registry/node/strategy.go | 8 ++++++++ pkg/registry/persistentvolume/strategy.go | 4 ++++ pkg/registry/persistentvolumeclaim/strategy.go | 4 ++++ pkg/registry/pod/strategy.go | 4 ++++ pkg/registry/podtemplate/strategy.go | 4 ++++ pkg/registry/resourcequota/strategy.go | 4 ++++ pkg/registry/secret/strategy.go | 3 +++ pkg/registry/service/rest.go | 1 + pkg/registry/service/strategy.go | 4 ++++ pkg/registry/serviceaccount/strategy.go | 4 ++++ pkg/registry/thirdpartyresource/strategy.go | 4 ++++ .../thirdpartyresourcedata/strategy.go | 4 ++++ 25 files changed, 113 insertions(+), 11 deletions(-) diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go index 2886d37533..d896e3954c 100644 --- a/pkg/api/rest/create.go +++ b/pkg/api/rest/create.go @@ -37,11 +37,15 @@ type RESTCreateStrategy interface { NamespaceScoped() bool // 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. + // sort order-insensitive list fields, etc. This should not remove fields + // whose presence would be considered a validation error. PrepareForCreate(obj runtime.Object) // Validate is invoked after default fields in the object have been filled in before - // the object is persisted. + // the object is persisted. This method should not mutate the object. Validate(ctx api.Context, obj runtime.Object) fielderrors.ValidationErrorList + // Canonicalize is invoked after validation has succeeded but before the + // object has been persisted. This method may mutate the object. + Canonicalize(obj runtime.Object) } // BeforeCreate ensures that common operations for all resources are performed on creation. It only returns @@ -77,6 +81,8 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx api.Context, obj runtime.Obje return errors.NewInvalid(kind, objectMeta.Name, errs) } + strategy.Canonicalize(obj) + return nil } diff --git a/pkg/api/rest/update.go b/pkg/api/rest/update.go index 394cb3d5ff..9601f64cdc 100644 --- a/pkg/api/rest/update.go +++ b/pkg/api/rest/update.go @@ -36,12 +36,19 @@ type RESTUpdateStrategy interface { 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. + // sort order-insensitive list fields, etc. This should not remove fields + // whose presence would be considered a validation error. PrepareForUpdate(obj, old runtime.Object) - // ValidateUpdate is invoked after default fields in the object have been filled in before - // the object is persisted. + // ValidateUpdate is invoked after default fields in the object have been + // filled in before the object is persisted. This method should not mutate + // the object. ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList - // AllowUnconditionalUpdate returns true if the object can be updated unconditionally (irrespective of the latest resource version), when there is no resource version specified in the object. + // Canonicalize is invoked after validation has succeeded but before the + // object has been persisted. This method may mutate the object. + Canonicalize(obj runtime.Object) + // AllowUnconditionalUpdate returns true if the object can be updated + // unconditionally (irrespective of the latest resource version), when + // there is no resource version specified in the object. AllowUnconditionalUpdate() bool } @@ -86,5 +93,8 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx api.Context, obj, old runtime if len(errs) > 0 { return errors.NewInvalid(kind, objectMeta.Name, errs) } + + strategy.Canonicalize(obj) + return nil } diff --git a/pkg/registry/controller/strategy.go b/pkg/registry/controller/strategy.go index 59dbb5db5c..a740778a90 100644 --- a/pkg/registry/controller/strategy.go +++ b/pkg/registry/controller/strategy.go @@ -81,6 +81,10 @@ func (rcStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Vali return validation.ValidateReplicationController(controller) } +// Canonicalize normalizes the object after validation. +func (rcStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for replication controllers; this means a POST is // needed to create one. func (rcStrategy) AllowCreateOnUpdate() bool { diff --git a/pkg/registry/daemonset/strategy.go b/pkg/registry/daemonset/strategy.go index b2a9b14c79..79d8f45a92 100644 --- a/pkg/registry/daemonset/strategy.go +++ b/pkg/registry/daemonset/strategy.go @@ -82,6 +82,10 @@ func (daemonSetStrategy) Validate(ctx api.Context, obj runtime.Object) fielderro return validation.ValidateDaemonSet(daemonSet) } +// Canonicalize normalizes the object after validation. +func (daemonSetStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for daemon set; this means a POST is // needed to create one func (daemonSetStrategy) AllowCreateOnUpdate() bool { diff --git a/pkg/registry/deployment/strategy.go b/pkg/registry/deployment/strategy.go index dd08aee3d3..8d9e87ba89 100644 --- a/pkg/registry/deployment/strategy.go +++ b/pkg/registry/deployment/strategy.go @@ -56,6 +56,10 @@ func (deploymentStrategy) Validate(ctx api.Context, obj runtime.Object) errs.Val return validation.ValidateDeployment(deployment) } +// Canonicalize normalizes the object after validation. +func (deploymentStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for deployments. func (deploymentStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/endpoint/strategy.go b/pkg/registry/endpoint/strategy.go index a67f5e624b..9fad2d1e9a 100644 --- a/pkg/registry/endpoint/strategy.go +++ b/pkg/registry/endpoint/strategy.go @@ -46,15 +46,10 @@ func (endpointsStrategy) NamespaceScoped() bool { // PrepareForCreate clears fields that are not allowed to be set by end users on creation. func (endpointsStrategy) PrepareForCreate(obj runtime.Object) { - endpoints := obj.(*api.Endpoints) - endpoints.Subsets = endptspkg.RepackSubsets(endpoints.Subsets) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. func (endpointsStrategy) PrepareForUpdate(obj, old runtime.Object) { - newEndpoints := obj.(*api.Endpoints) - _ = old.(*api.Endpoints) - newEndpoints.Subsets = endptspkg.RepackSubsets(newEndpoints.Subsets) } // Validate validates a new endpoints. @@ -62,6 +57,12 @@ func (endpointsStrategy) Validate(ctx api.Context, obj runtime.Object) fielderro return validation.ValidateEndpoints(obj.(*api.Endpoints)) } +// Canonicalize normalizes the object after validation. +func (endpointsStrategy) Canonicalize(obj runtime.Object) { + endpoints := obj.(*api.Endpoints) + endpoints.Subsets = endptspkg.RepackSubsets(endpoints.Subsets) +} + // AllowCreateOnUpdate is true for endpoints. func (endpointsStrategy) AllowCreateOnUpdate() bool { return true diff --git a/pkg/registry/event/strategy.go b/pkg/registry/event/strategy.go index 9d033f5662..a11ddb819b 100644 --- a/pkg/registry/event/strategy.go +++ b/pkg/registry/event/strategy.go @@ -53,6 +53,10 @@ func (eventStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.V return validation.ValidateEvent(event) } +// Canonicalize normalizes the object after validation. +func (eventStrategy) Canonicalize(obj runtime.Object) { +} + func (eventStrategy) AllowCreateOnUpdate() bool { return true } diff --git a/pkg/registry/generic/etcd/etcd_test.go b/pkg/registry/generic/etcd/etcd_test.go index 9db13a4416..2630eae322 100644 --- a/pkg/registry/generic/etcd/etcd_test.go +++ b/pkg/registry/generic/etcd/etcd_test.go @@ -55,6 +55,7 @@ func (t *testRESTStrategy) Validate(ctx api.Context, obj runtime.Object) fielder func (t *testRESTStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { return nil } +func (t *testRESTStrategy) Canonicalize(obj runtime.Object) {} func hasCreated(t *testing.T, pod *api.Pod) func(runtime.Object) bool { return func(obj runtime.Object) bool { diff --git a/pkg/registry/horizontalpodautoscaler/strategy.go b/pkg/registry/horizontalpodautoscaler/strategy.go index 955da299c4..725b4ea135 100644 --- a/pkg/registry/horizontalpodautoscaler/strategy.go +++ b/pkg/registry/horizontalpodautoscaler/strategy.go @@ -58,6 +58,10 @@ func (autoscalerStrategy) Validate(ctx api.Context, obj runtime.Object) errs.Val return validation.ValidateHorizontalPodAutoscaler(autoscaler) } +// Canonicalize normalizes the object after validation. +func (autoscalerStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for autoscalers. func (autoscalerStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/ingress/strategy.go b/pkg/registry/ingress/strategy.go index f53931116c..8ffe645a22 100644 --- a/pkg/registry/ingress/strategy.go +++ b/pkg/registry/ingress/strategy.go @@ -76,6 +76,10 @@ func (ingressStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors return err } +// Canonicalize normalizes the object after validation. +func (ingressStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for Ingress; this means POST is needed to create one. func (ingressStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/job/strategy.go b/pkg/registry/job/strategy.go index 7ce425f09c..28fabe6a82 100644 --- a/pkg/registry/job/strategy.go +++ b/pkg/registry/job/strategy.go @@ -63,6 +63,10 @@ func (jobStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Val return validation.ValidateJob(job) } +// Canonicalize normalizes the object after validation. +func (jobStrategy) Canonicalize(obj runtime.Object) { +} + func (jobStrategy) AllowUnconditionalUpdate() bool { return true } diff --git a/pkg/registry/limitrange/strategy.go b/pkg/registry/limitrange/strategy.go index d14b04e8eb..e0de8cdb45 100644 --- a/pkg/registry/limitrange/strategy.go +++ b/pkg/registry/limitrange/strategy.go @@ -57,6 +57,10 @@ func (limitrangeStrategy) Validate(ctx api.Context, obj runtime.Object) fielderr return validation.ValidateLimitRange(limitRange) } +// Canonicalize normalizes the object after validation. +func (limitrangeStrategy) Canonicalize(obj runtime.Object) { +} + func (limitrangeStrategy) AllowCreateOnUpdate() bool { return true } diff --git a/pkg/registry/namespace/strategy.go b/pkg/registry/namespace/strategy.go index bfe95d5df1..d3f989b5fc 100644 --- a/pkg/registry/namespace/strategy.go +++ b/pkg/registry/namespace/strategy.go @@ -82,6 +82,10 @@ func (namespaceStrategy) Validate(ctx api.Context, obj runtime.Object) fielderro return validation.ValidateNamespace(namespace) } +// Canonicalize normalizes the object after validation. +func (namespaceStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for namespaces. func (namespaceStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/node/strategy.go b/pkg/registry/node/strategy.go index a3a5b9a75c..651622d70a 100644 --- a/pkg/registry/node/strategy.go +++ b/pkg/registry/node/strategy.go @@ -76,6 +76,10 @@ func (nodeStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Va return validation.ValidateNode(node) } +// Canonicalize normalizes the object after validation. +func (nodeStrategy) Canonicalize(obj runtime.Object) { +} + // ValidateUpdate is the default update validation for an end user. func (nodeStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { errorList := validation.ValidateNode(obj.(*api.Node)) @@ -107,6 +111,10 @@ func (nodeStatusStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Objec return validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node)) } +// Canonicalize normalizes the object after validation. +func (nodeStatusStrategy) Canonicalize(obj runtime.Object) { +} + // ResourceGetter is an interface for retrieving resources by ResourceLocation. type ResourceGetter interface { Get(api.Context, string) (runtime.Object, error) diff --git a/pkg/registry/persistentvolume/strategy.go b/pkg/registry/persistentvolume/strategy.go index 35ff51aa70..e0c0e737fd 100644 --- a/pkg/registry/persistentvolume/strategy.go +++ b/pkg/registry/persistentvolume/strategy.go @@ -53,6 +53,10 @@ func (persistentvolumeStrategy) Validate(ctx api.Context, obj runtime.Object) fi return validation.ValidatePersistentVolume(persistentvolume) } +// Canonicalize normalizes the object after validation. +func (persistentvolumeStrategy) Canonicalize(obj runtime.Object) { +} + func (persistentvolumeStrategy) AllowCreateOnUpdate() bool { return false } diff --git a/pkg/registry/persistentvolumeclaim/strategy.go b/pkg/registry/persistentvolumeclaim/strategy.go index 26b99a2f4f..8c32223da6 100644 --- a/pkg/registry/persistentvolumeclaim/strategy.go +++ b/pkg/registry/persistentvolumeclaim/strategy.go @@ -53,6 +53,10 @@ func (persistentvolumeclaimStrategy) Validate(ctx api.Context, obj runtime.Objec return validation.ValidatePersistentVolumeClaim(pvc) } +// Canonicalize normalizes the object after validation. +func (persistentvolumeclaimStrategy) Canonicalize(obj runtime.Object) { +} + func (persistentvolumeclaimStrategy) AllowCreateOnUpdate() bool { return false } diff --git a/pkg/registry/pod/strategy.go b/pkg/registry/pod/strategy.go index a753f7ece6..69f13f659a 100644 --- a/pkg/registry/pod/strategy.go +++ b/pkg/registry/pod/strategy.go @@ -72,6 +72,10 @@ func (podStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Val return validation.ValidatePod(pod) } +// Canonicalize normalizes the object after validation. +func (podStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for pods. func (podStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/podtemplate/strategy.go b/pkg/registry/podtemplate/strategy.go index a3868254db..43433521a8 100644 --- a/pkg/registry/podtemplate/strategy.go +++ b/pkg/registry/podtemplate/strategy.go @@ -54,6 +54,10 @@ func (podTemplateStrategy) Validate(ctx api.Context, obj runtime.Object) errs.Va return validation.ValidatePodTemplate(pod) } +// Canonicalize normalizes the object after validation. +func (podTemplateStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for pod templates. func (podTemplateStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/resourcequota/strategy.go b/pkg/registry/resourcequota/strategy.go index 4780e7d9bc..5a82dcc280 100644 --- a/pkg/registry/resourcequota/strategy.go +++ b/pkg/registry/resourcequota/strategy.go @@ -62,6 +62,10 @@ func (resourcequotaStrategy) Validate(ctx api.Context, obj runtime.Object) field return validation.ValidateResourceQuota(resourcequota) } +// Canonicalize normalizes the object after validation. +func (resourcequotaStrategy) Canonicalize(obj runtime.Object) { +} + // AllowCreateOnUpdate is false for resourcequotas. func (resourcequotaStrategy) AllowCreateOnUpdate() bool { return false diff --git a/pkg/registry/secret/strategy.go b/pkg/registry/secret/strategy.go index 6f1ff04b4c..fd2d3930f8 100644 --- a/pkg/registry/secret/strategy.go +++ b/pkg/registry/secret/strategy.go @@ -54,6 +54,9 @@ func (strategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Valida return validation.ValidateSecret(obj.(*api.Secret)) } +func (strategy) Canonicalize(obj runtime.Object) { +} + func (strategy) AllowCreateOnUpdate() bool { return false } diff --git a/pkg/registry/service/rest.go b/pkg/registry/service/rest.go index b7ceb7a56d..6699c00022 100644 --- a/pkg/registry/service/rest.go +++ b/pkg/registry/service/rest.go @@ -67,6 +67,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err return nil, err } + // TODO: this should probably move to strategy.PrepareForCreate() releaseServiceIP := false defer func() { if releaseServiceIP { diff --git a/pkg/registry/service/strategy.go b/pkg/registry/service/strategy.go index c43f3dba89..3072053f70 100644 --- a/pkg/registry/service/strategy.go +++ b/pkg/registry/service/strategy.go @@ -63,6 +63,10 @@ func (svcStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Val return validation.ValidateService(service) } +// Canonicalize normalizes the object after validation. +func (svcStrategy) Canonicalize(obj runtime.Object) { +} + func (svcStrategy) AllowCreateOnUpdate() bool { return true } diff --git a/pkg/registry/serviceaccount/strategy.go b/pkg/registry/serviceaccount/strategy.go index 915c8b7ca7..2e53c922de 100644 --- a/pkg/registry/serviceaccount/strategy.go +++ b/pkg/registry/serviceaccount/strategy.go @@ -50,6 +50,10 @@ func (strategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Valida return validation.ValidateServiceAccount(obj.(*api.ServiceAccount)) } +// Canonicalize normalizes the object after validation. +func (strategy) Canonicalize(obj runtime.Object) { +} + func (strategy) AllowCreateOnUpdate() bool { return false } diff --git a/pkg/registry/thirdpartyresource/strategy.go b/pkg/registry/thirdpartyresource/strategy.go index 9ee5081e4e..5a2487bfa5 100644 --- a/pkg/registry/thirdpartyresource/strategy.go +++ b/pkg/registry/thirdpartyresource/strategy.go @@ -55,6 +55,10 @@ func (strategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Valida return validation.ValidateThirdPartyResource(obj.(*extensions.ThirdPartyResource)) } +// Canonicalize normalizes the object after validation. +func (strategy) Canonicalize(obj runtime.Object) { +} + func (strategy) AllowCreateOnUpdate() bool { return false } diff --git a/pkg/registry/thirdpartyresourcedata/strategy.go b/pkg/registry/thirdpartyresourcedata/strategy.go index 8bf22044ee..9a3e3856e5 100644 --- a/pkg/registry/thirdpartyresourcedata/strategy.go +++ b/pkg/registry/thirdpartyresourcedata/strategy.go @@ -55,6 +55,10 @@ func (strategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.Valida return validation.ValidateThirdPartyResourceData(obj.(*extensions.ThirdPartyResourceData)) } +// Canonicalize normalizes the object after validation. +func (strategy) Canonicalize(obj runtime.Object) { +} + func (strategy) AllowCreateOnUpdate() bool { return false }