Merge pull request #3353 from thockin/errors

Cleaner aggregate errors API
This commit is contained in:
Clayton Coleman 2015-01-09 11:50:57 -05:00
commit 68298f08a4
13 changed files with 405 additions and 133 deletions

View File

@ -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)),
}}
}

View File

@ -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))
}
}

View File

@ -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 {

View File

@ -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

View File

@ -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))
}
}
}

View File

@ -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.

View File

@ -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)
}

View File

@ -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())
}
}
}

18
pkg/util/errors/doc.go Normal file
View File

@ -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

133
pkg/util/errors/errors.go Normal file
View File

@ -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)
}

View File

@ -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)
}
}
}

View File

@ -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)
}

View File

@ -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,