From 296ceadda61b82fe47e22e71ca85d01b00dc7c67 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sun, 9 May 2021 16:03:18 +0100 Subject: [PATCH] fs: add --all to rclone config create/update to ask all the config questions #3455 This also factors the config questions into a state based mechanism so a backend can be configured using the same dialog as rclone config but remotely. --- cmd/config/config.go | 8 +- fs/backend_config.go | 180 ++++++++++++++++++++++++++++++++-- fs/backend_config_test.go | 22 +++++ fs/config.go | 3 + fs/config/config.go | 69 +++++++------ fs/config/rc.go | 1 + fs/config/ui.go | 103 +++++-------------- fs/config/ui_internal_test.go | 29 ------ fs/config/ui_test.go | 2 +- fs/fs.go | 18 ++++ 10 files changed, 290 insertions(+), 145 deletions(-) delete mode 100644 fs/config/ui_internal_test.go diff --git a/cmd/config/config.go b/cmd/config/config.go index f5af810e3..1de6a0a31 100644 --- a/cmd/config/config.go +++ b/cmd/config/config.go @@ -167,10 +167,15 @@ time as the question. rclone config update name --continue state "*oauth-islocal,teamdrive,," result "true" Note that when using |--continue| all passwords should be passed in -the clear (not obscured). +the clear (not obscured). Any default config values should be passed +in with each invocation of |--continue|. At the end of the non interactive process, rclone will return a result with |State| as empty string. + +If |--all| is passed then rclone will ask all the config questions, +not just the post config questions. Any parameters are used as +defaults for questions as usual. `, "|", "`") var configCreateCommand = &cobra.Command{ Use: "create `name` `type` [`key` `value`]*", @@ -229,6 +234,7 @@ func init() { flags.BoolVarP(cmdFlags, &updateRemoteOpt.NoObscure, "no-obscure", "", false, "Force any passwords not to be obscured.") flags.BoolVarP(cmdFlags, &updateRemoteOpt.NonInteractive, "non-interactive", "", false, "Don't interact with user and return questions.") flags.BoolVarP(cmdFlags, &updateRemoteOpt.Continue, "continue", "", false, "Continue the configuration process with an answer.") + flags.BoolVarP(cmdFlags, &updateRemoteOpt.All, "all", "", false, "Ask the full set of config questions.") } } diff --git a/fs/backend_config.go b/fs/backend_config.go index 9516ac250..6d8e2fad9 100644 --- a/fs/backend_config.go +++ b/fs/backend_config.go @@ -7,6 +7,7 @@ package fs import ( "context" "fmt" + "strconv" "strings" "github.com/pkg/errors" @@ -16,6 +17,9 @@ import ( const ( // ConfigToken is the key used to store the token under ConfigToken = "token" + + // ConfigKeyEphemeralPrefix marks config keys which shouldn't be stored in the config file + ConfigKeyEphemeralPrefix = "config_" ) // ConfigOAuth should be called to do the OAuth @@ -52,7 +56,10 @@ var ConfigOAuth func(ctx context.Context, name string, m configmap.Mapper, ri *R // // Where the questions ask for a name then this should start with // "config_" to show it is an ephemeral config input rather than the -// actual value stored in the config file. +// actual value stored in the config file. Names beginning with +// "config_fs_" are reserved for internal use. +// +// State names starting with "*" are reserved for internal use. type ConfigIn struct { State string // State to run Result string // Result from previous Option @@ -258,10 +265,11 @@ func StatePop(state string) (newState string, value string) { // BackendConfig calls the config for the backend in ri // -// It wraps any OAuth transactions as necessary so only straight forward config questions are emitted -func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) { +// It wraps any OAuth transactions as necessary so only straight +// forward config questions are emitted +func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, choices configmap.Getter, in ConfigIn) (out *ConfigOut, err error) { for { - out, err = backendConfigStep(ctx, name, m, ri, in) + out, err = backendConfigStep(ctx, name, m, ri, choices, in) if err != nil { break } @@ -286,24 +294,130 @@ func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *Reg return out, err } -func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) { - ci := GetConfig(ctx) - if ri.Config == nil { - return nil, nil +// ConfigAll should be passed in as the initial state to run the +// entire config +const ConfigAll = "*all" + +// Run the config state machine for the normal config +func configAll(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) { + if len(ri.Options) == 0 { + return ConfigGoto("*postconfig") } + + // States are encoded + // + // *all-ACTION,NUMBER,ADVANCED + // + // Where NUMBER is the curent state, ADVANCED is a flag true or false + // to say whether we are asking about advanced config and + // ACTION is what the state should be doing next. + stateParams, state := StatePop(in.State) + stateParams, stateNumber := StatePop(stateParams) + _, stateAdvanced := StatePop(stateParams) + + optionNumber := 0 + advanced := stateAdvanced == "true" + if stateNumber != "" { + optionNumber, err = strconv.Atoi(stateNumber) + if err != nil { + return nil, errors.Wrap(err, "internal error: bad state number") + } + } + + // Detect if reached the end of the questions + if optionNumber == len(ri.Options) { + if ri.Options.HasAdvanced() { + return ConfigConfirm("*all-advanced", false, "config_fs_advanced", "Edit advanced config?") + } + return ConfigGoto("*postconfig") + } else if optionNumber < 0 || optionNumber > len(ri.Options) { + return nil, errors.New("internal error: option out of range") + } + + // Make the next state + newState := func(state string, i int, advanced bool) string { + return StatePush("", state, fmt.Sprint(i), fmt.Sprint(advanced)) + } + + // Find the current option + option := &ri.Options[optionNumber] + + switch state { + case "*all": + // If option is hidden or doesn't match advanced setting then skip it + if option.Hide&OptionHideConfigurator != 0 || option.Advanced != advanced { + return ConfigGoto(newState("*all", optionNumber+1, advanced)) + } + + // Skip this question if it isn't the correct provider + provider, _ := m.Get(ConfigProvider) + if !MatchProvider(option.Provider, provider) { + return ConfigGoto(newState("*all", optionNumber+1, advanced)) + } + + out = &ConfigOut{ + State: newState("*all-set", optionNumber, advanced), + Option: option, + } + + // Filter examples by provider if necessary + if provider != "" && len(option.Examples) > 0 { + optionCopy := option.Copy() + optionCopy.Examples = OptionExamples{} + for _, example := range option.Examples { + if MatchProvider(example.Provider, provider) { + optionCopy.Examples = append(optionCopy.Examples, example) + } + } + out.Option = optionCopy + } + + return out, nil + case "*all-set": + // Set the value if not different to current + // Note this won't set blank values in the config file + // if the default is blank + currentValue, _ := m.Get(option.Name) + if currentValue != in.Result { + m.Set(option.Name, in.Result) + } + // Find the next question + return ConfigGoto(newState("*all", optionNumber+1, advanced)) + case "*all-advanced": + // Reply to edit advanced question + if in.Result == "true" { + return ConfigGoto(newState("*all", 0, true)) + } + return ConfigGoto("*postconfig") + } + return nil, errors.Errorf("internal error: bad state %q", state) +} + +func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, choices configmap.Getter, in ConfigIn) (out *ConfigOut, err error) { + ci := GetConfig(ctx) Debugf(name, "config in: state=%q, result=%q", in.State, in.Result) defer func() { Debugf(name, "config out: out=%+v, err=%v", out, err) }() switch { + case strings.HasPrefix(in.State, ConfigAll): + // Do all config + out, err = configAll(ctx, name, m, ri, in) case strings.HasPrefix(in.State, "*oauth"): // Do internal oauth states out, err = ConfigOAuth(ctx, name, m, ri, in) + case strings.HasPrefix(in.State, "*postconfig"): + // Do the post config starting from state "" + in.State = "" + return backendConfigStep(ctx, name, m, ri, choices, in) case strings.HasPrefix(in.State, "*"): err = errors.Errorf("unknown internal state %q", in.State) default: // Otherwise pass to backend + if ri.Config == nil { + return nil, nil + } out, err = ri.Config(ctx, name, m, in) } if err != nil { @@ -325,8 +439,8 @@ func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri if out.Option.Name == "" { return nil, errors.New("internal error: no name set in Option") } - // If override value is set in the config then use that - if result, ok := m.Get(out.Option.Name); ok { + // If override value is set in the choices then use that + if result, ok := choices.Get(out.Option.Name); ok { Debugf(nil, "Override value found, choosing value %q for state %q", result, out.State) return ConfigResult(out.State, result) } @@ -336,6 +450,52 @@ func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri Debugf(nil, "Auto confirm is set, choosing default %q for state %q, override by setting config parameter %q", result, out.State, out.Option.Name) return ConfigResult(out.State, result) } + // If fs.ConfigEdit is set then make the default value + // in the config the current value. + if result, ok := choices.Get(ConfigEdit); ok && result == "true" { + if value, ok := m.Get(out.Option.Name); ok { + newOption := out.Option.Copy() + oldValue := newOption.Value + err = newOption.Set(value) + if err != nil { + Errorf(nil, "Failed to set %q from %q - using default: %v", out.Option.Name, value, err) + } else { + newOption.Default = newOption.Value + newOption.Value = oldValue + out.Option = newOption + } + } + } } return out, nil } + +// MatchProvider returns true if provider matches the providerConfig string. +// +// The providerConfig string can either be a list of providers to +// match, or if it starts with "!" it will be a list of providers not +// to match. +// +// If either providerConfig or provider is blank then it will return true +func MatchProvider(providerConfig, provider string) bool { + if providerConfig == "" || provider == "" { + return true + } + negate := false + if strings.HasPrefix(providerConfig, "!") { + providerConfig = providerConfig[1:] + negate = true + } + providers := strings.Split(providerConfig, ",") + matched := false + for _, p := range providers { + if p == provider { + matched = true + break + } + } + if negate { + return !matched + } + return matched +} diff --git a/fs/backend_config_test.go b/fs/backend_config_test.go index 2bb5de45d..9c373693f 100644 --- a/fs/backend_config_test.go +++ b/fs/backend_config_test.go @@ -1,6 +1,7 @@ package fs import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -35,3 +36,24 @@ func TestStatePop(t *testing.T) { assert.Equal(t, "1,2,3", value) assert.Equal(t, "a", state) } + +func TestMatchProvider(t *testing.T) { + for _, test := range []struct { + config string + provider string + want bool + }{ + {"", "", true}, + {"one", "one", true}, + {"one,two", "two", true}, + {"one,two,three", "two", true}, + {"one", "on", false}, + {"one,two,three", "tw", false}, + {"!one,two,three", "two", false}, + {"!one,two,three", "four", true}, + } { + what := fmt.Sprintf("%q,%q", test.config, test.provider) + got := MatchProvider(test.config, test.provider) + assert.Equal(t, test.want, got, what) + } +} diff --git a/fs/config.go b/fs/config.go index eece951f8..8c31d5a73 100644 --- a/fs/config.go +++ b/fs/config.go @@ -37,6 +37,9 @@ var ( // ConfigProvider is the config key used for provider options ConfigProvider = "provider" + + // ConfigEdit is the config key used to show we wish to edit existing entries + ConfigEdit = "config_fs_edit" ) // ConfigInfo is filesystem config options diff --git a/fs/config/config.go b/fs/config/config.go index de9ceb086..081c154fc 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -19,6 +19,7 @@ import ( "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/cache" + "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fs/config/obscure" "github.com/rclone/rclone/fs/fspath" "github.com/rclone/rclone/fs/rc" @@ -418,6 +419,8 @@ type UpdateRemoteOpt struct { NonInteractive bool `json:"nonInteractive"` // If set then supply state and result parameters to continue the process Continue bool `json:"continue"` + // If set then ask all the questions, not just the post config questions + All bool `json:"all"` } // UpdateRemote adds the keyValues passed in to the remote of name. @@ -431,7 +434,7 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd return nil, err } interactive := !(opt.NonInteractive || opt.Continue) - if interactive { + if interactive && !opt.All { ctx = suppressConfirm(ctx) } @@ -445,41 +448,48 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd return nil, errors.Errorf("couldn't find backend for type %q", fsType) } - if !opt.Continue { - // Work out which options need to be obscured - needsObscure := map[string]struct{}{} - if !opt.NoObscure { - for _, option := range ri.Options { - if option.IsPassword { - needsObscure[option.Name] = struct{}{} + // Work out which options need to be obscured + needsObscure := map[string]struct{}{} + if !opt.NoObscure { + for _, option := range ri.Options { + if option.IsPassword { + needsObscure[option.Name] = struct{}{} + } + } + } + + choices := configmap.Simple{} + m := fs.ConfigMap(ri, name, nil) + + // Set the config + for k, v := range keyValues { + vStr := fmt.Sprint(v) + // Obscure parameter if necessary + if _, ok := needsObscure[k]; ok { + _, err := obscure.Reveal(vStr) + if err != nil || opt.Obscure { + // If error => not already obscured, so obscure it + // or we are forced to obscure + vStr, err = obscure.Obscure(vStr) + if err != nil { + return nil, errors.Wrap(err, "UpdateRemote: obscure failed") } } } - - // Set the config - for k, v := range keyValues { - vStr := fmt.Sprint(v) - // Obscure parameter if necessary - if _, ok := needsObscure[k]; ok { - _, err := obscure.Reveal(vStr) - if err != nil || opt.Obscure { - // If error => not already obscured, so obscure it - // or we are forced to obscure - vStr, err = obscure.Obscure(vStr) - if err != nil { - return nil, errors.Wrap(err, "UpdateRemote: obscure failed") - } - } - } - LoadedData().SetValue(name, k, vStr) + choices.Set(k, vStr) + if !strings.HasPrefix(k, fs.ConfigKeyEphemeralPrefix) { + m.Set(k, vStr) } } if interactive { - err = RemoteConfig(ctx, name) + var state = "" + if opt.All { + state = fs.ConfigAll + } + err = backendConfig(ctx, name, m, ri, choices, state) } else { // Start the config state machine - m := fs.ConfigMap(ri, name, nil) in := fs.ConfigIn{} if opt.Continue { if state, ok := keyValues["state"]; ok { @@ -493,7 +503,10 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd return nil, errors.New("UpdateRemote: need result parameter with --continue") } } - out, err = fs.BackendConfig(ctx, name, m, ri, in) + if in.State == "" && opt.All { + in.State = fs.ConfigAll + } + out, err = fs.BackendConfig(ctx, name, m, ri, choices, in) } if err != nil { return nil, err diff --git a/fs/config/rc.go b/fs/config/rc.go index d2f8ffc38..fe8dacec9 100644 --- a/fs/config/rc.go +++ b/fs/config/rc.go @@ -118,6 +118,7 @@ func init() { - noObscure - declare passwords are already obscured and don't need obscuring - nonInteractive - don't interact with a user, return questions - continue - continue the config process with an answer + - all - ask all the config questions not just the post config ones ` } rc.Add(rc.Call{ diff --git a/fs/config/ui.go b/fs/config/ui.go index dbd4a512c..2b72f3ba3 100644 --- a/fs/config/ui.go +++ b/fs/config/ui.go @@ -241,18 +241,15 @@ func OkRemote(name string) bool { return false } -// PostConfig configures the backend after the main config has been done +// backendConfig configures the backend starting from the state passed in // // The is the user interface loop that drives the post configuration backend config. -func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo) error { - if ri.Config == nil { - return errors.New("backend doesn't support reconnect or authorize") - } +func backendConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo, choices configmap.Getter, startState string) error { in := fs.ConfigIn{ - State: "", + State: startState, } for { - out, err := fs.BackendConfig(ctx, name, m, ri, in) + out, err := fs.BackendConfig(ctx, name, m, ri, choices, in) if err != nil { return err } @@ -297,6 +294,16 @@ func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.Reg return nil } +// PostConfig configures the backend after the main config has been done +// +// The is the user interface loop that drives the post configuration backend config. +func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo) error { + if ri.Config == nil { + return errors.New("backend doesn't support reconnect or authorize") + } + return backendConfig(ctx, name, m, ri, configmap.Simple{}, "") +} + // RemoteConfig runs the config helper for the remote if needed func RemoteConfig(ctx context.Context, name string) error { fmt.Printf("Remote config\n") @@ -308,39 +315,8 @@ func RemoteConfig(ctx context.Context, name string) error { return PostConfig(ctx, name, m, ri) } -// matchProvider returns true if provider matches the providerConfig string. -// -// The providerConfig string can either be a list of providers to -// match, or if it starts with "!" it will be a list of providers not -// to match. -// -// If either providerConfig or provider is blank then it will return true -func matchProvider(providerConfig, provider string) bool { - if providerConfig == "" || provider == "" { - return true - } - negate := false - if strings.HasPrefix(providerConfig, "!") { - providerConfig = providerConfig[1:] - negate = true - } - providers := strings.Split(providerConfig, ",") - matched := false - for _, p := range providers { - if p == provider { - matched = true - break - } - } - if negate { - return !matched - } - return matched -} - // ChooseOption asks the user to choose an option func ChooseOption(o *fs.Option, name string) string { - var subProvider = getWithDefault(name, fs.ConfigProvider, "") fmt.Println(o.Help) if o.IsPassword { actions := []string{"yYes type in my own password", "gGenerate random password"} @@ -397,10 +373,8 @@ func ChooseOption(o *fs.Option, name string) string { var values []string var help []string for _, example := range o.Examples { - if matchProvider(example.Provider, subProvider) { - values = append(values, example.Value) - help = append(help, example.Help) - } + values = append(values, example.Value) + help = append(help, example.Help) } in = Choose(o.Name, values, help, !o.Exclusive) } else { @@ -450,38 +424,13 @@ func NewRemoteName() (name string) { // editOptions edits the options. If new is true then it just allows // entry and doesn't show any old values. -func editOptions(ri *fs.RegInfo, name string, isNew bool) { +func editOptions(ctx context.Context, ri *fs.RegInfo, name string, isNew bool) error { fmt.Printf("** See help for %s backend at: https://rclone.org/%s/ **\n\n", ri.Name, ri.FileName()) - hasAdvanced := false - for _, advanced := range []bool{false, true} { - if advanced { - if !hasAdvanced { - break - } - fmt.Printf("Edit advanced config? (y/n)\n") - if !Confirm(false) { - break - } - } - for _, option := range ri.Options { - isVisible := option.Hide&fs.OptionHideConfigurator == 0 - hasAdvanced = hasAdvanced || (option.Advanced && isVisible) - if option.Advanced != advanced { - continue - } - subProvider := getWithDefault(name, fs.ConfigProvider, "") - if matchProvider(option.Provider, subProvider) && isVisible { - if !isNew { - fmt.Printf("Value %q = %q\n", option.Name, FileGet(name, option.Name)) - fmt.Printf("Edit? (y/n)>\n") - if !Confirm(false) { - continue - } - } - FileSet(name, option.Name, ChooseOption(&option, name)) - } - } + m := fs.ConfigMap(ri, name, nil) + choices := configmap.Simple{ + fs.ConfigEdit: fmt.Sprint(isNew), } + return backendConfig(ctx, name, m, ri, choices, fs.ConfigAll) } // NewRemote make a new remote from its name @@ -504,8 +453,7 @@ func NewRemote(ctx context.Context, name string) error { } LoadedData().SetValue(name, "type", newType) - editOptions(ri, name, true) - err = RemoteConfig(ctx, name) + err = editOptions(ctx, ri, name, true) if err != nil { return err } @@ -521,13 +469,16 @@ func EditRemote(ctx context.Context, ri *fs.RegInfo, name string) error { ShowRemote(name) fmt.Printf("Edit remote\n") for { - editOptions(ri, name, false) + err := editOptions(ctx, ri, name, true) + if err != nil { + return err + } if OkRemote(name) { break } } SaveConfig() - return RemoteConfig(ctx, name) + return nil } // DeleteRemote gets the user to delete a remote diff --git a/fs/config/ui_internal_test.go b/fs/config/ui_internal_test.go deleted file mode 100644 index 98568d47a..000000000 --- a/fs/config/ui_internal_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package config - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestMatchProvider(t *testing.T) { - for _, test := range []struct { - config string - provider string - want bool - }{ - {"", "", true}, - {"one", "one", true}, - {"one,two", "two", true}, - {"one,two,three", "two", true}, - {"one", "on", false}, - {"one,two,three", "tw", false}, - {"!one,two,three", "two", false}, - {"!one,two,three", "four", true}, - } { - what := fmt.Sprintf("%q,%q", test.config, test.provider) - got := matchProvider(test.config, test.provider) - assert.Equal(t, test.want, got, what) - } -} diff --git a/fs/config/ui_test.go b/fs/config/ui_test.go index 02a16cf5d..550383914 100644 --- a/fs/config/ui_test.go +++ b/fs/config/ui_test.go @@ -148,7 +148,7 @@ func TestChooseOption(t *testing.T) { } require.NoError(t, config.NewRemote(ctx, "test")) - assert.Equal(t, "false", config.FileGet("test", "bool")) + assert.Equal(t, "", config.FileGet("test", "bool")) // this is the default now assert.Equal(t, "not very random password", obscure.MustReveal(config.FileGet("test", "pass"))) // script for creating remote diff --git a/fs/fs.go b/fs/fs.go index 0532b24ec..5056b64e2 100644 --- a/fs/fs.go +++ b/fs/fs.go @@ -157,6 +157,17 @@ func (os Options) NonDefault(m configmap.Getter) configmap.Simple { return nonDefault } +// HasAdvanced discovers if any options have an Advanced setting +func (os Options) HasAdvanced() bool { + for i := range os { + opt := &os[i] + if opt.Advanced { + return true + } + } + return false +} + // OptionVisibility controls whether the options are visible in the // configurator or the command line. type OptionVisibility byte @@ -256,6 +267,13 @@ func (o *Option) EnvVarName(prefix string) string { return OptionToEnv(prefix + "-" + o.Name) } +// Copy makes a shallow copy of the option +func (o *Option) Copy() *Option { + copy := new(Option) + *copy = *o + return copy +} + // OptionExamples is a slice of examples type OptionExamples []OptionExample