From 4e857ea1339e4575425b3c558926bd5f43ad23a1 Mon Sep 17 00:00:00 2001 From: Michal Niewrzal Date: Mon, 7 Jan 2019 12:06:10 +0100 Subject: [PATCH] Add `setup` tag for config structs (#968) * Fix ignored setup arguments * fix linter errors * small params cleanup * fix integration tests * cleanup in configs * Add `setup` tag for config structs * fix broken if statement * cleanup captplanet config * remove reduntant return * add missing Signer config * review comments * local variable renamed * remove unused var --- cmd/captplanet/main.go | 61 ++++++++++++++++++++++++++++++++++++++ cmd/captplanet/run.go | 47 ----------------------------- cmd/captplanet/setup.go | 22 ++------------ cmd/satellite/main.go | 19 +++++------- cmd/storagenode/main.go | 21 +++++-------- cmd/uplink/cmd/setup.go | 16 +++++----- pkg/cfgstruct/bind.go | 64 +++++++++++++++++++++++++++------------- pkg/process/exec_conf.go | 5 ++++ 8 files changed, 134 insertions(+), 121 deletions(-) diff --git a/cmd/captplanet/main.go b/cmd/captplanet/main.go index 19a175eed..027a137b4 100644 --- a/cmd/captplanet/main.go +++ b/cmd/captplanet/main.go @@ -18,10 +18,71 @@ import ( "storj.io/storj/internal/fpath" "storj.io/storj/internal/memory" + "storj.io/storj/pkg/accounting/rollup" + "storj.io/storj/pkg/accounting/tally" + "storj.io/storj/pkg/audit" + "storj.io/storj/pkg/bwagreement" "storj.io/storj/pkg/cfgstruct" + "storj.io/storj/pkg/datarepair/checker" + "storj.io/storj/pkg/datarepair/repairer" + "storj.io/storj/pkg/discovery" + "storj.io/storj/pkg/inspector" + "storj.io/storj/pkg/kademlia" + "storj.io/storj/pkg/miniogw" + "storj.io/storj/pkg/overlay" + "storj.io/storj/pkg/piecestore/psserver" + "storj.io/storj/pkg/pointerdb" "storj.io/storj/pkg/process" + "storj.io/storj/pkg/provider" + "storj.io/storj/pkg/satellite/satelliteweb" + "storj.io/storj/pkg/server" ) +// Captplanet defines Captain Planet configuration +type Captplanet struct { + SatelliteCA provider.CASetupConfig `setup:"true"` + SatelliteIdentity provider.IdentitySetupConfig `setup:"true"` + UplinkCA provider.CASetupConfig `setup:"true"` + UplinkIdentity provider.IdentitySetupConfig `setup:"true"` + StorageNodeCA provider.CASetupConfig `setup:"true"` + StorageNodeIdentity provider.IdentitySetupConfig `setup:"true"` + ListenHost string `help:"the host for providers to listen on" default:"127.0.0.1" setup:"true"` + StartingPort int `help:"all providers will listen on ports consecutively starting with this one" default:"7777" setup:"true"` + APIKey string `default:"abc123" help:"the api key to use for the satellite" setup:"true"` + EncKey string `default:"insecure-default-encryption-key" help:"your root encryption key" setup:"true"` + Overwrite bool `help:"whether to overwrite pre-existing configuration files" default:"false" setup:"true"` + GenerateMinioCerts bool `default:"false" help:"generate sample TLS certs for Minio GW" setup:"true"` + + Satellite Satellite + StorageNodes [storagenodeCount]StorageNode + Uplink miniogw.Config +} + +// Satellite configuration +type Satellite struct { + Server server.Config + Kademlia kademlia.SatelliteConfig + PointerDB pointerdb.Config + Overlay overlay.Config + Inspector inspector.Config + Checker checker.Config + Repairer repairer.Config + Audit audit.Config + BwAgreement bwagreement.Config + Web satelliteweb.Config + Discovery discovery.Config + Tally tally.Config + Rollup rollup.Config + Database string `help:"satellite database connection string" default:"sqlite3://$CONFDIR/master.db"` +} + +// StorageNode configuration +type StorageNode struct { + Server server.Config + Kademlia kademlia.StorageNodeConfig + Storage psserver.Config +} + var ( mon = monkit.Package() diff --git a/cmd/captplanet/run.go b/cmd/captplanet/run.go index 27c31bfdd..cb201de56 100644 --- a/cmd/captplanet/run.go +++ b/cmd/captplanet/run.go @@ -12,24 +12,9 @@ import ( "github.com/spf13/cobra" "github.com/zeebo/errs" - "storj.io/storj/pkg/accounting/rollup" - "storj.io/storj/pkg/accounting/tally" - "storj.io/storj/pkg/audit" "storj.io/storj/pkg/auth/grpcauth" - "storj.io/storj/pkg/bwagreement" "storj.io/storj/pkg/cfgstruct" - "storj.io/storj/pkg/datarepair/checker" - "storj.io/storj/pkg/datarepair/repairer" - "storj.io/storj/pkg/discovery" - "storj.io/storj/pkg/inspector" - "storj.io/storj/pkg/kademlia" - "storj.io/storj/pkg/miniogw" - "storj.io/storj/pkg/overlay" - "storj.io/storj/pkg/piecestore/psserver" - "storj.io/storj/pkg/pointerdb" "storj.io/storj/pkg/process" - "storj.io/storj/pkg/satellite/satelliteweb" - "storj.io/storj/pkg/server" "storj.io/storj/pkg/utils" "storj.io/storj/satellite/satellitedb" ) @@ -38,38 +23,6 @@ const ( storagenodeCount = 10 ) -// Captplanet defines Captain Planet runtime configuration -type Captplanet struct { - Satellite Satellite - StorageNodes [storagenodeCount]StorageNode - Uplink miniogw.Config -} - -// Satellite is for configuring client -type Satellite struct { - Server server.Config - Kademlia kademlia.SatelliteConfig - PointerDB pointerdb.Config - Overlay overlay.Config - Inspector inspector.Config - Checker checker.Config - Repairer repairer.Config - Audit audit.Config - BwAgreement bwagreement.Config - Web satelliteweb.Config - Discovery discovery.Config - Tally tally.Config - Rollup rollup.Config - Database string `help:"satellite database connection string" default:"sqlite3://$CONFDIR/master.db"` -} - -// StorageNode is for configuring storage nodes -type StorageNode struct { - Server server.Config - Kademlia kademlia.StorageNodeConfig - Storage psserver.Config -} - var ( runCmd = &cobra.Command{ Use: "run", diff --git a/cmd/captplanet/setup.go b/cmd/captplanet/setup.go index 37614f6ee..caf1a53c1 100644 --- a/cmd/captplanet/setup.go +++ b/cmd/captplanet/setup.go @@ -17,24 +17,6 @@ import ( "storj.io/storj/pkg/provider" ) -// SetupCaptplanet defines Captain Planet setup configuration -type SetupCaptplanet struct { - SatelliteCA provider.CASetupConfig - SatelliteIdentity provider.IdentitySetupConfig - UplinkCA provider.CASetupConfig - UplinkIdentity provider.IdentitySetupConfig - StorageNodeCA provider.CASetupConfig - StorageNodeIdentity provider.IdentitySetupConfig - ListenHost string `help:"the host for providers to listen on" default:"127.0.0.1"` - StartingPort int `help:"all providers will listen on ports consecutively starting with this one" default:"7777"` - APIKey string `default:"abc123" help:"the api key to use for the satellite"` - EncKey string `default:"insecure-default-encryption-key" help:"your root encryption key"` - Overwrite bool `help:"whether to overwrite pre-existing configuration files" default:"false"` - GenerateMinioCerts bool `default:"false" help:"generate sample TLS certs for Minio GW"` - - Captplanet -} - var ( setupCmd = &cobra.Command{ Use: "setup", @@ -42,12 +24,12 @@ var ( RunE: cmdSetup, Annotations: map[string]string{"type": "setup"}, } - setupCfg SetupCaptplanet + setupCfg Captplanet ) func init() { rootCmd.AddCommand(setupCmd) - cfgstruct.Bind(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) + cfgstruct.BindSetup(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) } func cmdSetup(cmd *cobra.Command, args []string) (err error) { diff --git a/cmd/satellite/main.go b/cmd/satellite/main.go index a9a4b3670..61eeb0562 100644 --- a/cmd/satellite/main.go +++ b/cmd/satellite/main.go @@ -34,8 +34,12 @@ import ( "storj.io/storj/satellite/satellitedb" ) -// Satellite defines satellite runtime configuration +// Satellite defines satellite configuration type Satellite struct { + CA identity.CASetupConfig `setup:"true"` + Identity identity.SetupConfig `setup:"true"` + Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files" setup:"true"` + Server server.Config Kademlia kademlia.SatelliteConfig PointerDB pointerdb.Config @@ -48,15 +52,6 @@ type Satellite struct { Database string `help:"satellite database connection string" default:"sqlite3://$CONFDIR/master.db"` } -// SetupSatellite defines satellite setup configuration -type SetupSatellite struct { - CA identity.CASetupConfig - Identity identity.SetupConfig - Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files"` - - Satellite -} - var ( rootCmd = &cobra.Command{ Use: "satellite", @@ -85,7 +80,7 @@ var ( } runCfg Satellite - setupCfg SetupSatellite + setupCfg Satellite diagCfg struct { Database string `help:"satellite database connection string" default:"sqlite3://$CONFDIR/master.db"` @@ -114,7 +109,7 @@ func init() { rootCmd.AddCommand(diagCmd) rootCmd.AddCommand(qdiagCmd) cfgstruct.Bind(runCmd.Flags(), &runCfg, cfgstruct.ConfDir(defaultConfDir)) - cfgstruct.Bind(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) + cfgstruct.BindSetup(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) cfgstruct.Bind(diagCmd.Flags(), &diagCfg, cfgstruct.ConfDir(defaultConfDir)) cfgstruct.Bind(qdiagCmd.Flags(), &qdiagCfg, cfgstruct.ConfDir(defaultConfDir)) } diff --git a/cmd/storagenode/main.go b/cmd/storagenode/main.go index 410834853..aab0d51c2 100644 --- a/cmd/storagenode/main.go +++ b/cmd/storagenode/main.go @@ -29,21 +29,16 @@ import ( "storj.io/storj/pkg/storj" ) -// StorageNode defines storage node runtime configuration +// StorageNode defines storage node configuration type StorageNode struct { + CA identity.CASetupConfig `setup:"true"` + Identity identity.SetupConfig `setup:"true"` + Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files" setup:"true"` + Server server.Config Kademlia kademlia.StorageNodeConfig Storage psserver.Config -} - -// SetupStorageNode defines storage node setup configuration -type SetupStorageNode struct { - CA identity.CASetupConfig - Identity identity.SetupConfig - Signer certificates.CertSigningConfig - Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files"` - - StorageNode + Signer certificates.CertSigningConfig } var ( @@ -75,7 +70,7 @@ var ( } runCfg StorageNode - setupCfg SetupStorageNode + setupCfg StorageNode diagCfg struct { } @@ -106,7 +101,7 @@ func init() { rootCmd.AddCommand(configCmd) rootCmd.AddCommand(diagCmd) cfgstruct.Bind(runCmd.Flags(), &runCfg, cfgstruct.ConfDir(defaultConfDir)) - cfgstruct.Bind(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) + cfgstruct.BindSetup(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) cfgstruct.Bind(diagCmd.Flags(), &diagCfg, cfgstruct.ConfDir(defaultDiagDir)) } diff --git a/cmd/uplink/cmd/setup.go b/cmd/uplink/cmd/setup.go index e744aad56..6cd6e562a 100644 --- a/cmd/uplink/cmd/setup.go +++ b/cmd/uplink/cmd/setup.go @@ -26,13 +26,13 @@ var ( Annotations: map[string]string{"type": "setup"}, } setupCfg struct { - CA provider.CASetupConfig - Identity provider.IdentitySetupConfig - Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files"` - SatelliteAddr string `default:"localhost:7778" help:"the address to use for the satellite"` - APIKey string `default:"" help:"the api key to use for the satellite"` - EncKey string `default:"" help:"your root encryption key"` - GenerateMinioCerts bool `default:"false" help:"generate sample TLS certs for Minio GW"` + CA provider.CASetupConfig `setup:"true"` + Identity provider.IdentitySetupConfig `setup:"true"` + Overwrite bool `default:"false" help:"whether to overwrite pre-existing configuration files" setup:"true"` + APIKey string `default:"" help:"the api key to use for the satellite" setup:"true"` + EncKey string `default:"" help:"your root encryption key" setup:"true"` + GenerateMinioCerts bool `default:"false" help:"generate sample TLS certs for Minio GW" setup:"true"` + SatelliteAddr string `default:"localhost:7778" help:"the address to use for the satellite" setup:"true"` } cliConfDir *string @@ -52,7 +52,7 @@ func init() { CLICmd.AddCommand(setupCmd) GWCmd.AddCommand(setupCmd) - cfgstruct.Bind(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) + cfgstruct.BindSetup(setupCmd.Flags(), &setupCfg, cfgstruct.ConfDir(defaultConfDir)) } func cmdSetup(cmd *cobra.Command, args []string) (err error) { diff --git a/pkg/cfgstruct/bind.go b/pkg/cfgstruct/bind.go index a0f08ad4f..8b1be0833 100644 --- a/pkg/cfgstruct/bind.go +++ b/pkg/cfgstruct/bind.go @@ -8,10 +8,11 @@ import ( "os" "path/filepath" "reflect" - "regexp" "strconv" "strings" "time" + + "github.com/spf13/pflag" ) // BindOpt is an option for the Bind method @@ -44,29 +45,33 @@ type confVar struct { // Bind sets flags on a FlagSet that match the configuration struct // 'config'. This works by traversing the config struct using the 'reflect' -// package. +// package. Will ignore fields with `setup:"true"` tag. func Bind(flags FlagSet, config interface{}, opts ...BindOpt) { + bind(flags, config, false, opts...) +} + +// BindSetup sets flags on a FlagSet that match the configuration struct +// 'config'. This works by traversing the config struct using the 'reflect' +// package. +func BindSetup(flags FlagSet, config interface{}, opts ...BindOpt) { + bind(flags, config, true, opts...) +} + +func bind(flags FlagSet, config interface{}, setupCommand bool, opts ...BindOpt) { ptrtype := reflect.TypeOf(config) if ptrtype.Kind() != reflect.Ptr { - panic(fmt.Sprintf("invalid config type: %#v. "+ - "Expecting pointer to struct.", config)) + panic(fmt.Sprintf("invalid config type: %#v. Expecting pointer to struct.", config)) } vars := map[string]confVar{} for _, opt := range opts { opt(vars) } - bindConfig(flags, "", reflect.ValueOf(config).Elem(), vars) + bindConfig(flags, "", reflect.ValueOf(config).Elem(), vars, setupCommand, false) } -var ( - whitespace = regexp.MustCompile(`\s+`) -) - -func bindConfig(flags FlagSet, prefix string, val reflect.Value, - vars map[string]confVar) { +func bindConfig(flags FlagSet, prefix string, val reflect.Value, vars map[string]confVar, setupCommand, setupStruct bool) { if val.Kind() != reflect.Struct { - panic(fmt.Sprintf("invalid config type: %#v. Expecting struct.", - val.Interface())) + panic(fmt.Sprintf("invalid config type: %#v. Expecting struct.", val.Interface())) } typ := val.Type() @@ -86,26 +91,28 @@ func bindConfig(flags FlagSet, prefix string, val reflect.Value, field := typ.Field(i) fieldval := val.Field(i) flagname := prefix + hyphenate(snakeCase(field.Name)) + onlyForSetup := (field.Tag.Get("setup") == "true") || setupStruct + // ignore setup params for non setup commands + if !setupCommand && onlyForSetup { + continue + } switch field.Type.Kind() { case reflect.Struct: if field.Anonymous { - bindConfig(flags, prefix, fieldval, vars) + bindConfig(flags, prefix, fieldval, vars, setupCommand, onlyForSetup) } else { - bindConfig(flags, flagname+".", fieldval, vars) + bindConfig(flags, flagname+".", fieldval, vars, setupCommand, onlyForSetup) } case reflect.Array, reflect.Slice: digits := len(fmt.Sprint(fieldval.Len())) for j := 0; j < fieldval.Len(); j++ { padding := strings.Repeat("0", digits-len(fmt.Sprint(j))) - bindConfig(flags, fmt.Sprintf("%s.%s%d.", flagname, padding, j), - fieldval.Index(j), vars) + bindConfig(flags, fmt.Sprintf("%s.%s%d.", flagname, padding, j), fieldval.Index(j), vars, setupCommand, onlyForSetup) } default: - tag := reflect.StructTag( - whitespace.ReplaceAllString(string(field.Tag), " ")) - help := tag.Get("help") - def := tag.Get("default") + help := field.Tag.Get("help") + def := field.Tag.Get("default") fieldaddr := fieldval.Addr().Interface() check := func(err error) { if err != nil { @@ -147,10 +154,25 @@ func bindConfig(flags FlagSet, prefix string, val reflect.Value, default: panic(fmt.Sprintf("invalid field type: %s", field.Type.String())) } + if onlyForSetup { + setSetupAnnotation(flags, flagname) + } } } } +func setSetupAnnotation(flagset interface{}, name string) { + flags, ok := flagset.(*pflag.FlagSet) + if !ok { + return + } + + err := flags.SetAnnotation(name, "setup", []string{"true"}) + if err != nil { + panic(fmt.Sprintf("unable to set annotation: %v", err)) + } +} + func expand(vars map[string]string, val string) string { return os.Expand(val, func(key string) string { return vars[key] }) } diff --git a/pkg/process/exec_conf.go b/pkg/process/exec_conf.go index 6e7d197b0..d22160449 100644 --- a/pkg/process/exec_conf.go +++ b/pkg/process/exec_conf.go @@ -67,6 +67,11 @@ func SaveConfig(flagset *pflag.FlagSet, outfile string, overrides map[string]int w := &sb for _, k := range keys { f := flagset.Lookup(k) + setup := f.Annotations["setup"] + if len(setup) > 0 && setup[0] == "true" { + continue + } + value := f.Value.String() if v, ok := overrides[k]; ok { value = fmt.Sprintf("%v", v)