Change how one-of blocks are validated

I took a hard look at error output and played until I was happier.  This now
prints JSON for structs in the error, rather than go's format.

Also made the error message easier to read.

Fixed tests.

pkg/api/validation/validation.go

@@ -360,74 +360,132 @@ func validateVolumes(volumes []api.Volume, fldPath *field.Path) (sets.String, fi
 func validateVolumeSource(source *api.VolumeSource, fldPath *field.Path) field.ErrorList {
 	numVolumes := 0
 	allErrs := field.ErrorList{}
-	if source.HostPath != nil {
-		numVolumes++
-		allErrs = append(allErrs, validateHostPathVolumeSource(source.HostPath, fldPath.Child("hostPath"))...)
-	}
 	if source.EmptyDir != nil {
 		numVolumes++
 		// EmptyDirs have nothing to validate
 	}
+	if source.HostPath != nil {
+		if numVolumes > 0 {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("hostPath"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateHostPathVolumeSource(source.HostPath, fldPath.Child("hostPath"))...)
+		}
+	}
 	if source.GitRepo != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateGitRepoVolumeSource(source.GitRepo, fldPath.Child("gitRepo"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("gitRepo"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateGitRepoVolumeSource(source.GitRepo, fldPath.Child("gitRepo"))...)
+		}
 	}
 	if source.GCEPersistentDisk != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(source.GCEPersistentDisk, fldPath.Child("persistentDisk"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("gcePersistentDisk"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(source.GCEPersistentDisk, fldPath.Child("persistentDisk"))...)
+		}
 	}
 	if source.AWSElasticBlockStore != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(source.AWSElasticBlockStore, fldPath.Child("awsElasticBlockStore"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("awsElasticBlockStore"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(source.AWSElasticBlockStore, fldPath.Child("awsElasticBlockStore"))...)
+		}
 	}
 	if source.Secret != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateSecretVolumeSource(source.Secret, fldPath.Child("secret"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("secret"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateSecretVolumeSource(source.Secret, fldPath.Child("secret"))...)
+		}
 	}
 	if source.NFS != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateNFSVolumeSource(source.NFS, fldPath.Child("nfs"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("nfs"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateNFSVolumeSource(source.NFS, fldPath.Child("nfs"))...)
+		}
 	}
 	if source.ISCSI != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateISCSIVolumeSource(source.ISCSI, fldPath.Child("iscsi"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("iscsi"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateISCSIVolumeSource(source.ISCSI, fldPath.Child("iscsi"))...)
+		}
 	}
 	if source.Glusterfs != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateGlusterfs(source.Glusterfs, fldPath.Child("glusterfs"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("glusterfs"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateGlusterfs(source.Glusterfs, fldPath.Child("glusterfs"))...)
+		}
 	}
 	if source.Flocker != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateFlockerVolumeSource(source.Flocker, fldPath.Child("flocker"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("flocker"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateFlockerVolumeSource(source.Flocker, fldPath.Child("flocker"))...)
+		}
 	}
 	if source.PersistentVolumeClaim != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validatePersistentClaimVolumeSource(source.PersistentVolumeClaim, fldPath.Child("persistentVolumeClaim"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("persistentVolumeClaim"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validatePersistentClaimVolumeSource(source.PersistentVolumeClaim, fldPath.Child("persistentVolumeClaim"))...)
+		}
 	}
 	if source.RBD != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateRBDVolumeSource(source.RBD, fldPath.Child("rbd"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("rbd"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateRBDVolumeSource(source.RBD, fldPath.Child("rbd"))...)
+		}
 	}
 	if source.Cinder != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateCinderVolumeSource(source.Cinder, fldPath.Child("cinder"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("cinder"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateCinderVolumeSource(source.Cinder, fldPath.Child("cinder"))...)
+		}
 	}
 	if source.CephFS != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateCephFSVolumeSource(source.CephFS, fldPath.Child("cephfs"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("cephFS"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateCephFSVolumeSource(source.CephFS, fldPath.Child("cephfs"))...)
+		}
 	}
 	if source.DownwardAPI != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateDownwardAPIVolumeSource(source.DownwardAPI, fldPath.Child("downwardAPI"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("downwarAPI"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateDownwardAPIVolumeSource(source.DownwardAPI, fldPath.Child("downwardAPI"))...)
+		}
 	}
 	if source.FC != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateFCVolumeSource(source.FC, fldPath.Child("fc"))...)
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("fc"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateFCVolumeSource(source.FC, fldPath.Child("fc"))...)
+		}
 	}
 	if numVolumes == 0 {
 		allErrs = append(allErrs, field.Required(fldPath, "must specify a volume type"))
-	} else if numVolumes > 1 {
-		allErrs = append(allErrs, field.Invalid(fldPath, source, "may not specify more than 1 volume type"))
 	}
 
 	return allErrs
@@ -672,53 +730,95 @@ func ValidatePersistentVolume(pv *api.PersistentVolume) field.ErrorList {
 
 	numVolumes := 0
 	if pv.Spec.HostPath != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateHostPathVolumeSource(pv.Spec.HostPath, specPath.Child("hostPath"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("hostPath"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateHostPathVolumeSource(pv.Spec.HostPath, specPath.Child("hostPath"))...)
+		}
 	}
 	if pv.Spec.GCEPersistentDisk != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(pv.Spec.GCEPersistentDisk, specPath.Child("persistentDisk"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("gcePersistentDisk"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(pv.Spec.GCEPersistentDisk, specPath.Child("persistentDisk"))...)
+		}
 	}
 	if pv.Spec.AWSElasticBlockStore != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(pv.Spec.AWSElasticBlockStore, specPath.Child("awsElasticBlockStore"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("awsElasticBlockStore"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(pv.Spec.AWSElasticBlockStore, specPath.Child("awsElasticBlockStore"))...)
+		}
 	}
 	if pv.Spec.Glusterfs != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateGlusterfs(pv.Spec.Glusterfs, specPath.Child("glusterfs"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("glusterfs"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateGlusterfs(pv.Spec.Glusterfs, specPath.Child("glusterfs"))...)
+		}
 	}
 	if pv.Spec.Flocker != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateFlockerVolumeSource(pv.Spec.Flocker, specPath.Child("flocker"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("flocker"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateFlockerVolumeSource(pv.Spec.Flocker, specPath.Child("flocker"))...)
+		}
 	}
 	if pv.Spec.NFS != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateNFSVolumeSource(pv.Spec.NFS, specPath.Child("nfs"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("nfs"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateNFSVolumeSource(pv.Spec.NFS, specPath.Child("nfs"))...)
+		}
 	}
 	if pv.Spec.RBD != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateRBDVolumeSource(pv.Spec.RBD, specPath.Child("rbd"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("rbd"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateRBDVolumeSource(pv.Spec.RBD, specPath.Child("rbd"))...)
+		}
 	}
 	if pv.Spec.CephFS != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateCephFSVolumeSource(pv.Spec.CephFS, specPath.Child("cephfs"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("cephFS"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateCephFSVolumeSource(pv.Spec.CephFS, specPath.Child("cephfs"))...)
+		}
 	}
 	if pv.Spec.ISCSI != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateISCSIVolumeSource(pv.Spec.ISCSI, specPath.Child("iscsi"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("iscsi"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateISCSIVolumeSource(pv.Spec.ISCSI, specPath.Child("iscsi"))...)
+		}
 	}
 	if pv.Spec.Cinder != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateCinderVolumeSource(pv.Spec.Cinder, specPath.Child("cinder"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("cinder"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateCinderVolumeSource(pv.Spec.Cinder, specPath.Child("cinder"))...)
+		}
 	}
 	if pv.Spec.FC != nil {
-		numVolumes++
+		if numVolumes > 0 {
-		allErrs = append(allErrs, validateFCVolumeSource(pv.Spec.FC, specPath.Child("fc"))...)
+			allErrs = append(allErrs, field.Forbidden(specPath.Child("fc"), "may not specify more than 1 volume type"))
+		} else {
+			numVolumes++
+			allErrs = append(allErrs, validateFCVolumeSource(pv.Spec.FC, specPath.Child("fc"))...)
+		}
 	}
 	if numVolumes == 0 {
 		allErrs = append(allErrs, field.Required(specPath, "must specify a volume type"))
-	} else if numVolumes > 1 {
-		allErrs = append(allErrs, field.Invalid(specPath, pv.Spec.PersistentVolumeSource, "may not specify more than 1 volume type"))
 	}
 	return allErrs
 }
@@ -979,21 +1079,31 @@ func validateHandler(handler *api.Handler, fldPath *field.Path) field.ErrorList
 	numHandlers := 0
 	allErrors := field.ErrorList{}
 	if handler.Exec != nil {
-		numHandlers++
+		if numHandlers > 0 {
-		allErrors = append(allErrors, validateExecAction(handler.Exec, fldPath.Child("exec"))...)
+			allErrors = append(allErrors, field.Forbidden(fldPath.Child("exec"), "may not specify more than 1 handler type"))
+		} else {
+			numHandlers++
+			allErrors = append(allErrors, validateExecAction(handler.Exec, fldPath.Child("exec"))...)
+		}
 	}
 	if handler.HTTPGet != nil {
-		numHandlers++
+		if numHandlers > 0 {
-		allErrors = append(allErrors, validateHTTPGetAction(handler.HTTPGet, fldPath.Child("httpGet"))...)
+			allErrors = append(allErrors, field.Forbidden(fldPath.Child("httpGet"), "may not specify more than 1 handler type"))
+		} else {
+			numHandlers++
+			allErrors = append(allErrors, validateHTTPGetAction(handler.HTTPGet, fldPath.Child("httpGet"))...)
+		}
 	}
 	if handler.TCPSocket != nil {
-		numHandlers++
+		if numHandlers > 0 {
-		allErrors = append(allErrors, validateTCPSocketAction(handler.TCPSocket, fldPath.Child("tcpSocket"))...)
+			allErrors = append(allErrors, field.Forbidden(fldPath.Child("tcpSocket"), "may not specify more than 1 handler type"))
+		} else {
+			numHandlers++
+			allErrors = append(allErrors, validateTCPSocketAction(handler.TCPSocket, fldPath.Child("tcpSocket"))...)
+		}
 	}
 	if numHandlers == 0 {
 		allErrors = append(allErrors, field.Required(fldPath, "must specify a handler type"))
-	} else if numHandlers > 1 {
-		allErrors = append(allErrors, field.Invalid(fldPath, handler, "may not specify more than 1 handler type"))
 	}
 	return allErrors
 }

pkg/api/validation/validation_test.go

@@ -73,7 +73,7 @@ func TestValidateObjectMetaNamespaces(t *testing.T) {
 	if len(errs) != 1 {
 		t.Fatalf("unexpected errors: %v", errs)
 	}
-	if !strings.Contains(errs[0].Error(), "invalid value 'foo.bar'") {
+	if !strings.Contains(errs[0].Error(), `Invalid value: "foo.bar"`) {
 		t.Errorf("unexpected error message: %v", errs)
 	}
 	maxLength := 63
@@ -92,7 +92,7 @@ func TestValidateObjectMetaNamespaces(t *testing.T) {
 	if len(errs) != 1 {
 		t.Fatalf("unexpected errors: %v", errs)
 	}
-	if !strings.Contains(errs[0].Error(), "invalid value") {
+	if !strings.Contains(errs[0].Error(), "Invalid value") {
 		t.Errorf("unexpected error message: %v", errs)
 	}
 }
@@ -824,12 +824,12 @@ func TestValidateEnv(t *testing.T) {
 		{
 			name:          "zero-length name",
 			envs:          []api.EnvVar{{Name: ""}},
-			expectedError: "[0].name: required value",
+			expectedError: "[0].name: Required value",
 		},
 		{
 			name:          "name not a C identifier",
 			envs:          []api.EnvVar{{Name: "a.b.c"}},
-			expectedError: `[0].name: invalid value 'a.b.c', Details: must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName"`,
+			expectedError: `[0].name: Invalid value: "a.b.c": must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName"`,
 		},
 		{
 			name: "value and valueFrom specified",
@@ -843,7 +843,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom: invalid value '', Details: may not be specified when `value` is not empty",
+			expectedError: "[0].valueFrom: Invalid value: \"\": may not be specified when `value` is not empty",
 		},
 		{
 			name: "missing FieldPath on ObjectFieldSelector",
@@ -855,7 +855,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom.fieldRef.fieldPath: required value",
+			expectedError: `[0].valueFrom.fieldRef.fieldPath: Required value`,
 		},
 		{
 			name: "missing APIVersion on ObjectFieldSelector",
@@ -867,7 +867,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom.fieldRef.apiVersion: required value",
+			expectedError: `[0].valueFrom.fieldRef.apiVersion: Required value`,
 		},
 		{
 			name: "invalid fieldPath",
@@ -880,7 +880,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom.fieldRef.fieldPath: invalid value 'metadata.whoops', Details: error converting fieldPath",
+			expectedError: `[0].valueFrom.fieldRef.fieldPath: Invalid value: "metadata.whoops": error converting fieldPath`,
 		},
 		{
 			name: "invalid fieldPath labels",
@@ -893,7 +893,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom.fieldRef.fieldPath: unsupported value 'metadata.labels', Details: supported values: metadata.name, metadata.namespace, status.podIP",
+			expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.labels": supported values: metadata.name, metadata.namespace, status.podIP`,
 		},
 		{
 			name: "invalid fieldPath annotations",
@@ -906,7 +906,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "[0].valueFrom.fieldRef.fieldPath: unsupported value 'metadata.annotations', Details: supported values: metadata.name, metadata.namespace, status.podIP",
+			expectedError: `[0].valueFrom.fieldRef.fieldPath: Unsupported value: "metadata.annotations": supported values: metadata.name, metadata.namespace, status.podIP`,
 		},
 		{
 			name: "unsupported fieldPath",
@@ -919,7 +919,7 @@ func TestValidateEnv(t *testing.T) {
 					},
 				},
 			}},
-			expectedError: "valueFrom.fieldRef.fieldPath: unsupported value 'status.phase', Details: supported values: metadata.name, metadata.namespace, status.podIP",
+			expectedError: `valueFrom.fieldRef.fieldPath: Unsupported value: "status.phase": supported values: metadata.name, metadata.namespace, status.podIP`,
 		},
 	}
 	for _, tc := range errorCases {

pkg/apis/extensions/validation/validation_test.go

@@ -80,7 +80,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.kind: required",
+			msg: "scaleRef.kind: Required",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -92,7 +92,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.kind: invalid",
+			msg: "scaleRef.kind: Invalid",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -104,7 +104,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.name: required",
+			msg: "scaleRef.name: Required",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -116,7 +116,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.name: invalid",
+			msg: "scaleRef.name: Invalid",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -128,7 +128,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.subresource: required",
+			msg: "scaleRef.subresource: Required",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -140,7 +140,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.subresource: invalid",
+			msg: "scaleRef.subresource: Invalid",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -152,7 +152,7 @@ func TestValidateHorizontalPodAutoscaler(t *testing.T) {
 					CPUUtilization: &extensions.CPUTargetUtilization{TargetPercentage: 70},
 				},
 			},
-			msg: "scaleRef.subresource: unsupported",
+			msg: "scaleRef.subresource: Unsupported",
 		},
 		{
 			horizontalPodAutoscaler: extensions.HorizontalPodAutoscaler{
@@ -736,7 +736,7 @@ func TestValidateDeployment(t *testing.T) {
 	}
 
 	errorCases := map[string]*extensions.Deployment{}
-	errorCases["metadata.name: required value"] = &extensions.Deployment{
+	errorCases["metadata.name: Required value"] = &extensions.Deployment{
 		ObjectMeta: api.ObjectMeta{
 			Namespace: api.NamespaceDefault,
 		},
@@ -751,12 +751,12 @@ func TestValidateDeployment(t *testing.T) {
 	// RestartPolicy should be always.
 	invalidRestartPolicyDeployment := validDeployment()
 	invalidRestartPolicyDeployment.Spec.Template.Spec.RestartPolicy = api.RestartPolicyNever
-	errorCases["unsupported value 'Never'"] = invalidRestartPolicyDeployment
+	errorCases["Unsupported value: \"Never\""] = invalidRestartPolicyDeployment
 
 	// invalid unique label key.
 	invalidUniqueLabelDeployment := validDeployment()
 	invalidUniqueLabelDeployment.Spec.UniqueLabelKey = "abc/def/ghi"
-	errorCases["spec.uniqueLabel: invalid value"] = invalidUniqueLabelDeployment
+	errorCases["spec.uniqueLabel: Invalid value"] = invalidUniqueLabelDeployment
 
 	// rollingUpdate should be nil for recreate.
 	invalidRecreateDeployment := validDeployment()
@@ -802,7 +802,7 @@ func TestValidateDeployment(t *testing.T) {
 		if len(errs) == 0 {
 			t.Errorf("[%s] expected failure", k)
 		} else if !strings.Contains(errs[0].Error(), k) {
-			t.Errorf("unexpected error: %v, expected: %q", errs[0], k)
+			t.Errorf("unexpected error: %q, expected: %q", errs[0].Error(), k)
 		}
 	}
 }
@@ -874,7 +874,7 @@ func TestValidateJob(t *testing.T) {
 				Template:              validPodTemplateSpec,
 			},
 		},
-		"spec.selector:required value": {
+		"spec.selector:Required value": {
 			ObjectMeta: api.ObjectMeta{
 				Name:      "myjob",
 				Namespace: api.NamespaceDefault,
@@ -883,7 +883,7 @@ func TestValidateJob(t *testing.T) {
 				Template: validPodTemplateSpec,
 			},
 		},
-		"spec.template.metadata.labels: invalid value 'map[y:z]', Details: selector does not match template `labels`": {
+		"spec.template.metadata.labels: Invalid value: {\"y\":\"z\"}: `selector` does not match template `labels`": {
 			ObjectMeta: api.ObjectMeta{
 				Name:      "myjob",
 				Namespace: api.NamespaceDefault,
@@ -902,7 +902,7 @@ func TestValidateJob(t *testing.T) {
 				},
 			},
 		},
-		"spec.template.spec.restartPolicy:unsupported value": {
+		"spec.template.spec.restartPolicy: Unsupported value": {
 			ObjectMeta: api.ObjectMeta{
 				Name:      "myjob",
 				Namespace: api.NamespaceDefault,
@@ -1006,19 +1006,19 @@ func TestValidateIngress(t *testing.T) {
 			Backend: defaultBackend,
 		},
 	}
-	badPathErr := fmt.Sprintf("spec.rules[0].http.paths[0].path: invalid value '%v'", badPathExpr)
+	badPathErr := fmt.Sprintf("spec.rules[0].http.paths[0].path: Invalid value: '%v'", badPathExpr)
 	hostIP := "127.0.0.1"
 	badHostIP := newValid()
 	badHostIP.Spec.Rules[0].Host = hostIP
-	badHostIPErr := fmt.Sprintf("spec.rules[0].host: invalid value '%v'", hostIP)
+	badHostIPErr := fmt.Sprintf("spec.rules[0].host: Invalid value: '%v'", hostIP)
 
 	errorCases := map[string]extensions.Ingress{
-		"spec.backend.serviceName: required value":        servicelessBackend,
+		"spec.backend.serviceName: Required value":        servicelessBackend,
-		"spec.backend.serviceName: invalid value":         invalidNameBackend,
+		"spec.backend.serviceName: Invalid value":         invalidNameBackend,
-		"spec.backend.servicePort: invalid value":         noPortBackend,
+		"spec.backend.servicePort: Invalid value":         noPortBackend,
-		"spec.rules[0].host: invalid value":               badHost,
+		"spec.rules[0].host: Invalid value":               badHost,
-		"spec.rules[0].http.paths: required value":        noPaths,
+		"spec.rules[0].http.paths: Required value":        noPaths,
-		"spec.rules[0].http.paths[0].path: invalid value": noForwardSlashPath,
+		"spec.rules[0].http.paths[0].path: Invalid value": noForwardSlashPath,
 	}
 	errorCases[badPathErr] = badRegexPath
 	errorCases[badHostIPErr] = badHostIP
@@ -1112,8 +1112,8 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
 	}
 
 	errorCases := map[string]extensions.Ingress{
-		"status.loadBalancer.ingress[0].ip: invalid value":       invalidIP,
+		"status.loadBalancer.ingress[0].ip: Invalid value":       invalidIP,
-		"status.loadBalancer.ingress[0].hostname: invalid value": invalidHostname,
+		"status.loadBalancer.ingress[0].hostname: Invalid value": invalidHostname,
 	}
 	for k, v := range errorCases {
 		errs := ValidateIngressStatusUpdate(&v, &oldValue)
@@ -1220,7 +1220,7 @@ func TestValidateClusterAutoscaler(t *testing.T) {
 				},
 			},
 		},
-		"required value": {
+		"Required value": {
 			ObjectMeta: api.ObjectMeta{
 				Name:      "ClusterAutoscaler",
 				Namespace: api.NamespaceDefault,

pkg/kubectl/cmd/util/helpers_test.go

@@ -275,11 +275,11 @@ func TestCheckInvalidErr(t *testing.T) {
 	}{
 		{
 			errors.NewInvalid(api.Kind("Invalid1"), "invalidation", field.ErrorList{field.Invalid(field.NewPath("field"), "single", "details")}),
-			`Error from server: Invalid1 "invalidation" is invalid: field: invalid value 'single', Details: details`,
+			`Error from server: Invalid1 "invalidation" is invalid: field: Invalid value: "single": details`,
 		},
 		{
 			errors.NewInvalid(api.Kind("Invalid2"), "invalidation", field.ErrorList{field.Invalid(field.NewPath("field1"), "multi1", "details"), field.Invalid(field.NewPath("field2"), "multi2", "details")}),
-			`Error from server: Invalid2 "invalidation" is invalid: [field1: invalid value 'multi1', Details: details, field2: invalid value 'multi2', Details: details]`,
+			`Error from server: Invalid2 "invalidation" is invalid: [field1: Invalid value: "multi1": details, field2: Invalid value: "multi2": details]`,
 		},
 		{
 			errors.NewInvalid(api.Kind("Invalid3"), "invalidation", field.ErrorList{}),

pkg/util/validation/field/errors.go

@@ -17,12 +17,11 @@ limitations under the License.
 package field
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
-
-	"github.com/davecgh/go-spew/spew"
 )
 
 // Error is an implementation of the 'error' interface, which represents a
@@ -46,13 +45,20 @@ func (v *Error) Error() string {
 func (v *Error) ErrorBody() string {
 	var s string
 	switch v.Type {
-	case ErrorTypeRequired, ErrorTypeTooLong, ErrorTypeInternal:
+	case ErrorTypeRequired, ErrorTypeForbidden, ErrorTypeTooLong, ErrorTypeInternal:
 		s = fmt.Sprintf("%s", v.Type)
 	default:
-		s = spew.Sprintf("%s '%+v'", v.Type, v.BadValue)
+		var bad string
+		badBytes, err := json.Marshal(v.BadValue)
+		if err != nil {
+			bad = err.Error()
+		} else {
+			bad = string(badBytes)
+		}
+		s = fmt.Sprintf("%s: %s", v.Type, bad)
 	}
 	if len(v.Detail) != 0 {
-		s += fmt.Sprintf(", Details: %s", v.Detail)
+		s += fmt.Sprintf(": %s", v.Detail)
 	}
 	return s
 }
@@ -98,21 +104,21 @@ const (
 func (t ErrorType) String() string {
 	switch t {
 	case ErrorTypeNotFound:
-		return "not found"
+		return "Not found"
 	case ErrorTypeRequired:
-		return "required value"
+		return "Required value"
 	case ErrorTypeDuplicate:
-		return "duplicate value"
+		return "Duplicate value"
 	case ErrorTypeInvalid:
-		return "invalid value"
+		return "Invalid value"
 	case ErrorTypeNotSupported:
-		return "unsupported value"
+		return "Unsupported value"
 	case ErrorTypeForbidden:
-		return "forbidden"
+		return "Forbidden"
 	case ErrorTypeTooLong:
-		return "too long"
+		return "Too long"
 	case ErrorTypeInternal:
-		return "internal error"
+		return "Internal error"
 	default:
 		panic(fmt.Sprintf("unrecognized validation error: %q", t))
 		return ""

test/e2e/service.go

@@ -643,7 +643,7 @@ var _ = Describe("Services", func() {
 		if err == nil {
 			Failf("Created service with conflicting NodePort: %v", result2)
 		}
-		expectedErr := fmt.Sprintf("Service \"%s\" is invalid: spec.ports[0].nodePort: invalid value '%d', Details: provided port is already allocated",
+		expectedErr := fmt.Sprintf("Service \"%s\" is invalid: spec.ports[0].nodePort: Invalid value: %d: provided port is already allocated",
 			serviceName2, port.NodePort)
 		Expect(fmt.Sprintf("%v", err)).To(Equal(expectedErr))
 
@@ -704,7 +704,7 @@ var _ = Describe("Services", func() {
 		if err == nil {
 			Failf("failed to prevent update of service with out-of-range NodePort: %v", result)
 		}
-		expectedErr := fmt.Sprintf("Service \"%s\" is invalid: spec.ports[0].nodePort: invalid value '%d', Details: provided port is not in the valid range", serviceName, outOfRangeNodePort)
+		expectedErr := fmt.Sprintf("Service \"%s\" is invalid: spec.ports[0].nodePort: Invalid value: %d: provided port is not in the valid range", serviceName, outOfRangeNodePort)
 		Expect(fmt.Sprintf("%v", err)).To(Equal(expectedErr))
 
 		By("deleting original service " + serviceName)