diff --git a/cmd/aws.go b/cmd/aws.go index 5572c247..280ac850 100644 --- a/cmd/aws.go +++ b/cmd/aws.go @@ -19,6 +19,9 @@ import ( ) func newAWSCmd(cfg *env.Env) *cobra.Command { + // DisableFlagParsing means Cobra won't strip lstk's own flags; PreRunE does + // that and stashes the remaining args here for RunE to forward to aws. + var passthrough []string return &cobra.Command{ Use: "aws [args...]", Short: "Run AWS CLI commands against LocalStack", @@ -35,10 +38,21 @@ Examples: lstk aws sqs list-queues lstk aws s3 mb s3://my-bucket`, DisableFlagParsing: true, - PreRunE: initConfig(nil), - RunE: func(cmd *cobra.Command, args []string) error { - args, nonInteractive := stripNonInteractiveFlag(args) - + PreRunE: func(cmd *cobra.Command, args []string) error { + var gf globalFlags + passthrough, gf = stripGlobalFlags(args) + if gf.nonInteractive { + cfg.NonInteractive = true + } + if gf.configPath != "" { + // initConfig reads the "config" flag, so feed the value back to it. + if err := cmd.Flags().Set("config", gf.configPath); err != nil { + return err + } + } + return initConfig(nil)(cmd, args) + }, + RunE: func(cmd *cobra.Command, _ []string) error { rt, err := runtime.NewDockerRuntime(cfg.DockerHost) if err != nil { return err @@ -87,7 +101,7 @@ Examples: } stdout, stderr := io.Writer(os.Stdout), io.Writer(os.Stderr) - if !nonInteractive && terminal.IsTerminal(os.Stderr) { + if !cfg.NonInteractive && terminal.IsTerminal(os.Stderr) { s := terminal.NewSpinner(os.Stderr, "Loading service...", 4*time.Second) s.Start() defer s.Stop() @@ -95,24 +109,7 @@ Examples: stderr = &terminal.StopOnWriteWriter{W: os.Stderr, Spinner: s} } - return awscli.Exec(cmd.Context(), "http://"+host, profileExists, stdout, stderr, args) + return awscli.Exec(cmd.Context(), "http://"+host, profileExists, stdout, stderr, passthrough) }, } } - -// stripNonInteractiveFlag pulls lstk's --non-interactive flag out of the AWS CLI -// passthrough args and reports whether it was set. The aws command uses -// DisableFlagParsing, so Cobra never parses the flag here — left in place it would -// be forwarded to the aws binary and rejected as an unknown option. -func stripNonInteractiveFlag(args []string) ([]string, bool) { - out := make([]string, 0, len(args)) - nonInteractive := false - for _, a := range args { - if a == "--non-interactive" { - nonInteractive = true - continue - } - out = append(out, a) - } - return out, nonInteractive -} diff --git a/cmd/aws_test.go b/cmd/aws_test.go deleted file mode 100644 index b6fee834..00000000 --- a/cmd/aws_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package cmd - -import ( - "reflect" - "testing" -) - -func TestStripNonInteractiveFlag(t *testing.T) { - tests := []struct { - name string - args []string - wantArgs []string - wantNonInteract bool - }{ - { - name: "absent", - args: []string{"s3", "ls"}, - wantArgs: []string{"s3", "ls"}, - wantNonInteract: false, - }, - { - name: "bare flag is stripped and enables non-interactive", - args: []string{"--non-interactive", "s3", "ls"}, - wantArgs: []string{"s3", "ls"}, - wantNonInteract: true, - }, - { - name: "flag among aws args is stripped", - args: []string{"s3", "ls", "--non-interactive", "--recursive"}, - wantArgs: []string{"s3", "ls", "--recursive"}, - wantNonInteract: true, - }, - { - name: "does not strip a similarly named aws flag", - args: []string{"s3", "ls", "--non-interactive-mode"}, - wantArgs: []string{"s3", "ls", "--non-interactive-mode"}, - wantNonInteract: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotArgs, gotNonInteract := stripNonInteractiveFlag(tt.args) - if gotNonInteract != tt.wantNonInteract { - t.Errorf("nonInteractive = %v, want %v", gotNonInteract, tt.wantNonInteract) - } - if !reflect.DeepEqual(gotArgs, tt.wantArgs) { - t.Errorf("args = %v, want %v", gotArgs, tt.wantArgs) - } - }) - } -} diff --git a/cmd/proxy.go b/cmd/proxy.go new file mode 100644 index 00000000..e2e96bd0 --- /dev/null +++ b/cmd/proxy.go @@ -0,0 +1,44 @@ +package cmd + +import ( + "strconv" + "strings" +) + +type globalFlags struct { + nonInteractive bool + configPath string +} + +// stripGlobalFlags removes lstk's persistent flags (--non-interactive and +// --config) from a proxy command's arguments, returning the remaining args and +// the extracted values. Proxy commands set DisableFlagParsing, so without this +// these flags would be forwarded to the wrapped binary (which rejects them as +// unknown) and their effect silently lost. Both --flag value and --flag=value +// forms are recognized, in any position. +func stripGlobalFlags(args []string) ([]string, globalFlags) { + out := make([]string, 0, len(args)) + var gf globalFlags + for i := 0; i < len(args); i++ { + arg := args[i] + switch { + case arg == "--non-interactive": + gf.nonInteractive = true + case strings.HasPrefix(arg, "--non-interactive="): + // A malformed value still strips the flag (it must never reach the + // wrapped binary) and enables the mode, matching the user's intent. + v, err := strconv.ParseBool(strings.TrimPrefix(arg, "--non-interactive=")) + gf.nonInteractive = err != nil || v + case arg == "--config": + if i+1 < len(args) { + gf.configPath = args[i+1] + i++ + } + case strings.HasPrefix(arg, "--config="): + gf.configPath = strings.TrimPrefix(arg, "--config=") + default: + out = append(out, arg) + } + } + return out, gf +} diff --git a/cmd/proxy_test.go b/cmd/proxy_test.go new file mode 100644 index 00000000..0b9a5a2f --- /dev/null +++ b/cmd/proxy_test.go @@ -0,0 +1,96 @@ +package cmd + +import ( + "reflect" + "testing" +) + +func TestStripGlobalFlags(t *testing.T) { + tests := []struct { + name string + args []string + wantArgs []string + wantNonInteract bool + wantConfigPath string + }{ + { + name: "no global flags", + args: []string{"s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + }, + { + name: "bare non-interactive is stripped", + args: []string{"--non-interactive", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantNonInteract: true, + }, + { + name: "non-interactive among aws args is stripped", + args: []string{"s3", "ls", "--non-interactive", "--recursive"}, + wantArgs: []string{"s3", "ls", "--recursive"}, + wantNonInteract: true, + }, + { + name: "non-interactive with explicit true value", + args: []string{"--non-interactive=true", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantNonInteract: true, + }, + { + name: "non-interactive with explicit false value", + args: []string{"--non-interactive=false", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantNonInteract: false, + }, + { + name: "config with separate value", + args: []string{"--config", "/tmp/c.toml", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantConfigPath: "/tmp/c.toml", + }, + { + name: "config with equals value", + args: []string{"--config=/tmp/c.toml", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantConfigPath: "/tmp/c.toml", + }, + { + name: "config among aws args", + args: []string{"s3", "ls", "--config", "/tmp/c.toml"}, + wantArgs: []string{"s3", "ls"}, + wantConfigPath: "/tmp/c.toml", + }, + { + name: "both flags together", + args: []string{"--non-interactive", "--config=/tmp/c.toml", "s3", "ls"}, + wantArgs: []string{"s3", "ls"}, + wantNonInteract: true, + wantConfigPath: "/tmp/c.toml", + }, + { + name: "trailing config without value is dropped", + args: []string{"s3", "ls", "--config"}, + wantArgs: []string{"s3", "ls"}, + }, + { + name: "similarly named flags are left untouched", + args: []string{"s3", "ls", "--non-interactive-mode", "--config-file", "x"}, + wantArgs: []string{"s3", "ls", "--non-interactive-mode", "--config-file", "x"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotArgs, gf := stripGlobalFlags(tt.args) + if !reflect.DeepEqual(gotArgs, tt.wantArgs) { + t.Errorf("args = %v, want %v", gotArgs, tt.wantArgs) + } + if gf.nonInteractive != tt.wantNonInteract { + t.Errorf("nonInteractive = %v, want %v", gf.nonInteractive, tt.wantNonInteract) + } + if gf.configPath != tt.wantConfigPath { + t.Errorf("configPath = %q, want %q", gf.configPath, tt.wantConfigPath) + } + }) + } +} diff --git a/test/integration/aws_cmd_test.go b/test/integration/aws_cmd_test.go index 1e0dca99..8c099036 100644 --- a/test/integration/aws_cmd_test.go +++ b/test/integration/aws_cmd_test.go @@ -69,6 +69,31 @@ func TestAWSCommandInjectsEndpointAndArgs(t *testing.T) { assertCommandTelemetry(t, events, "aws", 0) } +func TestAWSCommandStripsGlobalFlagsFromPassthrough(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + ctx := testContext(t) + startTestContainer(t, ctx) + + fakeDir := writeFakeAWS(t) + homeDir := t.TempDir() + writeAWSProfile(t, homeDir) + + // --config must resolve to this file, not be forwarded to the aws binary. + configPath := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte("# lstk test config\n"), 0600)) + + e := env.With(env.DisableEvents, "1").With("PATH", fakeDir).With(env.Home, homeDir) + + stdout, stderr, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "aws", "s3", "ls") + require.NoError(t, err, "lstk aws failed: %s", stderr) + + assert.Contains(t, stdout, "ARGS:--profile localstack s3 ls") + assert.NotContains(t, stdout, "--config") + assert.NotContains(t, stdout, "--non-interactive") +} + func TestAWSCommandInjectsCredentials(t *testing.T) { requireDocker(t) cleanup()