From c22f78c913356981a78d4e982b3b9736d8afc553 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Thu, 28 May 2020 23:55:30 -0400 Subject: [PATCH] feat: simplify flag parsing in proxy plugin Commands should be written such that they take arguments as is. --- docs/appendices/0.21.0-migration-guide.md | 1 + plugins/proxy/src/subcommands/subcommands.go | 33 ++++-- plugins/proxy/subcommands.go | 103 +++++++------------ 3 files changed, 60 insertions(+), 77 deletions(-) diff --git a/docs/appendices/0.21.0-migration-guide.md b/docs/appendices/0.21.0-migration-guide.md index 77785ea8d..cf45eed59 100644 --- a/docs/appendices/0.21.0-migration-guide.md +++ b/docs/appendices/0.21.0-migration-guide.md @@ -12,3 +12,4 @@ The `tls` name is no longer a reserved app name, and can be used by applications The `apps:destroy` command no longer takes a second argument `force`. Instead, this can be passed as the `--force` flag, either globally or as a flag to the command itself. +The `proxy:enable` and `proxy:disable` no longer restart apps when setting the `DOKKU_DISABLE_PROXY` environment variable. A future release will disable usage of `DOKKU_DISABLE_PROXY` altogether. diff --git a/plugins/proxy/src/subcommands/subcommands.go b/plugins/proxy/src/subcommands/subcommands.go index e15730b8d..bc2c49fa7 100644 --- a/plugins/proxy/src/subcommands/subcommands.go +++ b/plugins/proxy/src/subcommands/subcommands.go @@ -19,40 +19,55 @@ func main() { switch subcommand { case "disable": args := flag.NewFlagSet("proxy:disable", flag.ExitOnError) + skipRestart := args.Bool("no-restart", false, "--no-restart: skip restart of the app") args.Parse(os.Args[2:]) - err = proxy.CommandDisable(args.Args()) + appName := args.Arg(0) + err = proxy.CommandDisable(appName, *skipRestart) case "enable": args := flag.NewFlagSet("proxy:enable", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandEnable(args.Args()) + appName := args.Arg(0) + err = proxy.CommandEnable(appName) case "ports": args := flag.NewFlagSet("proxy:ports", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandPorts(args.Args()) + appName := args.Arg(0) + err = proxy.CommandPorts(appName) case "ports-add": args := flag.NewFlagSet("proxy:ports-add", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandPortsAdd(args.Args()) + appName := args.Arg(0) + _, portMaps := common.ShiftString(args.Args()) + err = proxy.CommandPortsAdd(appName, portMaps) case "ports-clear": args := flag.NewFlagSet("proxy:ports-clear", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandPortsClear(args.Args()) + appName := args.Arg(0) + err = proxy.CommandPortsClear(appName) case "ports-remove": args := flag.NewFlagSet("proxy:ports-remove", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandPortsRemove(args.Args()) + appName := args.Arg(0) + _, portMaps := common.ShiftString(args.Args()) + err = proxy.CommandPortsRemove(appName, portMaps) case "ports-set": args := flag.NewFlagSet("proxy:ports-set", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandPortsSet(args.Args()) + appName := args.Arg(0) + _, portMaps := common.ShiftString(args.Args()) + err = proxy.CommandPortsSet(appName, portMaps) case "report": args := flag.NewFlagSet("proxy:report", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandReport(args.Args()) + appName := args.Arg(0) + infoFlag := args.Arg(1) + err = proxy.CommandReport(appName, infoFlag) case "set": args := flag.NewFlagSet("proxy:set", flag.ExitOnError) args.Parse(os.Args[2:]) - err = proxy.CommandSet(args.Args()) + appName := args.Arg(0) + proxyType := args.Arg(1) + err = proxy.CommandSet(appName, proxyType) default: common.LogFail(fmt.Sprintf("Invalid plugin subcommand call: %s", subcommand)) } diff --git a/plugins/proxy/subcommands.go b/plugins/proxy/subcommands.go index e3a5abccf..7252fb19d 100644 --- a/plugins/proxy/subcommands.go +++ b/plugins/proxy/subcommands.go @@ -9,10 +9,9 @@ import ( ) // CommandDisable disables the proxy for app via command line -func CommandDisable(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandDisable(appName string, skipRestart bool) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } if !IsAppProxyEnabled(appName) { @@ -21,16 +20,11 @@ func CommandDisable(args []string) error { } common.LogInfo1("Disabling proxy for app") - restart := true - if len(args) >= 2 && args[1] == "--no-restart" { - restart = false - } - entries := map[string]string{ "DOKKU_DISABLE_PROXY": "1", } - if err := config.SetMany(appName, entries, restart); err != nil { + if err := config.SetMany(appName, entries, false); err != nil { return err } @@ -38,10 +32,9 @@ func CommandDisable(args []string) error { } // CommandEnable enables the proxy for app via command line -func CommandEnable(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandEnable(appName string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } if IsAppProxyEnabled(appName) { @@ -50,13 +43,8 @@ func CommandEnable(args []string) error { } common.LogInfo1("Enabling proxy for app") - restart := true - if len(args) >= 2 && args[1] == "--no-restart" { - restart = false - } - keys := []string{"DOKKU_DISABLE_PROXY"} - if err := config.UnsetMany(appName, keys, restart); err != nil { + if err := config.UnsetMany(appName, keys, false); err != nil { return err } @@ -64,28 +52,25 @@ func CommandEnable(args []string) error { } // CommandPorts is a cmd wrapper to list proxy port mappings for an app -func CommandPorts(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandPorts(appName string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } return listAppProxyPorts(appName) } // CommandPortsAdd adds proxy port mappings to an app -func CommandPortsAdd(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandPortsAdd(appName string, portMaps []string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } - if len(args) == 1 { + if len(portMaps) == 0 { return errors.New("No port mapping specified") } - stringPortMap := strings.Join(args[1:], " ") - proxyPortMap, err := parseProxyPortMapString(stringPortMap) + proxyPortMap, err := parseProxyPortMapString(strings.Join(portMaps, " ")) if err != nil { return err } @@ -98,15 +83,13 @@ func CommandPortsAdd(args []string) error { } // CommandPortsClear clears all proxy port mappings for an app -func CommandPortsClear(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandPortsClear(appName string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } keys := []string{"DOKKU_PROXY_PORT_MAP"} - err = config.UnsetMany(appName, keys, false) - if err != nil { + if err := config.UnsetMany(appName, keys, false); err != nil { return err } @@ -114,18 +97,16 @@ func CommandPortsClear(args []string) error { } // CommandPortsRemove removes specific proxy port mappings from an app -func CommandPortsRemove(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandPortsRemove(appName string, portMaps []string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } - if len(args) == 1 { + if len(portMaps) == 0 { return errors.New("No port mapping specified") } - stringPortMap := strings.Join(args[1:], " ") - proxyPortMap, err := parseProxyPortMapString(stringPortMap) + proxyPortMap, err := parseProxyPortMapString(strings.Join(portMaps, " ")) if err != nil { return err } @@ -138,18 +119,16 @@ func CommandPortsRemove(args []string) error { } // CommandPortsSet sets proxy port mappings for an app -func CommandPortsSet(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandPortsSet(appName string, portMaps []string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } - if len(args) == 1 { + if len(portMaps) == 0 { return errors.New("No port mapping specified") } - stringPortMap := strings.Join(args[1:], " ") - proxyPortMap, err := parseProxyPortMapString(stringPortMap) + proxyPortMap, err := parseProxyPortMapString(strings.Join(portMaps, " ")) if err != nil { return err } @@ -162,17 +141,7 @@ func CommandPortsSet(args []string) error { } // CommandReport displays a proxy report for one or more apps -func CommandReport(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err - } - - infoFlag := "" - if len(args) > 1 { - infoFlag = args[1] - } - +func CommandReport(appName string, infoFlag string) error { if strings.HasPrefix(appName, "--") { infoFlag = appName appName = "" @@ -195,17 +164,15 @@ func CommandReport(args []string) error { } // CommandSet sets a proxy for an app -func CommandSet(args []string) error { - appName, err := getAppName(args) - if err != nil { - return err +func CommandSet(appName string, proxyType string) error { + if appName == "" { + return errors.New("Please specify an app to run the command on") } - if len(appName) < 2 { + if len(proxyType) < 2 { return errors.New("Please specify a proxy type") } - proxyType := args[1] entries := map[string]string{ "DOKKU_APP_PROXY_TYPE": proxyType, }