From d59a8ff7f8070c5676fe31fa8473cf8e8fbfb71d Mon Sep 17 00:00:00 2001 From: Jason Sommer Date: Mon, 8 Jun 2015 02:56:22 -0500 Subject: [PATCH] Improve signature consistency for ValidateObjectMetaUpdate Fixes #9340 Signed-off-by: Jason Sommer --- pkg/api/validation/validation.go | 34 +++++++++++++-------------- pkg/api/validation/validation_test.go | 14 +++++------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index c112b927a3d..f5fead4ba1e 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -239,7 +239,7 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val } // ValidateObjectMetaUpdate validates an object's metadata when updated -func ValidateObjectMetaUpdate(old, meta *api.ObjectMeta) errs.ValidationErrorList { +func ValidateObjectMetaUpdate(meta, old *api.ObjectMeta) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} // in the event it is left empty, set it, to allow clients more flexibility @@ -541,7 +541,7 @@ func ValidatePersistentVolumeUpdate(newPv, oldPv *api.PersistentVolume) errs.Val // newPv is updated with fields that cannot be changed. func ValidatePersistentVolumeStatusUpdate(newPv, oldPv *api.PersistentVolume) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldPv.ObjectMeta, &newPv.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newPv.ObjectMeta, &oldPv.ObjectMeta).Prefix("metadata")...) if newPv.ResourceVersion == "" { allErrs = append(allErrs, errs.NewFieldRequired("resourceVersion")) } @@ -569,7 +569,7 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *api.PersistentVolumeCla func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *api.PersistentVolumeClaim) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldPvc.ObjectMeta, &newPvc.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newPvc.ObjectMeta, &oldPvc.ObjectMeta).Prefix("metadata")...) if newPvc.ResourceVersion == "" { allErrs = append(allErrs, errs.NewFieldRequired("resourceVersion")) } @@ -977,7 +977,7 @@ func ValidatePodSpec(spec *api.PodSpec) errs.ValidationErrorList { func ValidatePodUpdate(newPod, oldPod *api.Pod) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldPod.ObjectMeta, &newPod.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta).Prefix("metadata")...) if len(newPod.Spec.Containers) != len(oldPod.Spec.Containers) { allErrs = append(allErrs, errs.NewFieldInvalid("spec.containers", newPod.Spec.Containers, "may not add or remove containers")) @@ -1004,7 +1004,7 @@ func ValidatePodUpdate(newPod, oldPod *api.Pod) errs.ValidationErrorList { func ValidatePodStatusUpdate(newPod, oldPod *api.Pod) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldPod.ObjectMeta, &newPod.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta).Prefix("metadata")...) // TODO: allow change when bindings are properly decoupled from pods if newPod.Spec.NodeName != oldPod.Spec.NodeName { @@ -1030,7 +1030,7 @@ func ValidatePodTemplate(pod *api.PodTemplate) errs.ValidationErrorList { func ValidatePodTemplateUpdate(newPod, oldPod *api.PodTemplate) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldPod.ObjectMeta, &newPod.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newPod.ObjectMeta, &oldPod.ObjectMeta).Prefix("metadata")...) allErrs = append(allErrs, ValidatePodTemplateSpec(&newPod.Template, 0).Prefix("template")...) return allErrs } @@ -1155,7 +1155,7 @@ func validateServicePort(sp *api.ServicePort, requireName bool, allNames *util.S // ValidateServiceUpdate tests if required fields in the service are set during an update func ValidateServiceUpdate(oldService, service *api.Service) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldService.ObjectMeta, &service.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&service.ObjectMeta, &oldService.ObjectMeta).Prefix("metadata")...) if api.IsServiceIPSet(oldService) && service.Spec.ClusterIP != oldService.Spec.ClusterIP { allErrs = append(allErrs, errs.NewFieldInvalid("spec.clusterIP", service.Spec.ClusterIP, "field is immutable")) @@ -1177,7 +1177,7 @@ func ValidateReplicationController(controller *api.ReplicationController) errs.V // ValidateReplicationControllerUpdate tests if required fields in the replication controller are set. func ValidateReplicationControllerUpdate(oldController, controller *api.ReplicationController) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldController.ObjectMeta, &controller.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&controller.ObjectMeta, &oldController.ObjectMeta).Prefix("metadata")...) allErrs = append(allErrs, ValidateReplicationControllerSpec(&controller.Spec).Prefix("spec")...) return allErrs } @@ -1254,7 +1254,7 @@ func ValidateNode(node *api.Node) errs.ValidationErrorList { // ValidateNodeUpdate tests to make sure a node update can be applied. Modifies oldNode. func ValidateNodeUpdate(oldNode *api.Node, node *api.Node) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldNode.ObjectMeta, &node.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&node.ObjectMeta, &oldNode.ObjectMeta).Prefix("metadata")...) // TODO: Enable the code once we have better api object.status update model. Currently, // anyone can update node status. @@ -1337,7 +1337,7 @@ func ValidateServiceAccount(serviceAccount *api.ServiceAccount) errs.ValidationE // ValidateServiceAccountUpdate tests if required fields in the ServiceAccount are set. func ValidateServiceAccountUpdate(oldServiceAccount, newServiceAccount *api.ServiceAccount) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldServiceAccount.ObjectMeta, &newServiceAccount.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newServiceAccount.ObjectMeta, &oldServiceAccount.ObjectMeta).Prefix("metadata")...) allErrs = append(allErrs, ValidateServiceAccount(newServiceAccount)...) return allErrs } @@ -1401,7 +1401,7 @@ func ValidateSecret(secret *api.Secret) errs.ValidationErrorList { // ValidateSecretUpdate tests if required fields in the Secret are set. func ValidateSecretUpdate(oldSecret, newSecret *api.Secret) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldSecret.ObjectMeta, &newSecret.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newSecret.ObjectMeta, &oldSecret.ObjectMeta).Prefix("metadata")...) if len(newSecret.Type) == 0 { newSecret.Type = oldSecret.Type @@ -1464,7 +1464,7 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro // newResourceQuota is updated with fields that cannot be changed. func ValidateResourceQuotaUpdate(newResourceQuota, oldResourceQuota *api.ResourceQuota) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldResourceQuota.ObjectMeta, &newResourceQuota.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newResourceQuota.ObjectMeta, &oldResourceQuota.ObjectMeta).Prefix("metadata")...) for k := range newResourceQuota.Spec.Hard { allErrs = append(allErrs, validateResourceName(string(k), string(newResourceQuota.TypeMeta.Kind))...) } @@ -1476,7 +1476,7 @@ func ValidateResourceQuotaUpdate(newResourceQuota, oldResourceQuota *api.Resourc // newResourceQuota is updated with fields that cannot be changed. func ValidateResourceQuotaStatusUpdate(newResourceQuota, oldResourceQuota *api.ResourceQuota) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldResourceQuota.ObjectMeta, &newResourceQuota.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newResourceQuota.ObjectMeta, &oldResourceQuota.ObjectMeta).Prefix("metadata")...) if newResourceQuota.ResourceVersion == "" { allErrs = append(allErrs, errs.NewFieldRequired("resourceVersion")) } @@ -1521,7 +1521,7 @@ func validateFinalizerName(stringValue string) errs.ValidationErrorList { // TODO The syntax here is the reverse of the (old, new) pattern in most other validation. Fix this. func ValidateNamespaceUpdate(newNamespace *api.Namespace, oldNamespace *api.Namespace) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldNamespace.ObjectMeta, &newNamespace.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newNamespace.ObjectMeta, &oldNamespace.ObjectMeta).Prefix("metadata")...) newNamespace.Spec.Finalizers = oldNamespace.Spec.Finalizers newNamespace.Status = oldNamespace.Status return allErrs @@ -1531,7 +1531,7 @@ func ValidateNamespaceUpdate(newNamespace *api.Namespace, oldNamespace *api.Name // that cannot be changed. func ValidateNamespaceStatusUpdate(newNamespace, oldNamespace *api.Namespace) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldNamespace.ObjectMeta, &newNamespace.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newNamespace.ObjectMeta, &oldNamespace.ObjectMeta).Prefix("metadata")...) newNamespace.Spec = oldNamespace.Spec if newNamespace.DeletionTimestamp.IsZero() { if newNamespace.Status.Phase != api.NamespaceActive { @@ -1549,7 +1549,7 @@ func ValidateNamespaceStatusUpdate(newNamespace, oldNamespace *api.Namespace) er // newNamespace is updated with fields that cannot be changed. func ValidateNamespaceFinalizeUpdate(newNamespace, oldNamespace *api.Namespace) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldNamespace.ObjectMeta, &newNamespace.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newNamespace.ObjectMeta, &oldNamespace.ObjectMeta).Prefix("metadata")...) for i := range newNamespace.Spec.Finalizers { allErrs = append(allErrs, validateFinalizerName(string(newNamespace.Spec.Finalizers[i]))...) } @@ -1636,7 +1636,7 @@ func validateEndpointPort(port *api.EndpointPort, requireName bool) errs.Validat // ValidateEndpointsUpdate tests to make sure an endpoints update can be applied. func ValidateEndpointsUpdate(oldEndpoints, newEndpoints *api.Endpoints) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldEndpoints.ObjectMeta, &newEndpoints.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&newEndpoints.ObjectMeta, &oldEndpoints.ObjectMeta).Prefix("metadata")...) allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets).Prefix("subsets")...) return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 4af153cce0d..21868c6d713 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -84,12 +84,6 @@ func TestValidateObjectMetaNamespaces(t *testing.T) { } func TestValidateObjectMetaUpdateIgnoresCreationTimestamp(t *testing.T) { - if errs := ValidateObjectMetaUpdate( - &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, - &api.ObjectMeta{Name: "test", ResourceVersion: "1"}, - ); len(errs) != 0 { - t.Fatalf("unexpected errors: %v", errs) - } if errs := ValidateObjectMetaUpdate( &api.ObjectMeta{Name: "test", ResourceVersion: "1"}, &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, @@ -97,8 +91,14 @@ func TestValidateObjectMetaUpdateIgnoresCreationTimestamp(t *testing.T) { t.Fatalf("unexpected errors: %v", errs) } if errs := ValidateObjectMetaUpdate( + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1"}, + ); len(errs) != 0 { + t.Fatalf("unexpected errors: %v", errs) + } + if errs := ValidateObjectMetaUpdate( + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(11, 0))}, - &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, ); len(errs) != 0 { t.Fatalf("unexpected errors: %v", errs) }