Skip to content
This repository was archived by the owner on Feb 8, 2021. It is now read-only.

Commit 682f2a5

Browse files
thockinthockin
committed
Stronger typing for validation ErrorList
1 parent c974e07 commit 682f2a5

File tree

12 files changed

+122
-78
lines changed

12 files changed

+122
-78
lines changed

‎examples/examples_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,13 @@ import (
3333
expvalidation "k8s.io/kubernetes/pkg/apis/extensions/validation"
3434
"k8s.io/kubernetes/pkg/capabilities"
3535
"k8s.io/kubernetes/pkg/runtime"
36+
utilvalidation "k8s.io/kubernetes/pkg/util/validation"
3637
"k8s.io/kubernetes/pkg/util/yaml"
3738
schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api"
3839
schedulerapilatest "k8s.io/kubernetes/plugin/pkg/scheduler/api/latest"
3940
)
4041

41-
func validateObject(obj runtime.Object) (errors []error) {
42+
func validateObject(obj runtime.Object) (errors utilvalidation.ErrorList) {
4243
switch t := obj.(type) {
4344
case *api.ReplicationController:
4445
if t.Namespace == "" {
@@ -122,7 +123,7 @@ func validateObject(obj runtime.Object) (errors []error) {
122123
}
123124
errors = expvalidation.ValidateDaemonSet(t)
124125
default:
125-
return []error{fmt.Errorf("no validation defined for %#v", obj)}
126+
return utilvalidation.ErrorList{utilvalidation.NewInternalError("", fmt.Errorf("no validation defined for %#v", obj))}
126127
}
127128
return errors
128129
}

‎pkg/api/errors/errors.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424

2525
"k8s.io/kubernetes/pkg/api/unversioned"
2626
"k8s.io/kubernetes/pkg/runtime"
27-
utilerrors "k8s.io/kubernetes/pkg/util/errors"
2827
"k8s.io/kubernetes/pkg/util/validation"
2928
)
3029

@@ -162,13 +161,12 @@ func NewConflict(kind, name string, err error) error {
162161
func NewInvalid(kind, name string, errs validation.ErrorList) error {
163162
causes := make([]unversioned.StatusCause, 0, len(errs))
164163
for i := range errs {
165-
if err, ok := errs[i].(*validation.Error); ok {
166-
causes = append(causes, unversioned.StatusCause{
167-
Type: unversioned.CauseType(err.Type),
168-
Message: err.ErrorBody(),
169-
Field: err.Field,
170-
})
171-
}
164+
err := errs[i]
165+
causes = append(causes, unversioned.StatusCause{
166+
Type: unversioned.CauseType(err.Type),
167+
Message: err.ErrorBody(),
168+
Field: err.Field,
169+
})
172170
}
173171
return &StatusError{unversioned.Status{
174172
Status: unversioned.StatusFailure,
@@ -179,7 +177,7 @@ func NewInvalid(kind, name string, errs validation.ErrorList) error {
179177
Name: name,
180178
Causes: causes,
181179
},
182-
Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, utilerrors.NewAggregate(errs)),
180+
Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, errs.ToAggregate()),
183181
}}
184182
}
185183

‎pkg/api/rest/update.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ func validateCommonFields(obj, old runtime.Object) utilvalidation.ErrorList {
5757
allErrs := utilvalidation.ErrorList{}
5858
objectMeta, err := api.ObjectMetaFor(obj)
5959
if err != nil {
60-
return append(allErrs, errors.NewInternalError(err))
60+
return append(allErrs, utilvalidation.NewInternalError("metadata", err))
6161
}
6262
oldObjectMeta, err := api.ObjectMetaFor(old)
6363
if err != nil {
64-
return append(allErrs, errors.NewInternalError(err))
64+
return append(allErrs, utilvalidation.NewInternalError("metadata", err))
6565
}
6666
allErrs = append(allErrs, validation.ValidateObjectMetaUpdate(objectMeta, oldObjectMeta)...)
6767

‎pkg/api/validation/schema.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/golang/glog"
2828
apiutil "k8s.io/kubernetes/pkg/api/util"
2929
utilerrors "k8s.io/kubernetes/pkg/util/errors"
30-
"k8s.io/kubernetes/pkg/util/validation"
3130
"k8s.io/kubernetes/pkg/util/yaml"
3231
)
3332

@@ -67,11 +66,11 @@ func NewSwaggerSchemaFromBytes(data []byte) (Schema, error) {
6766
return schema, nil
6867
}
6968

70-
// validateList unpack a list and validate every item in the list.
69+
// validateList unpacks a list and validate every item in the list.
7170
// It return nil if every item is ok.
7271
// Otherwise it return an error list contain errors of every item.
73-
func (s *SwaggerSchema) validateList(obj map[string]interface{}) validation.ErrorList {
74-
allErrs := validation.ErrorList{}
72+
func (s *SwaggerSchema) validateList(obj map[string]interface{}) []error {
73+
allErrs := []error{}
7574
items, exists := obj["items"]
7675
if !exists {
7776
return append(allErrs, fmt.Errorf("no items field in %#v", obj))
@@ -160,8 +159,8 @@ func (s *SwaggerSchema) ValidateBytes(data []byte) error {
160159
return utilerrors.NewAggregate(allErrs)
161160
}
162161

163-
func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName string) validation.ErrorList {
164-
allErrs := validation.ErrorList{}
162+
func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName string) []error {
163+
allErrs := []error{}
165164
models := s.api.Models
166165
model, ok := models.At(typeName)
167166
if !ok {
@@ -215,7 +214,7 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, fieldName, typeName stri
215214
// This matches type name in the swagger spec, such as "v1.Binding".
216215
var versionRegexp = regexp.MustCompile(`^v.+\..*`)
217216

218-
func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) validation.ErrorList {
217+
func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) []error {
219218
// TODO: caesarxuchao: because we have multiple group/versions and objects
220219
// may reference objects in other group, the commented out way of checking
221220
// 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
229228
// if strings.HasPrefix(fieldType, apiVersion) {
230229
return s.ValidateObject(value, fieldName, fieldType)
231230
}
232-
allErrs := validation.ErrorList{}
231+
allErrs := []error{}
233232
switch fieldType {
234233
case "string":
235234
// Be loose about what we accept for 'string' since we use IntOrString in a couple of places

‎pkg/api/validation/validation.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1480,7 +1480,7 @@ func ValidateNodeUpdate(node, oldNode *api.Node) validation.ErrorList {
14801480
addresses := make(map[api.NodeAddress]bool)
14811481
for _, address := range node.Status.Addresses {
14821482
if _, ok := addresses[address]; ok {
1483-
allErrs = append(allErrs, fmt.Errorf("duplicate node addresses found"))
1483+
allErrs = append(allErrs, validation.NewFieldDuplicate("addresses", address))
14841484
}
14851485
addresses[address] = true
14861486
}
@@ -1500,7 +1500,7 @@ func ValidateNodeUpdate(node, oldNode *api.Node) validation.ErrorList {
15001500
// TODO: Add a 'real' error type for this error and provide print actual diffs.
15011501
if !api.Semantic.DeepEqual(oldNode, node) {
15021502
glog.V(4).Infof("Update failed validation %#v vs %#v", oldNode, node)
1503-
allErrs = append(allErrs, fmt.Errorf("update contains more than labels or capacity changes"))
1503+
allErrs = append(allErrs, validation.NewFieldForbidden("", "update contains more than labels or capacity changes"))
15041504
}
15051505

15061506
return allErrs

‎pkg/api/validation/validation_test.go

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,14 @@ import (
2828
"k8s.io/kubernetes/pkg/api/testapi"
2929
"k8s.io/kubernetes/pkg/api/unversioned"
3030
"k8s.io/kubernetes/pkg/capabilities"
31-
utilerrors "k8s.io/kubernetes/pkg/util/errors"
3231
"k8s.io/kubernetes/pkg/util/intstr"
3332
"k8s.io/kubernetes/pkg/util/sets"
3433
"k8s.io/kubernetes/pkg/util/validation"
3534
)
3635

3736
func expectPrefix(t *testing.T, prefix string, errs validation.ErrorList) {
3837
for i := range errs {
39-
if f, p := errs[i].(*validation.Error).Field, prefix; !strings.HasPrefix(f, p) {
38+
if f, p := errs[i].Field, prefix; !strings.HasPrefix(f, p) {
4039
t.Errorf("expected prefix '%s' for field '%s' (%v)", p, f, errs[i])
4140
}
4241
}
@@ -150,7 +149,7 @@ func TestValidateLabels(t *testing.T) {
150149
if len(errs) != 1 {
151150
t.Errorf("case[%d] expected failure", i)
152151
} else {
153-
detail := errs[0].(*validation.Error).Detail
152+
detail := errs[0].Detail
154153
if detail != qualifiedNameErrorMsg {
155154
t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
156155
}
@@ -168,7 +167,7 @@ func TestValidateLabels(t *testing.T) {
168167
if len(errs) != 1 {
169168
t.Errorf("case[%d] expected failure", i)
170169
} else {
171-
detail := errs[0].(*validation.Error).Detail
170+
detail := errs[0].Detail
172171
if detail != labelValueErrorMsg {
173172
t.Errorf("error detail %s should be equal %s", detail, labelValueErrorMsg)
174173
}
@@ -215,7 +214,7 @@ func TestValidateAnnotations(t *testing.T) {
215214
if len(errs) != 1 {
216215
t.Errorf("case[%d] expected failure", i)
217216
}
218-
detail := errs[0].(*validation.Error).Detail
217+
detail := errs[0].Detail
219218
if detail != qualifiedNameErrorMsg {
220219
t.Errorf("error detail %s should be equal %s", detail, qualifiedNameErrorMsg)
221220
}
@@ -568,13 +567,13 @@ func TestValidateVolumes(t *testing.T) {
568567
continue
569568
}
570569
for i := range errs {
571-
if errs[i].(*validation.Error).Type != v.T {
570+
if errs[i].Type != v.T {
572571
t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i])
573572
}
574-
if errs[i].(*validation.Error).Field != v.F {
573+
if errs[i].Field != v.F {
575574
t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
576575
}
577-
detail := errs[i].(*validation.Error).Detail
576+
detail := errs[i].Detail
578577
if detail != v.D {
579578
t.Errorf("%s: expected error detail \"%s\", got \"%s\"", k, v.D, detail)
580579
}
@@ -627,13 +626,13 @@ func TestValidatePorts(t *testing.T) {
627626
t.Errorf("expected failure for %s", k)
628627
}
629628
for i := range errs {
630-
if errs[i].(*validation.Error).Type != v.T {
629+
if errs[i].Type != v.T {
631630
t.Errorf("%s: expected errors to have type %s: %v", k, v.T, errs[i])
632631
}
633-
if errs[i].(*validation.Error).Field != v.F {
632+
if errs[i].Field != v.F {
634633
t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
635634
}
636-
detail := errs[i].(*validation.Error).Detail
635+
detail := errs[i].Detail
637636
if detail != v.D {
638637
t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
639638
}
@@ -772,7 +771,7 @@ func TestValidateEnv(t *testing.T) {
772771
t.Errorf("expected failure for %s", tc.name)
773772
} else {
774773
for i := range errs {
775-
str := errs[i].(*validation.Error).Error()
774+
str := errs[i].Error()
776775
if str != "" && str != tc.expectedError {
777776
t.Errorf("%s: expected error detail either empty or %s, got %s", tc.name, tc.expectedError, str)
778777
}
@@ -2108,7 +2107,7 @@ func TestValidateService(t *testing.T) {
21082107
tc.tweakSvc(&svc)
21092108
errs := ValidateService(&svc)
21102109
if len(errs) != tc.numErrs {
2111-
t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs))
2110+
t.Errorf("Unexpected error list for case %q: %v", tc.name, errs.ToAggregate())
21122111
}
21132112
}
21142113
}
@@ -2560,7 +2559,7 @@ func TestValidateReplicationController(t *testing.T) {
25602559
t.Errorf("expected failure for %s", k)
25612560
}
25622561
for i := range errs {
2563-
field := errs[i].(*validation.Error).Field
2562+
field := errs[i].Field
25642563
if !strings.HasPrefix(field, "spec.template.") &&
25652564
field != "metadata.name" &&
25662565
field != "metadata.namespace" &&
@@ -2676,7 +2675,7 @@ func TestValidateNode(t *testing.T) {
26762675
t.Errorf("expected failure for %s", k)
26772676
}
26782677
for i := range errs {
2679-
field := errs[i].(*validation.Error).Field
2678+
field := errs[i].Field
26802679
expectedFields := map[string]bool{
26812680
"metadata.name": true,
26822681
"metadata.labels": true,
@@ -2974,7 +2973,7 @@ func TestValidateServiceUpdate(t *testing.T) {
29742973
tc.tweakSvc(&oldSvc, &newSvc)
29752974
errs := ValidateServiceUpdate(&newSvc, &oldSvc)
29762975
if len(errs) != tc.numErrs {
2977-
t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs))
2976+
t.Errorf("Unexpected error list for case %q: %v", tc.name, errs.ToAggregate())
29782977
}
29792978
}
29802979
}
@@ -3008,7 +3007,7 @@ func TestValidateResourceNames(t *testing.T) {
30083007
} else if len(err) == 0 && !item.success {
30093008
t.Errorf("expected failure for input %q", item.input)
30103009
for i := range err {
3011-
detail := err[i].(*validation.Error).Detail
3010+
detail := err[i].Detail
30123011
if detail != "" && detail != qualifiedNameErrorMsg {
30133012
t.Errorf("%d: expected error detail either empty or %s, got %s", k, qualifiedNameErrorMsg, detail)
30143013
}
@@ -3224,7 +3223,7 @@ func TestValidateLimitRange(t *testing.T) {
32243223
t.Errorf("expected failure for %s", k)
32253224
}
32263225
for i := range errs {
3227-
detail := errs[i].(*validation.Error).Detail
3226+
detail := errs[i].Detail
32283227
if detail != v.D {
32293228
t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)
32303229
}
@@ -3329,8 +3328,8 @@ func TestValidateResourceQuota(t *testing.T) {
33293328
t.Errorf("expected failure for %s", k)
33303329
}
33313330
for i := range errs {
3332-
field := errs[i].(*validation.Error).Field
3333-
detail := errs[i].(*validation.Error).Detail
3331+
field := errs[i].Field
3332+
detail := errs[i].Detail
33343333
if field != "metadata.name" && field != "metadata.namespace" && !api.IsStandardResourceName(field) {
33353334
t.Errorf("%s: missing prefix for: %v", k, field)
33363335
}
@@ -3937,7 +3936,7 @@ func TestValidateEndpoints(t *testing.T) {
39373936
}
39383937

39393938
for k, v := range errorCases {
3940-
if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].(*validation.Error).Type != v.errorType || !strings.Contains(errs[0].(*validation.Error).Detail, v.errorDetail) {
3939+
if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].Type != v.errorType || !strings.Contains(errs[0].Detail, v.errorDetail) {
39413940
t.Errorf("Expected error type %s with detail %s for %s, got %v", v.errorType, v.errorDetail, k, errs)
39423941
}
39433942
}
@@ -4017,7 +4016,7 @@ func TestValidateSecurityContext(t *testing.T) {
40174016
},
40184017
}
40194018
for k, v := range errorCases {
4020-
if errs := ValidateSecurityContext(v.sc); len(errs) == 0 || errs[0].(*validation.Error).Type != v.errorType || errs[0].(*validation.Error).Detail != v.errorDetail {
4019+
if errs := ValidateSecurityContext(v.sc); len(errs) == 0 || errs[0].Type != v.errorType || errs[0].Detail != v.errorDetail {
40214020
t.Errorf("Expected error type %s with detail %s for %s, got %v", v.errorType, v.errorDetail, k, errs)
40224021
}
40234022
}

‎pkg/apis/extensions/validation/validation_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"k8s.io/kubernetes/pkg/api"
2525
"k8s.io/kubernetes/pkg/apis/extensions"
2626
"k8s.io/kubernetes/pkg/util/intstr"
27-
"k8s.io/kubernetes/pkg/util/validation"
2827
)
2928

3029
func TestValidateHorizontalPodAutoscaler(t *testing.T) {
@@ -675,7 +674,7 @@ func TestValidateDaemonSet(t *testing.T) {
675674
t.Errorf("expected failure for %s", k)
676675
}
677676
for i := range errs {
678-
field := errs[i].(*validation.Error).Field
677+
field := errs[i].Field
679678
if !strings.HasPrefix(field, "spec.template.") &&
680679
field != "metadata.name" &&
681680
field != "metadata.namespace" &&
@@ -918,9 +917,9 @@ func TestValidateJob(t *testing.T) {
918917
t.Errorf("expected failure for %s", k)
919918
} else {
920919
s := strings.Split(k, ":")
921-
err := errs[0].(*validation.Error)
920+
err := errs[0]
922921
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
923-
t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
922+
t.Errorf("unexpected error: %v, expected: %s", err, k)
924923
}
925924
}
926925
}
@@ -1019,9 +1018,9 @@ func TestValidateIngress(t *testing.T) {
10191018
t.Errorf("expected failure for %s", k)
10201019
} else {
10211020
s := strings.Split(k, ":")
1022-
err := errs[0].(*validation.Error)
1021+
err := errs[0]
10231022
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
1024-
t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
1023+
t.Errorf("unexpected error: %v, expected: %s", err, k)
10251024
}
10261025
}
10271026
}
@@ -1111,9 +1110,9 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
11111110
t.Errorf("expected failure for %s", k)
11121111
} else {
11131112
s := strings.Split(k, ":")
1114-
err := errs[0].(*validation.Error)
1113+
err := errs[0]
11151114
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
1116-
t.Errorf("unexpected error: %v, expected: %s", errs[0], k)
1115+
t.Errorf("unexpected error: %v, expected: %s", err, k)
11171116
}
11181117
}
11191118
}

‎pkg/kubectl/cmd/log.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
3333
"k8s.io/kubernetes/pkg/kubectl/resource"
3434
"k8s.io/kubernetes/pkg/runtime"
35-
kerrors "k8s.io/kubernetes/pkg/util/errors"
3635
)
3736

3837
const (
@@ -169,7 +168,7 @@ func (o LogsOptions) Validate() error {
169168
return errors.New("unexpected log options object")
170169
}
171170
if errs := validation.ValidatePodLogOptions(logOptions); len(errs) > 0 {
172-
return kerrors.NewAggregate(errs)
171+
return errs.ToAggregate()
173172
}
174173

175174
return nil

0 commit comments

Comments
 (0)