Merge pull request kubernetes#17204 from thockin/airplane-rest-canoni…

…calize Auto commit by PR queue bot
nikhiljindal · Nov 14, 2015 · 961a02a · 961a02a
2 parents c377b32 + 35ab5c6
commit 961a02a
Show file tree

Hide file tree

Showing 25 changed files with 113 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -46,22 +46,23 @@ 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.
 func (endpointsStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.ValidationErrorList {
 	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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 }