Merge pull request #17247 from thockin/airplane_validation_pt3

examples/examples_test.go

@@ -33,12 +33,13 @@ import (
 	expvalidation "k8s.io/kubernetes/pkg/apis/extensions/validation"
 	"k8s.io/kubernetes/pkg/capabilities"
 	"k8s.io/kubernetes/pkg/runtime"
+	utilvalidation "k8s.io/kubernetes/pkg/util/validation"
 	"k8s.io/kubernetes/pkg/util/yaml"
 	schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api"
 	schedulerapilatest "k8s.io/kubernetes/plugin/pkg/scheduler/api/latest"
 )
 
-func validateObject(obj runtime.Object) (errors []error) {
+func validateObject(obj runtime.Object) (errors utilvalidation.ErrorList) {
 	switch t := obj.(type) {
 	case *api.ReplicationController:
 		if t.Namespace == "" {
@@ -122,7 +123,7 @@ func validateObject(obj runtime.Object) (errors []error) {
 		}
 		errors = expvalidation.ValidateDaemonSet(t)
 	default:
-		return []error{fmt.Errorf("no validation defined for %#v", obj)}
+		return utilvalidation.ErrorList{utilvalidation.NewInternalError("", fmt.Errorf("no validation defined for %#v", obj))}
 	}
 	return errors
 }

pkg/api/errors/errors.go

@@ -24,7 +24,6 @@ import (
 
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/runtime"
-	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/validation"
 )
 
@@ -162,13 +161,12 @@ func NewConflict(kind, name string, err error) error {
 func NewInvalid(kind, name string, errs validation.ErrorList) error {
 	causes := make([]unversioned.StatusCause, 0, len(errs))
 	for i := range errs {
-		if err, ok := errs[i].(*validation.Error); ok {
-			causes = append(causes, unversioned.StatusCause{
-				Type:    unversioned.CauseType(err.Type),
-				Message: err.ErrorBody(),
-				Field:   err.Field,
-			})
-		}
+		err := errs[i]
+		causes = append(causes, unversioned.StatusCause{
+			Type:    unversioned.CauseType(err.Type),
+			Message: err.ErrorBody(),
+			Field:   err.Field,
+		})
 	}
 	return &StatusError{unversioned.Status{
 		Status: unversioned.StatusFailure,
@@ -179,7 +177,7 @@ func NewInvalid(kind, name string, errs validation.ErrorList) error {
 			Name:   name,
 			Causes: causes,
 		},
-		Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, utilerrors.NewAggregate(errs)),
+		Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, errs.ToAggregate()),
 	}}
 }
 

pkg/api/errors/errors_test.go

@@ -92,7 +92,7 @@ func TestNewInvalid(t *testing.T) {
 		Details *unversioned.StatusDetails
 	}{
 		{
-			validation.NewFieldDuplicate("field[0].name", "bar"),
+			validation.NewDuplicateError("field[0].name", "bar"),
 			&unversioned.StatusDetails{
 				Kind: "kind",
 				Name: "name",
@@ -103,7 +103,7 @@ func TestNewInvalid(t *testing.T) {
 			},
 		},
 		{
-			validation.NewFieldInvalid("field[0].name", "bar", "detail"),
+			validation.NewInvalidError("field[0].name", "bar", "detail"),
 			&unversioned.StatusDetails{
 				Kind: "kind",
 				Name: "name",
@@ -114,7 +114,7 @@ func TestNewInvalid(t *testing.T) {
 			},
 		},
 		{
-			validation.NewFieldNotFound("field[0].name", "bar"),
+			validation.NewNotFoundError("field[0].name", "bar"),
 			&unversioned.StatusDetails{
 				Kind: "kind",
 				Name: "name",
@@ -125,7 +125,7 @@ func TestNewInvalid(t *testing.T) {
 			},
 		},
 		{
-			validation.NewFieldNotSupported("field[0].name", "bar", nil),
+			validation.NewNotSupportedError("field[0].name", "bar", nil),
 			&unversioned.StatusDetails{
 				Kind: "kind",
 				Name: "name",
@@ -136,7 +136,7 @@ func TestNewInvalid(t *testing.T) {
 			},
 		},
 		{
-			validation.NewFieldRequired("field[0].name"),
+			validation.NewRequiredError("field[0].name"),
 			&unversioned.StatusDetails{
 				Kind: "kind",
 				Name: "name",

pkg/api/rest/update.go

@@ -57,11 +57,11 @@ func validateCommonFields(obj, old runtime.Object) utilvalidation.ErrorList {
 	allErrs := utilvalidation.ErrorList{}
 	objectMeta, err := api.ObjectMetaFor(obj)
 	if err != nil {
-		return append(allErrs, errors.NewInternalError(err))
+		return append(allErrs, utilvalidation.NewInternalError("metadata", err))
 	}
 	oldObjectMeta, err := api.ObjectMetaFor(old)
 	if err != nil {
-		return append(allErrs, errors.NewInternalError(err))
+		return append(allErrs, utilvalidation.NewInternalError("metadata", err))
 	}
 	allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(objectMeta, oldObjectMeta)...)
 

pkg/api/validation/events.go

@@ -27,10 +27,10 @@ func ValidateEvent(event *api.Event) validation.ErrorList {
 	// TODO: There is no namespace required for node.
 	if event.InvolvedObject.Kind != "Node" &&
 		event.Namespace != event.InvolvedObject.Namespace {
-		allErrs = append(allErrs, validation.NewFieldInvalid("involvedObject.namespace", event.InvolvedObject.Namespace, "namespace does not match involvedObject"))
+		allErrs = append(allErrs, validation.NewInvalidError("involvedObject.namespace", event.InvolvedObject.Namespace, "namespace does not match involvedObject"))
 	}
 	if !validation.IsDNS1123Subdomain(event.Namespace) {
-		allErrs = append(allErrs, validation.NewFieldInvalid("namespace", event.Namespace, ""))
+		allErrs = append(allErrs, validation.NewInvalidError("namespace", event.Namespace, ""))
 	}
 	return allErrs
 }

pkg/api/validation/schema.go

@@ -27,7 +27,6 @@ import (
 	"github.com/golang/glog"
 	apiutil "k8s.io/kubernetes/pkg/api/util"
 	utilerrors "k8s.io/kubernetes/pkg/util/errors"
-	"k8s.io/kubernetes/pkg/util/validation"
 	"k8s.io/kubernetes/pkg/util/yaml"
 )
 
@@ -67,11 +66,11 @@ func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) {
 	return schema, nil
 }
 
-// validateList unpack a list and validate every item in the list.
+// validateList unpacks a list and validate every item in the list.
 // It return nil if every item is ok.
 // Otherwise it return an error list contain errors of every item.
-func (s *SwaggerSchema) validateList(obj map[string]interface{}) validation.ErrorList {
-	allErrs := validation.ErrorList{}
+func (s *SwaggerSchema) validateList(obj map[string]interface{}) []error {
+	allErrs := []error{}
 	items, exists := obj["items"]
 	if !exists {
 		return append(allErrs, fmt.Errorf("no items field in %#v", obj))
@@ -160,8 +159,8 @@ func (s *SwaggerSchema) ValidateBytes(data []byte) error {
 	return utilerrors.NewAggregate(allErrs)
 }
 
-func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName string) validation.ErrorList {
-	allErrs := validation.ErrorList{}
+func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName string) []error {
+	allErrs := []error{}
 	models := s.api.Models
 	model, ok := models.At(typeName)
 	if !ok {
@@ -215,7 +214,7 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName stri
 // This matches type name in the swagger spec, such as "v1.Binding".
 var versionRegexp = regexp.MustCompile(`^v.+\..*`)
 
-func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) validation.ErrorList {
+func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) []error {
 	// TODO: caesarxuchao: because we have multiple group/versions and objects
 	// may reference objects in other group, the commented out way of checking
 	// if a filedType is a type defined by us is outdated. We use a hacky way
@@ -229,7 +228,7 @@ func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType st
 		// if strings.HasPrefix(fieldType, apiVersion) {
 		return s.ValidateObject(value, fieldName, fieldType)
 	}
-	allErrs := validation.ErrorList{}
+	allErrs := []error{}
 	switch fieldType {
 	case "string":
 		// Be loose about what we accept for 'string' since we use IntOrString in a couple of places

pkg/api/validation/validation_test.go

@@ -28,7 +28,6 @@ import (
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/capabilities"
-	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/intstr"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/util/validation"
@@ -36,7 +35,7 @@ import (
 
 func expectPrefix(t *testing.T, prefix string, errs validation.ErrorList) {
 	for i := range errs {
-		if f, p := errs[i].(*validation.Error).Field, prefix; !strings.HasPrefix(f, p) {
+		if f, p := errs[i].Field, prefix; !strings.HasPrefix(f, p) {
 			t.Errorf("expected prefix '%s' for field '%s' (%v)", p, f, errs[i])
 		}
 	}
@@ -150,7 +149,7 @@ func TestValidateLabels(t *testing.T) {
 		if len(errs) != 1 {
 			t.Errorf("case[%d] expected failure", i)
 		} else {
-			detail := errs[0].(*validation.Error).Detail
+			detail := errs[0].Detail
 			if detail != qualifiedNameErrorMsg {
 				t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
 			}
@@ -168,7 +167,7 @@ func TestValidateLabels(t *testing.T) {
 		if len(errs) != 1 {
 			t.Errorf("case[%d] expected failure", i)
 		} else {
-			detail := errs[0].(*validation.Error).Detail
+			detail := errs[0].Detail
 			if detail != labelValueErrorMsg {
 				t.Errorf("error detail %s should be equal %s", detail, labelValueErrorMsg)
 			}
@@ -215,7 +214,7 @@ func TestValidateAnnotations(t *testing.T) {
 		if len(errs) != 1 {
 			t.Errorf("case[%d] expected failure", i)
 		}
-		detail := errs[0].(*validation.Error).Detail
+		detail := errs[0].Detail
 		if detail != qualifiedNameErrorMsg {
 			t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
 		}
@@ -568,13 +567,13 @@ func TestValidateVolumes(t *testing.T) {
 			continue
 		}
 		for i := range errs {
-			if errs[i].(*validation.Error).Type != v.T {
+			if errs[i].Type != v.T {
 				t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i])
 			}
-			if errs[i].(*validation.Error).Field != v.F {
+			if errs[i].Field != v.F {
 				t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
 			}
-			detail := errs[i].(*validation.Error).Detail
+			detail := errs[i].Detail
 			if detail != v.D {
 				t.Errorf("%s: expected error detail \"%s\", got \"%s\"", k, v.D, detail)
 			}
@@ -627,13 +626,13 @@ func TestValidatePorts(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			if errs[i].(*validation.Error).Type != v.T {
+			if errs[i].Type != v.T {
 				t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i])
 			}
-			if errs[i].(*validation.Error).Field != v.F {
+			if errs[i].Field != v.F {
 				t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
 			}
-			detail := errs[i].(*validation.Error).Detail
+			detail := errs[i].Detail
 			if detail != v.D {
 				t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
 			}
@@ -772,7 +771,7 @@ func TestValidateEnv(t *testing.T) {
 			t.Errorf("expected failure for %s", tc.name)
 		} else {
 			for i := range errs {
-				str := errs[i].(*validation.Error).Error()
+				str := errs[i].Error()
 				if str != "" && str != tc.expectedError {
 					t.Errorf("%s: expected error detail either empty or %s, got %s", tc.name, tc.expectedError, str)
 				}
@@ -2108,7 +2107,7 @@ func TestValidateService(t *testing.T) {
 		tc.tweakSvc(&svc)
 		errs := ValidateService(&svc)
 		if len(errs) != tc.numErrs {
-			t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs))
+			t.Errorf("Unexpected error list for case %q: %v", tc.name, errs.ToAggregate())
 		}
 	}
 }
@@ -2560,7 +2559,7 @@ func TestValidateReplicationController(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			field := errs[i].(*validation.Error).Field
+			field := errs[i].Field
 			if !strings.HasPrefix(field, "spec.template.") &&
 				field != "metadata.name" &&
 				field != "metadata.namespace" &&
@@ -2676,7 +2675,7 @@ func TestValidateNode(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			field := errs[i].(*validation.Error).Field
+			field := errs[i].Field
 			expectedFields := map[string]bool{
 				"metadata.name":        true,
 				"metadata.labels":      true,
@@ -2974,7 +2973,7 @@ func TestValidateServiceUpdate(t *testing.T) {
 		tc.tweakSvc(&oldSvc, &newSvc)
 		errs := ValidateServiceUpdate(&newSvc, &oldSvc)
 		if len(errs) != tc.numErrs {
-			t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs))
+			t.Errorf("Unexpected error list for case %q: %v", tc.name, errs.ToAggregate())
 		}
 	}
 }
@@ -3008,7 +3007,7 @@ func TestValidateResourceNames(t *testing.T) {
 		} else if len(err) == 0 && !item.success {
 			t.Errorf("expected failure for input %q", item.input)
 			for i := range err {
-				detail := err[i].(*validation.Error).Detail
+				detail := err[i].Detail
 				if detail != "" && detail != qualifiedNameErrorMsg {
 					t.Errorf("%d: expected error detail either empty or %s, got %s", k, qualifiedNameErrorMsg, detail)
 				}
@@ -3224,7 +3223,7 @@ func TestValidateLimitRange(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			detail := errs[i].(*validation.Error).Detail
+			detail := errs[i].Detail
 			if detail != v.D {
 				t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
 			}
@@ -3329,8 +3328,8 @@ func TestValidateResourceQuota(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			field := errs[i].(*validation.Error).Field
-			detail := errs[i].(*validation.Error).Detail
+			field := errs[i].Field
+			detail := errs[i].Detail
 			if field != "metadata.name" && field != "metadata.namespace" && !api.IsStandardResourceName(field) {
 				t.Errorf("%s: missing prefix for: %v", k, field)
 			}
@@ -3937,7 +3936,7 @@ func TestValidateEndpoints(t *testing.T) {
 	}
 
 	for k, v := range errorCases {
-		if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].(*validation.Error).Type != v.errorType || !strings.Contains(errs[0].(*validation.Error).Detail, v.errorDetail) {
+		if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) {
 			t.Errorf("Expected error type %s with detail %s for %s, got %v", v.errorType, v.errorDetail, k, errs)
 		}
 	}
@@ -4017,7 +4016,7 @@ func TestValidateSecurityContext(t *testing.T) {
 		},
 	}
 	for k, v := range errorCases {
-		if errs := ValidateSecurityContext(v.sc); len(errs) == 0 || errs[0].(*validation.Error).Type != v.errorType || errs[0].(*validation.Error).Detail != v.errorDetail {
+		if errs := ValidateSecurityContext(v.sc); len(errs) == 0 || errs[0].Type != v.errorType || errs[0].Detail != v.errorDetail {
 			t.Errorf("Expected error type %s with detail %s for %s, got %v", v.errorType, v.errorDetail, k, errs)
 		}
 	}

pkg/apis/extensions/validation/validation_test.go

@@ -24,7 +24,6 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/extensions"
 	"k8s.io/kubernetes/pkg/util/intstr"
-	"k8s.io/kubernetes/pkg/util/validation"
 )
 
 func TestValidateHorizontalPodAutoscaler(t *testing.T) {
@@ -675,7 +674,7 @@ func TestValidateDaemonSet(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		}
 		for i := range errs {
-			field := errs[i].(*validation.Error).Field
+			field := errs[i].Field
 			if !strings.HasPrefix(field, "spec.template.") &&
 				field != "metadata.name" &&
 				field != "metadata.namespace" &&
@@ -918,9 +917,9 @@ func TestValidateJob(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		} else {
 			s := strings.Split(k, ":")
-			err := errs[0].(*validation.Error)
+			err := errs[0]
 			if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
-				t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
+				t.Errorf("unexpected error: %v, expected: %s", err, k)
 			}
 		}
 	}
@@ -1019,9 +1018,9 @@ func TestValidateIngress(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		} else {
 			s := strings.Split(k, ":")
-			err := errs[0].(*validation.Error)
+			err := errs[0]
 			if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
-				t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
+				t.Errorf("unexpected error: %v, expected: %s", err, k)
 			}
 		}
 	}
@@ -1111,9 +1110,9 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
 			t.Errorf("expected failure for %s", k)
 		} else {
 			s := strings.Split(k, ":")
-			err := errs[0].(*validation.Error)
+			err := errs[0]
 			if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
-				t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
+				t.Errorf("unexpected error: %v, expected: %s", err, k)
 			}
 		}
 	}

pkg/kubectl/cmd/log.go

@@ -32,7 +32,6 @@ import (
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/kubectl/resource"
 	"k8s.io/kubernetes/pkg/runtime"
-	kerrors "k8s.io/kubernetes/pkg/util/errors"
 )
 
 const (
@@ -169,7 +168,7 @@ func (o LogsOptions) Validate() error {
 		return errors.New("unexpected log options object")
 	}
 	if errs := validation.ValidatePodLogOptions(logOptions); len(errs) > 0 {
-		return kerrors.NewAggregate(errs)
+		return errs.ToAggregate()
 	}
 
 	return nil

pkg/kubectl/cmd/util/helpers_test.go

@@ -274,11 +274,11 @@ func TestCheckInvalidErr(t *testing.T) {
 		expected string
 	}{
 		{
-			errors.NewInvalid("Invalid1", "invalidation", validation.ErrorList{validation.NewFieldInvalid("Cause", "single", "details")}),
+			errors.NewInvalid("Invalid1", "invalidation", validation.ErrorList{validation.NewInvalidError("Cause", "single", "details")}),
 			`Error from server: Invalid1 "invalidation" is invalid: Cause: invalid value 'single', Details: details`,
 		},
 		{
-			errors.NewInvalid("Invalid2", "invalidation", validation.ErrorList{validation.NewFieldInvalid("Cause", "multi1", "details"), validation.NewFieldInvalid("Cause", "multi2", "details")}),
+			errors.NewInvalid("Invalid2", "invalidation", validation.ErrorList{validation.NewInvalidError("Cause", "multi1", "details"), validation.NewInvalidError("Cause", "multi2", "details")}),
 			`Error from server: Invalid2 "invalidation" is invalid: [Cause: invalid value 'multi1', Details: details, Cause: invalid value 'multi2', Details: details]`,
 		},
 		{

pkg/kubelet/config/config.go

@@ -29,7 +29,6 @@ import (
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
 	kubeletutil "k8s.io/kubernetes/pkg/kubelet/util"
 	"k8s.io/kubernetes/pkg/util/config"
-	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 	"k8s.io/kubernetes/pkg/util/sets"
 	utilvalidation "k8s.io/kubernetes/pkg/util/validation"
 )
@@ -310,7 +309,7 @@ func (s *podStorage) seenSources(sources ...string) bool {
 func filterInvalidPods(pods []*api.Pod, source string, recorder record.EventRecorder) (filtered []*api.Pod) {
 	names := sets.String{}
 	for i, pod := range pods {
-		var errlist []error
+		var errlist utilvalidation.ErrorList
 		if errs := validation.ValidatePod(pod); len(errs) != 0 {
 			errlist = append(errlist, errs...)
 			// If validation fails, don't trust it any further -
@@ -318,14 +317,14 @@ func filterInvalidPods(pods []*api.Pod, source string, recorder record.EventReco
 		} else {
 			name := kubecontainer.GetPodFullName(pod)
 			if names.Has(name) {
-				errlist = append(errlist, utilvalidation.NewFieldDuplicate("name", pod.Name))
+				errlist = append(errlist, utilvalidation.NewDuplicateError("name", pod.Name))
 			} else {
 				names.Insert(name)
 			}
 		}
 		if len(errlist) > 0 {
 			name := bestPodIdentString(pod)
-			err := utilerrors.NewAggregate(errlist)
+			err := errlist.ToAggregate()
 			glog.Warningf("Pod[%d] (%s) from %s failed validation, ignoring: %v", i+1, name, source, err)
 			recorder.Eventf(pod, api.EventTypeWarning, kubecontainer.FailedValidation, "Error validating pod %s from %s, ignoring: %v", name, source, err)
 			continue

pkg/kubelet/config/http_test.go

@@ -30,7 +30,6 @@ import (
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
 	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util"
-	utilerrors "k8s.io/kubernetes/pkg/util/errors"
 )
 
 func TestURLErrorNotExistNoUpdate(t *testing.T) {
@@ -286,7 +285,7 @@ func TestExtractPodsFromHTTP(t *testing.T) {
 		}
 		for _, pod := range update.Pods {
 			if errs := validation.ValidatePod(pod); len(errs) != 0 {
-				t.Errorf("%s: Expected no validation errors on %#v, Got %v", testCase.desc, pod, utilerrors.NewAggregate(errs))
+				t.Errorf("%s: Expected no validation errors on %#v, Got %v", testCase.desc, pod, errs.ToAggregate())
 			}
 		}
 	}

pkg/labels/selector.go

@@ -701,14 +701,14 @@ const qualifiedNameErrorMsg string = "must match regex [" + validation.DNS1123Su
 
 func validateLabelKey(k string) error {
 	if !validation.IsQualifiedName(k) {
-		return validation.NewFieldInvalid("label key", k, qualifiedNameErrorMsg)
+		return validation.NewInvalidError("label key", k, qualifiedNameErrorMsg)
 	}
 	return nil
 }
 
 func validateLabelValue(v string) error {
 	if !validation.IsValidLabelValue(v) {
-		return validation.NewFieldInvalid("label value", v, qualifiedNameErrorMsg)
+		return validation.NewInvalidError("label value", v, qualifiedNameErrorMsg)
 	}
 	return nil
 }

pkg/registry/pod/etcd/etcd.go

@@ -129,10 +129,10 @@ func (r *BindingREST) Create(ctx api.Context, obj runtime.Object) (out runtime.O
 	binding := obj.(*api.Binding)
 	// TODO: move me to a binding strategy
 	if len(binding.Target.Kind) != 0 && binding.Target.Kind != "Node" {
-		return nil, errors.NewInvalid("binding", binding.Name, validation.ErrorList{validation.NewFieldInvalid("to.kind", binding.Target.Kind, "must be empty or 'Node'")})
+		return nil, errors.NewInvalid("binding", binding.Name, validation.ErrorList{validation.NewInvalidError("to.kind", binding.Target.Kind, "must be empty or 'Node'")})
 	}
 	if len(binding.Target.Name) == 0 {
-		return nil, errors.NewInvalid("binding", binding.Name, validation.ErrorList{validation.NewFieldRequired("to.name")})
+		return nil, errors.NewInvalid("binding", binding.Name, validation.ErrorList{validation.NewRequiredError("to.name")})
 	}
 	err = r.assignPod(ctx, binding.Name, binding.Target.Name, binding.Annotations)
 	out = &unversioned.Status{Status: unversioned.StatusSuccess}

pkg/registry/service/rest.go

@@ -84,7 +84,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err
 		// Allocate next available.
 		ip, err := rs.serviceIPs.AllocateNext()
 		if err != nil {
-			el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("spec.clusterIP", service.Spec.ClusterIP, err.Error())}
+			el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("spec.clusterIP", service.Spec.ClusterIP, err.Error())}
 			return nil, errors.NewInvalid("Service", service.Name, el)
 		}
 		service.Spec.ClusterIP = ip.String()
@@ -92,7 +92,7 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err
 	} else if api.IsServiceIPSet(service) {
 		// Try to respect the requested IP.
 		if err := rs.serviceIPs.Allocate(net.ParseIP(service.Spec.ClusterIP)); err != nil {
-			el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("spec.clusterIP", service.Spec.ClusterIP, err.Error())}
+			el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("spec.clusterIP", service.Spec.ClusterIP, err.Error())}
 			return nil, errors.NewInvalid("Service", service.Name, el)
 		}
 		releaseServiceIP = true
@@ -104,13 +104,13 @@ func (rs *REST) Create(ctx api.Context, obj runtime.Object) (runtime.Object, err
 		if servicePort.NodePort != 0 {
 			err := nodePortOp.Allocate(servicePort.NodePort)
 			if err != nil {
-				el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("nodePort", servicePort.NodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
+				el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("nodePort", servicePort.NodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
 				return nil, errors.NewInvalid("Service", service.Name, el)
 			}
 		} else if assignNodePorts {
 			nodePort, err := nodePortOp.AllocateNext()
 			if err != nil {
-				el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("nodePort", servicePort.NodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
+				el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("nodePort", servicePort.NodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
 				return nil, errors.NewInvalid("Service", service.Name, el)
 			}
 			servicePort.NodePort = nodePort
@@ -223,14 +223,14 @@ func (rs *REST) Update(ctx api.Context, obj runtime.Object) (runtime.Object, boo
 				if !contains(oldNodePorts, nodePort) {
 					err := nodePortOp.Allocate(nodePort)
 					if err != nil {
-						el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("nodePort", nodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
+						el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("nodePort", nodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
 						return nil, false, errors.NewInvalid("Service", service.Name, el)
 					}
 				}
 			} else {
 				nodePort, err = nodePortOp.AllocateNext()
 				if err != nil {
-					el := utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("nodePort", nodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
+					el := utilvalidation.ErrorList{utilvalidation.NewInvalidError("nodePort", nodePort, err.Error())}.PrefixIndex(i).Prefix("spec.ports")
 					return nil, false, errors.NewInvalid("Service", service.Name, el)
 				}
 				servicePort.NodePort = nodePort

pkg/storage/util.go

@@ -48,7 +48,7 @@ func ParseWatchResourceVersion(resourceVersion, kind string) (uint64, error) {
 	version, err := strconv.ParseUint(resourceVersion, 10, 64)
 	if err != nil {
 		// TODO: Does this need to be a ErrorList?  I can't convince myself it does.
-		return 0, errors.NewInvalid(kind, "", utilvalidation.ErrorList{utilvalidation.NewFieldInvalid("resourceVersion", resourceVersion, err.Error())})
+		return 0, errors.NewInvalid(kind, "", utilvalidation.ErrorList{utilvalidation.NewInvalidError("resourceVersion", resourceVersion, err.Error())})
 	}
 	return version + 1, nil
 }

pkg/util/validation/errors.go

@@ -46,7 +46,7 @@ func (v *Error) Error() string {
 func (v *Error) ErrorBody() string {
 	var s string
 	switch v.Type {
-	case ErrorTypeRequired, ErrorTypeTooLong:
+	case ErrorTypeRequired, ErrorTypeTooLong, ErrorTypeInternal:
 		s = spew.Sprintf("%s", v.Type)
 	default:
 		s = spew.Sprintf("%s '%+v'", v.Type, v.BadValue)
@@ -65,30 +65,33 @@ type ErrorType string
 // TODO: These values are duplicated in api/types.go, but there's a circular dep.  Fix it.
 const (
 	// ErrorTypeNotFound is used to report failure to find a requested value
-	// (e.g. looking up an ID).  See NewFieldNotFound.
+	// (e.g. looking up an ID).  See NewNotFoundError.
 	ErrorTypeNotFound ErrorType = "FieldValueNotFound"
 	// ErrorTypeRequired is used to report required values that are not
 	// provided (e.g. empty strings, null values, or empty arrays).  See
-	// NewFieldRequired.
+	// NewRequiredError.
 	ErrorTypeRequired ErrorType = "FieldValueRequired"
 	// ErrorTypeDuplicate is used to report collisions of values that must be
-	// unique (e.g. unique IDs).  See NewFieldDuplicate.
+	// unique (e.g. unique IDs).  See NewDuplicateError.
 	ErrorTypeDuplicate ErrorType = "FieldValueDuplicate"
 	// ErrorTypeInvalid is used to report malformed values (e.g. failed regex
-	// match, too long, out of bounds).  See NewFieldInvalid.
+	// match, too long, out of bounds).  See NewInvalidError.
 	ErrorTypeInvalid ErrorType = "FieldValueInvalid"
 	// ErrorTypeNotSupported is used to report unknown values for enumerated
-	// fields (e.g. a list of valid values).  See NewFieldNotSupported.
+	// fields (e.g. a list of valid values).  See NewNotSupportedError.
 	ErrorTypeNotSupported ErrorType = "FieldValueNotSupported"
 	// ErrorTypeForbidden is used to report valid (as per formatting rules)
 	// values which would be accepted under some conditions, but which are not
 	// permitted by the current conditions (such as security policy).  See
-	// NewFieldForbidden.
+	// NewForbiddenError.
 	ErrorTypeForbidden ErrorType = "FieldValueForbidden"
 	// ErrorTypeTooLong is used to report that the given value is too long.
 	// This is similar to ErrorTypeInvalid, but the error will not include the
-	// too-long value.  See NewFieldTooLong.
+	// too-long value.  See NewTooLongError.
 	ErrorTypeTooLong ErrorType = "FieldValueTooLong"
+	// ErrorTypeInternal is used to report other errors that are not related
+	// to user input.
+	ErrorTypeInternal ErrorType = "InternalError"
 )
 
 // String converts a ErrorType into its corresponding canonical error message.
@@ -108,41 +111,43 @@ func (t ErrorType) String() string {
 		return "forbidden"
 	case ErrorTypeTooLong:
 		return "too long"
+	case ErrorTypeInternal:
+		return "internal error"
 	default:
 		panic(fmt.Sprintf("unrecognized validation error: %q", t))
 		return ""
 	}
 }
 
-// NewFieldNotFound returns a *Error indicating "value not found".  This is
+// NewNotFoundError returns a *Error indicating "value not found".  This is
 // used to report failure to find a requested value (e.g. looking up an ID).
-func NewFieldNotFound(field string, value interface{}) *Error {
+func NewNotFoundError(field string, value interface{}) *Error {
 	return &Error{ErrorTypeNotFound, field, value, ""}
 }
 
-// NewFieldRequired returns a *Error indicating "value required".  This is used
+// NewRequiredError returns a *Error indicating "value required".  This is used
 // to report required values that are not provided (e.g. empty strings, null
 // values, or empty arrays).
-func NewFieldRequired(field string) *Error {
+func NewRequiredError(field string) *Error {
 	return &Error{ErrorTypeRequired, field, "", ""}
 }
 
-// NewFieldDuplicate returns a *Error indicating "duplicate value".  This is
+// NewDuplicateError returns a *Error indicating "duplicate value".  This is
 // used to report collisions of values that must be unique (e.g. names or IDs).
-func NewFieldDuplicate(field string, value interface{}) *Error {
+func NewDuplicateError(field string, value interface{}) *Error {
 	return &Error{ErrorTypeDuplicate, field, value, ""}
 }
 
-// NewFieldInvalid returns a *Error indicating "invalid value".  This is used
+// NewInvalidError returns a *Error indicating "invalid value".  This is used
 // to report malformed values (e.g. failed regex match, too long, out of bounds).
-func NewFieldInvalid(field string, value interface{}, detail string) *Error {
+func NewInvalidError(field string, value interface{}, detail string) *Error {
 	return &Error{ErrorTypeInvalid, field, value, detail}
 }
 
-// NewFieldNotSupported returns a *Error indicating "unsupported value".
+// NewNotSupportedError returns a *Error indicating "unsupported value".
 // This is used to report unknown values for enumerated fields (e.g. a list of
 // valid values).
-func NewFieldNotSupported(field string, value interface{}, validValues []string) *Error {
+func NewNotSupportedError(field string, value interface{}, validValues []string) *Error {
 	detail := ""
 	if validValues != nil && len(validValues) > 0 {
 		detail = "supported values: " + strings.Join(validValues, ", ")
@@ -150,40 +155,43 @@ func NewFieldNotSupported(field string, value interface{}, validValues []string)
 	return &Error{ErrorTypeNotSupported, field, value, detail}
 }
 
-// NewFieldForbidden returns a *Error indicating "forbidden".  This is used to
+// NewForbiddenError returns a *Error indicating "forbidden".  This is used to
 // report valid (as per formatting rules) values which would be accepted under
 // some conditions, but which are not permitted by current conditions (e.g.
 // security policy).
-func NewFieldForbidden(field string, value interface{}) *Error {
+func NewForbiddenError(field string, value interface{}) *Error {
 	return &Error{ErrorTypeForbidden, field, value, ""}
 }
 
-// NewFieldTooLong returns a *Error indicating "too long".  This is used to
+// NewTooLongError returns a *Error indicating "too long".  This is used to
 // report that the given value is too long.  This is similar to
-// NewFieldInvalid, but the returned error will not include the too-long
+// NewInvalidError, but the returned error will not include the too-long
 // value.
-func NewFieldTooLong(field string, value interface{}, maxLength int) *Error {
+func NewTooLongError(field string, value interface{}, maxLength int) *Error {
 	return &Error{ErrorTypeTooLong, field, value, fmt.Sprintf("must have at most %d characters", maxLength)}
 }
 
+// NewInternalError returns a *Error indicating "internal error".  This is used
+// to signal that an error was found that was not directly related to user
+// input.  The err argument must be non-nil.
+func NewInternalError(field string, err error) *Error {
+	return &Error{ErrorTypeInternal, field, nil, err.Error()}
+}
+
 // ErrorList holds a set of errors.
-type ErrorList []error
+type ErrorList []*Error
 
 // Prefix adds a prefix to the Field of every Error in the list.
 // Returns the list for convenience.
 func (list ErrorList) Prefix(prefix string) ErrorList {
 	for i := range list {
-		if err, ok := list[i].(*Error); ok {
-			if strings.HasPrefix(err.Field, "[") {
-				err.Field = prefix + err.Field
-			} else if len(err.Field) != 0 {
-				err.Field = prefix + "." + err.Field
-			} else {
-				err.Field = prefix
-			}
-			list[i] = err
+		err := list[i]
+		if strings.HasPrefix(err.Field, "[") {
+			err.Field = prefix + err.Field
+		} else if len(err.Field) != 0 {
+			err.Field = prefix + "." + err.Field
 		} else {
-			panic(fmt.Sprintf("Programmer error: ErrorList holds non-Error: %#v", list[i]))
+			err.Field = prefix
 		}
 	}
 	return list
@@ -206,13 +214,30 @@ func NewErrorTypeMatcher(t ErrorType) utilerrors.Matcher {
 	}
 }
 
+// ToAggregate converts the ErrorList into an errors.Aggregate.
+func (list ErrorList) ToAggregate() utilerrors.Aggregate {
+	errs := make([]error, len(list))
+	for i := range list {
+		errs[i] = list[i]
+	}
+	return utilerrors.NewAggregate(errs)
+}
+
+func fromAggregate(agg utilerrors.Aggregate) ErrorList {
+	errs := agg.Errors()
+	list := make(ErrorList, len(errs))
+	for i := range errs {
+		list[i] = errs[i].(*Error)
+	}
+	return list
+}
+
 // Filter removes items from the ErrorList that match the provided fns.
 func (list ErrorList) Filter(fns ...utilerrors.Matcher) ErrorList {
-	err := utilerrors.FilterOut(utilerrors.NewAggregate(list), fns...)
+	err := utilerrors.FilterOut(list.ToAggregate(), fns...)
 	if err == nil {
 		return nil
 	}
-	// FilterOut that takes an Aggregate returns an Aggregate
-	agg := err.(utilerrors.Aggregate)
-	return ErrorList(agg.Errors())
+	// FilterOut takes an Aggregate and returns an Aggregate
+	return fromAggregate(err.(utilerrors.Aggregate))
 }

pkg/util/validation/errors_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package validation
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 )
@@ -27,25 +28,29 @@ func TestMakeFuncs(t *testing.T) {
 		expected ErrorType
 	}{
 		{
-			func() *Error { return NewFieldInvalid("f", "v", "d") },
+			func() *Error { return NewInvalidError("f", "v", "d") },
 			ErrorTypeInvalid,
 		},
 		{
-			func() *Error { return NewFieldNotSupported("f", "v", nil) },
+			func() *Error { return NewNotSupportedError("f", "v", nil) },
 			ErrorTypeNotSupported,
 		},
 		{
-			func() *Error { return NewFieldDuplicate("f", "v") },
+			func() *Error { return NewDuplicateError("f", "v") },
 			ErrorTypeDuplicate,
 		},
 		{
-			func() *Error { return NewFieldNotFound("f", "v") },
+			func() *Error { return NewNotFoundError("f", "v") },
 			ErrorTypeNotFound,
 		},
 		{
-			func() *Error { return NewFieldRequired("f") },
+			func() *Error { return NewRequiredError("f") },
 			ErrorTypeRequired,
 		},
+		{
+			func() *Error { return NewInternalError("f", fmt.Errorf("e")) },
+			ErrorTypeInternal,
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -57,7 +62,7 @@ func TestMakeFuncs(t *testing.T) {
 }
 
 func TestErrorUsefulMessage(t *testing.T) {
-	s := NewFieldInvalid("foo", "bar", "deet").Error()
+	s := NewInvalidError("foo", "bar", "deet").Error()
 	t.Logf("message: %v", s)
 	for _, part := range []string{"foo", "bar", "deet", ErrorTypeInvalid.String()} {
 		if !strings.Contains(s, part) {
@@ -71,7 +76,7 @@ func TestErrorUsefulMessage(t *testing.T) {
 		Inner interface{}
 		KV    map[string]int
 	}
-	s = NewFieldInvalid(
+	s = NewInvalidError(
 		"foo",
 		&complicated{
 			Baz:   1,
@@ -93,11 +98,32 @@ func TestErrorUsefulMessage(t *testing.T) {
 	}
 }
 
+func TestToAggregate(t *testing.T) {
+	testCases := []ErrorList{
+		nil,
+		{},
+		{NewInvalidError("f", "v", "d")},
+		{NewInvalidError("f", "v", "d"), NewInternalError("", fmt.Errorf("e"))},
+	}
+	for i, tc := range testCases {
+		agg := tc.ToAggregate()
+		if len(tc) == 0 {
+			if agg != nil {
+				t.Errorf("[%d] Expected nil, got %#v", i, agg)
+			}
+		} else if agg == nil {
+			t.Errorf("[%d] Expected non-nil", i)
+		} else if len(tc) != len(agg.Errors()) {
+			t.Errorf("[%d] Expected %d, got %d", i, len(tc), len(agg.Errors()))
+		}
+	}
+}
+
 func TestErrListFilter(t *testing.T) {
 	list := ErrorList{
-		NewFieldInvalid("test.field", "", ""),
-		NewFieldInvalid("field.test", "", ""),
-		NewFieldDuplicate("test", "value"),
+		NewInvalidError("test.field", "", ""),
+		NewInvalidError("field.test", "", ""),
+		NewDuplicateError("test", "value"),
 	}
 	if len(list.Filter(NewErrorTypeMatcher(ErrorTypeDuplicate))) != 2 {
 		t.Errorf("should not filter")
@@ -113,15 +139,15 @@ func TestErrListPrefix(t *testing.T) {
 		Expected string
 	}{
 		{
-			NewFieldNotFound("[0].bar", "value"),
+			NewNotFoundError("[0].bar", "value"),
 			"foo[0].bar",
 		},
 		{
-			NewFieldInvalid("field", "value", ""),
+			NewInvalidError("field", "value", ""),
 			"foo.field",
 		},
 		{
-			NewFieldDuplicate("", "value"),
+			NewDuplicateError("", "value"),
 			"foo",
 		},
 	}
@@ -131,7 +157,7 @@ func TestErrListPrefix(t *testing.T) {
 		if prefix == nil || len(prefix) != len(errList) {
 			t.Errorf("Prefix should return self")
 		}
-		if e, a := testCase.Expected, errList[0].(*Error).Field; e != a {
+		if e, a := testCase.Expected, errList[0].Field; e != a {
 			t.Errorf("expected %s, got %s", e, a)
 		}
 	}
@@ -143,15 +169,15 @@ func TestErrListPrefixIndex(t *testing.T) {
 		Expected string
 	}{
 		{
-			NewFieldNotFound("[0].bar", "value"),
+			NewNotFoundError("[0].bar", "value"),
 			"[1][0].bar",
 		},
 		{
-			NewFieldInvalid("field", "value", ""),
+			NewInvalidError("field", "value", ""),
 			"[1].field",
 		},
 		{
-			NewFieldDuplicate("", "value"),
+			NewDuplicateError("", "value"),
 			"[1]",
 		},
 	}
@@ -161,7 +187,7 @@ func TestErrListPrefixIndex(t *testing.T) {
 		if prefix == nil || len(prefix) != len(errList) {
 			t.Errorf("PrefixIndex should return self")
 		}
-		if e, a := testCase.Expected, errList[0].(*Error).Field; e != a {
+		if e, a := testCase.Expected, errList[0].Field; e != a {
 			t.Errorf("expected %s, got %s", e, a)
 		}
 	}