Skip to content

Remove global state #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carole-lavillonniere merged 4 commits into from
Mar 4, 2026
Merged

Remove global state #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f7a08b0
commands are not vars
carole-lavillonniere Mar 3, 2026
fc5caa1
inject envs vars
carole-lavillonniere Mar 3, 2026
64e95b8
Merge branch 'main' into deps-inj
carole-lavillonniere Mar 4, 2026
895fac8
address comments
carole-lavillonniere Mar 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Environment variables:
- Do not remove comments added by someone else than yourself.
- Errors returned by functions should always be checked unless in test files.
- Terminology: in user-facing CLI/help/docs, prefer `emulator` over `container`/`runtime`; use `container`/`runtime` only for internal implementation details.
- Avoid package-level global variables. Use constructor functions that return fresh instances and inject dependencies explicitly. This keeps packages testable in isolation and prevents shared mutable state between tests.

# Testing

Expand Down
37 changes: 19 additions & 18 deletions cmd/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,26 @@ import (
"github.com/spf13/cobra"
)

var configCmd = &cobra.Command{
Use: "config",
Short: "Manage configuration",
func newConfigCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "config",
Short: "Manage configuration",
}
cmd.AddCommand(newConfigPathCmd())
return cmd
}

var configPathCmd = &cobra.Command{
Use: "path",
Short: "Print the configuration file path",
RunE: func(cmd *cobra.Command, args []string) error {
configPath, err := config.ConfigFilePath()
if err != nil {
func newConfigPathCmd() *cobra.Command {
return &cobra.Command{
Use: "path",
Short: "Print the configuration file path",
RunE: func(cmd *cobra.Command, args []string) error {
configPath, err := config.ConfigFilePath()
if err != nil {
return err
}
_, err = fmt.Fprintln(cmd.OutOrStdout(), configPath)
return err
}
_, err = fmt.Fprintln(cmd.OutOrStdout(), configPath)
return err
},
}

func init() {
configCmd.AddCommand(configPathCmd)
rootCmd.AddCommand(configCmd)
},
}
}
43 changes: 7 additions & 36 deletions cmd/help_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,18 @@ import (
"strings"
"testing"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/localstack/lstk/internal/env"
)

func executeWithArgs(t *testing.T, args ...string) (string, error) {
t.Helper()

origOut := rootCmd.OutOrStdout()
origErr := rootCmd.ErrOrStderr()
resetCommandState(rootCmd)

buf := new(bytes.Buffer)
rootCmd.SetOut(buf)
rootCmd.SetErr(buf)
rootCmd.SetArgs(args)

err := rootCmd.ExecuteContext(context.Background())
out := buf.String()

rootCmd.SetArgs(nil)
rootCmd.SetOut(origOut)
rootCmd.SetErr(origErr)
resetCommandState(rootCmd)

return out, err
}

func resetCommandState(cmd *cobra.Command) {
resetFlagSet(cmd.Flags())
resetFlagSet(cmd.PersistentFlags())

for _, sub := range cmd.Commands() {
resetCommandState(sub)
}
}

func resetFlagSet(flags *pflag.FlagSet) {
flags.VisitAll(func(flag *pflag.Flag) {
_ = flag.Value.Set(flag.DefValue)
flag.Changed = false
})
cmd := NewRootCmd(&env.Env{})
cmd.SetOut(buf)
cmd.SetErr(buf)
cmd.SetArgs(args)
err := cmd.ExecuteContext(context.Background())
return buf.String(), err
}

func TestRootHelpOutputTemplate(t *testing.T) {
Expand Down
31 changes: 15 additions & 16 deletions cmd/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,24 @@ import (
"fmt"

"github.com/localstack/lstk/internal/api"
"github.com/localstack/lstk/internal/env"
"github.com/localstack/lstk/internal/ui"
"github.com/localstack/lstk/internal/version"
"github.com/spf13/cobra"
)

var loginCmd = &cobra.Command{
Use: "login",
Short: "Manage login",
Long: "Manage login and store credentials in system keyring",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
if !ui.IsInteractive() {
return fmt.Errorf("login requires an interactive terminal")
}
platformClient := api.NewPlatformClient()
return ui.RunLogin(cmd.Context(), version.Version(), platformClient)
},
}

func init() {
rootCmd.AddCommand(loginCmd)
func newLoginCmd(cfg *env.Env) *cobra.Command {
return &cobra.Command{
Use: "login",
Short: "Manage login",
Long: "Manage login and store credentials in system keyring",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
if !ui.IsInteractive() {
return fmt.Errorf("login requires an interactive terminal")
}
platformClient := api.NewPlatformClient(cfg.APIEndpoint)
return ui.RunLogin(cmd.Context(), version.Version(), platformClient, cfg.AuthToken, cfg.ForceFileKeyring, cfg.WebAppURL)
},
}
}
51 changes: 25 additions & 26 deletions cmd/logout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,36 @@ import (

"github.com/localstack/lstk/internal/api"
"github.com/localstack/lstk/internal/auth"
"github.com/localstack/lstk/internal/env"
"github.com/localstack/lstk/internal/output"
"github.com/localstack/lstk/internal/ui"
"github.com/spf13/cobra"
)

var logoutCmd = &cobra.Command{
Use: "logout",
Short: "Remove stored authentication credentials",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
if ui.IsInteractive() {
return ui.RunLogout(cmd.Context())
}

sink := output.NewPlainSink(os.Stdout)
platformClient := api.NewPlatformClient()
tokenStorage, err := auth.NewTokenStorage()
if err != nil {
return fmt.Errorf("failed to initialize token storage: %w", err)
}
a := auth.New(sink, platformClient, tokenStorage, false)
if err := a.Logout(); err != nil {
if errors.Is(err, auth.ErrNotLoggedIn) {
return nil
func newLogoutCmd(cfg *env.Env) *cobra.Command {
return &cobra.Command{
Use: "logout",
Short: "Remove stored authentication credentials",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
platformClient := api.NewPlatformClient(cfg.APIEndpoint)
if ui.IsInteractive() {
return ui.RunLogout(cmd.Context(), platformClient, cfg.AuthToken, cfg.ForceFileKeyring)
}
return fmt.Errorf("failed to logout: %w", err)
}
return nil
},
}

func init() {
rootCmd.AddCommand(logoutCmd)
sink := output.NewPlainSink(os.Stdout)
tokenStorage, err := auth.NewTokenStorage(cfg.ForceFileKeyring)
if err != nil {
return fmt.Errorf("failed to initialize token storage: %w", err)
}
a := auth.New(sink, platformClient, tokenStorage, cfg.AuthToken, "", false)
if err := a.Logout(); err != nil {
if errors.Is(err, auth.ErrNotLoggedIn) {
return nil
}
return fmt.Errorf("failed to logout: %w", err)
}
return nil
},
}
}
41 changes: 20 additions & 21 deletions cmd/logs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,24 @@ import (
"github.com/spf13/cobra"
)

var logsCmd = &cobra.Command{
Use: "logs",
Short: "Show emulator logs",
Long: "Show logs from the emulator. Use --follow to stream in real-time.",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
follow, err := cmd.Flags().GetBool("follow")
if err != nil {
return err
}
rt, err := runtime.NewDockerRuntime()
if err != nil {
return err
}
return container.Logs(cmd.Context(), rt, output.NewPlainSink(os.Stdout), follow)
},
}

func init() {
rootCmd.AddCommand(logsCmd)
logsCmd.Flags().BoolP("follow", "f", false, "Follow log output")
func newLogsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "logs",
Short: "Show emulator logs",
Long: "Show logs from the emulator. Use --follow to stream in real-time.",
PreRunE: initConfig,
RunE: func(cmd *cobra.Command, args []string) error {
follow, err := cmd.Flags().GetBool("follow")
if err != nil {
return err
}
rt, err := runtime.NewDockerRuntime()
if err != nil {
return err
}
return container.Logs(cmd.Context(), rt, output.NewPlainSink(os.Stdout), follow)
},
}
cmd.Flags().BoolP("follow", "f", false, "Follow log output")
return cmd
}
73 changes: 42 additions & 31 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,51 +16,62 @@ import (
"github.com/spf13/cobra"
)

var rootCmd = &cobra.Command{
Use: "lstk",
Short: "LocalStack CLI",
Long: "lstk is the command-line interface for LocalStack.",
PreRunE: initConfig,
Run: func(cmd *cobra.Command, args []string) {
rt, err := runtime.NewDockerRuntime()
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}

if err := runStart(cmd.Context(), rt); err != nil {
if !output.IsSilent(err) {
func NewRootCmd(cfg *env.Env) *cobra.Command {
root := &cobra.Command{
Use: "lstk",
Short: "LocalStack CLI",
Long: "lstk is the command-line interface for LocalStack.",
PreRunE: initConfig,
Run: func(cmd *cobra.Command, args []string) {
rt, err := runtime.NewDockerRuntime()
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
os.Exit(1)
}
},
}

func init() {
rootCmd.Version = version.Version()
rootCmd.SetVersionTemplate(versionLine() + "\n")
if err := runStart(cmd.Context(), rt, cfg); err != nil {
if !output.IsSilent(err) {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
}
os.Exit(1)
}
Comment on lines +25 to +37
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Check stderr write errors before exiting.

fmt.Fprintf results are ignored on Line 28 and Line 34. Please check and handle write errors explicitly.

🛠️ Minimal fix 
 			if err != nil {
-				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+				if _, writeErr := fmt.Fprintf(os.Stderr, "Error: %v\n", err); writeErr != nil {
+					// best-effort logging; continue exiting with original error
+				}
 				os.Exit(1)
 			}
@@
 			if err := runStart(cmd.Context(), rt, cfg); err != nil {
 				if !output.IsSilent(err) {
-					fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+					if _, writeErr := fmt.Fprintf(os.Stderr, "Error: %v\n", err); writeErr != nil {
+						// best-effort logging; continue exiting with original error
+					}
 				}
 				os.Exit(1)
 			}

As per coding guidelines: "Errors returned by functions should always be checked unless in test files."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` around lines 25 - 37, The fmt.Fprintf calls that write errors to
stderr after runtime.NewDockerRuntime() and after runStart() must have their
returned values checked; locate the two fmt.Fprintf(...) usages in the Run
function (surrounding runtime.NewDockerRuntime and the runStart error handling
that uses output.IsSilent) and replace the ignored write with a checked write
(capture the returned n, err := fmt.Fprintf(...)) and handle any write error
before calling os.Exit(1) — e.g., if fmt.Fprintf returns an error, attempt a
fallback write (such as fmt.Fprintln(os.Stderr, "failed to write to stderr:",
writeErr)) or log the write failure so the process still exits with the intended
status; ensure both occurrences are updated so no write errors are ignored.

},
}

root.Version = version.Version()
root.SetVersionTemplate(versionLine() + "\n")

configureHelp(root)

root.InitDefaultVersionFlag()
root.Flags().Lookup("version").Usage = "Show version"

configureHelp(rootCmd)
root.AddCommand(
newStartCmd(cfg),
newStopCmd(),
newLoginCmd(cfg),
newLogoutCmd(cfg),
newLogsCmd(),
newConfigCmd(),
newVersionCmd(),
)

rootCmd.InitDefaultVersionFlag()
rootCmd.Flags().Lookup("version").Usage = "Show version"
rootCmd.AddCommand(startCmd)
return root
}

func Execute(ctx context.Context) error {
return rootCmd.ExecuteContext(ctx)
cfg := env.Init()
return NewRootCmd(cfg).ExecuteContext(ctx)
Comment on lines +63 to +64
Copy link

@coderabbitai coderabbitai bot Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n cmd/root.go | head -80

Repository: localstack/lstk

Length of output: 2558

🏁 Script executed:

cat -n internal/env/env.go | head -100

Repository: localstack/lstk

Length of output: 1185

🏁 Script executed:

rg -n 'func Init|config\.|viper\.|os\.Getenv|LookupEnv' internal/env/env.go -A 2 -B 1

Repository: localstack/lstk

Length of output: 739

🏁 Script executed:

rg -n 'func initConfig|PreRunE' cmd/root.go -A 5

Repository: localstack/lstk

Length of output: 360

🏁 Script executed:

fd -e go internal/config | head -20

Repository: localstack/lstk

Length of output: 424

🏁 Script executed:

rg -n 'func Init' internal/config -A 10 | head -50

Repository: localstack/lstk

Length of output: 666

🏁 Script executed:

rg -n 'func loadConfig|func firstExistingConfigPath' internal/config -A 15

Repository: localstack/lstk

Length of output: 1763

🏁 Script executed:

cat -n internal/config/config.go | head -80

Repository: localstack/lstk

Length of output: 2186

Move env.Init() to after config.Init() is guaranteed to complete, or restructure to wire env initialization in the command's PreRunE.

The current code captures env values at line 63 before config.Init() has loaded any config file (which happens later via PreRunE at line 24). This means cfg captures defaults set by env.Init() only, not values that may exist in the config file for api_endpoint, web_app_url, or keyring. When runStart (lines 67–72) uses these stale values, it won't reflect config-file overrides.

Either call config.Init() before env.Init(), or move env initialization into PreRunE to ensure config is loaded first—consistent with the guideline: "When adding a new command that depends on configuration, wire config initialization explicitly in that command (PreRunE: initConfig)."

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@cmd/root.go` around lines 63 - 64, The code calls env.Init() before
config.Init(), causing cfg to capture defaults instead of config-file overrides;
update NewRootCmd/command wiring so environment initialization runs after
configuration is loaded—either call config.Init() before env.Init() or move
env.Init() into the command's PreRunE (e.g., after initConfig runs) so runStart
and any use of cfg reflect config-file values; locate env.Init(), config.Init(),
NewRootCmd, PreRunE, and runStart in the diff and reorder or invoke env.Init()
from PreRunE (after initConfig) accordingly.

}

func runStart(ctx context.Context, rt runtime.Runtime) error {
platformClient := api.NewPlatformClient()
func runStart(ctx context.Context, rt runtime.Runtime, cfg *env.Env) error {
platformClient := api.NewPlatformClient(cfg.APIEndpoint)
if ui.IsInteractive() {
return ui.Run(ctx, rt, version.Version(), platformClient)
return ui.Run(ctx, rt, version.Version(), platformClient, cfg.AuthToken, cfg.ForceFileKeyring, cfg.WebAppURL)
}
return container.Start(ctx, rt, output.NewPlainSink(os.Stdout), platformClient, false)
return container.Start(ctx, rt, output.NewPlainSink(os.Stdout), platformClient, cfg.AuthToken, cfg.ForceFileKeyring, cfg.WebAppURL, false)
}

func initConfig(_ *cobra.Command, _ []string) error {
env.Init()
return config.Init()
}
Loading