From cd29ba7ebc567c99bebe4c1ac4a424f56738d89e Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 8 Jan 2015 21:59:23 -0800 Subject: [PATCH 1/2] Add new util/errors pkg --- pkg/util/errors/doc.go | 18 +++ pkg/util/errors/errors.go | 133 ++++++++++++++++++++ pkg/util/errors/errors_test.go | 223 +++++++++++++++++++++++++++++++++ 3 files changed, 374 insertions(+) create mode 100644 pkg/util/errors/doc.go create mode 100644 pkg/util/errors/errors.go create mode 100644 pkg/util/errors/errors_test.go diff --git a/pkg/util/errors/doc.go b/pkg/util/errors/doc.go new file mode 100644 index 00000000000..15bd2ea03f5 --- /dev/null +++ b/pkg/util/errors/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2015 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package errors implements various utility functions and types around errors. +package errors diff --git a/pkg/util/errors/errors.go b/pkg/util/errors/errors.go new file mode 100644 index 00000000000..69e4ed9c823 --- /dev/null +++ b/pkg/util/errors/errors.go @@ -0,0 +1,133 @@ +/* +Copyright 2015 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import "fmt" + +// Aggregate represents an object that contains multiple errors, but does not +// necessarily have singular semantic meaning. +type Aggregate interface { + error + Errors() []error +} + +// NewAggregate converts a slice of errors into an Aggregate interface, which +// is itself an implementation of the error interface. If the slice is empty, +// this returs nil. +func NewAggregate(errlist []error) Aggregate { + if len(errlist) == 0 { + return nil + } + return aggregate(errlist) +} + +// This helper implements the error and Errors interfaces. Keeping it private +// prevents people from making an aggregate of 0 errors, which is not +// an error, but does satisfy the error interface. +type aggregate []error + +// Error is part of the error interface. +func (agg aggregate) Error() string { + if len(agg) == 0 { + // This should never happen, really. + return "" + } + if len(agg) == 1 { + return agg[0].Error() + } + result := fmt.Sprintf("[%s", agg[0].Error()) + for i := 1; i < len(agg); i++ { + result += fmt.Sprintf(", %s", agg[i].Error()) + } + result += "]" + return result +} + +// Errors is part of the Aggregate interface. +func (agg aggregate) Errors() []error { + return []error(agg) +} + +// Matcher is used to match errors. Returns true if the error matches. +type Matcher func(error) bool + +// FilterOut removes all errors that match any of the matchers from the input +// error. If the input is a singular error, only that error is tested. If the +// input implements the Aggregate interface, the list of errors will be +// processed recursively. +// +// This can be used, for example, to remove known-OK errors (such as io.EOF or +// os.PathNotFound) from a list of errors. +func FilterOut(err error, fns ...Matcher) error { + if err == nil { + return nil + } + if agg, ok := err.(Aggregate); ok { + return NewAggregate(filterErrors(agg.Errors(), fns...)) + } + if !matchesError(err, fns...) { + return err + } + return nil +} + +// matchesError returns true if any Matcher returns true +func matchesError(err error, fns ...Matcher) bool { + for _, fn := range fns { + if fn(err) { + return true + } + } + return false +} + +// filterErrors returns any errors (or nested errors, if the list contains +// nested Errors) for which all fns return false. If no errors +// remain a nil list is returned. The resulting silec will have all +// nested slices flattened as a side effect. +func filterErrors(list []error, fns ...Matcher) []error { + result := []error{} + for _, err := range list { + r := FilterOut(err, fns...) + if r != nil { + result = append(result, r) + } + } + return result +} + +// Flatten takes an Aggregate, which may hold other Aggregates in arbitrary +// nesting, and flattens them all into a single Aggregate, recursively. +func Flatten(agg Aggregate) Aggregate { + result := []error{} + if agg == nil { + return nil + } + for _, err := range agg.Errors() { + if a, ok := err.(Aggregate); ok { + r := Flatten(a) + if r != nil { + result = append(result, r.Errors()...) + } + } else { + if err != nil { + result = append(result, err) + } + } + } + return NewAggregate(result) +} diff --git a/pkg/util/errors/errors_test.go b/pkg/util/errors/errors_test.go new file mode 100644 index 00000000000..4269c8e0a03 --- /dev/null +++ b/pkg/util/errors/errors_test.go @@ -0,0 +1,223 @@ +/* +Copyright 2015 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import ( + "fmt" + "reflect" + "testing" +) + +func TestEmptyAggregate(t *testing.T) { + var slice []error + var agg Aggregate + var err error + + agg = NewAggregate(slice) + if agg != nil { + t.Errorf("expected nil, got %#v", agg) + } + err = NewAggregate(slice) + if err != nil { + t.Errorf("expected nil, got %#v", err) + } + + // This is not normally possible, but pedantry demands I test it. + agg = aggregate(slice) // empty aggregate + if s := agg.Error(); s != "" { + t.Errorf("expected empty string, got %q", s) + } + if s := agg.Errors(); len(s) != 0 { + t.Errorf("expected empty slice, got %#v", s) + } + err = agg.(error) + if s := err.Error(); s != "" { + t.Errorf("expected empty string, got %q", s) + } +} + +func TestSingularAggregate(t *testing.T) { + var slice []error = []error{fmt.Errorf("err")} + var agg Aggregate + var err error + + agg = NewAggregate(slice) + if agg == nil { + t.Errorf("expected non-nil") + } + if s := agg.Error(); s != "err" { + t.Errorf("expected 'err', got %q", s) + } + if s := agg.Errors(); len(s) != 1 { + t.Errorf("expected one-element slice, got %#v", s) + } + if s := agg.Errors()[0].Error(); s != "err" { + t.Errorf("expected 'err', got %q", s) + } + + err = agg.(error) + if err == nil { + t.Errorf("expected non-nil") + } + if s := err.Error(); s != "err" { + t.Errorf("expected 'err', got %q", s) + } +} + +func TestPluralAggregate(t *testing.T) { + var slice []error = []error{fmt.Errorf("abc"), fmt.Errorf("123")} + var agg Aggregate + var err error + + agg = NewAggregate(slice) + if agg == nil { + t.Errorf("expected non-nil") + } + if s := agg.Error(); s != "[abc, 123]" { + t.Errorf("expected '[abc, 123]', got %q", s) + } + if s := agg.Errors(); len(s) != 2 { + t.Errorf("expected one-element slice, got %#v", s) + } + if s := agg.Errors()[0].Error(); s != "abc" { + t.Errorf("expected '[abc, 123]', got %q", s) + } + + err = agg.(error) + if err == nil { + t.Errorf("expected non-nil") + } + if s := err.Error(); s != "[abc, 123]" { + t.Errorf("expected '[abc, 123]', got %q", s) + } +} + +func TestFilterOut(t *testing.T) { + testCases := []struct { + err error + filter []Matcher + expected error + }{ + { + nil, + []Matcher{}, + nil, + }, + { + aggregate{}, + []Matcher{}, + nil, + }, + { + aggregate{fmt.Errorf("abc")}, + []Matcher{}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{fmt.Errorf("abc")}, + []Matcher{func(err error) bool { return false }}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{fmt.Errorf("abc")}, + []Matcher{func(err error) bool { return true }}, + nil, + }, + { + aggregate{fmt.Errorf("abc")}, + []Matcher{func(err error) bool { return false }, func(err error) bool { return false }}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{fmt.Errorf("abc")}, + []Matcher{func(err error) bool { return false }, func(err error) bool { return true }}, + nil, + }, + { + aggregate{fmt.Errorf("abc"), fmt.Errorf("def"), fmt.Errorf("ghi")}, + []Matcher{func(err error) bool { return err.Error() == "def" }}, + aggregate{fmt.Errorf("abc"), fmt.Errorf("ghi")}, + }, + { + aggregate{aggregate{fmt.Errorf("abc")}}, + []Matcher{}, + aggregate{aggregate{fmt.Errorf("abc")}}, + }, + { + aggregate{aggregate{fmt.Errorf("abc"), aggregate{fmt.Errorf("def")}}}, + []Matcher{}, + aggregate{aggregate{fmt.Errorf("abc"), aggregate{fmt.Errorf("def")}}}, + }, + { + aggregate{aggregate{fmt.Errorf("abc"), aggregate{fmt.Errorf("def")}}}, + []Matcher{func(err error) bool { return err.Error() == "def" }}, + aggregate{aggregate{fmt.Errorf("abc")}}, + }, + } + for i, testCase := range testCases { + err := FilterOut(testCase.err, testCase.filter...) + if !reflect.DeepEqual(testCase.expected, err) { + t.Errorf("%d: expected %v, got %v", i, testCase.expected, err) + } + } +} + +func TestFlatten(t *testing.T) { + testCases := []struct { + agg Aggregate + expected Aggregate + }{ + { + nil, + nil, + }, + { + aggregate{}, + nil, + }, + { + aggregate{fmt.Errorf("abc")}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{fmt.Errorf("abc"), fmt.Errorf("def"), fmt.Errorf("ghi")}, + aggregate{fmt.Errorf("abc"), fmt.Errorf("def"), fmt.Errorf("ghi")}, + }, + { + aggregate{aggregate{fmt.Errorf("abc")}}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{aggregate{aggregate{fmt.Errorf("abc")}}}, + aggregate{fmt.Errorf("abc")}, + }, + { + aggregate{aggregate{fmt.Errorf("abc"), aggregate{fmt.Errorf("def")}}}, + aggregate{fmt.Errorf("abc"), fmt.Errorf("def")}, + }, + { + aggregate{aggregate{aggregate{fmt.Errorf("abc")}, fmt.Errorf("def"), aggregate{fmt.Errorf("ghi")}}}, + aggregate{fmt.Errorf("abc"), fmt.Errorf("def"), fmt.Errorf("ghi")}, + }, + } + for i, testCase := range testCases { + agg := Flatten(testCase.agg) + if !reflect.DeepEqual(testCase.expected, agg) { + t.Errorf("%d: expected %v, got %v", i, testCase.expected, agg) + } + } +} From 4fcd496d5995eee6d18c1a052443314fe31e0a02 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 8 Jan 2015 22:10:03 -0800 Subject: [PATCH 2/2] change everything to use new util/errors --- pkg/api/errors/errors.go | 4 +- pkg/api/validation/validation_test.go | 3 +- pkg/client/clientcmd/client_config.go | 4 +- pkg/client/clientcmd/validation.go | 5 +- pkg/client/clientcmd/validation_test.go | 20 +++---- pkg/proxy/proxier.go | 5 +- pkg/util/error_list.go | 54 ------------------- pkg/util/error_list_test.go | 51 ------------------ .../auth/authenticator/request/union/union.go | 8 +-- .../auth/authenticator/request/x509/x509.go | 10 ++-- 10 files changed, 31 insertions(+), 133 deletions(-) delete mode 100644 pkg/util/error_list.go delete mode 100644 pkg/util/error_list_test.go diff --git a/pkg/api/errors/errors.go b/pkg/api/errors/errors.go index 9f24248571a..116f8e05b09 100644 --- a/pkg/api/errors/errors.go +++ b/pkg/api/errors/errors.go @@ -22,7 +22,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) // StatusError is an error intended for consumption by a REST API server; it can also be @@ -141,7 +141,7 @@ func NewInvalid(kind, name string, errs ValidationErrorList) error { ID: name, Causes: causes, }, - Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, util.SliceToError(errs)), + Message: fmt.Sprintf("%s %q is invalid: %v", kind, name, errors.NewAggregate(errs)), }} } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 827aa40ad53..2757f268ff0 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/capabilities" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/registrytest" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + utilerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) func expectPrefix(t *testing.T, prefix string, errs errors.ValidationErrorList) { @@ -968,7 +969,7 @@ func TestValidateService(t *testing.T) { registry.List = tc.existing errs := ValidateService(&tc.svc, registry, api.NewDefaultContext()) if len(errs) != tc.numErrs { - t.Errorf("Unexpected error list for case %q: %v", tc.name, util.SliceToError(errs)) + t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs)) } } diff --git a/pkg/client/clientcmd/client_config.go b/pkg/client/clientcmd/client_config.go index 47a4d9ac76b..6f9c2323ebf 100644 --- a/pkg/client/clientcmd/client_config.go +++ b/pkg/client/clientcmd/client_config.go @@ -24,7 +24,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/clientauth" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) var ( @@ -227,7 +227,7 @@ func (config DirectClientConfig) ConfirmUsable() error { validationErrors = append(validationErrors, validateAuthInfo(config.getAuthInfoName(), config.getAuthInfo())...) validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...) - return util.SliceToError(validationErrors) + return errors.NewAggregate(validationErrors) } func (config DirectClientConfig) getContextName() string { diff --git a/pkg/client/clientcmd/validation.go b/pkg/client/clientcmd/validation.go index 4d1a52abd92..1732962a721 100644 --- a/pkg/client/clientcmd/validation.go +++ b/pkg/client/clientcmd/validation.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + utilerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) var ErrNoContext = errors.New("no context chosen") @@ -67,7 +68,7 @@ func Validate(config Config) error { validationErrors = append(validationErrors, validateClusterInfo(clusterName, clusterInfo)...) } - return util.SliceToError(validationErrors) + return utilerrors.NewAggregate(validationErrors) } // ConfirmUsable looks a particular context and determines if that particular part of the config is useable. There might still be errors in the config, @@ -97,7 +98,7 @@ func ConfirmUsable(config Config, passedContextName string) error { validationErrors = append(validationErrors, validateClusterInfo(context.Cluster, config.Clusters[context.Cluster])...) } - return util.SliceToError(validationErrors) + return utilerrors.NewAggregate(validationErrors) } // validateClusterInfo looks for conflicts and errors in the cluster info diff --git a/pkg/client/clientcmd/validation_test.go b/pkg/client/clientcmd/validation_test.go index 7408cbe134c..23c0d4b38e4 100644 --- a/pkg/client/clientcmd/validation_test.go +++ b/pkg/client/clientcmd/validation_test.go @@ -22,7 +22,7 @@ import ( "strings" "testing" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) func TestConfirmUsableBadInfoButOkConfig(t *testing.T) { @@ -300,14 +300,14 @@ func (c configValidationTest) testContext(contextName string, t *testing.T) { t.Errorf("Expected error containing: %v", c.expectedErrorSubstring) } for _, curr := range c.expectedErrorSubstring { - if len(errs) != 0 && !strings.Contains(util.SliceToError(errs).Error(), curr) { - t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, util.SliceToError(errs)) + if len(errs) != 0 && !strings.Contains(errors.NewAggregate(errs).Error(), curr) { + t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, errors.NewAggregate(errs)) } } } else { if len(errs) != 0 { - t.Errorf("Unexpected error: %v", util.SliceToError(errs)) + t.Errorf("Unexpected error: %v", errors.NewAggregate(errs)) } } } @@ -357,14 +357,14 @@ func (c configValidationTest) testCluster(clusterName string, t *testing.T) { t.Errorf("Expected error containing: %v", c.expectedErrorSubstring) } for _, curr := range c.expectedErrorSubstring { - if len(errs) != 0 && !strings.Contains(util.SliceToError(errs).Error(), curr) { - t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, util.SliceToError(errs)) + if len(errs) != 0 && !strings.Contains(errors.NewAggregate(errs).Error(), curr) { + t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, errors.NewAggregate(errs)) } } } else { if len(errs) != 0 { - t.Errorf("Unexpected error: %v", util.SliceToError(errs)) + t.Errorf("Unexpected error: %v", errors.NewAggregate(errs)) } } } @@ -377,14 +377,14 @@ func (c configValidationTest) testAuthInfo(authInfoName string, t *testing.T) { t.Errorf("Expected error containing: %v", c.expectedErrorSubstring) } for _, curr := range c.expectedErrorSubstring { - if len(errs) != 0 && !strings.Contains(util.SliceToError(errs).Error(), curr) { - t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, util.SliceToError(errs)) + if len(errs) != 0 && !strings.Contains(errors.NewAggregate(errs).Error(), curr) { + t.Errorf("Expected error containing: %v, but got %v", c.expectedErrorSubstring, errors.NewAggregate(errs)) } } } else { if len(errs) != 0 { - t.Errorf("Unexpected error: %v", util.SliceToError(errs)) + t.Errorf("Unexpected error: %v", errors.NewAggregate(errs)) } } } diff --git a/pkg/proxy/proxier.go b/pkg/proxy/proxier.go index 22d9cae83e7..f0e2c9fa312 100644 --- a/pkg/proxy/proxier.go +++ b/pkg/proxy/proxier.go @@ -27,6 +27,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/util/iptables" "github.com/golang/glog" ) @@ -575,7 +576,7 @@ func (proxier *Proxier) closePortal(service string, info *serviceInfo) error { } else { glog.Errorf("Some errors closing iptables portals for service %q", service) } - return util.SliceToError(el) + return errors.NewAggregate(el) } func (proxier *Proxier) closeOnePortal(portalIP net.IP, portalPort int, protocol api.Protocol, proxyIP net.IP, proxyPort int, name string) []error { @@ -646,7 +647,7 @@ func iptablesFlush(ipt iptables.Interface) error { if len(el) != 0 { glog.Errorf("Some errors flushing old iptables portals: %v", el) } - return util.SliceToError(el) + return errors.NewAggregate(el) } // Used below. diff --git a/pkg/util/error_list.go b/pkg/util/error_list.go deleted file mode 100644 index e39ee7dfdce..00000000000 --- a/pkg/util/error_list.go +++ /dev/null @@ -1,54 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "fmt" -) - -// []error is a collection of errors. This does not implement the error -// interface to avoid confusion where an empty []error would still be an -// error (non-nil). To produce a single error instance from an []error, use -// the SliceToError() method, which will return nil for an empty []error. - -// This helper implements the error interface for []error, but prevents -// accidental conversion of []error to error. -type errorList []error - -// Error is part of the error interface. -func (list errorList) Error() string { - if len(list) == 0 { - return "" - } - if len(list) == 1 { - return list[0].Error() - } - result := fmt.Sprintf("[%s", list[0].Error()) - for i := 1; i < len(list); i++ { - result += fmt.Sprintf(", %s", list[i].Error()) - } - result += "]" - return result -} - -// SliceToError converts an []error into a "normal" error, or nil if the slice is empty. -func SliceToError(errs []error) error { - if len(errs) == 0 { - return nil - } - return errorList(errs) -} diff --git a/pkg/util/error_list_test.go b/pkg/util/error_list_test.go deleted file mode 100644 index 2162b5570b8..00000000000 --- a/pkg/util/error_list_test.go +++ /dev/null @@ -1,51 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package util - -import ( - "fmt" - "testing" -) - -func TestErrorList(t *testing.T) { - var errList []error - err := SliceToError(errList) - if err != nil { - t.Errorf("expected nil, got %v", err) - } - if a := errorList(errList).Error(); a != "" { - t.Errorf("expected empty string, got %q", a) - } - - testCases := []struct { - errs []error - expected string - }{ - {[]error{fmt.Errorf("abc")}, "abc"}, - {[]error{fmt.Errorf("abc"), fmt.Errorf("123")}, "[abc, 123]"}, - } - for _, testCase := range testCases { - err := SliceToError(testCase.errs) - if err == nil { - t.Errorf("expected an error, got nil: %v", testCase) - continue - } - if err.Error() != testCase.expected { - t.Errorf("expected %q, got %q", testCase.expected, err.Error()) - } - } -} diff --git a/plugin/pkg/auth/authenticator/request/union/union.go b/plugin/pkg/auth/authenticator/request/union/union.go index 4d90f63afbe..6ef831ffb8e 100644 --- a/plugin/pkg/auth/authenticator/request/union/union.go +++ b/plugin/pkg/auth/authenticator/request/union/union.go @@ -21,7 +21,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/authenticator" "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) // unionAuthRequestHandler authenticates requests using a chain of authenticator.Requests @@ -35,11 +35,11 @@ func New(authRequestHandlers ...authenticator.Request) authenticator.Request { // AuthenticateRequest authenticates the request using a chain of authenticator.Request objects. The first // success returns that identity. Errors are only returned if no matches are found. func (authHandler unionAuthRequestHandler) AuthenticateRequest(req *http.Request) (user.Info, bool, error) { - var errors []error + var errlist []error for _, currAuthRequestHandler := range authHandler { info, ok, err := currAuthRequestHandler.AuthenticateRequest(req) if err != nil { - errors = append(errors, err) + errlist = append(errlist, err) continue } @@ -48,5 +48,5 @@ func (authHandler unionAuthRequestHandler) AuthenticateRequest(req *http.Request } } - return nil, false, util.SliceToError(errors) + return nil, false, errors.NewAggregate(errlist) } diff --git a/plugin/pkg/auth/authenticator/request/x509/x509.go b/plugin/pkg/auth/authenticator/request/x509/x509.go index f9159e8d202..9b0a0e5d250 100644 --- a/plugin/pkg/auth/authenticator/request/x509/x509.go +++ b/plugin/pkg/auth/authenticator/request/x509/x509.go @@ -21,7 +21,7 @@ import ( "net/http" "github.com/GoogleCloudPlatform/kubernetes/pkg/auth/user" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) // UserConversion defines an interface for extracting user info from a client certificate chain @@ -55,18 +55,18 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, return nil, false, nil } - var errors []error + var errlist []error for _, cert := range req.TLS.PeerCertificates { chains, err := cert.Verify(a.opts) if err != nil { - errors = append(errors, err) + errlist = append(errlist, err) continue } for _, chain := range chains { user, ok, err := a.user.User(chain) if err != nil { - errors = append(errors, err) + errlist = append(errlist, err) continue } @@ -75,7 +75,7 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, } } } - return nil, false, util.SliceToError(errors) + return nil, false, errors.NewAggregate(errlist) } // DefaultVerifyOptions returns VerifyOptions that use the system root certificates, current time,