Skip to content

Commit 2205871

Browse files
authored
feat: Handling of incorrectly used flag (gruntwork-io#3901)
* feat: handling incorrectly used flag * feat: supporting nested commands * chore: add test * fix: go-lint * chore: small code improvement
1 parent ffab77b commit 2205871

File tree

13 files changed

+195
-10
lines changed

13 files changed

+195
-10
lines changed

Diff for: cli/app.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/gruntwork-io/terragrunt/cli/commands"
2222
"github.com/gruntwork-io/terragrunt/cli/commands/graph"
23+
"github.com/gruntwork-io/terragrunt/cli/flags"
2324
"github.com/gruntwork-io/terragrunt/cli/flags/global"
2425

2526
"github.com/gruntwork-io/go-commons/version"
@@ -49,6 +50,8 @@ type App struct {
4950

5051
// NewApp creates the Terragrunt CLI App.
5152
func NewApp(opts *options.TerragruntOptions) *App {
53+
terragruntCommands := commands.New(opts)
54+
5255
app := cli.NewApp()
5356
app.Name = "terragrunt"
5457
app.Usage = "Terragrunt is a flexible orchestration tool that allows Infrastructure as Code written in OpenTofu/Terraform to scale.\nFor documentation, see https://terragrunt.gruntwork.io/."
@@ -57,10 +60,11 @@ func NewApp(opts *options.TerragruntOptions) *App {
5760
app.Writer = opts.Writer
5861
app.ErrWriter = opts.ErrWriter
5962
app.Flags = global.NewFlagsWithDeprecatedMovedFlags(opts)
60-
app.Commands = commands.New(opts).WrapAction(WrapWithTelemetry(opts))
63+
app.Commands = terragruntCommands.WrapAction(WrapWithTelemetry(opts))
6164
app.Before = beforeAction(opts)
6265
app.OsExiter = OSExiter
6366
app.ExitErrHandler = ExitErrHandler
67+
app.FlagErrHandler = flags.ErrorHandler(terragruntCommands)
6468

6569
return &App{app, opts}
6670
}

Diff for: cli/app_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@ func TestParseTerragruntOptionsFromArgs(t *testing.T) {
6161
{
6262
[]string{"--foo", "--bar"},
6363
mockOptions(t, util.JoinPath(workingDir, config.DefaultTerragruntConfigPath), workingDir, []string{"-foo", "-bar"}, false, "", false, false, defaultLogLevel, false),
64-
clipkg.UndefinedFlagError("flag provided but not defined: -foo"),
64+
clipkg.UndefinedFlagError("foo"),
6565
},
6666

6767
{
6868
[]string{"--foo", "apply", "--bar"},
6969
mockOptions(t, util.JoinPath(workingDir, config.DefaultTerragruntConfigPath), workingDir, []string{"apply", "-foo", "-bar"}, false, "", false, false, defaultLogLevel, false),
70-
clipkg.UndefinedFlagError("flag provided but not defined: -foo"),
70+
clipkg.UndefinedFlagError("foo"),
7171
},
7272

7373
{

Diff for: cli/flags/error_handler.go

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package flags
2+
3+
import (
4+
"slices"
5+
"strings"
6+
7+
"github.com/gruntwork-io/terragrunt/internal/cli"
8+
"github.com/gruntwork-io/terragrunt/internal/errors"
9+
"github.com/gruntwork-io/terragrunt/util"
10+
)
11+
12+
// ErrorHandler returns `FlagErrHandlerFunc` which takes a flag parsing error
13+
// and tries to suggest the correct command to use with this flag. Otherwise returns the error as is.
14+
func ErrorHandler(commands cli.Commands) cli.FlagErrHandlerFunc {
15+
return func(ctx *cli.Context, err error) error {
16+
var undefinedFlagErr cli.UndefinedFlagError
17+
if !errors.As(err, &undefinedFlagErr) {
18+
return err
19+
}
20+
21+
undefFlag := string(undefinedFlagErr)
22+
23+
if cmds, flag := findFlagInCommands(commands, undefFlag); cmds != nil {
24+
var (
25+
flagHint = util.FirstElement(util.RemoveEmptyElements(flag.Names()))
26+
cmdHint = strings.Join(cmds.Names(), " ")
27+
)
28+
29+
if ctx.Parent().Command == nil {
30+
return NewGlobalFlagHintError(undefFlag, cmdHint, flagHint)
31+
}
32+
33+
return NewCommandFlagHintError(ctx.Command.Name, undefFlag, cmdHint, flagHint)
34+
}
35+
36+
return err
37+
}
38+
}
39+
40+
func findFlagInCommands(commands cli.Commands, undefFlag string) (cli.Commands, cli.Flag) {
41+
if len(commands) == 0 {
42+
return nil, nil
43+
}
44+
45+
for _, cmd := range commands {
46+
for _, flag := range cmd.Flags {
47+
flagNames := flag.Names()
48+
49+
if flag, ok := flag.(interface{ DeprecatedNames() []string }); ok {
50+
flagNames = append(flagNames, flag.DeprecatedNames()...)
51+
}
52+
53+
if slices.Contains(flagNames, undefFlag) {
54+
return cli.Commands{cmd}, flag
55+
}
56+
}
57+
58+
if cmds, flag := findFlagInCommands(cmd.Subcommands, undefFlag); cmds != nil {
59+
return append(cli.Commands{cmd}, cmds...), flag
60+
}
61+
}
62+
63+
return nil, nil
64+
}

Diff for: cli/flags/errors.go

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package flags
2+
3+
import "fmt"
4+
5+
var _ error = new(GlobalFlagHintError)
6+
7+
type GlobalFlagHintError struct {
8+
undefFlag string
9+
cmdHint string
10+
flagHint string
11+
}
12+
13+
func NewGlobalFlagHintError(undefFlag, cmdHint, flagHint string) *GlobalFlagHintError {
14+
return &GlobalFlagHintError{
15+
undefFlag: undefFlag,
16+
cmdHint: cmdHint,
17+
flagHint: flagHint,
18+
}
19+
}
20+
21+
func (err GlobalFlagHintError) Error() string {
22+
return fmt.Sprintf("flag `--%s` is not a valid global flag. Did you mean to use `%s --%s`?", err.undefFlag, err.cmdHint, err.flagHint)
23+
}
24+
25+
var _ error = new(CommandFlagHintError)
26+
27+
type CommandFlagHintError struct {
28+
undefFlag string
29+
wrongCmd string
30+
cmdHint string
31+
flagHint string
32+
}
33+
34+
func NewCommandFlagHintError(wrongCmd, undefFlag, cmdHint, flagHint string) *CommandFlagHintError {
35+
return &CommandFlagHintError{
36+
undefFlag: undefFlag,
37+
wrongCmd: wrongCmd,
38+
cmdHint: cmdHint,
39+
flagHint: flagHint,
40+
}
41+
}
42+
43+
func (err CommandFlagHintError) Error() string {
44+
return fmt.Sprintf("flag `--%s` is not a valid flag for `%s`. Did you mean to use `%s --%s`?", err.undefFlag, err.wrongCmd, err.cmdHint, err.flagHint)
45+
}

Diff for: cli/flags/flag.go

+15
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,21 @@ func (newFlag *Flag) TakesValue() bool {
5151
return !ok || !val
5252
}
5353

54+
// DeprecatedNames returns all deprecated names for this flag.
55+
func (newFlag *Flag) DeprecatedNames() []string {
56+
var names []string
57+
58+
if flag, ok := newFlag.Flag.(interface{ DeprecatedNames() []string }); ok {
59+
names = flag.DeprecatedNames()
60+
}
61+
62+
for _, deprecated := range newFlag.deprecatedFlags {
63+
names = append(names, deprecated.Names()...)
64+
}
65+
66+
return names
67+
}
68+
5469
// Value implements `cli.Flag` interface.
5570
func (newFlag *Flag) Value() cli.FlagValue {
5671
for _, deprecatedFlag := range newFlag.deprecatedFlags {

Diff for: internal/cli/app.go

+3
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ type App struct {
4949
// is used as the default behavior.
5050
ExitErrHandler ExitErrHandlerFunc
5151

52+
// FlagErrHandler processes any error encountered while parsing flags.
53+
FlagErrHandler FlagErrHandlerFunc
54+
5255
// Autocomplete enables or disables subcommand auto-completion support.
5356
// This is enabled by default when NewApp is called. Otherwise, this
5457
// must enabled explicitly.

Diff for: internal/cli/command.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"strings"
77
)
88

9-
const ErrFlagUndefined = "flag provided but not defined:"
10-
119
type Command struct {
1210
// Name is the command name.
1311
Name string
@@ -102,12 +100,16 @@ func (cmd *Command) VisibleSubcommands() Commands {
102100
// If this is the final command, starts its execution.
103101
func (cmd *Command) Run(ctx *Context, args Args) (err error) {
104102
args, err = cmd.parseFlags(ctx, args.Slice())
103+
ctx = ctx.NewCommandContext(cmd, args)
104+
105105
if err != nil {
106+
if flagErrHandler := ctx.App.FlagErrHandler; flagErrHandler != nil {
107+
err = flagErrHandler(ctx, err)
108+
}
109+
106110
return NewExitError(err, ExitCodeGeneralError)
107111
}
108112

109-
ctx = ctx.NewCommandContext(cmd, args)
110-
111113
subCmdName := ctx.Args().CommandName()
112114
subCmdArgs := ctx.Args().Remove(subCmdName)
113115
subCmd := cmd.Subcommand(subCmdName)
@@ -223,8 +225,8 @@ func (cmd *Command) flagSetParse(ctx *Context, flagSet *libflag.FlagSet, args Ar
223225
}
224226

225227
if errStr := err.Error(); strings.HasPrefix(errStr, ErrFlagUndefined) {
226-
err = UndefinedFlagError(errStr)
227228
undefArg = strings.Trim(strings.TrimPrefix(errStr, ErrFlagUndefined), " -")
229+
err = UndefinedFlagError(undefArg)
228230
} else {
229231
break
230232
}

Diff for: internal/cli/commands.go

+11
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ func (commands Commands) Get(name string) *Command {
1919
return nil
2020
}
2121

22+
// Names returns names of the commands.
23+
func (commands Commands) Names() []string {
24+
var names = make([]string, len(commands))
25+
26+
for i, cmd := range commands {
27+
names[i] = cmd.Name
28+
}
29+
30+
return names
31+
}
32+
2233
// Add adds a new cmd to the list.
2334
func (commands *Commands) Add(cmd *Command) {
2435
*commands = append(*commands, cmd)

Diff for: internal/cli/errors.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ func (err InvalidValueError) Unwrap() error {
107107
return err.underlyingError
108108
}
109109

110+
const ErrFlagUndefined = "flag provided but not defined:"
111+
110112
type UndefinedFlagError string
111113

112-
func (err UndefinedFlagError) Error() string {
113-
return string(err)
114+
func (flag UndefinedFlagError) Error() string {
115+
return ErrFlagUndefined + " -" + string(flag)
114116
}

Diff for: internal/cli/funcs.go

+3
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,6 @@ type SplitterFunc func(s, sep string) []string
2121
// ExitErrHandlerFunc is executed if provided in order to handle exitError values
2222
// returned by Actions and Before/After functions.
2323
type ExitErrHandlerFunc func(ctx *Context, err error) error
24+
25+
// FlagErrHandlerFunc is executed if an error occurs while parsing flags.
26+
type FlagErrHandlerFunc func(ctx *Context, err error) error

Diff for: test/fixtures/cli-flag-hints/main.hcl

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Intentionally empty

Diff for: test/fixtures/cli-flag-hints/terragrunt.hcl

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Intentionally empty

Diff for: test/integration_test.go

+34
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/gruntwork-io/terragrunt/cli/commands/run"
1818
runall "github.com/gruntwork-io/terragrunt/cli/commands/run-all"
1919
terragruntinfo "github.com/gruntwork-io/terragrunt/cli/commands/terragrunt-info"
20+
"github.com/gruntwork-io/terragrunt/cli/flags"
2021
"github.com/gruntwork-io/terragrunt/codegen"
2122
"github.com/gruntwork-io/terragrunt/config"
2223
"github.com/gruntwork-io/terragrunt/internal/errors"
@@ -107,6 +108,7 @@ const (
107108
testFixtureExecCmd = "fixtures/exec-cmd"
108109
textFixtureDisjointSymlinks = "fixtures/stack/disjoint-symlinks"
109110
testFixtureLogStreaming = "fixtures/streaming"
111+
testFixtureCLIFlagHints = "fixtures/cli-flag-hints"
110112

111113
terraformFolder = ".terraform"
112114

@@ -116,6 +118,38 @@ const (
116118
terragruntCache = ".terragrunt-cache"
117119
)
118120

121+
func TestCLIFlagHints(t *testing.T) {
122+
t.Parallel()
123+
124+
testCases := []struct {
125+
args string
126+
expectedError error
127+
}{
128+
{
129+
"-raw init",
130+
flags.NewGlobalFlagHintError("raw", "stack output", "raw"),
131+
},
132+
{
133+
"run --no-include-root",
134+
flags.NewCommandFlagHintError("run", "no-include-root", "catalog", "no-include-root"),
135+
},
136+
}
137+
138+
for i, testCase := range testCases {
139+
t.Run(fmt.Sprintf("testCase-%d", i), func(t *testing.T) {
140+
t.Parallel()
141+
142+
helpers.CleanupTerraformFolder(t, testFixtureCLIFlagHints)
143+
rootPath := helpers.CopyEnvironment(t, testFixtureCLIFlagHints)
144+
rootPath, err := filepath.EvalSymlinks(rootPath)
145+
require.NoError(t, err)
146+
147+
_, _, err = helpers.RunTerragruntCommandWithOutput(t, "terragrunt "+testCase.args+" --working-dir "+rootPath)
148+
assert.EqualError(t, err, testCase.expectedError.Error())
149+
})
150+
}
151+
}
152+
119153
func TestExecCommand(t *testing.T) {
120154
t.Parallel()
121155

0 commit comments

Comments
 (0)