fix duplicate validation/field/errors

Related PR: https://github.com/kubernetes/kubernetes/pull/30313

PR #30313 fixed duplicate errors for invalid aggregate errors in
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/util/helpers.go

However, duplicate aggregate errors that went through
https://github.com/kubernetes/kubernetes/blob/master/pkg/util/validation/field/errors.go
were not affected by that patch.

This patch adds duplicate aggregate error checking to
`pkg/util/validation/field/errors.go`

\##### Before
`$ kubectl set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName", spec.template.spec.containers[0].env[0].name:
Invalid value: "test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ kubectl set env rc/node-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

\##### After
`$ kubectl set env rc/idling-echo-1 test-abc=1234`
```
error: ReplicationController "idling-echo-1" is invalid:
[spec.template.spec.containers[0].env[0].name: Invalid value:
"test-abc": must be a C identifier (matching regex
[A-Za-z_][A-Za-z0-9_]*): e.g. "my_name" or "MyName",
spec.template.spec.containers[1].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"]
```

`$ kubectl set env rc/node-1 test-abc=1234`
```
error: ReplicationController "node-1" is invalid:
spec.template.spec.containers[0].env[0].name: Invalid value: "test-abc":
must be a C identifier (matching regex [A-Za-z_][A-Za-z0-9_]*): e.g.
"my_name" or "MyName"
```
This commit is contained in:
juanvallejo 2016-08-24 12:38:02 -04:00
parent 45a436ac24
commit a7286e8afa
3 changed files with 46 additions and 12 deletions

View File

@ -17,7 +17,10 @@ go_library(
"path.go", "path.go",
], ],
tags = ["automanaged"], tags = ["automanaged"],
deps = ["//pkg/util/errors:go_default_library"], deps = [
"//pkg/util/errors:go_default_library",
"//pkg/util/sets:go_default_library",
],
) )
go_test( go_test(

View File

@ -22,6 +22,7 @@ import (
"strings" "strings"
utilerrors "k8s.io/kubernetes/pkg/util/errors" utilerrors "k8s.io/kubernetes/pkg/util/errors"
"k8s.io/kubernetes/pkg/util/sets"
) )
// Error is an implementation of the 'error' interface, which represents a // Error is an implementation of the 'error' interface, which represents a
@ -201,9 +202,15 @@ func NewErrorTypeMatcher(t ErrorType) utilerrors.Matcher {
// ToAggregate converts the ErrorList into an errors.Aggregate. // ToAggregate converts the ErrorList into an errors.Aggregate.
func (list ErrorList) ToAggregate() utilerrors.Aggregate { func (list ErrorList) ToAggregate() utilerrors.Aggregate {
errs := make([]error, len(list)) errs := make([]error, 0, len(list))
for i := range list { errorMsgs := sets.NewString()
errs[i] = list[i] for _, err := range list {
msg := fmt.Sprintf("%v", err)
if errorMsgs.Has(msg) {
continue
}
errorMsgs.Insert(msg)
errs = append(errs, err)
} }
return utilerrors.NewAggregate(errs) return utilerrors.NewAggregate(errs)
} }

View File

@ -99,22 +99,46 @@ func TestErrorUsefulMessage(t *testing.T) {
} }
func TestToAggregate(t *testing.T) { func TestToAggregate(t *testing.T) {
testCases := []ErrorList{ testCases := struct {
nil, ErrList []ErrorList
{}, NumExpectedErrs []int
{Invalid(NewPath("f"), "v", "d")}, }{
{Invalid(NewPath("f"), "v", "d"), InternalError(NewPath(""), fmt.Errorf("e"))}, []ErrorList{
nil,
{},
{Invalid(NewPath("f"), "v", "d")},
{Invalid(NewPath("f"), "v", "d"), Invalid(NewPath("f"), "v", "d")},
{Invalid(NewPath("f"), "v", "d"), InternalError(NewPath(""), fmt.Errorf("e"))},
},
[]int{
0,
0,
1,
1,
2,
},
} }
for i, tc := range testCases {
if len(testCases.ErrList) != len(testCases.NumExpectedErrs) {
t.Errorf("Mismatch: length of NumExpectedErrs does not match length of ErrList")
}
for i, tc := range testCases.ErrList {
agg := tc.ToAggregate() agg := tc.ToAggregate()
numErrs := 0
if agg != nil {
numErrs = len(agg.Errors())
}
if numErrs != testCases.NumExpectedErrs[i] {
t.Errorf("[%d] Expected %d, got %d", i, testCases.NumExpectedErrs[i], numErrs)
}
if len(tc) == 0 { if len(tc) == 0 {
if agg != nil { if agg != nil {
t.Errorf("[%d] Expected nil, got %#v", i, agg) t.Errorf("[%d] Expected nil, got %#v", i, agg)
} }
} else if agg == nil { } else if agg == nil {
t.Errorf("[%d] Expected non-nil", i) 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()))
} }
} }
} }