Skip to content

Commit

Permalink
Fix runtime panic caused by missing global flags (#693)
Browse files Browse the repository at this point in the history
* fix runtime panic caused by missing global flags

* incorrect conditional
  • Loading branch information
Integralist authored Oct 26, 2022
1 parent d31860f commit 6b3b3a6
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 15 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ debug:
.PHONY: config
config:
@$(CONFIG_SCRIPT)
@cat $(CONFIG_FILE)

.PHONY: all
all: config dependencies tidy fmt vet staticcheck gosec test build install
Expand Down
1 change: 1 addition & 0 deletions pkg/app/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func Run(opts RunOpts) error {
// WARNING: kingpin has no way of decorating flags as being "global"
// therefore if you add/remove a global flag you will also need to update
// the globalFlags map in pkg/app/usage.go which is used for usage rendering.
// You should also update `IsGlobalFlagsOnly` in ../cmd/cmd.go
//
// NOTE: Global flags, unlike command flags, must be unique. This means BOTH
// the long flag and the short flag identifiers must be unique. If you try to
Expand Down
14 changes: 6 additions & 8 deletions pkg/app/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,18 +292,16 @@ func processCommandInput(
return command, cmdName, help(vars, err)
}

// NOTE: The `fastly help` and `fastly --help` behaviours need to avoid
// trying to call cmd.Select() as the context object will not return a useful
// value for FullCommand(). The former will fail to find a match as it will
// be set to `help [<command>...]` as it's a built-in command that we don't
// control, and the latter --help flag variation will be an empty string as
// there were no actual 'command' specified.
// NOTE: `fastly help`, no flags, or only globals, should skip conditional.
//
// This is because the `ctx` variable will be assigned a
// `kingpin.ParseContext` whose `SelectedCommand` will be nil.
//
// Additionally we don't want to use the ctx if dealing with a shell
// completion flag, as that depends on kingpin.Parse() being called, and so
// the ctx is otherwise empty.
// the `ctx` is otherwise empty.
var found bool
if !cmd.IsHelpOnly(opts.Args) && !cmd.IsHelpFlagOnly(opts.Args) && !cmd.IsCompletion(opts.Args) && !cmd.IsCompletionScript(opts.Args) {
if !noargs && !globalFlagsOnly && !cmd.IsHelpOnly(opts.Args) && !cmd.IsHelpFlagOnly(opts.Args) && !cmd.IsCompletion(opts.Args) && !cmd.IsCompletionScript(opts.Args) {
command, found = cmd.Select(ctx.SelectedCommand.FullCommand(), commands)
if !found {
return command, cmdName, help(vars, err)
Expand Down
22 changes: 16 additions & 6 deletions pkg/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,23 @@ func IsVerboseAndQuiet(args []string) bool {
// args: [--verbose -v --endpoint ... --token ... -t ... --endpoint ...] 10
// total: 10
func IsGlobalFlagsOnly(args []string) bool {
// Global flags are defined in pkg/app/run.go#84
// Global flags are defined in ../app/run.go
globals := map[string]int{
"--verbose": 0,
"-v": 0,
"--token": 1,
"-t": 1,
"--endpoint": 1,
"--accept-defaults": 0,
"-d": 0,
"--auto-yes": 0,
"-y": 0,
"--endpoint": 1,
"--non-interactive": 0,
"-i": 0,
"--profile": 1,
"-o": 1,
"--quiet": 0,
"-q": 0,
"--token": 1,
"-t": 1,
"--verbose": 0,
"-v": 0,
}
var total int
for _, a := range args {
Expand Down

0 comments on commit 6b3b3a6

Please sign in to comment.