diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go index d1172d03fa9..6c8d3980c06 100644 --- a/pkg/api/errors/errors.go +++ b/pkg/api/errors/errors.go @@ -30,6 +30,8 @@ type StatusError struct { ErrStatus api.Status } +var _ error = &statusError{} + // Error implements the Error interface. func (e *StatusError) Error() string { return e.ErrStatus.Message @@ -107,7 +109,7 @@ func NewConflict(kind, name string, err error) error { func NewInvalid(kind, name string, errs ValidationErrorList) error { causes := make([]api.StatusCause, 0, len(errs)) for i := range errs { - if err, ok := errs[i].(ValidationError); ok { + if err, ok := errs[i].(*ValidationError); ok { causes = append(causes, api.StatusCause{ Type: api.CauseType(err.Type), Message: err.Error(), @@ -130,18 +132,16 @@ func NewInvalid(kind, name string, errs ValidationErrorList) error { // NewBadRequest creates an error that indicates that the request is invalid and can not be processed. func NewBadRequest(reason string) error { - return &StatusError{ - api.Status{ - Status: api.StatusFailure, - Code: http.StatusBadRequest, - Reason: api.StatusReasonBadRequest, - Details: &api.StatusDetails{ - Causes: []api.StatusCause{ - {Message: reason}, - }, + return &StatusError{api.Status{ + Status: api.StatusFailure, + Code: http.StatusBadRequest, + Reason: api.StatusReasonBadRequest, + Details: &api.StatusDetails{ + Causes: []api.StatusCause{ + {Message: reason}, }, }, - } + }} } // NewInternalError returns an error indicating the item is invalid and cannot be processed. diff --git a/pkg/api/errors/errors_test.go b/pkg/api/errors/errors_test.go index 1e56ef94acc..c1def08febe 100644 --- a/pkg/api/errors/errors_test.go +++ b/pkg/api/errors/errors_test.go @@ -54,7 +54,7 @@ func TestErrorNew(t *testing.T) { func TestNewInvalid(t *testing.T) { testCases := []struct { - Err ValidationError + Err *ValidationError Details *api.StatusDetails }{ { diff --git a/pkg/api/errors/validation.go b/pkg/api/errors/validation.go index a9d711466a9..2ec06ceecf9 100644 --- a/pkg/api/errors/validation.go +++ b/pkg/api/errors/validation.go @@ -80,38 +80,40 @@ type ValidationError struct { BadValue interface{} } -func (v ValidationError) Error() string { +var _ error = &ValidationError{} + +func (v *ValidationError) Error() string { return fmt.Sprintf("%s: %v '%v'", v.Field, ValueOf(v.Type), v.BadValue) } -// NewFieldRequired returns a ValidationError indicating "value required" -func NewFieldRequired(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeRequired, field, value} +// NewFieldRequired returns a *ValidationError indicating "value required" +func NewFieldRequired(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeRequired, field, value} } -// NewFieldInvalid returns a ValidationError indicating "invalid value" -func NewFieldInvalid(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeInvalid, field, value} +// NewFieldInvalid returns a *ValidationError indicating "invalid value" +func NewFieldInvalid(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeInvalid, field, value} } -// NewFieldNotSupported returns a ValidationError indicating "unsupported value" -func NewFieldNotSupported(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeNotSupported, field, value} +// NewFieldNotSupported returns a *ValidationError indicating "unsupported value" +func NewFieldNotSupported(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeNotSupported, field, value} } -// NewFieldForbidden returns a ValidationError indicating "forbidden" -func NewFieldForbidden(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeForbidden, field, value} +// NewFieldForbidden returns a *ValidationError indicating "forbidden" +func NewFieldForbidden(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeForbidden, field, value} } -// NewFieldDuplicate returns a ValidationError indicating "duplicate value" -func NewFieldDuplicate(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeDuplicate, field, value} +// NewFieldDuplicate returns a *ValidationError indicating "duplicate value" +func NewFieldDuplicate(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeDuplicate, field, value} } -// NewFieldNotFound returns a ValidationError indicating "value not found" -func NewFieldNotFound(field string, value interface{}) ValidationError { - return ValidationError{ValidationErrorTypeNotFound, field, value} +// NewFieldNotFound returns a *ValidationError indicating "value not found" +func NewFieldNotFound(field string, value interface{}) *ValidationError { + return &ValidationError{ValidationErrorTypeNotFound, field, value} } // ValidationErrorList is a collection of ValidationErrors. This does not @@ -131,7 +133,7 @@ func (list ValidationErrorList) ToError() error { // Returns the list for convenience. func (list ValidationErrorList) Prefix(prefix string) ValidationErrorList { for i := range list { - if err, ok := list[i].(ValidationError); ok { + if err, ok := list[i].(*ValidationError); ok { if strings.HasPrefix(err.Field, "[") { err.Field = prefix + err.Field } else if len(err.Field) != 0 { diff --git a/pkg/api/errors/validation_test.go b/pkg/api/errors/validation_test.go index 0ed29359a30..4044f2b4eb0 100644 --- a/pkg/api/errors/validation_test.go +++ b/pkg/api/errors/validation_test.go @@ -23,27 +23,27 @@ import ( func TestMakeFuncs(t *testing.T) { testCases := []struct { - fn func() ValidationError + fn func() *ValidationError expected ValidationErrorType }{ { - func() ValidationError { return NewFieldInvalid("f", "v") }, + func() *ValidationError { return NewFieldInvalid("f", "v") }, ValidationErrorTypeInvalid, }, { - func() ValidationError { return NewFieldNotSupported("f", "v") }, + func() *ValidationError { return NewFieldNotSupported("f", "v") }, ValidationErrorTypeNotSupported, }, { - func() ValidationError { return NewFieldDuplicate("f", "v") }, + func() *ValidationError { return NewFieldDuplicate("f", "v") }, ValidationErrorTypeDuplicate, }, { - func() ValidationError { return NewFieldNotFound("f", "v") }, + func() *ValidationError { return NewFieldNotFound("f", "v") }, ValidationErrorTypeNotFound, }, { - func() ValidationError { return NewFieldRequired("f", "v") }, + func() *ValidationError { return NewFieldRequired("f", "v") }, ValidationErrorTypeRequired, }, } @@ -65,7 +65,7 @@ func TestValidationError(t *testing.T) { func TestErrListPrefix(t *testing.T) { testCases := []struct { - Err ValidationError + Err *ValidationError Expected string }{ { @@ -87,7 +87,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].(ValidationError).Field; e != a { + if e, a := testCase.Expected, errList[0].(*ValidationError).Field; e != a { t.Errorf("expected %s, got %s", e, a) } } @@ -95,7 +95,7 @@ func TestErrListPrefix(t *testing.T) { func TestErrListPrefixIndex(t *testing.T) { testCases := []struct { - Err ValidationError + Err *ValidationError Expected string }{ { @@ -117,7 +117,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].(ValidationError).Field; e != a { + if e, a := testCase.Expected, errList[0].(*ValidationError).Field; e != a { t.Errorf("expected %s, got %s", e, a) } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index bedc22d226d..b2a3ee107d3 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -29,7 +29,7 @@ import ( func expectPrefix(t *testing.T, prefix string, errs errors.ValidationErrorList) { for i := range errs { - if f, p := errs[i].(errors.ValidationError).Field, prefix; !strings.HasPrefix(f, p) { + if f, p := errs[i].(*errors.ValidationError).Field, prefix; !strings.HasPrefix(f, p) { t.Errorf("expected prefix '%s' for field '%s' (%v)", p, f, errs[i]) } } @@ -69,10 +69,10 @@ func TestValidateVolumes(t *testing.T) { continue } for i := range errs { - if errs[i].(errors.ValidationError).Type != v.T { + if errs[i].(*errors.ValidationError).Type != v.T { t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i]) } - if errs[i].(errors.ValidationError).Field != v.F { + if errs[i].(*errors.ValidationError).Field != v.F { t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i]) } } @@ -125,10 +125,10 @@ func TestValidatePorts(t *testing.T) { t.Errorf("expected failure for %s", k) } for i := range errs { - if errs[i].(errors.ValidationError).Type != v.T { + if errs[i].(*errors.ValidationError).Type != v.T { t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i]) } - if errs[i].(errors.ValidationError).Field != v.F { + if errs[i].(*errors.ValidationError).Field != v.F { t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i]) } } @@ -995,7 +995,7 @@ func TestValidateReplicationController(t *testing.T) { t.Errorf("expected failure for %s", k) } for i := range errs { - field := errs[i].(errors.ValidationError).Field + field := errs[i].(*errors.ValidationError).Field if !strings.HasPrefix(field, "spec.template.") && field != "name" && field != "namespace" && @@ -1078,7 +1078,7 @@ func TestValidateMinion(t *testing.T) { t.Errorf("expected failure for %s", k) } for i := range errs { - field := errs[i].(errors.ValidationError).Field + field := errs[i].(*errors.ValidationError).Field if field != "name" && field != "label" { t.Errorf("%s: missing prefix for: %v", k, errs[i]) diff --git a/pkg/config/config.go b/pkg/config/config.go index a74f2a48b6a..a215e805a5a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -52,7 +52,7 @@ func CreateObjects(typer runtime.ObjectTyper, mapper meta.RESTMapper, clientFor } if err := CreateObject(client, mapping, obj); err != nil { - reportError(&allErrors, i, *err) + reportError(&allErrors, i, err) } } @@ -65,21 +65,18 @@ func CreateObjects(typer runtime.ObjectTyper, mapper meta.RESTMapper, clientFor func CreateObject(client *client.RESTClient, mapping *meta.RESTMapping, obj runtime.Object) *errs.ValidationError { name, err := mapping.MetadataAccessor.Name(obj) if err != nil || name == "" { - e := errs.NewFieldRequired("name", err) - return &e + return errs.NewFieldRequired("name", err) } namespace, err := mapping.Namespace(obj) if err != nil { - e := errs.NewFieldRequired("namespace", err) - return &e + return errs.NewFieldRequired("namespace", err) } // TODO: This should be using RESTHelper err = client.Post().Path(mapping.Resource).Namespace(namespace).Body(obj).Do().Error() if err != nil { - e := errs.NewFieldInvalid(name, err) - return &e + return errs.NewFieldInvalid(name, err) } return nil @@ -87,7 +84,7 @@ func CreateObject(client *client.RESTClient, mapping *meta.RESTMapping, obj runt // reportError reports the single item validation error and properly set the // prefix and index to match the Config item JSON index -func reportError(allErrs *errs.ValidationErrorList, index int, err errs.ValidationError) { +func reportError(allErrs *errs.ValidationErrorList, index int, err *errs.ValidationError) { i := errs.ValidationErrorList{} *allErrs = append(*allErrs, append(i, err).PrefixIndex(index).Prefix("item")...) } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 0c318f4e212..5f140be0553 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -92,7 +92,7 @@ func TestCreateNoNameItem(t *testing.T) { t.Errorf("Expected required value error for missing name") } - e := errs[0].(errors.ValidationError) + e := errs[0].(*errors.ValidationError) if errors.ValueOf(e.Type) != "required value" { t.Errorf("Expected ValidationErrorTypeRequired error, got %#v", e) } @@ -121,7 +121,7 @@ func TestCreateInvalidItem(t *testing.T) { t.Errorf("Expected invalid value error for kind") } - e := errs[0].(errors.ValidationError) + e := errs[0].(*errors.ValidationError) if errors.ValueOf(e.Type) != "invalid value" { t.Errorf("Expected ValidationErrorTypeInvalid error, got %#v", e) } @@ -153,7 +153,7 @@ func TestCreateNoClientItems(t *testing.T) { t.Errorf("Expected invalid value error for client") } - e := errs[0].(errors.ValidationError) + e := errs[0].(*errors.ValidationError) if errors.ValueOf(e.Type) != "unsupported value" { t.Errorf("Expected ValidationErrorTypeUnsupported error, got %#v", e) }