From 99ff40a1bb3541e2d9169eb08d1ca412e6bd06cb Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Wed, 26 Jul 2017 08:52:25 -0700 Subject: [PATCH 1/3] Update vendor of gopkg.in/gcfg from v1 to v1.2.0 --- Godeps/Godeps.json | 21 ++- Godeps/LICENSES | 32 ++++ vendor/BUILD | 1 + vendor/gopkg.in/gcfg.v1/BUILD | 2 + vendor/gopkg.in/gcfg.v1/doc.go | 27 ++++ vendor/gopkg.in/gcfg.v1/errors.go | 41 +++++ vendor/gopkg.in/gcfg.v1/read.go | 94 +++++++++--- vendor/gopkg.in/gcfg.v1/set.go | 56 ++++++- vendor/gopkg.in/warnings.v0/BUILD | 27 ++++ vendor/gopkg.in/warnings.v0/LICENSE | 24 +++ vendor/gopkg.in/warnings.v0/README | 74 +++++++++ vendor/gopkg.in/warnings.v0/warnings.go | 191 ++++++++++++++++++++++++ 12 files changed, 552 insertions(+), 38 deletions(-) create mode 100644 vendor/gopkg.in/gcfg.v1/errors.go create mode 100644 vendor/gopkg.in/warnings.v0/BUILD create mode 100644 vendor/gopkg.in/warnings.v0/LICENSE create mode 100644 vendor/gopkg.in/warnings.v0/README create mode 100644 vendor/gopkg.in/warnings.v0/warnings.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 5b026bc5c73..88175fa58ca 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -2951,23 +2951,23 @@ }, { "ImportPath": "gopkg.in/gcfg.v1", - "Comment": "v1.0.0", - "Rev": "083575c3955c85df16fe9590cceab64d03f5eb6e" + "Comment": "v1.2.0", + "Rev": "27e4946190b4a327b539185f2b5b1f7c84730728" }, { "ImportPath": "gopkg.in/gcfg.v1/scanner", - "Comment": "v1.0.0", - "Rev": "083575c3955c85df16fe9590cceab64d03f5eb6e" + "Comment": "v1.2.0", + "Rev": "27e4946190b4a327b539185f2b5b1f7c84730728" }, { "ImportPath": "gopkg.in/gcfg.v1/token", - "Comment": "v1.0.0", - "Rev": "083575c3955c85df16fe9590cceab64d03f5eb6e" + "Comment": "v1.2.0", + "Rev": "27e4946190b4a327b539185f2b5b1f7c84730728" }, { "ImportPath": "gopkg.in/gcfg.v1/types", - "Comment": "v1.0.0", - "Rev": "083575c3955c85df16fe9590cceab64d03f5eb6e" + "Comment": "v1.2.0", + "Rev": "27e4946190b4a327b539185f2b5b1f7c84730728" }, { "ImportPath": "gopkg.in/inf.v0", @@ -2979,6 +2979,11 @@ "Comment": "v1.0-16-g20b71e5", "Rev": "20b71e5b60d756d3d2f80def009790325acc2b23" }, + { + "ImportPath": "gopkg.in/warnings.v0", + "Comment": "v0.1.1", + "Rev": "8a331561fe74dadba6edfc59f3be66c22c3b065d" + }, { "ImportPath": "gopkg.in/yaml.v2", "Rev": "53feefa2559fb8dfa8d81baad31be332c97d6c77" diff --git a/Godeps/LICENSES b/Godeps/LICENSES index 07323c35393..859199fa920 100644 --- a/Godeps/LICENSES +++ b/Godeps/LICENSES @@ -87638,6 +87638,38 @@ SOFTWARE. ================================================================================ +================================================================================ += vendor/gopkg.in/warnings.v0 licensed under: = + +Copyright (c) 2016 Péter Surányi. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + += vendor/gopkg.in/warnings.v0/LICENSE c6775875c9d604beb22447dfae3d7049 - +================================================================================ + + ================================================================================ = vendor/gopkg.in/yaml.v2 licensed under: = diff --git a/vendor/BUILD b/vendor/BUILD index a5ad0b17304..e337a6281a8 100644 --- a/vendor/BUILD +++ b/vendor/BUILD @@ -370,6 +370,7 @@ filegroup( "//vendor/gopkg.in/gcfg.v1:all-srcs", "//vendor/gopkg.in/inf.v0:all-srcs", "//vendor/gopkg.in/natefinch/lumberjack.v2:all-srcs", + "//vendor/gopkg.in/warnings.v0:all-srcs", "//vendor/gopkg.in/yaml.v2:all-srcs", "//vendor/k8s.io/gengo/args:all-srcs", "//vendor/k8s.io/gengo/examples/deepcopy-gen/generators:all-srcs", diff --git a/vendor/gopkg.in/gcfg.v1/BUILD b/vendor/gopkg.in/gcfg.v1/BUILD index 81095af2880..7f584416c68 100644 --- a/vendor/gopkg.in/gcfg.v1/BUILD +++ b/vendor/gopkg.in/gcfg.v1/BUILD @@ -11,6 +11,7 @@ go_library( name = "go_default_library", srcs = [ "doc.go", + "errors.go", "go1_2.go", "read.go", "set.go", @@ -20,6 +21,7 @@ go_library( "//vendor/gopkg.in/gcfg.v1/scanner:go_default_library", "//vendor/gopkg.in/gcfg.v1/token:go_default_library", "//vendor/gopkg.in/gcfg.v1/types:go_default_library", + "//vendor/gopkg.in/warnings.v0:go_default_library", ], ) diff --git a/vendor/gopkg.in/gcfg.v1/doc.go b/vendor/gopkg.in/gcfg.v1/doc.go index 99687b46f41..40c80fe8439 100644 --- a/vendor/gopkg.in/gcfg.v1/doc.go +++ b/vendor/gopkg.in/gcfg.v1/doc.go @@ -48,6 +48,9 @@ // When using a map, and there is a section with the same section name but // without a subsection name, its values are stored with the empty string used // as the key. +// It is possible to provide default values for subsections in the section +// "default-" (or by setting values in the corresponding struct +// field "Default_"). // // The functions in this package panic if config is not a pointer to a struct, // or when a field is not of a suitable type (either a struct or a map with @@ -95,6 +98,30 @@ // The types subpackage for provides helpers for parsing "enum-like" and integer // types. // +// Error handling +// +// There are 3 types of errors: +// +// - programmer errors / panics: +// - invalid configuration structure +// - data errors: +// - fatal errors: +// - invalid configuration syntax +// - warnings: +// - data that doesn't belong to any part of the config structure +// +// Programmer errors trigger panics. These are should be fixed by the programmer +// before releasing code that uses gcfg. +// +// Data errors cause gcfg to return a non-nil error value. This includes the +// case when there are extra unknown key-value definitions in the configuration +// data (extra data). +// However, in some occasions it is desirable to be able to proceed in +// situations when the only data error is that of extra data. +// These errors are handled at a different (warning) priority and can be +// filtered out programmatically. To ignore extra data warnings, wrap the +// gcfg.Read*Into invocation into a call to gcfg.FatalOnly. +// // TODO // // The following is a list of changes under consideration: diff --git a/vendor/gopkg.in/gcfg.v1/errors.go b/vendor/gopkg.in/gcfg.v1/errors.go new file mode 100644 index 00000000000..853c76021de --- /dev/null +++ b/vendor/gopkg.in/gcfg.v1/errors.go @@ -0,0 +1,41 @@ +package gcfg + +import ( + "gopkg.in/warnings.v0" +) + +// FatalOnly filters the results of a Read*Into invocation and returns only +// fatal errors. That is, errors (warnings) indicating data for unknown +// sections / variables is ignored. Example invocation: +// +// err := gcfg.FatalOnly(gcfg.ReadFileInto(&cfg, configFile)) +// if err != nil { +// ... +// +func FatalOnly(err error) error { + return warnings.FatalOnly(err) +} + +func isFatal(err error) bool { + _, ok := err.(extraData) + return !ok +} + +type extraData struct { + section string + subsection *string + variable *string +} + +func (e extraData) Error() string { + s := "can't store data at section \"" + e.section + "\"" + if e.subsection != nil { + s += ", subsection \"" + *e.subsection + "\"" + } + if e.variable != nil { + s += ", variable \"" + *e.variable + "\"" + } + return s +} + +var _ error = extraData{} diff --git a/vendor/gopkg.in/gcfg.v1/read.go b/vendor/gopkg.in/gcfg.v1/read.go index fdfb5f3a2c8..8400cf124e9 100644 --- a/vendor/gopkg.in/gcfg.v1/read.go +++ b/vendor/gopkg.in/gcfg.v1/read.go @@ -6,11 +6,10 @@ import ( "io/ioutil" "os" "strings" -) -import ( "gopkg.in/gcfg.v1/scanner" "gopkg.in/gcfg.v1/token" + "gopkg.in/warnings.v0" ) var unescape = map[rune]rune{'\\': '\\', '"': '"', 'n': '\n', 't': '\t'} @@ -49,7 +48,9 @@ func unquote(s string) string { return string(u) } -func readInto(config interface{}, fset *token.FileSet, file *token.File, src []byte) error { +func readIntoPass(c *warnings.Collector, config interface{}, fset *token.FileSet, + file *token.File, src []byte, subsectPass bool) error { + // var s scanner.Scanner var errs scanner.ErrorList s.Init(file, src, func(p token.Position, m string) { errs.Add(p, m) }, 0) @@ -60,7 +61,9 @@ func readInto(config interface{}, fset *token.FileSet, file *token.File, src []b } for { if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } switch tok { case token.EOF: @@ -70,46 +73,64 @@ func readInto(config interface{}, fset *token.FileSet, file *token.File, src []b case token.LBRACK: pos, tok, lit = s.Scan() if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } if tok != token.IDENT { - return errfn("expected section name") + if err := c.Collect(errfn("expected section name")); err != nil { + return err + } } sect, sectsub = lit, "" pos, tok, lit = s.Scan() if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } if tok == token.STRING { sectsub = unquote(lit) if sectsub == "" { - return errfn("empty subsection name") + if err := c.Collect(errfn("empty subsection name")); err != nil { + return err + } } pos, tok, lit = s.Scan() if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } } if tok != token.RBRACK { if sectsub == "" { - return errfn("expected subsection name or right bracket") + if err := c.Collect(errfn("expected subsection name or right bracket")); err != nil { + return err + } + } + if err := c.Collect(errfn("expected right bracket")); err != nil { + return err } - return errfn("expected right bracket") } pos, tok, lit = s.Scan() if tok != token.EOL && tok != token.EOF && tok != token.COMMENT { - return errfn("expected EOL, EOF, or comment") + if err := c.Collect(errfn("expected EOL, EOF, or comment")); err != nil { + return err + } } // If a section/subsection header was found, ensure a // container object is created, even if there are no // variables further down. - err := set(config, sect, sectsub, "", true, "") + err := c.Collect(set(c, config, sect, sectsub, "", true, "", subsectPass)) if err != nil { return err } case token.IDENT: if sect == "" { - return errfn("expected section header") + if err := c.Collect(errfn("expected section header")); err != nil { + return err + } } n := lit pos, tok, lit = s.Scan() @@ -119,38 +140,67 @@ func readInto(config interface{}, fset *token.FileSet, file *token.File, src []b blank, v := tok == token.EOF || tok == token.EOL || tok == token.COMMENT, "" if !blank { if tok != token.ASSIGN { - return errfn("expected '='") + if err := c.Collect(errfn("expected '='")); err != nil { + return err + } } pos, tok, lit = s.Scan() if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } if tok != token.STRING { - return errfn("expected value") + if err := c.Collect(errfn("expected value")); err != nil { + return err + } } v = unquote(lit) pos, tok, lit = s.Scan() if errs.Len() > 0 { - return errs.Err() + if err := c.Collect(errs.Err()); err != nil { + return err + } } if tok != token.EOL && tok != token.EOF && tok != token.COMMENT { - return errfn("expected EOL, EOF, or comment") + if err := c.Collect(errfn("expected EOL, EOF, or comment")); err != nil { + return err + } } } - err := set(config, sect, sectsub, n, blank, v) + err := set(c, config, sect, sectsub, n, blank, v, subsectPass) if err != nil { return err } default: if sect == "" { - return errfn("expected section header") + if err := c.Collect(errfn("expected section header")); err != nil { + return err + } + } + if err := c.Collect(errfn("expected section header or variable declaration")); err != nil { + return err } - return errfn("expected section header or variable declaration") } } panic("never reached") } +func readInto(config interface{}, fset *token.FileSet, file *token.File, + src []byte) error { + // + c := warnings.NewCollector(isFatal) + err := readIntoPass(c, config, fset, file, src, false) + if err != nil { + return err + } + err = readIntoPass(c, config, fset, file, src, true) + if err != nil { + return err + } + return c.Done() +} + // ReadInto reads gcfg formatted data from reader and sets the values into the // corresponding fields in config. func ReadInto(config interface{}, reader io.Reader) error { diff --git a/vendor/gopkg.in/gcfg.v1/set.go b/vendor/gopkg.in/gcfg.v1/set.go index 7252b689465..e85ec155dca 100644 --- a/vendor/gopkg.in/gcfg.v1/set.go +++ b/vendor/gopkg.in/gcfg.v1/set.go @@ -1,6 +1,8 @@ package gcfg import ( + "bytes" + "encoding/gob" "fmt" "math/big" "reflect" @@ -9,6 +11,7 @@ import ( "unicode/utf8" "gopkg.in/gcfg.v1/types" + "gopkg.in/warnings.v0" ) type tag struct { @@ -189,7 +192,30 @@ func scanSetter(d interface{}, blank bool, val string, tt tag) error { return types.ScanFully(d, val, 'v') } -func set(cfg interface{}, sect, sub, name string, blank bool, value string) error { +func newValue(sect string, vCfg reflect.Value, vType reflect.Type) (reflect.Value, error) { + pv := reflect.New(vType) + dfltName := "default-" + sect + dfltField, _ := fieldFold(vCfg, dfltName) + var err error + if dfltField.IsValid() { + b := bytes.NewBuffer(nil) + ge := gob.NewEncoder(b) + err = ge.EncodeValue(dfltField) + if err != nil { + return pv, err + } + gd := gob.NewDecoder(bytes.NewReader(b.Bytes())) + err = gd.DecodeValue(pv.Elem()) + if err != nil { + return pv, err + } + } + return pv, nil +} + +func set(c *warnings.Collector, cfg interface{}, sect, sub, name string, + blank bool, value string, subsectPass bool) error { + // vPCfg := reflect.ValueOf(cfg) if vPCfg.Kind() != reflect.Ptr || vPCfg.Elem().Kind() != reflect.Struct { panic(fmt.Errorf("config must be a pointer to a struct")) @@ -197,9 +223,14 @@ func set(cfg interface{}, sect, sub, name string, blank bool, value string) erro vCfg := vPCfg.Elem() vSect, _ := fieldFold(vCfg, sect) if !vSect.IsValid() { - return fmt.Errorf("invalid section: section %q", sect) + err := extraData{section: sect} + return c.Collect(err) } - if vSect.Kind() == reflect.Map { + isSubsect := vSect.Kind() == reflect.Map + if subsectPass != isSubsect { + return nil + } + if isSubsect { vst := vSect.Type() if vst.Key().Kind() != reflect.String || vst.Elem().Kind() != reflect.Ptr || @@ -214,7 +245,11 @@ func set(cfg interface{}, sect, sub, name string, blank bool, value string) erro pv := vSect.MapIndex(k) if !pv.IsValid() { vType := vSect.Type().Elem().Elem() - pv = reflect.New(vType) + var err error + pv, err = newValue(sect, vCfg, vType) + if err != nil { + return err + } vSect.SetMapIndex(k, pv) } vSect = pv.Elem() @@ -222,8 +257,8 @@ func set(cfg interface{}, sect, sub, name string, blank bool, value string) erro panic(fmt.Errorf("field for section must be a map or a struct: "+ "section %q", sect)) } else if sub != "" { - return fmt.Errorf("invalid subsection: "+ - "section %q subsection %q", sect, sub) + err := extraData{section: sect, subsection: &sub} + return c.Collect(err) } // Empty name is a special value, meaning that only the // section/subsection object is to be created, with no values set. @@ -232,8 +267,13 @@ func set(cfg interface{}, sect, sub, name string, blank bool, value string) erro } vVar, t := fieldFold(vSect, name) if !vVar.IsValid() { - return fmt.Errorf("invalid variable: "+ - "section %q subsection %q variable %q", sect, sub, name) + var err error + if isSubsect { + err = extraData{section: sect, subsection: &sub, variable: &name} + } else { + err = extraData{section: sect, variable: &name} + } + return c.Collect(err) } // vVal is either single-valued var, or newly allocated value within multi-valued var var vVal reflect.Value diff --git a/vendor/gopkg.in/warnings.v0/BUILD b/vendor/gopkg.in/warnings.v0/BUILD new file mode 100644 index 00000000000..c2d0a12d102 --- /dev/null +++ b/vendor/gopkg.in/warnings.v0/BUILD @@ -0,0 +1,27 @@ +package(default_visibility = ["//visibility:public"]) + +licenses(["notice"]) + +load( + "@io_bazel_rules_go//go:def.bzl", + "go_library", +) + +go_library( + name = "go_default_library", + srcs = ["warnings.go"], + tags = ["automanaged"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], +) diff --git a/vendor/gopkg.in/warnings.v0/LICENSE b/vendor/gopkg.in/warnings.v0/LICENSE new file mode 100644 index 00000000000..d65f7e9d8cd --- /dev/null +++ b/vendor/gopkg.in/warnings.v0/LICENSE @@ -0,0 +1,24 @@ +Copyright (c) 2016 Péter Surányi. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/gopkg.in/warnings.v0/README b/vendor/gopkg.in/warnings.v0/README new file mode 100644 index 00000000000..77f46dd2c69 --- /dev/null +++ b/vendor/gopkg.in/warnings.v0/README @@ -0,0 +1,74 @@ +Package warnings implements error handling with non-fatal errors (warnings). + +import path: "gopkg.in/warnings.v0" +package docs: https://godoc.org/gopkg.in/warnings.v0 +issues: https://github.com/go-warnings/warnings/issues +pull requests: https://github.com/go-warnings/warnings/pulls + +A recurring pattern in Go programming is the following: + + func myfunc(params) error { + if err := doSomething(...); err != nil { + return err + } + if err := doSomethingElse(...); err != nil { + return err + } + if ok := doAnotherThing(...); !ok { + return errors.New("my error") + } + ... + return nil + } + +This pattern allows interrupting the flow on any received error. But what if +there are errors that should be noted but still not fatal, for which the flow +should not be interrupted? Implementing such logic at each if statement would +make the code complex and the flow much harder to follow. + +Package warnings provides the Collector type and a clean and simple pattern +for achieving such logic. The Collector takes care of deciding when to break +the flow and when to continue, collecting any non-fatal errors (warnings) +along the way. The only requirement is that fatal and non-fatal errors can be +distinguished programmatically; that is a function such as + + IsFatal(error) bool + +must be implemented. The following is an example of what the above snippet +could look like using the warnings package: + + import "gopkg.in/warnings.v0" + + func isFatal(err error) bool { + _, ok := err.(WarningType) + return !ok + } + + func myfunc(params) error { + c := warnings.NewCollector(isFatal) + c.FatalWithWarnings = true + if err := c.Collect(doSomething()); err != nil { + return err + } + if err := c.Collect(doSomethingElse(...)); err != nil { + return err + } + if ok := doAnotherThing(...); !ok { + if err := c.Collect(errors.New("my error")); err != nil { + return err + } + } + ... + return c.Done() + } + +Rules for using warnings + + - ensure that warnings are programmatically distinguishable from fatal + errors (i.e. implement an isFatal function and any necessary error types) + - ensure that there is a single Collector instance for a call of each + exported function + - ensure that all errors (fatal or warning) are fed through Collect + - ensure that every time an error is returned, it is one returned by a + Collector (from Collect or Done) + - ensure that Collect is never called after Done diff --git a/vendor/gopkg.in/warnings.v0/warnings.go b/vendor/gopkg.in/warnings.v0/warnings.go new file mode 100644 index 00000000000..0e678004849 --- /dev/null +++ b/vendor/gopkg.in/warnings.v0/warnings.go @@ -0,0 +1,191 @@ +// Package warnings implements error handling with non-fatal errors (warnings). +// +// A recurring pattern in Go programming is the following: +// +// func myfunc(params) error { +// if err := doSomething(...); err != nil { +// return err +// } +// if err := doSomethingElse(...); err != nil { +// return err +// } +// if ok := doAnotherThing(...); !ok { +// return errors.New("my error") +// } +// ... +// return nil +// } +// +// This pattern allows interrupting the flow on any received error. But what if +// there are errors that should be noted but still not fatal, for which the flow +// should not be interrupted? Implementing such logic at each if statement would +// make the code complex and the flow much harder to follow. +// +// Package warnings provides the Collector type and a clean and simple pattern +// for achieving such logic. The Collector takes care of deciding when to break +// the flow and when to continue, collecting any non-fatal errors (warnings) +// along the way. The only requirement is that fatal and non-fatal errors can be +// distinguished programmatically; that is a function such as +// +// IsFatal(error) bool +// +// must be implemented. The following is an example of what the above snippet +// could look like using the warnings package: +// +// import "gopkg.in/warnings.v0" +// +// func isFatal(err error) bool { +// _, ok := err.(WarningType) +// return !ok +// } +// +// func myfunc(params) error { +// c := warnings.NewCollector(isFatal) +// c.FatalWithWarnings = true +// if err := c.Collect(doSomething()); err != nil { +// return err +// } +// if err := c.Collect(doSomethingElse(...)); err != nil { +// return err +// } +// if ok := doAnotherThing(...); !ok { +// if err := c.Collect(errors.New("my error")); err != nil { +// return err +// } +// } +// ... +// return c.Done() +// } +// +// Rules for using warnings +// +// - ensure that warnings are programmatically distinguishable from fatal +// errors (i.e. implement an isFatal function and any necessary error types) +// - ensure that there is a single Collector instance for a call of each +// exported function +// - ensure that all errors (fatal or warning) are fed through Collect +// - ensure that every time an error is returned, it is one returned by a +// Collector (from Collect or Done) +// - ensure that Collect is never called after Done +// +// TODO +// +// - optionally limit the number of warnings (e.g. stop after 20 warnings) (?) +// - consider interaction with contexts +// - go vet-style invocations verifier +// - semi-automatic code converter +// +package warnings + +import ( + "bytes" + "fmt" +) + +// List holds a collection of warnings and optionally one fatal error. +type List struct { + Warnings []error + Fatal error +} + +// Error implements the error interface. +func (l List) Error() string { + b := bytes.NewBuffer(nil) + if l.Fatal != nil { + fmt.Fprintln(b, "fatal:") + fmt.Fprintln(b, l.Fatal) + } + switch len(l.Warnings) { + case 0: + // nop + case 1: + fmt.Fprintln(b, "warning:") + default: + fmt.Fprintln(b, "warnings:") + } + for _, err := range l.Warnings { + fmt.Fprintln(b, err) + } + return b.String() +} + +// A Collector collects errors up to the first fatal error. +type Collector struct { + // IsFatal distinguishes between warnings and fatal errors. + IsFatal func(error) bool + // FatalWithWarnings set to true means that a fatal error is returned as + // a List together with all warnings so far. The default behavior is to + // only return the fatal error and discard any warnings that have been + // collected. + FatalWithWarnings bool + + l List + done bool +} + +// NewCollector returns a new Collector; it uses isFatal to distinguish between +// warnings and fatal errors. +func NewCollector(isFatal func(error) bool) *Collector { + return &Collector{IsFatal: isFatal} +} + +// Collect collects a single error (warning or fatal). It returns nil if +// collection can continue (only warnings so far), or otherwise the errors +// collected. Collect mustn't be called after the first fatal error or after +// Done has been called. +func (c *Collector) Collect(err error) error { + if c.done { + panic("warnings.Collector already done") + } + if err == nil { + return nil + } + if c.IsFatal(err) { + c.done = true + c.l.Fatal = err + } else { + c.l.Warnings = append(c.l.Warnings, err) + } + if c.l.Fatal != nil { + return c.erorr() + } + return nil +} + +// Done ends collection and returns the collected error(s). +func (c *Collector) Done() error { + c.done = true + return c.erorr() +} + +func (c *Collector) erorr() error { + if !c.FatalWithWarnings && c.l.Fatal != nil { + return c.l.Fatal + } + if c.l.Fatal == nil && len(c.l.Warnings) == 0 { + return nil + } + // Note that a single warning is also returned as a List. This is to make it + // easier to determine fatal-ness of the returned error. + return c.l +} + +// FatalOnly returns the fatal error, if any, **in an error returned by a +// Collector**. It returns nil if and only if err is nil or err is a List +// with err.Fatal == nil. +func FatalOnly(err error) error { + l, ok := err.(List) + if !ok { + return err + } + return l.Fatal +} + +// WarningsOnly returns the warnings **in an error returned by a Collector**. +func WarningsOnly(err error) []error { + l, ok := err.(List) + if !ok { + return nil + } + return l.Warnings +} From cde038b9be44bfdf64a48cd47307bbf61952fe35 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Wed, 26 Jul 2017 08:52:55 -0700 Subject: [PATCH 2/3] Wrap gce.conf parse with FatalOnly error filter --- pkg/cloudprovider/providers/gce/gce.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index bf5f1ee0dbb..8cdc89a95c3 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -199,7 +199,7 @@ func newGCECloud(config io.Reader) (*GCECloud, error) { var nodeInstancePrefix string if config != nil { var cfg Config - if err := gcfg.ReadInto(&cfg, config); err != nil { + if err := gcfg.FatalOnly(gcfg.ReadInto(&cfg, config)); err != nil { glog.Errorf("Couldn't read config: %v", err) return nil, err } From 3f01685943b9cfa2b548ec98c2d1f1ae2a2c13d7 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Wed, 26 Jul 2017 12:22:37 -0700 Subject: [PATCH 3/3] Unit test unknown value in config --- pkg/cloudprovider/providers/gce/gce.go | 15 ++++++++++++--- pkg/cloudprovider/providers/gce/gce_test.go | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index 8cdc89a95c3..aaa7101c1a4 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -198,11 +198,11 @@ func newGCECloud(config io.Reader) (*GCECloud, error) { var nodeTags []string var nodeInstancePrefix string if config != nil { - var cfg Config - if err := gcfg.FatalOnly(gcfg.ReadInto(&cfg, config)); err != nil { - glog.Errorf("Couldn't read config: %v", err) + cfg, err := readConfig(config) + if err != nil { return nil, err } + glog.Infof("Using GCE provider config %+v", cfg) if cfg.Global.ApiEndpoint != "" { apiEndpoint = cfg.Global.ApiEndpoint @@ -241,6 +241,15 @@ func newGCECloud(config io.Reader) (*GCECloud, error) { nodeTags, nodeInstancePrefix, tokenSource, true /* useMetadataServer */) } +func readConfig(reader io.Reader) (*Config, error) { + cfg := &Config{} + if err := gcfg.FatalOnly(gcfg.ReadInto(cfg, reader)); err != nil { + glog.Errorf("Couldn't read config: %v", err) + return nil, err + } + return cfg, nil +} + // Creates a GCECloud object using the specified parameters. // If no networkUrl is specified, loads networkName via rest call. // If no tokenSource is specified, uses oauth2.DefaultTokenSource. diff --git a/pkg/cloudprovider/providers/gce/gce_test.go b/pkg/cloudprovider/providers/gce/gce_test.go index c04473f9380..0eff53dbdf7 100644 --- a/pkg/cloudprovider/providers/gce/gce_test.go +++ b/pkg/cloudprovider/providers/gce/gce_test.go @@ -18,9 +18,26 @@ package gce import ( "reflect" + "strings" "testing" ) +func TestExtraKeyInConfig(t *testing.T) { + const s = `[Global] +project-id = my-project +unknown-key = abc +network-name = my-network + ` + reader := strings.NewReader(s) + config, err := readConfig(reader) + if err != nil { + t.Fatalf("Unexpected config parsing error %v", err) + } + if config.Global.ProjectID != "my-project" || config.Global.NetworkName != "my-network" { + t.Fatalf("Expected config values to continue to be read despite extra key-value pair.") + } +} + func TestGetRegion(t *testing.T) { zoneName := "us-central1-b" regionName, err := GetGCERegion(zoneName)