From 21da625fd7bddda4e3ef6130c13b9e5f9555c4c5 Mon Sep 17 00:00:00 2001 From: urso Date: Wed, 8 Nov 2017 16:45:46 +0100 Subject: [PATCH] Improve custom config flag handlers - move support for flags collecting usage into a []string to libbeat/common - fix printing default value of `-c` flag - move flags manipulating config objects from config.go to flags.go - change flag namings to always end with xxxFlag - introduce unit tests for config flags - check the flags usage string in unit tests - fix potential panic when printing defaults on flag collecting into []string (nil pointer being passed) - fix printing defaults and setting types - add `Type` method improving cobra flag parameter in help message Old flags usage messages for `filebeat -h`: ``` -E, --E Flag Configuration overwrite (default null) -M, --M Flag Module configuration overwrite (default null) -N, --N Disable actual publishing for testing -c, --c argList Configuration file, relative to path.config (default beat.yml) --cpuprofile string Write cpu profile to file -d, --d string Enable certain debug selectors -e, --e Log to stderr and disable syslog/file output -h, --help help for filebeat --httpprof string Start pprof http server --memprofile string Write memory profile to this file --modules string List of enabled modules (comma separated) --once Run filebeat only once until all harvesters reach EOF --path.config flagOverwrite Configuration path --path.data flagOverwrite Data path --path.home flagOverwrite Home path --path.logs flagOverwrite Logs path --setup Load the sample Kibana dashboards --strict.perms Strict permission checking on config files (default true) -v, --v Log at INFO level ``` New flags usage messages for `filebeat -h`: ``` -E, --E setting=value Configuration overwrite -M, --M setting=value Module configuration overwrite -N, --N Disable actual publishing for testing -c, --c string Configuration file, relative to path.config (default "filebeat.yml") --cpuprofile string Write cpu profile to file -d, --d string Enable certain debug selectors -e, --e Log to stderr and disable syslog/file output -h, --help help for filebeat --httpprof string Start pprof http server --memprofile string Write memory profile to this file --modules string List of enabled modules (comma separated) --once Run filebeat only once until all harvesters reach EOF --path.config string Configuration path --path.data string Data path --path.home string Home path --path.logs string Logs path --setup Load the sample Kibana dashboards --strict.perms Strict permission checking on config files (default true) -v, --v Log at INFO level ``` --- CHANGELOG.asciidoc | 1 + filebeat/fileset/flags.go | 2 +- libbeat/cfgfile/cfgfile.go | 8 +- libbeat/cfgfile/flags.go | 58 -------- libbeat/cmd/instance/beat.go | 6 - libbeat/cmd/root.go | 10 ++ libbeat/common/config.go | 92 +----------- libbeat/common/flags.go | 273 +++++++++++++++++++++++++++++++++++ libbeat/common/flags_test.go | 177 +++++++++++++++++++++++ 9 files changed, 469 insertions(+), 158 deletions(-) delete mode 100644 libbeat/cfgfile/flags.go create mode 100644 libbeat/common/flags.go create mode 100644 libbeat/common/flags_test.go diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 6aa5f2107b2..cd344b4eedc 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -85,6 +85,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Add Azure VM support for add_cloud_metadata processor {pull}5355[5355] - Add `output.file.permission` config option. {pull}4638[4638] - Refactor add_kubernetes_metadata to support autodiscovery {pull}5434[5434] +- Improve custom flag handling and CLI flags usage message. {pull}5543[5543] *Auditbeat* diff --git a/filebeat/fileset/flags.go b/filebeat/fileset/flags.go index 816f27f3da0..9258d4c6d55 100644 --- a/filebeat/fileset/flags.go +++ b/filebeat/fileset/flags.go @@ -11,7 +11,7 @@ import ( // Modules related command line flags. var ( modulesFlag = flag.String("modules", "", "List of enabled modules (comma separated)") - moduleOverrides = common.NewFlagConfig(nil, nil, "M", "Module configuration overwrite") + moduleOverrides = common.SettingFlag(nil, "M", "Module configuration overwrite") ) type ModuleOverrides map[string]map[string]*common.Config // module -> fileset -> Config diff --git a/libbeat/cfgfile/cfgfile.go b/libbeat/cfgfile/cfgfile.go index 886e74197b9..66459e04bf6 100644 --- a/libbeat/cfgfile/cfgfile.go +++ b/libbeat/cfgfile/cfgfile.go @@ -15,8 +15,8 @@ var ( // The default config cannot include the beat name as it is not initialized // when this variable is created. See ChangeDefaultCfgfileFlag which should // be called prior to flags.Parse(). - configfiles = flagArgList("c", "beat.yml", "Configuration file, relative to path.config") - overwrites = common.NewFlagConfig(nil, nil, "E", "Configuration overwrite") + configfiles = common.StringArrFlag(nil, "c", "beat.yml", "Configuration file, relative to path.config") + overwrites = common.SettingFlag(nil, "E", "Configuration overwrite") testConfig = flag.Bool("configtest", false, "Test configuration and exit.") // Additional default settings, that must be available for variable expansion @@ -37,7 +37,7 @@ var ( func init() { // add '-path.x' options overwriting paths in 'overwrites' config makePathFlag := func(name, usage string) *string { - return common.NewFlagOverwrite(nil, overwrites, name, name, "", usage) + return common.ConfigOverwriteFlag(nil, overwrites, name, name, "", usage) } homePath = makePathFlag("path.home", "Home path") @@ -107,7 +107,7 @@ func Load(path string) (*common.Config, error) { if path == "" { list := []string{} - for _, cfg := range configfiles.list { + for _, cfg := range configfiles.List() { if !filepath.IsAbs(cfg) { list = append(list, filepath.Join(cfgpath, cfg)) } else { diff --git a/libbeat/cfgfile/flags.go b/libbeat/cfgfile/flags.go deleted file mode 100644 index 53af53f342a..00000000000 --- a/libbeat/cfgfile/flags.go +++ /dev/null @@ -1,58 +0,0 @@ -package cfgfile - -import ( - "flag" - "strings" -) - -type argList struct { - list []string - isDefault bool - f *flag.Flag -} - -func flagArgList(name string, def string, usage string) *argList { - l := &argList{ - list: []string{def}, - isDefault: true, - } - flag.Var(l, name, usage) - l.f = flag.Lookup(name) - if l.f == nil { - panic("Failed to lookup registered flag") - } - return l -} - -func (l *argList) SetDefault(v string) { - l.f.DefValue = v - // Only update value if we are still in the default - if l.isDefault { - l.list = []string{v} - l.isDefault = true - } -} - -func (l *argList) String() string { - return strings.Join(l.list, ", ") -} - -func (l *argList) Set(v string) error { - if l.isDefault { - l.list = []string{v} - } else { - // Ignore duplicates, can be caused by multiple flag parses - for _, f := range l.list { - if f == v { - return nil - } - } - l.list = append(l.list, v) - } - l.isDefault = false - return nil -} - -func (l *argList) Get() interface{} { - return l.list -} diff --git a/libbeat/cmd/instance/beat.go b/libbeat/cmd/instance/beat.go index 7da448f2d1c..b3da62d8830 100644 --- a/libbeat/cmd/instance/beat.go +++ b/libbeat/cmd/instance/beat.go @@ -364,12 +364,6 @@ func (b *Beat) Setup(bt beat.Creator, template, dashboards, machineLearning bool // handleFlags parses the command line flags. It handles the '-version' flag // and invokes the HandleFlags callback if implemented by the Beat. func (b *Beat) handleFlags() error { - // Due to a dependence upon the beat name, the default config file path - // must be updated prior to CLI flag handling. - err := cfgfile.ChangeDefaultCfgfileFlag(b.Info.Beat) - if err != nil { - return fmt.Errorf("failed to set default config file path: %v", err) - } flag.Parse() if printVersion { diff --git a/libbeat/cmd/root.go b/libbeat/cmd/root.go index 9bc0a9b226d..aefada18e63 100644 --- a/libbeat/cmd/root.go +++ b/libbeat/cmd/root.go @@ -2,6 +2,7 @@ package cmd import ( "flag" + "fmt" "os" "strings" @@ -9,6 +10,7 @@ import ( "github.com/spf13/pflag" "github.com/elastic/beats/libbeat/beat" + "github.com/elastic/beats/libbeat/cfgfile" ) func init() { @@ -51,6 +53,14 @@ func GenRootCmdWithIndexPrefixWithRunFlags(name, indexPrefix, version string, be rootCmd := &BeatsRootCmd{} rootCmd.Use = name + // Due to a dependence upon the beat name, the default config file path + err := cfgfile.ChangeDefaultCfgfileFlag(name) + if err != nil { + panic(fmt.Errorf("failed to set default config file path: %v", err)) + } + + // must be updated prior to CLI flag handling. + rootCmd.RunCmd = genRunCmd(name, indexPrefix, version, beatCreator, runFlags) rootCmd.SetupCmd = genSetupCmd(name, indexPrefix, version, beatCreator) rootCmd.VersionCmd = genVersionCmd(name, version) diff --git a/libbeat/common/config.go b/libbeat/common/config.go index 137cab5273c..072e3962c2a 100644 --- a/libbeat/common/config.go +++ b/libbeat/common/config.go @@ -10,12 +10,12 @@ import ( "runtime" "strings" + ucfg "github.com/elastic/go-ucfg" + "github.com/elastic/go-ucfg/yaml" + "github.com/elastic/beats/libbeat/common/file" "github.com/elastic/beats/libbeat/logp" - ucfg "github.com/elastic/go-ucfg" "github.com/elastic/go-ucfg/cfgutil" - cfgflag "github.com/elastic/go-ucfg/flag" - "github.com/elastic/go-ucfg/yaml" ) var flagStrictPerms = flag.Bool("strict.perms", true, "Strict permission checking on config files") @@ -39,12 +39,6 @@ type ConfigNamespace struct { config *Config } -type flagOverwrite struct { - config *ucfg.Config - path string - value string -} - var configOpts = []ucfg.Option{ ucfg.PathSep("."), ucfg.ResolveEnv, @@ -102,62 +96,6 @@ func NewConfigWithYAML(in []byte, source string) (*Config, error) { return fromConfig(c), err } -func NewFlagConfig( - set *flag.FlagSet, - def *Config, - name string, - usage string, -) *Config { - opts := append( - []ucfg.Option{ - ucfg.MetaData(ucfg.Meta{Source: "command line flag"}), - }, - configOpts..., - ) - - var to *ucfg.Config - if def != nil { - to = def.access() - } - - config := cfgflag.ConfigVar(set, to, name, usage, opts...) - return fromConfig(config) -} - -func NewFlagOverwrite( - set *flag.FlagSet, - config *Config, - name, path, def, usage string, -) *string { - if config == nil { - panic("Missing configuration") - } - if path == "" { - panic("empty path") - } - - if def != "" { - err := config.SetString(path, -1, def) - if err != nil { - panic(err) - } - } - - f := &flagOverwrite{ - config: config.access(), - path: path, - value: def, - } - - if set == nil { - flag.Var(f, name, usage) - } else { - set.Var(f, name, usage) - } - - return &f.value -} - func LoadFile(path string) (*Config, error) { if IsStrictPerms() { if err := ownerHasExclusiveWritePerms(path); err != nil { @@ -304,30 +242,6 @@ func (c *Config) GetFields() []string { return c.access().GetFields() } -func (f *flagOverwrite) String() string { - return f.value -} - -func (f *flagOverwrite) Set(v string) error { - opts := append( - []ucfg.Option{ - ucfg.MetaData(ucfg.Meta{Source: "command line flag"}), - }, - configOpts..., - ) - - err := f.config.SetString(f.path, -1, v, opts...) - if err != nil { - return err - } - f.value = v - return nil -} - -func (f *flagOverwrite) Get() interface{} { - return f.value -} - // Unpack unpacks a configuration with at most one sub object. An sub object is // ignored if it is disabled by setting `enabled: false`. If the configuration // passed contains multiple active sub objects, Unpack will return an error. diff --git a/libbeat/common/flags.go b/libbeat/common/flags.go new file mode 100644 index 00000000000..a78201d6b3f --- /dev/null +++ b/libbeat/common/flags.go @@ -0,0 +1,273 @@ +package common + +import ( + "flag" + "strings" + + ucfg "github.com/elastic/go-ucfg" + cfgflag "github.com/elastic/go-ucfg/flag" +) + +// StringsFlag collects multiple usages of the same flag into an array of strings. +// Duplicate values will be ignored. +type StringsFlag struct { + list *[]string + isDefault bool + flag *flag.Flag +} + +// SettingsFlag captures key/values pairs into an Config object. +// The flag backed by SettingsFlag can be used multiple times. +// Values are overwritten by the last usage of a key. +type SettingsFlag cfgflag.FlagValue + +// flagOverwrite provides a flag value, which always overwrites the same setting +// in an Config object. +type flagOverwrite struct { + config *ucfg.Config + path string + value string +} + +// StringArrFlag creates and registers a new StringsFlag with the given FlagSet. +// If no FlagSet is passed, flag.CommandLine will be used as target FlagSet. +func StringArrFlag(fs *flag.FlagSet, name, def, usage string) *StringsFlag { + var arr *[]string + if def != "" { + arr = &[]string{def} + } else { + arr = &[]string{} + } + + return StringArrVarFlag(fs, arr, name, usage) +} + +// StringArrVarFlag creates and registers a new StringsFlag with the given +// FlagSet. Results of the flag usage will be appended to `arr`. If the slice +// is not initially empty, its first value will be used as default. If the flag +// is used, the slice will be emptied first. If no FlagSet is passed, +// flag.CommandLine will be used as target FlagSet. +func StringArrVarFlag(fs *flag.FlagSet, arr *[]string, name, usage string) *StringsFlag { + if fs == nil { + fs = flag.CommandLine + } + f := NewStringsFlag(arr) + f.Register(fs, name, usage) + return f +} + +// NewStringsFlag creates a new, but unregistered StringsFlag instance. +// Results of the flag usage will be appended to `arr`. If the slice is not +// initially empty, its first value will be used as default. If the flag is +// used, the slice will be emptied first. +func NewStringsFlag(arr *[]string) *StringsFlag { + if arr == nil { + panic("No target array") + } + return &StringsFlag{list: arr, isDefault: true} +} + +// Register registers the StringsFlag instance with a FlagSet. +// A valid FlagSet must be used. +// Register panics if the flag is already registered. +func (f *StringsFlag) Register(fs *flag.FlagSet, name, usage string) { + if f.flag != nil { + panic("StringsFlag is already registered") + } + + fs.Var(f, name, usage) + f.flag = fs.Lookup(name) + if f.flag == nil { + panic("Failed to lookup registered flag") + } + + if len(*f.list) > 0 { + f.flag.DefValue = (*f.list)[0] + } +} + +// String joins all it's values set into a comma-separated string. +func (f *StringsFlag) String() string { + if f == nil || f.list == nil { + return "" + } + + l := *f.list + return strings.Join(l, ", ") +} + +// SetDefault sets the flags new default value. +// This overwrites the contents in the backing array. +func (f *StringsFlag) SetDefault(v string) { + if f.flag != nil { + f.flag.DefValue = v + } + + *f.list = []string{v} + f.isDefault = true +} + +// Set is used to pass usage of the flag to StringsFlag. Set adds the new value +// to the backing array. The array will be emptied on Set, if the backing array +// still contains the default value. +func (f *StringsFlag) Set(v string) error { + // Ignore duplicates, can be caused by multiple flag parses + if f.isDefault { + *f.list = []string{v} + } else { + for _, old := range *f.list { + if old == v { + return nil + } + } + *f.list = append(*f.list, v) + } + f.isDefault = false + return nil +} + +// Get returns the backing slice its contents as interface{}. The type used is +// `[]string`. +func (f *StringsFlag) Get() interface{} { + return f.List() +} + +// List returns the current set values. +func (f *StringsFlag) List() []string { + return *f.list +} + +// Type reports the type of contents (string) expected to be parsed by Set. +// It is used to build the CLI usage string. +func (f *StringsFlag) Type() string { + return "string" +} + +// SettingFlag defines a setting flag, name and it's usage. The return value is +// the Config object settings are applied to. +func SettingFlag(fs *flag.FlagSet, name, usage string) *Config { + cfg := NewConfig() + SettingVarFlag(fs, cfg, name, usage) + return cfg +} + +// SettingVarFlag defines a setting flag, name and it's usage. +// Settings are applied to the Config object passed. +func SettingVarFlag(fs *flag.FlagSet, def *Config, name, usage string) { + if fs == nil { + fs = flag.CommandLine + } + + f := NewSettingsFlag(def) + fs.Var(f, name, usage) +} + +// NewSettingsFlag creates a new SettingsFlag instance, not registered with any +// FlagSet. +func NewSettingsFlag(def *Config) *SettingsFlag { + opts := append( + []ucfg.Option{ + ucfg.MetaData(ucfg.Meta{Source: "command line flag"}), + }, + configOpts..., + ) + + tmp := cfgflag.NewFlagKeyValue(def.access(), true, opts...) + return (*SettingsFlag)(tmp) +} + +func (f *SettingsFlag) access() *cfgflag.FlagValue { + return (*cfgflag.FlagValue)(f) +} + +// Config returns the config object the SettingsFlag stores applied settings to. +func (f *SettingsFlag) Config() *Config { + return fromConfig(f.access().Config()) +} + +// Set sets a settings value in the Config object. The input string must be a +// key-value pair like `key=value`. If the value is missing, the value is set +// to the boolean value `true`. +func (f *SettingsFlag) Set(s string) error { + return f.access().Set(s) +} + +// Get returns the Config object used to store values. +func (f *SettingsFlag) Get() interface{} { + return f.Config() +} + +// String always returns an empty string. It is required to fulfil +// the flag.Value interface. +func (f *SettingsFlag) String() string { + return "" +} + +// Type reports the type of contents (setting=value) expected to be parsed by Set. +// It is used to build the CLI usage string. +func (f *SettingsFlag) Type() string { + return "setting=value" +} + +// ConfigOverwriteFlag defines a new flag updating a setting in an Config +// object. The name is used as the flag its name the path parameter is the +// full setting name to be used when the flag is set. +func ConfigOverwriteFlag( + fs *flag.FlagSet, + config *Config, + name, path, def, usage string, +) *string { + if config == nil { + panic("Missing configuration") + } + if path == "" { + panic("empty path") + } + + if fs == nil { + fs = flag.CommandLine + } + + if def != "" { + err := config.SetString(path, -1, def) + if err != nil { + panic(err) + } + } + + f := newOverwriteFlag(config, path, def) + fs.Var(f, name, usage) + return &f.value +} + +func newOverwriteFlag(config *Config, path, def string) *flagOverwrite { + return &flagOverwrite{config: config.access(), path: path, value: def} +} + +func (f *flagOverwrite) String() string { + return f.value +} + +func (f *flagOverwrite) Set(v string) error { + opts := append( + []ucfg.Option{ + ucfg.MetaData(ucfg.Meta{Source: "command line flag"}), + }, + configOpts..., + ) + + err := f.config.SetString(f.path, -1, v, opts...) + if err != nil { + return err + } + f.value = v + return nil +} + +func (f *flagOverwrite) Get() interface{} { + return f.value +} + +func (f *flagOverwrite) Type() string { + return "string" +} diff --git a/libbeat/common/flags_test.go b/libbeat/common/flags_test.go new file mode 100644 index 00000000000..d77a9882e4f --- /dev/null +++ b/libbeat/common/flags_test.go @@ -0,0 +1,177 @@ +package common + +import ( + "bytes" + "flag" + "fmt" + "io" + "os" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" +) + +func TestStringArrFlag(t *testing.T) { + tests := []struct { + init []string + def string + in []string + expected []string + }{ + {nil, "test", nil, []string{"test"}}, + {nil, "test", []string{"new"}, []string{"new"}}, + {nil, "test", []string{"a", "b"}, []string{"a", "b"}}, + {[]string{"default"}, "newdefault", nil, []string{"newdefault"}}, + {[]string{"default"}, "newdefault", []string{"arg"}, []string{"arg"}}, + {[]string{"default"}, "newdefault", []string{"a", "b"}, []string{"a", "b"}}, + {[]string{"default"}, "newdefault", []string{"a", "b", "a", "b"}, []string{"a", "b"}}, + } + + for _, test := range tests { + test := test + name := fmt.Sprintf("init=%v,default=%v,in=%v,out=%v", test.init, test.def, test.in, test.expected) + + t.Run(name, func(t *testing.T) { + init := make([]string, len(test.init)) + copy(init, test.init) + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + flag := StringArrVarFlag(fs, &init, "a", "add") + + if test.def != "" { + flag.SetDefault(test.def) + } + + defaultValue := flag.String() + + goflagUsage, _ := withStderr(fs.PrintDefaults) + goflagExpectedUsage := fmt.Sprintf(" -a value\n \tadd (default %v)\n", defaultValue) + + cmd := cobra.Command{} + cmd.PersistentFlags().AddGoFlag(fs.Lookup("a")) + cobraUsage := cmd.LocalFlags().FlagUsages() + cobraExpectedUsage := fmt.Sprintf(" -a, --a string add (default \"%v\")\n", defaultValue) + + for _, v := range test.in { + err := flag.Set(v) + if err != nil { + t.Error(err) + } + } + + assert.Equal(t, goflagExpectedUsage, goflagUsage) + assert.Equal(t, cobraExpectedUsage, cobraUsage) + assert.Equal(t, test.expected, init) + assert.Equal(t, test.expected, flag.List()) + }) + } +} + +func TestSettingsFlag(t *testing.T) { + tests := []struct { + in []string + expected map[string]interface{} + }{ + {nil, nil}, + {[]string{"a=1"}, map[string]interface{}{"a": uint64(1)}}, + {[]string{"a=1", "b=false"}, map[string]interface{}{"a": uint64(1), "b": false}}, + {[]string{"a=1", "b"}, map[string]interface{}{"a": uint64(1), "b": true}}, + {[]string{"a=1", "c=${a}"}, map[string]interface{}{"a": uint64(1), "c": uint64(1)}}, + } + + for _, test := range tests { + test := test + name := strings.Join(test.in, ",") + + t.Run(name, func(t *testing.T) { + config := NewConfig() + f := NewSettingsFlag(config) + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + fs.Var(f, "s", "message") + + goflagUsage, _ := withStderr(fs.PrintDefaults) + goflagExpectedUsage := " -s value\n \tmessage\n" + + cmd := cobra.Command{} + cmd.PersistentFlags().AddGoFlag(fs.Lookup("s")) + cobraUsage := cmd.LocalFlags().FlagUsages() + cobraExpectedUsage := " -s, --s setting=value message\n" + + for _, in := range test.in { + err := f.Set(in) + if err != nil { + t.Error(err) + } + } + + var result map[string]interface{} + err := config.Unpack(&result) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, goflagExpectedUsage, goflagUsage) + assert.Equal(t, cobraExpectedUsage, cobraUsage) + assert.Equal(t, test.expected, result) + }) + } +} + +func TestOverwriteFlag(t *testing.T) { + config, err := NewConfigFrom(map[string]interface{}{ + "a": "test", + }) + if err != nil { + panic(err) + } + + fs := flag.NewFlagSet("test", flag.ContinueOnError) + ConfigOverwriteFlag(fs, config, "a", "a", "", "message") + + goflagUsage, _ := withStderr(fs.PrintDefaults) + goflagExpectedUsage := " -a value\n \tmessage\n" + assert.Equal(t, goflagExpectedUsage, goflagUsage) + + cmd := cobra.Command{} + cmd.PersistentFlags().AddGoFlag(fs.Lookup("a")) + cobraUsage := cmd.LocalFlags().FlagUsages() + cobraExpectedUsage := " -a, --a string message\n" + assert.Equal(t, cobraExpectedUsage, cobraUsage) + + fs.Set("a", "overwrite") + final, err := config.String("a", -1) + assert.NoError(t, err) + assert.Equal(t, "overwrite", final) +} + +// capture stderr and return captured string +func withStderr(fn func()) (string, error) { + stderr := os.Stderr + + r, w, err := os.Pipe() + if err != nil { + return "", err + } + + os.Stderr = w + defer func() { + os.Stderr = stderr + }() + + outC := make(chan string) + go func() { + // capture all output + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + r.Close() + outC <- buf.String() + }() + + fn() + w.Close() + result := <-outC + return result, err +}