From d4096514d695e264a64eac95ce65720ca7122782 Mon Sep 17 00:00:00 2001 From: Anisa Oshafi Date: Tue, 13 Oct 2026 18:36:37 +0200 Subject: [PATCH 1/3] Add lstk inspect to turn App Inspector on or off --- CLAUDE.md | 5 +- README.md | 1 + cmd/inspect.go | 160 ++++++ cmd/root.go | 1 + docs/structured-output.md | 21 + internal/config/emulator_type.go | 11 +- internal/config/env_profile.go | 184 +++++++ internal/config/env_profile_test.go | 102 ++++ internal/emulator/aws/appinspector.go | 105 ++++ internal/emulator/aws/appinspector_test.go | 102 ++++ internal/inspect/inspect.go | 219 ++++++++ internal/inspect/inspect_test.go | 35 ++ internal/ui/run_inspect.go | 20 + openspec/changes/json-output-schema/tasks.md | 1 + .../__snapshots__/exit_code_test.snap | 27 + .../__snapshots__/extension_test.snap | 3 + test/integration/endpoint_url_test.go | 1 + test/integration/exit_code_test.go | 3 +- test/integration/inspect_test.go | 482 ++++++++++++++++++ 19 files changed, 1472 insertions(+), 11 deletions(-) create mode 100644 cmd/inspect.go create mode 100644 internal/config/env_profile.go create mode 100644 internal/config/env_profile_test.go create mode 100644 internal/emulator/aws/appinspector.go create mode 100644 internal/emulator/aws/appinspector_test.go create mode 100644 internal/inspect/inspect.go create mode 100644 internal/inspect/inspect_test.go create mode 100644 internal/ui/run_inspect.go create mode 100644 test/integration/inspect_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 698cc66f6..b7f1ce5aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,6 +63,7 @@ Notes: - `env/` - Process environment snapshot/injection helper (also used to isolate test envs) - `extension/` - Git-style `lstk-` extension resolution and exec - `iac/` - Wrappers for third-party infrastructure as code tools (`terraform/`, `cdk/`, `sam/`) + - `inspect/` - `lstk inspect` domain logic: turns App Inspector on/off through the AWS emulator's runtime switch; `--persist` saves `APP_INSPECTOR` to an lstk-owned `[env.app-inspector]` profile (via `config.SetEnvProfileVar`) so it survives restarts - `log/` - Internal diagnostic logging (not for user-facing output — use `output/` for that) - `output/` - Generic event and sink abstractions for CLI/TUI/non-interactive rendering - `ports/` - Port availability checks @@ -79,7 +80,7 @@ Notes: - `version/` - Version info - `volume/` - `lstk volume` domain logic -Commands are registered in `cmd/root.go` in two Cobra groups: the `commands` group (start, stop, restart, login, logout, status, logs, setup, config, volume, update, docs, snapshot, reset, save, load) and the `tools` group of proxy commands (aws, terraform/tf, cdk, sam, az). Shared helpers: `cmd/root.go` (wiring, groups, `requireSubcommand`, `initConfig`), `cmd/help.go` (help template), `cmd/iac.go` (IaC command boundary), `cmd/extension.go` (extension dispatch). +Commands are registered in `cmd/root.go` in two Cobra groups: the `commands` group (start, stop, restart, login, logout, status, logs, inspect, setup, config, volume, update, docs, snapshot, reset, save, load) and the `tools` group of proxy commands (aws, terraform/tf, cdk, sam, az). Shared helpers: `cmd/root.go` (wiring, groups, `requireSubcommand`, `initConfig`), `cmd/help.go` (help template), `cmd/iac.go` (IaC command boundary), `cmd/extension.go` (extension dispatch). ## Container runtime discovery @@ -144,7 +145,7 @@ There is no `--offline` flag. Instead `container.Start` degrades gracefully when # Targeting an External Emulator (`--endpoint-url`) -`--endpoint-url ` (a root persistent flag) lets `aws`, `az`, `terraform`/`tf`, `cdk`, `sam`, `snapshot save`/`load` (incl. the `lstk save`/`lstk load` aliases), `snapshot remove`, `snapshot list s3://...`, `reset`, and `status` target an emulator lstk did not start — docker compose, host-network mode, CI, a different machine, or a LocalStack cloud-hosted **ephemeral instance** — instead of discovering one via local Docker. `LSTK_ENDPOINT_URL` is the equivalent environment variable. Both `http://` and `https://` are accepted (any other scheme is rejected); the resolved scheme is preserved end-to-end all the way to whatever ultimately makes the request — the wrapped `aws`/`terraform`/`cdk`/`sam` tools, and the emulator API calls behind `snapshot`/`reset`/`status` — never normalized to `http`, which is what makes `https://` ephemeral instances work. +`--endpoint-url ` (a root persistent flag) lets `aws`, `az`, `terraform`/`tf`, `cdk`, `sam`, `snapshot save`/`load` (incl. the `lstk save`/`lstk load` aliases), `snapshot remove`, `snapshot list s3://...`, `reset`, `inspect` (except `--persist`, which only edits lstk's config), and `status` target an emulator lstk did not start — docker compose, host-network mode, CI, a different machine, or a LocalStack cloud-hosted **ephemeral instance** — instead of discovering one via local Docker. `LSTK_ENDPOINT_URL` is the equivalent environment variable. Both `http://` and `https://` are accepted (any other scheme is rejected); the resolved scheme is preserved end-to-end all the way to whatever ultimately makes the request — the wrapped `aws`/`terraform`/`cdk`/`sam` tools, and the emulator API calls behind `snapshot`/`reset`/`status` — never normalized to `http`, which is what makes `https://` ephemeral instances work. `lstk logs`, `stop`, `restart`, `volume` (`path`/`clear`), `start`, and the bare `lstk` root command (which starts the emulator the same way `start` does) have no remote equivalent and reject *any* resolved endpoint source. diff --git a/README.md b/README.md index ec7316438..a2491121b 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ Running `lstk` will automatically handle authentication, configuration, and cont - **Interactive TUI** — a Bubble Tea-powered terminal UI in interactive terminals, plain output for CI/CD and scripting - **Browser-based login** — authenticate via browser and store credentials securely in the system keyring, or use `LOCALSTACK_AUTH_TOKEN` for CI (it takes precedence over stored credentials) - **Snapshots** — save, load, and manage emulator state as local files, cloud snapshots, or in your own S3 bucket +- **App Inspector**: `lstk inspect enable` turns on App Inspector for the AWS emulator; add `--persist` to keep it on after restarts - **Cloud CLI proxies** — run `aws`, `az`, `terraform`, `cdk`, and `sam` commands against LocalStack with the endpoint, credentials, and region pre-configured - **Target an external emulator** — pass `--endpoint-url ` (or set `LSTK_ENDPOINT_URL`) to point most commands at an already-running LocalStack instance — docker compose, host-network mode, CI, a different machine, or a cloud-hosted ephemeral instance (`https://` is supported) — instead of one lstk manages locally - **Extensions** — Git-style `lstk-` executables extend the CLI with new commands; see [extension authoring](https://github.com/localstack/lstk/blob/main/docs/extensions-authoring.md) diff --git a/cmd/inspect.go b/cmd/inspect.go new file mode 100644 index 000000000..53eda6738 --- /dev/null +++ b/cmd/inspect.go @@ -0,0 +1,160 @@ +package cmd + +import ( + "errors" + "fmt" + "os" + + "github.com/localstack/lstk/internal/config" + "github.com/localstack/lstk/internal/emulator/aws" + "github.com/localstack/lstk/internal/endpoint" + "github.com/localstack/lstk/internal/env" + "github.com/localstack/lstk/internal/inspect" + "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/runtime" + "github.com/localstack/lstk/internal/ui" + "github.com/spf13/cobra" +) + +func newInspectCmd(cfg *env.Env) *cobra.Command { + cmd := &cobra.Command{ + Use: "inspect", + Short: "Turn App Inspector on or off", + Long: "Turn App Inspector on or off for the AWS emulator. App Inspector records the operations your application sends to the emulator, with their payloads, the services they connect, and IAM permission results. View them in the LocalStack Web Application or the LocalStack Toolkit for VS Code.", + } + requireSubcommand(cmd) + cmd.AddCommand(newInspectSetCmd(cfg, true)) + cmd.AddCommand(newInspectSetCmd(cfg, false)) + cmd.AddCommand(newInspectStatusCmd(cfg)) + return cmd +} + +func newInspectSetCmd(cfg *env.Env, enabled bool) *cobra.Command { + var persist bool + use, title, state := "enable", "Enable", "enabled" + if !enabled { + use, title, state = "disable", "Disable", "disabled" + } + + cmd := &cobra.Command{ + Use: use, + Short: title + " App Inspector", + Long: title + " App Inspector on the running AWS emulator.\n\n" + + "The change lasts until the emulator restarts. With --persist, lstk also saves it to the config file (as APP_INSPECTOR in an [env.app-inspector] profile), so App Inspector is " + state + " on every start. --persist works while the emulator is stopped too.", + Args: cobra.NoArgs, + PreRunE: initConfigDeferCreate(nil), + RunE: func(cmd *cobra.Command, args []string) error { + sink := output.NewPlainSink(os.Stdout) + t, err := resolveInspectTarget(cmd, cfg, sink, persist) + if err != nil { + return err + } + if isInteractiveMode(cfg) { + return ui.RunInspectSet(cmd.Context(), t, enabled, persist) + } + return inspect.Set(cmd.Context(), sink, t, enabled, persist) + }, + } + cmd.Flags().BoolVar(&persist, "persist", false, "Also save the setting to the config file so it applies on every start") + return cmd +} + +func newInspectStatusCmd(cfg *env.Env) *cobra.Command { + return &cobra.Command{ + Use: "status", + Short: "Show whether App Inspector is enabled", + Args: cobra.NoArgs, + PreRunE: initConfigDeferCreate(nil), + RunE: func(cmd *cobra.Command, args []string) error { + sink := output.NewPlainSink(os.Stdout) + t, err := resolveInspectTarget(cmd, cfg, sink, false) + if err != nil { + return err + } + if isInteractiveMode(cfg) { + return ui.RunInspectStatus(cmd.Context(), t) + } + return inspect.Status(cmd.Context(), sink, t) + }, + } +} + +// resolveInspectTarget picks the AWS emulator to act on: the --endpoint-url +// target when one resolves, otherwise the AWS emulator in the lstk config. +func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, persist bool) (inspect.Target, error) { + fail := func(e output.ErrorEvent) (inspect.Target, error) { + sink.Emit(e) + return inspect.Target{}, output.NewSilentError(errors.New(e.Title)) + } + + remote, err := endpoint.Resolve(cmd.Context(), cmd) + if err != nil { + return inspect.Target{}, err + } + if remote != nil { + if persist { + source, _, _ := endpoint.ResolvedSource(cmd) + return fail(output.ErrorEvent{ + Title: fmt.Sprintf("--persist cannot be used with %s", source), + Summary: fmt.Sprintf("--persist saves to the lstk config, which does not start the emulator at %s.", remote.URL), + }) + } + if remote.Type != config.EmulatorAWS { + return fail(output.ErrorEvent{ + Title: fmt.Sprintf("App Inspector is only available for the AWS emulator, but the endpoint at %s is a %s", remote.URL, remote.Type.DisplayName()), + Code: output.ErrEmulatorWrongType, + }) + } + c := config.ContainerConfig{Type: config.EmulatorAWS, Port: config.DefaultPort} + return inspect.Target{ + Runtime: runtime.NewExternalRuntime(c.Name()), + Containers: []config.ContainerConfig{c}, + Client: aws.NewClient(), + Host: remote.URL, + }, nil + } + + appConfig, err := config.Get() + if err != nil { + return inspect.Target{}, fmt.Errorf("failed to get config: %w", err) + } + var awsContainer *config.ContainerConfig + for i := range appConfig.Containers { + if appConfig.Containers[i].Type == config.EmulatorAWS { + awsContainer = &appConfig.Containers[i] + break + } + } + if awsContainer == nil { + return fail(output.ErrorEvent{Title: "App Inspector is only available for the AWS emulator", Code: output.ErrEmulatorNotConfigured}) + } + if persist { + if !config.HasFile() { + return fail(output.ErrorEvent{ + Title: "No config file to save the setting to yet", + Actions: []output.ErrorAction{{Label: "Create it by starting the emulator once:", Value: "lstk start"}}, + }) + } + // SetEnvProfileVar edits the first [[containers]] block. + if appConfig.Containers[0].Type != config.EmulatorAWS { + return fail(output.ErrorEvent{Title: "--persist needs the AWS emulator to be the first [[containers]] block in the config"}) + } + } + + rt, err := runtime.NewDockerRuntime(cfg.DockerHost) + if err != nil { + return inspect.Target{}, err + } + host, _ := endpoint.ResolveHost(cmd.Context(), awsContainer.Port, cfg.LocalStackHost) + path, err := config.FriendlyConfigPath() + if err != nil { + path = "the config file" + } + return inspect.Target{ + Runtime: rt, + Containers: []config.ContainerConfig{*awsContainer}, + Client: aws.NewClient(), + Host: "http://" + host, + Config: &inspect.LocalConfig{Container: *awsContainer, Env: appConfig.Env, Path: path}, + }, nil +} diff --git a/cmd/root.go b/cmd/root.go index ea4530398..fb0be1510 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -200,6 +200,7 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C newLogoutCmd(cfg, logger), newStatusCmd(cfg), newLogsCmd(cfg), + newInspectCmd(cfg), newSetupCmd(cfg), newConfigCmd(), newVolumeCmd(cfg), diff --git a/docs/structured-output.md b/docs/structured-output.md index 4b8ae6597..cdf39807e 100644 --- a/docs/structured-output.md +++ b/docs/structured-output.md @@ -604,6 +604,27 @@ Codes: `AUTH_REQUIRED` (platform), `CREDENTIALS_MISSING`/`SNAPSHOT_BUCKET_NOT_FO ``` Codes: `AUTH_REQUIRED`, `SNAPSHOT_NOT_FOUND`, `SNAPSHOT_INVALID_REF`, `CONFIRMATION_REQUIRED`, `SNAPSHOT_REMOTE_ERROR`. +#### App Inspector + +**`lstk inspect enable`** / **`lstk inspect disable`** / **`lstk inspect status`**: App Inspector's state on the running emulator, and what the config sets for the next start. `changed` and `persisted` are omitted for `status`; `running` is `false` only for `enable --persist`/`disable --persist` while the emulator is stopped, in which case `enabled` is `null`. `enabledOnStart` is omitted for an `--endpoint-url` target, since lstk does not start that emulator. +```json +{ + "schemaVersion": 1, + "command": "inspect enable", + "status": "ok", + "data": { + "running": true, + "enabled": true, + "changed": true, + "persisted": false, + "enabledOnStart": false + }, + "warnings": [], + "error": null +} +``` +Codes: `RUNTIME_UNAVAILABLE`, `EMULATOR_NOT_RUNNING`, `EMULATOR_NOT_CONFIGURED`, `EMULATOR_WRONG_TYPE`, `CONFIG_INVALID`. + ## Commands that will never support `--json` - **`login`** — requires an interactive terminal unconditionally (browser-based OAuth) and has no defined non-interactive behavior at all today, so there's no output to render as JSON. diff --git a/internal/config/emulator_type.go b/internal/config/emulator_type.go index ff5af81cc..6d927940f 100644 --- a/internal/config/emulator_type.go +++ b/internal/config/emulator_type.go @@ -55,16 +55,11 @@ func SetEmulatorType(to EmulatorType) error { // profile) is never mistaken for the emulator type. Only the captured value // is replaced, leaving commented-out example blocks and the original // formatting untouched. - header := containersHeaderRe.FindIndex(data) - if header == nil { + blockStart, blockEnd, ok := firstContainersBlock(data) + if !ok { return fmt.Errorf("no [[containers]] block found in config") } - blockStart := header[1] - block := data[blockStart:] - if next := tableHeaderRe.FindIndex(block); next != nil { - block = block[:next[0]] - } - loc := typeLineRe.FindSubmatchIndex(block) + loc := typeLineRe.FindSubmatchIndex(data[blockStart:blockEnd]) if loc == nil { return fmt.Errorf("no emulator type field found in config") } diff --git a/internal/config/env_profile.go b/internal/config/env_profile.go new file mode 100644 index 000000000..774da8542 --- /dev/null +++ b/internal/config/env_profile.go @@ -0,0 +1,184 @@ +package config + +import ( + "bytes" + "errors" + "fmt" + "os" + "regexp" + "strings" +) + +// envListRe matches the opening of an `env = [` assignment at line start, so a +// commented-out `# env = []` never matches. quotedRe matches one TOML string +// in that list. +var ( + envListRe = regexp.MustCompile(`(?m)^[ \t]*env[ \t]*=[ \t]*\[`) + quotedRe = regexp.MustCompile(`"[^"\n]*"|'[^'\n]*'`) +) + +// SetEnvProfileVar makes the first [[containers]] block apply the [env.] +// profile with key set to value. The profile is appended to the block's env +// list, so it is applied after (and wins over) the profiles already there, and +// the table is created at the end of the file when missing. +// +// The file is edited in place so comments and formatting survive. The edit is +// rolled back if the result does not load, or if a profile listed after this +// one still overrides the value. +func SetEnvProfileVar(profile, key, value string) error { + path := resolvedConfigPath() + if path == "" { + return errors.New("no config file to write to yet") + } + original, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("failed to read config file: %w", err) + } + updated, err := withEnvReference(original, profile) + if err != nil { + return err + } + updated = withProfileVar(updated, profile, key, value) + if bytes.Equal(updated, original) { + return nil + } + + info, err := os.Stat(path) + if err != nil { + return fmt.Errorf("failed to read config file: %w", err) + } + if err := os.WriteFile(path, updated, info.Mode().Perm()); err != nil { + return fmt.Errorf("failed to write config file: %w", err) + } + if err := checkEnvProfileVar(path, profile, key, value); err != nil { + if restoreErr := os.WriteFile(path, original, info.Mode().Perm()); restoreErr != nil { + return fmt.Errorf("%w (restoring %s also failed: %v)", err, path, restoreErr) + } + return errors.Join(err, loadConfig(path)) + } + return nil +} + +// checkEnvProfileVar reloads the config and confirms the first container now +// resolves key to value. +func checkEnvProfileVar(path, profile, key, value string) error { + if err := loadConfig(path); err != nil { + return err + } + cfg, err := Get() + if err != nil { + return err + } + if len(cfg.Containers) == 0 { + return errors.New("no [[containers]] block found in config") + } + winner := "" + for _, name := range cfg.Containers[0].Env { + for k := range cfg.Env[name] { + if strings.EqualFold(k, key) { + winner = name + } + } + } + if winner != profile { + return fmt.Errorf("[env.%s] is applied after [env.%s] and also sets %s; remove %s from [env.%s] or move %q to the end of the env list", winner, profile, key, key, winner, profile) + } + if got := cfg.Env[profile][strings.ToLower(key)]; got != value { + return fmt.Errorf("could not set %s in [env.%s]; edit %s by hand", key, profile, path) + } + return nil +} + +// firstContainersBlock returns the byte range of the first [[containers]] +// block's body: from just after its header to the next table header (or EOF). +func firstContainersBlock(data []byte) (start, end int, ok bool) { + header := containersHeaderRe.FindIndex(data) + if header == nil { + return 0, 0, false + } + start, end = header[1], len(data) + if next := tableHeaderRe.FindIndex(data[start:]); next != nil { + end = start + next[0] + } + return start, end, true +} + +// withEnvReference adds profile to the end of the first [[containers]] block's +// env list, adding an env line after the type line when the block has none. +func withEnvReference(data []byte, profile string) ([]byte, error) { + start, end, ok := firstContainersBlock(data) + if !ok { + return nil, errors.New("no [[containers]] block found in config") + } + block := data[start:end] + quoted := `"` + profile + `"` + + if loc := envListRe.FindIndex(block); loc != nil { + open := loc[1] + closeOffset := bytes.IndexByte(block[open:], ']') + if closeOffset < 0 { + return nil, errors.New("could not parse the env list in the [[containers]] block") + } + inner := block[open : open+closeOffset] + names := quotedRe.FindAllIndex(inner, -1) + for _, n := range names { + if string(inner[n[0]+1:n[1]-1]) == profile { + return data, nil + } + } + switch { + case len(names) > 0: + return splice(data, start+open+names[len(names)-1][1], ", "+quoted), nil + case len(bytes.TrimSpace(inner)) == 0: + return replace(data, start+open, start+open+closeOffset, quoted), nil + default: + return splice(data, start+open, quoted+","), nil + } + } + + at, indent := start, "" + if loc := typeLineRe.FindIndex(block); loc != nil { + at = start + loc[1] + line := block[loc[0]:loc[1]] + indent = string(line[:len(line)-len(bytes.TrimLeft(line, " \t"))]) + } + nl := bytes.IndexByte(data[at:], '\n') + if nl < 0 { + return append(append([]byte{}, data...), "\n"+indent+"env = ["+quoted+"]\n"...), nil + } + return splice(data, at+nl+1, indent+"env = ["+quoted+"]\n"), nil +} + +// withProfileVar sets key to value in the [env.] table, creating the +// table at the end of the file when it does not exist. +func withProfileVar(data []byte, profile, key, value string) []byte { + assignment := key + " = " + fmt.Sprintf("%q", value) + q := regexp.QuoteMeta(profile) + headerRe := regexp.MustCompile(`(?m)^[ \t]*\[env\.(?:` + q + `|"` + q + `")\][ \t]*(?:#.*)?$`) + header := headerRe.FindIndex(data) + if header == nil { + return append(bytes.TrimRight(data, "\n"), "\n\n[env."+profile+"]\n"+assignment+"\n"...) + } + + start, end := header[1], len(data) + if next := tableHeaderRe.FindIndex(data[start:]); next != nil { + end = start + next[0] + } + // Keys are matched case-insensitively because viper lowercases them. + keyRe := regexp.MustCompile(`(?mi)^[ \t]*` + regexp.QuoteMeta(key) + `[ \t]*=[ \t]*("[^"\n]*"|'[^'\n]*'|[^ \t\n#]+)`) + if loc := keyRe.FindSubmatchIndex(data[start:end]); loc != nil { + return replace(data, start+loc[2], start+loc[3], fmt.Sprintf("%q", value)) + } + return splice(data, start, "\n"+assignment) +} + +func splice(data []byte, at int, s string) []byte { + return replace(data, at, at, s) +} + +func replace(data []byte, from, to int, s string) []byte { + out := make([]byte, 0, len(data)-(to-from)+len(s)) + out = append(out, data[:from]...) + out = append(out, s...) + return append(out, data[to:]...) +} diff --git a/internal/config/env_profile_test.go b/internal/config/env_profile_test.go new file mode 100644 index 000000000..eaf0c3e6f --- /dev/null +++ b/internal/config/env_profile_test.go @@ -0,0 +1,102 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeAndLoad(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(path, []byte(content), 0644)) + require.NoError(t, loadConfig(path)) + t.Cleanup(viper.Reset) + return path +} + +func TestSetEnvProfileVar(t *testing.T) { + for _, tc := range []struct { + name string + in string + want string + }{ + { + name: "empty env list", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = []\n", + want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\"]\n\n[env.p]\nKEY = \"1\"\n", + }, + { + name: "env list holding only a comment", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [ # none yet\n]\n", + want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\", # none yet\n]\n\n[env.p]\nKEY = \"1\"\n", + }, + { + name: "indented block without a type line", + in: "[[containers]]\n port = \"4566\"\n", + want: "[[containers]]\nenv = [\"p\"]\n port = \"4566\"\n\n[env.p]\nKEY = \"1\"\n", + }, + { + name: "existing quoted table with the key in another case", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = ['p']\n\n[env.\"p\"] # mine\nkey = 0\nOTHER = \"x\"\n", + want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = ['p']\n\n[env.\"p\"] # mine\nkey = \"1\"\nOTHER = \"x\"\n", + }, + { + name: "existing table without the key", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\n\n[env.p]\nOTHER = \"x\"\n", + want: "[[containers]]\ntype = \"aws\"\nenv = [\"p\"]\nport = \"4566\"\n\n[env.p]\nKEY = \"1\"\nOTHER = \"x\"\n", + }, + { + name: "only the first containers block is edited", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\n\n[[containers]]\ntype = \"snowflake\"\nport = \"4567\"\nenv = [\"q\"]\n\n[env.q]\nA = \"b\"\n", + want: "[[containers]]\ntype = \"aws\"\nenv = [\"p\"]\nport = \"4566\"\n\n[[containers]]\ntype = \"snowflake\"\nport = \"4567\"\nenv = [\"q\"]\n\n[env.q]\nA = \"b\"\n\n[env.p]\nKEY = \"1\"\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + path := writeAndLoad(t, tc.in) + + require.NoError(t, SetEnvProfileVar("p", "KEY", "1")) + + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, tc.want, string(got)) + }) + } +} + +func TestSetEnvProfileVar_RollsBackWhenALaterProfileOverrides(t *testing.T) { + original := "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\", \"later\"]\n\n[env.p]\nKEY = \"0\"\n\n[env.later]\nKEY = \"0\"\n" + path := writeAndLoad(t, original) + + err := SetEnvProfileVar("p", "KEY", "1") + + require.Error(t, err) + assert.Contains(t, err.Error(), "[env.later]") + got, readErr := os.ReadFile(path) + require.NoError(t, readErr) + assert.Equal(t, original, string(got)) +} + +func TestSetEnvProfileVar_RollsBackWhenResultDoesNotParse(t *testing.T) { + // A dotted key already defines the profile, so appending a table header + // would define it twice. + original := "env.p.OTHER = \"x\"\n\n[[containers]]\ntype = \"aws\"\nport = \"4566\"\n" + path := writeAndLoad(t, original) + + require.Error(t, SetEnvProfileVar("p", "KEY", "1")) + + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, original, string(got)) +} + +func TestSetEnvProfileVar_NoConfigFile(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + assert.Error(t, SetEnvProfileVar("p", "KEY", "1")) +} diff --git a/internal/emulator/aws/appinspector.go b/internal/emulator/aws/appinspector.go new file mode 100644 index 000000000..366dfdaa4 --- /dev/null +++ b/internal/emulator/aws/appinspector.go @@ -0,0 +1,105 @@ +package aws + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + + "github.com/localstack/lstk/internal/inspect" +) + +// appInspectorStatusPath is App Inspector's runtime switch. The emulator does +// not keep the state across restarts; there APP_INSPECTOR decides. +const appInspectorStatusPath = "/_localstack/appinspector/status" + +type appInspectorStatus struct { + Status string `json:"status"` + Changed bool `json:"changed"` +} + +func (c *Client) AppInspectorEnabled(ctx context.Context, baseURL string) (bool, error) { + s, err := c.appInspectorStatus(ctx, http.MethodGet, baseURL, nil) + if err != nil { + return false, err + } + return s.enabled() +} + +func (c *Client) SetAppInspectorEnabled(ctx context.Context, baseURL string, enabled bool) (bool, error) { + status := "disabled" + if enabled { + status = "enabled" + } + body, err := json.Marshal(map[string]string{"status": status}) + if err != nil { + return false, err + } + s, err := c.appInspectorStatus(ctx, http.MethodPut, baseURL, body) + if err != nil { + return false, err + } + got, err := s.enabled() + if err != nil { + return false, err + } + if got != enabled { + return false, fmt.Errorf("emulator reported App Inspector status %s after the change", s.Status) + } + return s.Changed, nil +} + +func (c *Client) appInspectorStatus(ctx context.Context, method, baseURL string, body []byte) (appInspectorStatus, error) { + url := strings.TrimRight(baseURL, "/") + appInspectorStatusPath + req, err := http.NewRequestWithContext(ctx, method, url, bytes.NewReader(body)) + if err != nil { + return appInspectorStatus{}, fmt.Errorf("failed to create App Inspector request: %w", err) + } + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + + resp, err := c.http.Do(req) + if err != nil { + return appInspectorStatus{}, fmt.Errorf("failed to reach App Inspector: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + data, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return appInspectorStatus{}, fmt.Errorf("failed to read App Inspector response: %w", err) + } + // Every licensed emulator that ships App Inspector registers this route, so + // a 404 means this one predates it (or is the community image). + if resp.StatusCode == http.StatusNotFound { + return appInspectorStatus{}, inspect.ErrUnavailable + } + if resp.StatusCode != http.StatusOK { + var e struct { + Error string `json:"error"` + } + if json.Unmarshal(data, &e) == nil && e.Error != "" { + data = []byte(e.Error) + } + return appInspectorStatus{}, emulatorStatusError(fmt.Sprintf("LocalStack returned status %d", resp.StatusCode), data) + } + + var s appInspectorStatus + if err := json.Unmarshal(data, &s); err != nil { + return appInspectorStatus{}, fmt.Errorf("failed to decode App Inspector response: %w", err) + } + return s, nil +} + +func (s appInspectorStatus) enabled() (bool, error) { + switch strings.ToUpper(s.Status) { + case "ENABLED": + return true, nil + case "DISABLED": + return false, nil + } + return false, fmt.Errorf("unexpected App Inspector status %q", s.Status) +} diff --git a/internal/emulator/aws/appinspector_test.go b/internal/emulator/aws/appinspector_test.go new file mode 100644 index 000000000..0f26eb223 --- /dev/null +++ b/internal/emulator/aws/appinspector_test.go @@ -0,0 +1,102 @@ +package aws + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/localstack/lstk/internal/inspect" +) + +func appInspectorServer(t *testing.T, status int, body string) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, appInspectorStatusPath, r.URL.Path) + w.WriteHeader(status) + _, _ = io.WriteString(w, body) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestAppInspectorEnabled(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + body string + want bool + }{ + {`{"status": "ENABLED", "note": "", "epoch": "a3db29c4"}`, true}, + {`{"status": "DISABLED", "note": "", "epoch": null}`, false}, + } { + srv := appInspectorServer(t, http.StatusOK, tc.body) + + got, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + } +} + +func TestSetAppInspectorEnabled(t *testing.T) { + t.Parallel() + var gotBody, gotMethod, gotContentType string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b, _ := io.ReadAll(r.Body) + gotBody, gotMethod, gotContentType = string(b), r.Method, r.Header.Get("Content-Type") + _, _ = io.WriteString(w, `{"status": "DISABLED", "changed": true}`) + })) + t.Cleanup(srv.Close) + + changed, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL+"/", false) + + require.NoError(t, err) + assert.True(t, changed) + assert.Equal(t, http.MethodPut, gotMethod) + assert.Equal(t, "application/json", gotContentType) + assert.JSONEq(t, `{"status": "disabled"}`, gotBody) +} + +func TestAppInspectorErrors(t *testing.T) { + t.Parallel() + + t.Run("404 means the emulator has no App Inspector", func(t *testing.T) { + t.Parallel() + srv := appInspectorServer(t, http.StatusNotFound, "") + + _, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) + + assert.ErrorIs(t, err, inspect.ErrUnavailable) + }) + + t.Run("error body is shown without its JSON wrapper", func(t *testing.T) { + t.Parallel() + srv := appInspectorServer(t, http.StatusInternalServerError, `{"error": "Failed to change AppInspector status"}`) + + _, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL, true) + + assert.EqualError(t, err, "LocalStack returned status 500: Failed to change AppInspector status") + }) + + t.Run("state that does not match the request", func(t *testing.T) { + t.Parallel() + srv := appInspectorServer(t, http.StatusOK, `{"status": "DISABLED", "changed": false}`) + + _, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL, true) + + assert.Error(t, err) + }) + + t.Run("unknown status value", func(t *testing.T) { + t.Parallel() + srv := appInspectorServer(t, http.StatusOK, `{"status": "PAUSED"}`) + + _, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) + + assert.Error(t, err) + }) +} diff --git a/internal/inspect/inspect.go b/internal/inspect/inspect.go new file mode 100644 index 000000000..fbf8ee0b3 --- /dev/null +++ b/internal/inspect/inspect.go @@ -0,0 +1,219 @@ +package inspect + +import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/localstack/lstk/internal/config" + "github.com/localstack/lstk/internal/container" + "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/runtime" +) + +// ErrUnavailable means the emulator has no App Inspector, e.g. an image that +// predates it. +var ErrUnavailable = errors.New("this emulator has no App Inspector") + +const ( + // profileName is the [env.*] profile lstk owns for the saved setting. + profileName = "app-inspector" + // envVar turns App Inspector on at emulator startup. The runtime switch + // does not survive a restart. + envVar = "APP_INSPECTOR" +) + +// Client reads and flips App Inspector on a running AWS emulator. +type Client interface { + AppInspectorEnabled(ctx context.Context, host string) (bool, error) + SetAppInspectorEnabled(ctx context.Context, host string, enabled bool) (changed bool, err error) +} + +// Target is the emulator a command acts on. +type Target struct { + Runtime runtime.Runtime + Containers []config.ContainerConfig + Client Client + Host string + // Config is nil for an --endpoint-url target: lstk does not start that + // emulator, so its config says nothing about it. + Config *LocalConfig +} + +// LocalConfig is the part of the lstk config that decides App Inspector's +// state when lstk starts the emulator. +type LocalConfig struct { + Container config.ContainerConfig + Env map[string]map[string]string + Path string +} + +// Set enables or disables App Inspector on the running emulator. With persist +// it also saves the setting to the config file, which works while the +// emulator is stopped too. +func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) error { + running, err := isRunning(ctx, sink, t, persist) + if err != nil { + return err + } + if !running && !persist { + return emitNotRunning(sink, output.ErrorAction{Label: "Or apply it on the next start:", Value: "lstk inspect " + verb(enabled) + " --persist"}) + } + + if running { + changed, err := t.Client.SetAppInspectorEnabled(ctx, t.Host, enabled) + if err != nil { + return emitRequestError(sink, err) + } + if changed { + sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: "App Inspector " + state(enabled)}) + } else { + sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: "App Inspector is already " + state(enabled)}) + } + } + + if persist { + if err := save(sink, t.Config, enabled); err != nil { + return err + } + if !running { + sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "The emulator is not running. The setting applies on the next start."}) + } + return nil + } + emitRestartNote(sink, t.Config, enabled) + return nil +} + +// Status reports whether App Inspector is enabled on the running emulator. +func Status(ctx context.Context, sink output.Sink, t Target) error { + running, err := isRunning(ctx, sink, t, false) + if err != nil { + return err + } + if !running { + return emitNotRunning(sink) + } + enabled, err := t.Client.AppInspectorEnabled(ctx, t.Host) + if err != nil { + return emitRequestError(sink, err) + } + sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: "App Inspector is " + state(enabled)}) + emitRestartNote(sink, t.Config, enabled) + return nil +} + +// isRunning treats an unreachable Docker as "not running" when persisting, +// since saving the setting does not need the emulator. +func isRunning(ctx context.Context, sink output.Sink, t Target, persist bool) (bool, error) { + if err := t.Runtime.IsHealthy(ctx); err != nil { + if persist { + return false, nil + } + t.Runtime.EmitUnhealthyError(sink, err) + return false, output.NewSilentError(fmt.Errorf("runtime not healthy: %w", err)) + } + running, err := container.RunningEmulators(ctx, t.Runtime, t.Containers) + if err != nil { + return false, fmt.Errorf("checking emulator status: %w", err) + } + return len(running) > 0, nil +} + +func save(sink output.Sink, cfg *LocalConfig, enabled bool) error { + if cfg == nil { + return errors.New("no lstk config to save the App Inspector setting to") + } + fail := func(err error) error { + sink.Emit(output.ErrorEvent{Title: "Could not save the App Inspector setting to " + cfg.Path, Summary: err.Error()}) + return output.NewSilentError(err) + } + saved, err := enabledOnStart(cfg) + if err != nil { + return fail(err) + } + if saved == enabled { + sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: fmt.Sprintf("App Inspector is already %s on every start (%s)", state(enabled), cfg.Path)}) + return nil + } + value := "0" + if enabled { + value = "1" + } + if err := config.SetEnvProfileVar(profileName, envVar, value); err != nil { + return fail(err) + } + sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("App Inspector will be %s on every start (saved to %s)", state(enabled), cfg.Path)}) + return nil +} + +// emitRestartNote warns when the running state will not survive a restart, +// because the config says otherwise. +func emitRestartNote(sink output.Sink, cfg *LocalConfig, enabled bool) { + if cfg == nil { + return + } + onStart, err := enabledOnStart(cfg) + if err != nil || onStart == enabled { + return + } + text := "App Inspector turns off when the emulator restarts. To keep it on, run: lstk inspect enable --persist" + if !enabled { + text = fmt.Sprintf("App Inspector turns back on when the emulator restarts, as %s enables it. To keep it off, run: lstk inspect disable --persist", cfg.Path) + } + sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: text}) +} + +// enabledOnStart reports whether the config's env profiles turn App Inspector +// on at startup. The last profile setting the variable wins, and LocalStack +// reads "1" and "true" as on. +func enabledOnStart(cfg *LocalConfig) (bool, error) { + env, err := cfg.Container.ResolvedEnv(cfg.Env) + if err != nil { + return false, err + } + enabled := false + for _, kv := range env { + if k, v, _ := strings.Cut(kv, "="); k == envVar { + v = strings.ToLower(strings.TrimSpace(v)) + enabled = v == "1" || v == "true" + } + } + return enabled, nil +} + +func emitNotRunning(sink output.Sink, extra ...output.ErrorAction) error { + sink.Emit(output.ErrorEvent{ + Title: "LocalStack is not running", + Actions: append([]output.ErrorAction{{Label: "Start LocalStack:", Value: "lstk"}}, extra...), + Code: output.ErrEmulatorNotRunning, + }) + return output.NewSilentError(errors.New("LocalStack is not running")) +} + +func emitRequestError(sink output.Sink, err error) error { + if errors.Is(err, ErrUnavailable) { + sink.Emit(output.ErrorEvent{ + Title: "App Inspector is not available in this emulator", + Summary: "This emulator version predates App Inspector. Use a recent LocalStack image.", + }) + return output.NewSilentError(err) + } + sink.Emit(output.ErrorEvent{Title: "App Inspector request failed", Summary: err.Error()}) + return output.NewSilentError(err) +} + +func state(enabled bool) string { + if enabled { + return "enabled" + } + return "disabled" +} + +func verb(enabled bool) string { + if enabled { + return "enable" + } + return "disable" +} diff --git a/internal/inspect/inspect_test.go b/internal/inspect/inspect_test.go new file mode 100644 index 000000000..13009fb99 --- /dev/null +++ b/internal/inspect/inspect_test.go @@ -0,0 +1,35 @@ +package inspect + +import ( + "testing" + + "github.com/localstack/lstk/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEnabledOnStart(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + profiles []string + env map[string]map[string]string + want bool + }{ + {"unset", nil, nil, false}, + {"one", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": "1"}}, true}, + {"true in any case with spaces", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": " TRUE "}}, true}, + {"other truthy-looking words are off", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": "yes"}}, false}, + {"last profile wins", []string{"a", "b"}, map[string]map[string]string{"a": {"app_inspector": "1"}, "b": {"app_inspector": "0"}}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cfg := &LocalConfig{Container: config.ContainerConfig{Type: config.EmulatorAWS, Env: tc.profiles}, Env: tc.env} + + got, err := enabledOnStart(cfg) + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/ui/run_inspect.go b/internal/ui/run_inspect.go new file mode 100644 index 000000000..2b1200e3d --- /dev/null +++ b/internal/ui/run_inspect.go @@ -0,0 +1,20 @@ +package ui + +import ( + "context" + + "github.com/localstack/lstk/internal/inspect" + "github.com/localstack/lstk/internal/output" +) + +func RunInspectSet(parentCtx context.Context, t inspect.Target, enabled, persist bool) error { + return runWithTUI(parentCtx, withoutHeader(), func(ctx context.Context, sink output.Sink) error { + return inspect.Set(ctx, sink, t, enabled, persist) + }) +} + +func RunInspectStatus(parentCtx context.Context, t inspect.Target) error { + return runWithTUI(parentCtx, withoutHeader(), func(ctx context.Context, sink output.Sink) error { + return inspect.Status(ctx, sink, t) + }) +} diff --git a/openspec/changes/json-output-schema/tasks.md b/openspec/changes/json-output-schema/tasks.md index 237494481..de9f10686 100644 --- a/openspec/changes/json-output-schema/tasks.md +++ b/openspec/changes/json-output-schema/tasks.md @@ -35,6 +35,7 @@ Implemented first, ahead of every other command: each depends only on the shared - [ ] 4.3 `status`: add annotation; change the domain loop (`internal/container/status.go`) to report every configured emulator (running or not) instead of failing fast on the first non-running one, gated so plain-text behavior is unchanged; `{"emulators": [...]}` data shape. - [ ] 4.4 `logs` (bounded): add annotation, `{"lines": [...]}` data shape. - [ ] 4.5 `logs --follow`: wire the NDJSON stream writer from 1.5. +- [ ] 4.5b `inspect enable`/`disable`/`status`: add annotation, `{"running", "enabled", "changed", "persisted", "enabledOnStart"}` data shape, `EMULATOR_NOT_RUNNING`/`EMULATOR_NOT_CONFIGURED`/`EMULATOR_WRONG_TYPE` codes. - [ ] 4.6 Integration tests for each command in this wave, covering both the success payload and its documented error codes. ## 5. Remaining emulator lifecycle diff --git a/test/integration/__snapshots__/exit_code_test.snap b/test/integration/__snapshots__/exit_code_test.snap index 85bb0e626..e6aa556e9 100644 --- a/test/integration/__snapshots__/exit_code_test.snap +++ b/test/integration/__snapshots__/exit_code_test.snap @@ -80,6 +80,29 @@ Global Options: --non-interactive Disable interactive mode --- +[TestBareParentCommandExitsZero_inspect_1] +Turn App Inspector on or off for the AWS emulator. App Inspector records the +operations your application sends to the emulator, with their payloads, the +services they connect, and IAM permission results. View them in the LocalStack +Web Application or the LocalStack Toolkit for VS Code. + +Usage: lstk inspect [flags] + +Commands: + disable Disable App Inspector + enable Enable App Inspector + status Show whether App Inspector is enabled + +Options: + -h, --help help for inspect + +Global Options: + --config string Path to config file + --endpoint-url string Target an existing, externally-managed emulator at this URL + --json Output in JSON format (only supported by some commands) + --non-interactive Disable interactive mode +--- + [TestBareParentCommandExitsZero_setup_1] Set up emulator CLI integration for AWS or Azure. @@ -166,6 +189,10 @@ Error: unknown flag: --bogus-flag-xyz Error: unknown flag: --bogus-flag-xyz --- +[TestInvalidUsageExitsNonZero_unknown_inspect_subcommand_1] +Error: unknown command "bogus" for "lstk inspect" +--- + [TestInvalidUsageExitsNonZero_unknown_setup_subcommand_1] Error: unknown command "bogus" for "lstk setup" --- diff --git a/test/integration/__snapshots__/extension_test.snap b/test/integration/__snapshots__/extension_test.snap index 04fbeb785..29857973e 100644 --- a/test/integration/__snapshots__/extension_test.snap +++ b/test/integration/__snapshots__/extension_test.snap @@ -10,6 +10,7 @@ Commands: completion Set up tab completion for your shell config Manage configuration help Help about any command + inspect Turn App Inspector on or off load Load a snapshot into the running emulator login Manage login logout Remove stored authentication credentials @@ -71,6 +72,7 @@ Commands: completion Set up tab completion for your shell config Manage configuration help Help about any command + inspect Turn App Inspector on or off load Load a snapshot into the running emulator login Manage login logout Remove stored authentication credentials @@ -121,6 +123,7 @@ Commands: completion Set up tab completion for your shell config Manage configuration help Help about any command + inspect Turn App Inspector on or off load Load a snapshot into the running emulator login Manage login logout Remove stored authentication credentials diff --git a/test/integration/endpoint_url_test.go b/test/integration/endpoint_url_test.go index 29dd4a6e7..2646f4546 100644 --- a/test/integration/endpoint_url_test.go +++ b/test/integration/endpoint_url_test.go @@ -429,6 +429,7 @@ func TestEndpointURLWrongTypeErrorNamesEmulatorOnce(t *testing.T) { want string }{ {"reset", snowflake.URL, []string{"reset", "--force"}, "reset is only supported for the AWS emulator, but the endpoint at %s is a LocalStack Snowflake Emulator"}, + {"inspect", snowflake.URL, []string{"inspect", "enable"}, "App Inspector is only available for the AWS emulator, but the endpoint at %s is a LocalStack Snowflake Emulator"}, {"aws", snowflake.URL, []string{"aws", "s3", "ls"}, "lstk aws requires the AWS emulator, but the endpoint at %s is a LocalStack Snowflake Emulator"}, {"terraform", snowflake.URL, []string{"terraform", "plan"}, "lstk terraform requires the AWS emulator, but the endpoint at %s is a LocalStack Snowflake Emulator"}, {"sam", snowflake.URL, []string{"sam", "deploy"}, "lstk sam requires the AWS emulator, but the endpoint at %s is a LocalStack Snowflake Emulator"}, diff --git a/test/integration/exit_code_test.go b/test/integration/exit_code_test.go index 8e17e0189..109bb067a 100644 --- a/test/integration/exit_code_test.go +++ b/test/integration/exit_code_test.go @@ -29,6 +29,7 @@ func TestInvalidUsageExitsNonZero(t *testing.T) { {"unknown setup subcommand", []string{"setup", "bogus"}}, {"unknown volume subcommand", []string{"volume", "bogus"}}, {"unknown snapshot subcommand", []string{"snapshot", "bogus"}}, + {"unknown inspect subcommand", []string{"inspect", "bogus"}}, {"unknown completion shell", []string{"completion", "bogus"}}, } @@ -51,7 +52,7 @@ func TestInvalidUsageExitsNonZero(t *testing.T) { func TestBareParentCommandExitsZero(t *testing.T) { t.Parallel() - for _, parent := range []string{"config", "setup", "volume", "snapshot", "completion"} { + for _, parent := range []string{"config", "setup", "volume", "snapshot", "inspect", "completion"} { parent := parent t.Run(parent, func(t *testing.T) { t.Parallel() diff --git a/test/integration/inspect_test.go b/test/integration/inspect_test.go new file mode 100644 index 000000000..d3ede5286 --- /dev/null +++ b/test/integration/inspect_test.go @@ -0,0 +1,482 @@ +package integration_test + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/localstack/lstk/test/integration/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// appInspectorServer mimics an AWS emulator's /_localstack/appinspector/status +// endpoint (GET reports the state, PUT {"status": "enabled"|"disabled"} sets it) +// and records the status sent by each PUT. +type appInspectorServer struct { + *httptest.Server + mu sync.Mutex + enabled bool + puts []string +} + +func newAppInspectorServer(t *testing.T, enabled bool) *appInspectorServer { + t.Helper() + s := &appInspectorServer{enabled: enabled} + health := awsHealthHandler() + s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/_localstack/appinspector/status" { + health.ServeHTTP(w, r) + return + } + s.mu.Lock() + defer s.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + switch r.Method { + case http.MethodGet: + _ = json.NewEncoder(w).Encode(map[string]any{"status": statusWord(s.enabled), "note": "", "epoch": nil}) + case http.MethodPut: + body, _ := io.ReadAll(r.Body) + var req struct { + Status string `json:"status"` + } + if err := json.Unmarshal(body, &req); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + s.puts = append(s.puts, req.Status) + want := req.Status == "enabled" + changed := want != s.enabled + s.enabled = want + _ = json.NewEncoder(w).Encode(map[string]any{"status": statusWord(s.enabled), "changed": changed}) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } + })) + t.Cleanup(s.Close) + return s +} + +func statusWord(enabled bool) string { + if enabled { + return "ENABLED" + } + return "DISABLED" +} + +func (s *appInspectorServer) recordedPuts() []string { + s.mu.Lock() + defer s.mu.Unlock() + return append([]string(nil), s.puts...) +} + +// inspectEnv is an isolated environment whose Docker endpoint is unreachable, so +// any test using it proves the command never needed Docker. +func inspectEnv(t *testing.T) env.Environ { + t.Helper() + return env.Environ(testEnvWithHome(t.TempDir(), "")). + With(env.DisableEvents, "1"). + With(dockerHostKey, "tcp://localhost:1") +} + +func writeInspectConfig(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(path, []byte(content), 0644)) + return path +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + return string(data) +} + +const inspectBaseConfig = `# my settings +[[containers]] +type = "aws" # emulator type +tag = "latest" +port = "4566" +# env = [] # Named environment profiles + +[cli] +check_for_update_on_startup = false +` + +func TestInspectEnableEndpointURL(t *testing.T) { + t.Parallel() + srv := newAppInspectorServer(t, false) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector enabled") + assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) +} + +func TestInspectEnableWhenAlreadyEnabled(t *testing.T) { + t.Parallel() + srv := newAppInspectorServer(t, true) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector is already enabled") +} + +func TestInspectDisableEndpointURL(t *testing.T) { + t.Parallel() + srv := newAppInspectorServer(t, true) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "disable") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector disabled") + assert.Equal(t, []string{"disabled"}, srv.recordedPuts()) +} + +func TestInspectStatusEndpointURL(t *testing.T) { + t.Parallel() + for _, enabled := range []bool{true, false} { + want := "App Inspector is disabled" + if enabled { + want = "App Inspector is enabled" + } + t.Run(want, func(t *testing.T) { + t.Parallel() + srv := newAppInspectorServer(t, enabled) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "status") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, want) + assert.Empty(t, srv.recordedPuts(), "status must not change the state") + }) + } +} + +// An emulator older than App Inspector has no status route and answers 404. +func TestInspectFailsWhenEmulatorLacksAppInspector(t *testing.T) { + t.Parallel() + srv := awsHealthServer(t) + t.Cleanup(srv.Close) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "App Inspector is not available in this emulator") +} + +func TestInspectPersistRejectsEndpointURL(t *testing.T) { + t.Parallel() + srv := newAppInspectorServer(t, false) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable", "--persist") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "--persist") + assert.Empty(t, srv.recordedPuts(), "a rejected invocation must not change the emulator") +} + +// With the emulator stopped, --persist saves the setting so the next start +// picks it up, without needing Docker at all. +func TestInspectEnablePersistWhileStoppedWritesConfig(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, inspectBaseConfig) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "next start") + assert.Equal(t, `# my settings +[[containers]] +type = "aws" # emulator type +env = ["app-inspector"] +tag = "latest" +port = "4566" +# env = [] # Named environment profiles + +[cli] +check_for_update_on_startup = false + +[env.app-inspector] +APP_INSPECTOR = "1" +`, readFile(t, configPath)) +} + +func TestInspectEnablePersistKeepsExistingProfiles(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, `[[containers]] +type = "aws" +port = "4566" +env = ["debug"] + +[env.debug] +DEBUG = "1" +`) + + _, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + require.NoError(t, err, stderr) + assert.Equal(t, `[[containers]] +type = "aws" +port = "4566" +env = ["debug", "app-inspector"] + +[env.debug] +DEBUG = "1" + +[env.app-inspector] +APP_INSPECTOR = "1" +`, readFile(t, configPath)) +} + +func TestInspectEnablePersistIsIdempotent(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, inspectBaseConfig) + e := inspectEnv(t) + + _, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + require.NoError(t, err, stderr) + afterFirst := readFile(t, configPath) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "already") + assert.Equal(t, afterFirst, readFile(t, configPath)) +} + +// A profile the user wrote themselves already enables App Inspector, so there +// is nothing to save. +func TestInspectEnablePersistRespectsUserProfile(t *testing.T) { + t.Parallel() + original := `[[containers]] +type = "aws" +port = "4566" +env = ["mine"] + +[env.mine] +APP_INSPECTOR = "true" +` + configPath := writeInspectConfig(t, original) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "already") + assert.Equal(t, original, readFile(t, configPath)) +} + +func TestInspectDisablePersistAfterEnable(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, inspectBaseConfig) + e := inspectEnv(t) + + _, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + require.NoError(t, err, stderr) + + _, stderr, err = runLstk(t, testContext(t), t.TempDir(), e, + "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") + require.NoError(t, err, stderr) + + got := readFile(t, configPath) + assert.Contains(t, got, "[env.app-inspector]\nAPP_INSPECTOR = \"0\"\n") + assert.Equal(t, 1, strings.Count(got, `"app-inspector"`), "profile must be referenced once") +} + +// A profile that turns App Inspector on is overridden by lstk's own profile, +// which is applied last. +func TestInspectDisablePersistOverridesUserProfile(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, `[[containers]] +type = "aws" +port = "4566" +env = [ + "mine", +] + +[env.mine] +APP_INSPECTOR = "1" +`) + + _, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") + + require.NoError(t, err, stderr) + assert.Equal(t, `[[containers]] +type = "aws" +port = "4566" +env = [ + "mine", "app-inspector", +] + +[env.mine] +APP_INSPECTOR = "1" + +[env.app-inspector] +APP_INSPECTOR = "0" +`, readFile(t, configPath)) +} + +// lstk's profile is listed before one that also sets APP_INSPECTOR, so saving +// would have no effect; the file must be left as it was. +func TestInspectPersistFailsWhenALaterProfileOverrides(t *testing.T) { + t.Parallel() + original := `[[containers]] +type = "aws" +port = "4566" +env = ["app-inspector", "mine"] + +[env.app-inspector] +APP_INSPECTOR = "0" + +[env.mine] +APP_INSPECTOR = "0" +` + configPath := writeInspectConfig(t, original) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "Could not save the App Inspector setting") + assert.Contains(t, stdout, "[env.mine]") + assert.Equal(t, original, readFile(t, configPath)) +} + +func TestInspectDisablePersistWhenNeverEnabledLeavesConfigAlone(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, inspectBaseConfig) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "already") + assert.Equal(t, inspectBaseConfig, readFile(t, configPath)) +} + +func TestInspectPersistWithoutConfigFile(t *testing.T) { + t.Parallel() + e := inspectEnv(t) + configPath := resolvedConfigPath(t, e) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), e, + "--non-interactive", "inspect", "enable", "--persist") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "lstk start") + assert.NoFileExists(t, configPath) +} + +func TestInspectRejectsNonAWSConfig(t *testing.T) { + t.Parallel() + original := "[[containers]]\ntype = \"snowflake\"\nport = \"4566\"\n" + configPath := writeInspectConfig(t, original) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "App Inspector is only available for the AWS emulator") + assert.Equal(t, original, readFile(t, configPath)) +} + +func TestInspectEnableNotRunning(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), testEnvWithHome(t.TempDir(), ""), + "--non-interactive", "inspect", "enable") + + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "not running") + assert.Contains(t, stdout, "--persist") +} + +func TestInspectEnableLocalEmulator(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ctx := testContext(t) + startTestContainer(t, ctx) + srv := newAppInspectorServer(t, false) + analyticsSrv, events := mockAnalyticsServer(t) + + stdout, stderr, err := runLstk(t, ctx, t.TempDir(), + env.Environ(testEnvWithHome(t.TempDir(), "")). + With(env.LocalStackHost, lsHost(srv.Server)). + With(env.AnalyticsEndpoint, analyticsSrv.URL), + "--non-interactive", "inspect", "enable") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector enabled") + assert.Contains(t, stdout, "lstk inspect enable --persist", "should explain how to keep it on after a restart") + assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) + assertCommandTelemetry(t, events, "inspect enable", 0) +} + +func TestInspectEnablePersistLocalEmulator(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ctx := testContext(t) + startTestContainer(t, ctx) + srv := newAppInspectorServer(t, false) + configPath := writeInspectConfig(t, inspectBaseConfig) + + stdout, stderr, err := runLstk(t, ctx, t.TempDir(), + env.Environ(testEnvWithHome(t.TempDir(), "")).With(env.LocalStackHost, lsHost(srv.Server)), + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector enabled") + assert.NotContains(t, stdout, "next start") + assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) + assert.Contains(t, readFile(t, configPath), "[env.app-inspector]\nAPP_INSPECTOR = \"1\"\n") +} + +// The persisted setting must reach a real emulator on its next start. +func TestInspectPersistAppliesOnStart(t *testing.T) { + requireDocker(t) + token := env.Require(t, env.AuthToken) + cleanup() + t.Cleanup(cleanup) + + ctx := testContext(t) + configPath := writeInspectConfig(t, inspectBaseConfig) + e := env.Environ(testEnvWithHome(t.TempDir(), "")).With(env.AuthToken, token) + + _, stderr, err := runLstk(t, ctx, t.TempDir(), e, + "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + require.NoError(t, err, stderr) + + _, stderr, err = runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "start") + require.NoError(t, err, stderr) + + stdout, stderr, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "inspect", "status") + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "App Inspector is enabled") +} From a4423100fd97b8cdf8bf4eac1745c435b3c8fc55 Mon Sep 17 00:00:00 2001 From: Anisa Oshafi Date: Tue, 13 Oct 2026 18:45:11 +0200 Subject: [PATCH 2/3] Simplify lstk inspect and its tests --- CLAUDE.md | 4 +- cmd/inspect.go | 38 +- docs/structured-output.md | 21 - internal/config/emulator_type.go | 11 +- internal/config/env_profile.go | 171 +++----- internal/config/env_profile_test.go | 86 +--- internal/emulator/aws/appinspector.go | 67 +--- internal/emulator/aws/appinspector_test.go | 102 ----- internal/inspect/inspect.go | 136 ++----- internal/inspect/inspect_test.go | 35 -- openspec/changes/json-output-schema/tasks.md | 1 - test/integration/inspect_test.go | 400 +++++-------------- 12 files changed, 254 insertions(+), 818 deletions(-) delete mode 100644 internal/emulator/aws/appinspector_test.go delete mode 100644 internal/inspect/inspect_test.go diff --git a/CLAUDE.md b/CLAUDE.md index b7f1ce5aa..b15698672 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -63,7 +63,7 @@ Notes: - `env/` - Process environment snapshot/injection helper (also used to isolate test envs) - `extension/` - Git-style `lstk-` extension resolution and exec - `iac/` - Wrappers for third-party infrastructure as code tools (`terraform/`, `cdk/`, `sam/`) - - `inspect/` - `lstk inspect` domain logic: turns App Inspector on/off through the AWS emulator's runtime switch; `--persist` saves `APP_INSPECTOR` to an lstk-owned `[env.app-inspector]` profile (via `config.SetEnvProfileVar`) so it survives restarts + - `inspect/` - `lstk inspect` domain logic (App Inspector on/off; `--persist` saves it to config) - `log/` - Internal diagnostic logging (not for user-facing output — use `output/` for that) - `output/` - Generic event and sink abstractions for CLI/TUI/non-interactive rendering - `ports/` - Port availability checks @@ -145,7 +145,7 @@ There is no `--offline` flag. Instead `container.Start` degrades gracefully when # Targeting an External Emulator (`--endpoint-url`) -`--endpoint-url ` (a root persistent flag) lets `aws`, `az`, `terraform`/`tf`, `cdk`, `sam`, `snapshot save`/`load` (incl. the `lstk save`/`lstk load` aliases), `snapshot remove`, `snapshot list s3://...`, `reset`, `inspect` (except `--persist`, which only edits lstk's config), and `status` target an emulator lstk did not start — docker compose, host-network mode, CI, a different machine, or a LocalStack cloud-hosted **ephemeral instance** — instead of discovering one via local Docker. `LSTK_ENDPOINT_URL` is the equivalent environment variable. Both `http://` and `https://` are accepted (any other scheme is rejected); the resolved scheme is preserved end-to-end all the way to whatever ultimately makes the request — the wrapped `aws`/`terraform`/`cdk`/`sam` tools, and the emulator API calls behind `snapshot`/`reset`/`status` — never normalized to `http`, which is what makes `https://` ephemeral instances work. +`--endpoint-url ` (a root persistent flag) lets `aws`, `az`, `terraform`/`tf`, `cdk`, `sam`, `snapshot save`/`load` (incl. the `lstk save`/`lstk load` aliases), `snapshot remove`, `snapshot list s3://...`, `reset`, `inspect` (not `--persist`), and `status` target an emulator lstk did not start — docker compose, host-network mode, CI, a different machine, or a LocalStack cloud-hosted **ephemeral instance** — instead of discovering one via local Docker. `LSTK_ENDPOINT_URL` is the equivalent environment variable. Both `http://` and `https://` are accepted (any other scheme is rejected); the resolved scheme is preserved end-to-end all the way to whatever ultimately makes the request — the wrapped `aws`/`terraform`/`cdk`/`sam` tools, and the emulator API calls behind `snapshot`/`reset`/`status` — never normalized to `http`, which is what makes `https://` ephemeral instances work. `lstk logs`, `stop`, `restart`, `volume` (`path`/`clear`), `start`, and the bare `lstk` root command (which starts the emulator the same way `start` does) have no remote equivalent and reject *any* resolved endpoint source. diff --git a/cmd/inspect.go b/cmd/inspect.go index 53eda6738..53d879ff3 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -23,9 +23,7 @@ func newInspectCmd(cfg *env.Env) *cobra.Command { Long: "Turn App Inspector on or off for the AWS emulator. App Inspector records the operations your application sends to the emulator, with their payloads, the services they connect, and IAM permission results. View them in the LocalStack Web Application or the LocalStack Toolkit for VS Code.", } requireSubcommand(cmd) - cmd.AddCommand(newInspectSetCmd(cfg, true)) - cmd.AddCommand(newInspectSetCmd(cfg, false)) - cmd.AddCommand(newInspectStatusCmd(cfg)) + cmd.AddCommand(newInspectSetCmd(cfg, true), newInspectSetCmd(cfg, false), newInspectStatusCmd(cfg)) return cmd } @@ -79,8 +77,8 @@ func newInspectStatusCmd(cfg *env.Env) *cobra.Command { } } -// resolveInspectTarget picks the AWS emulator to act on: the --endpoint-url -// target when one resolves, otherwise the AWS emulator in the lstk config. +// resolveInspectTarget returns the --endpoint-url emulator when one is set, +// otherwise the AWS emulator from the lstk config. func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, persist bool) (inspect.Target, error) { fail := func(e output.ErrorEvent) (inspect.Target, error) { sink.Emit(e) @@ -94,10 +92,7 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe if remote != nil { if persist { source, _, _ := endpoint.ResolvedSource(cmd) - return fail(output.ErrorEvent{ - Title: fmt.Sprintf("--persist cannot be used with %s", source), - Summary: fmt.Sprintf("--persist saves to the lstk config, which does not start the emulator at %s.", remote.URL), - }) + return fail(output.ErrorEvent{Title: "--persist cannot be used with " + source, Summary: "--persist saves to the lstk config, which does not start the emulator at " + remote.URL + "."}) } if remote.Type != config.EmulatorAWS { return fail(output.ErrorEvent{ @@ -106,12 +101,7 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe }) } c := config.ContainerConfig{Type: config.EmulatorAWS, Port: config.DefaultPort} - return inspect.Target{ - Runtime: runtime.NewExternalRuntime(c.Name()), - Containers: []config.ContainerConfig{c}, - Client: aws.NewClient(), - Host: remote.URL, - }, nil + return inspect.Target{Runtime: runtime.NewExternalRuntime(c.Name()), Containers: []config.ContainerConfig{c}, Client: aws.NewClient(), Host: remote.URL}, nil } appConfig, err := config.Get() @@ -128,17 +118,11 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe if awsContainer == nil { return fail(output.ErrorEvent{Title: "App Inspector is only available for the AWS emulator", Code: output.ErrEmulatorNotConfigured}) } - if persist { - if !config.HasFile() { - return fail(output.ErrorEvent{ - Title: "No config file to save the setting to yet", - Actions: []output.ErrorAction{{Label: "Create it by starting the emulator once:", Value: "lstk start"}}, - }) - } - // SetEnvProfileVar edits the first [[containers]] block. - if appConfig.Containers[0].Type != config.EmulatorAWS { - return fail(output.ErrorEvent{Title: "--persist needs the AWS emulator to be the first [[containers]] block in the config"}) - } + if persist && !config.HasFile() { + return fail(output.ErrorEvent{ + Title: "No config file to save the setting to yet", + Actions: []output.ErrorAction{{Label: "Create it by starting the emulator once:", Value: "lstk start"}}, + }) } rt, err := runtime.NewDockerRuntime(cfg.DockerHost) @@ -155,6 +139,6 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe Containers: []config.ContainerConfig{*awsContainer}, Client: aws.NewClient(), Host: "http://" + host, - Config: &inspect.LocalConfig{Container: *awsContainer, Env: appConfig.Env, Path: path}, + ConfigPath: path, }, nil } diff --git a/docs/structured-output.md b/docs/structured-output.md index cdf39807e..4b8ae6597 100644 --- a/docs/structured-output.md +++ b/docs/structured-output.md @@ -604,27 +604,6 @@ Codes: `AUTH_REQUIRED` (platform), `CREDENTIALS_MISSING`/`SNAPSHOT_BUCKET_NOT_FO ``` Codes: `AUTH_REQUIRED`, `SNAPSHOT_NOT_FOUND`, `SNAPSHOT_INVALID_REF`, `CONFIRMATION_REQUIRED`, `SNAPSHOT_REMOTE_ERROR`. -#### App Inspector - -**`lstk inspect enable`** / **`lstk inspect disable`** / **`lstk inspect status`**: App Inspector's state on the running emulator, and what the config sets for the next start. `changed` and `persisted` are omitted for `status`; `running` is `false` only for `enable --persist`/`disable --persist` while the emulator is stopped, in which case `enabled` is `null`. `enabledOnStart` is omitted for an `--endpoint-url` target, since lstk does not start that emulator. -```json -{ - "schemaVersion": 1, - "command": "inspect enable", - "status": "ok", - "data": { - "running": true, - "enabled": true, - "changed": true, - "persisted": false, - "enabledOnStart": false - }, - "warnings": [], - "error": null -} -``` -Codes: `RUNTIME_UNAVAILABLE`, `EMULATOR_NOT_RUNNING`, `EMULATOR_NOT_CONFIGURED`, `EMULATOR_WRONG_TYPE`, `CONFIG_INVALID`. - ## Commands that will never support `--json` - **`login`** — requires an interactive terminal unconditionally (browser-based OAuth) and has no defined non-interactive behavior at all today, so there's no output to render as JSON. diff --git a/internal/config/emulator_type.go b/internal/config/emulator_type.go index 6d927940f..ff5af81cc 100644 --- a/internal/config/emulator_type.go +++ b/internal/config/emulator_type.go @@ -55,11 +55,16 @@ func SetEmulatorType(to EmulatorType) error { // profile) is never mistaken for the emulator type. Only the captured value // is replaced, leaving commented-out example blocks and the original // formatting untouched. - blockStart, blockEnd, ok := firstContainersBlock(data) - if !ok { + header := containersHeaderRe.FindIndex(data) + if header == nil { return fmt.Errorf("no [[containers]] block found in config") } - loc := typeLineRe.FindSubmatchIndex(data[blockStart:blockEnd]) + blockStart := header[1] + block := data[blockStart:] + if next := tableHeaderRe.FindIndex(block); next != nil { + block = block[:next[0]] + } + loc := typeLineRe.FindSubmatchIndex(block) if loc == nil { return fmt.Errorf("no emulator type field found in config") } diff --git a/internal/config/env_profile.go b/internal/config/env_profile.go index 774da8542..af76fd85b 100644 --- a/internal/config/env_profile.go +++ b/internal/config/env_profile.go @@ -9,59 +9,48 @@ import ( "strings" ) -// envListRe matches the opening of an `env = [` assignment at line start, so a -// commented-out `# env = []` never matches. quotedRe matches one TOML string -// in that list. var ( envListRe = regexp.MustCompile(`(?m)^[ \t]*env[ \t]*=[ \t]*\[`) quotedRe = regexp.MustCompile(`"[^"\n]*"|'[^'\n]*'`) ) -// SetEnvProfileVar makes the first [[containers]] block apply the [env.] -// profile with key set to value. The profile is appended to the block's env -// list, so it is applied after (and wins over) the profiles already there, and -// the table is created at the end of the file when missing. -// -// The file is edited in place so comments and formatting survive. The edit is -// rolled back if the result does not load, or if a profile listed after this -// one still overrides the value. -func SetEnvProfileVar(profile, key, value string) error { +// SetEnvProfileVar sets key = value in [env.] and appends the profile +// to the first [[containers]] block's env list, so it wins over the others. +// It edits the file in place to keep comments, and restores it if the result +// does not load or the value does not take effect. It reports whether the +// file changed. +func SetEnvProfileVar(profile, key, value string) (bool, error) { path := resolvedConfigPath() if path == "" { - return errors.New("no config file to write to yet") + return false, errors.New("no config file to write to yet") } original, err := os.ReadFile(path) if err != nil { - return fmt.Errorf("failed to read config file: %w", err) + return false, fmt.Errorf("failed to read config file: %w", err) } - updated, err := withEnvReference(original, profile) + updated, err := addEnvReference(original, profile) if err != nil { - return err + return false, err } - updated = withProfileVar(updated, profile, key, value) + updated = setProfileVar(updated, profile, key, value) if bytes.Equal(updated, original) { - return nil - } - - info, err := os.Stat(path) - if err != nil { - return fmt.Errorf("failed to read config file: %w", err) + return false, nil } - if err := os.WriteFile(path, updated, info.Mode().Perm()); err != nil { - return fmt.Errorf("failed to write config file: %w", err) + if err := os.WriteFile(path, updated, 0644); err != nil { + return false, fmt.Errorf("failed to write config file: %w", err) } - if err := checkEnvProfileVar(path, profile, key, value); err != nil { - if restoreErr := os.WriteFile(path, original, info.Mode().Perm()); restoreErr != nil { - return fmt.Errorf("%w (restoring %s also failed: %v)", err, path, restoreErr) + if err := checkResolvedEnv(path, key, value); err != nil { + if restoreErr := os.WriteFile(path, original, 0644); restoreErr != nil { + return false, errors.Join(err, restoreErr) } - return errors.Join(err, loadConfig(path)) + return false, errors.Join(err, loadConfig(path)) } - return nil + return true, nil } -// checkEnvProfileVar reloads the config and confirms the first container now -// resolves key to value. -func checkEnvProfileVar(path, profile, key, value string) error { +// checkResolvedEnv reloads the config and checks that the first container +// gets key=value. +func checkResolvedEnv(path, key, value string) error { if err := loadConfig(path); err != nil { return err } @@ -69,113 +58,79 @@ func checkEnvProfileVar(path, profile, key, value string) error { if err != nil { return err } - if len(cfg.Containers) == 0 { - return errors.New("no [[containers]] block found in config") + env, err := cfg.Containers[0].ResolvedEnv(cfg.Env) + if err != nil { + return err } - winner := "" - for _, name := range cfg.Containers[0].Env { - for k := range cfg.Env[name] { - if strings.EqualFold(k, key) { - winner = name - } + got := "" + for _, kv := range env { + if k, v, _ := strings.Cut(kv, "="); k == strings.ToUpper(key) { + got = v } } - if winner != profile { - return fmt.Errorf("[env.%s] is applied after [env.%s] and also sets %s; remove %s from [env.%s] or move %q to the end of the env list", winner, profile, key, key, winner, profile) - } - if got := cfg.Env[profile][strings.ToLower(key)]; got != value { - return fmt.Errorf("could not set %s in [env.%s]; edit %s by hand", key, profile, path) + if got != value { + return fmt.Errorf("another env profile also sets %s and overrides this change", key) } return nil } -// firstContainersBlock returns the byte range of the first [[containers]] -// block's body: from just after its header to the next table header (or EOF). -func firstContainersBlock(data []byte) (start, end int, ok bool) { +// addEnvReference appends profile to the env list of the first [[containers]] +// block, or adds an env line under the block header when there is none. +func addEnvReference(data []byte, profile string) ([]byte, error) { header := containersHeaderRe.FindIndex(data) if header == nil { - return 0, 0, false + return nil, errors.New("no [[containers]] block found in config") } - start, end = header[1], len(data) + start, end := header[1], len(data) if next := tableHeaderRe.FindIndex(data[start:]); next != nil { end = start + next[0] } - return start, end, true -} - -// withEnvReference adds profile to the end of the first [[containers]] block's -// env list, adding an env line after the type line when the block has none. -func withEnvReference(data []byte, profile string) ([]byte, error) { - start, end, ok := firstContainersBlock(data) - if !ok { - return nil, errors.New("no [[containers]] block found in config") - } - block := data[start:end] quoted := `"` + profile + `"` - - if loc := envListRe.FindIndex(block); loc != nil { - open := loc[1] - closeOffset := bytes.IndexByte(block[open:], ']') - if closeOffset < 0 { - return nil, errors.New("could not parse the env list in the [[containers]] block") - } - inner := block[open : open+closeOffset] - names := quotedRe.FindAllIndex(inner, -1) - for _, n := range names { - if string(inner[n[0]+1:n[1]-1]) == profile { - return data, nil - } - } - switch { - case len(names) > 0: - return splice(data, start+open+names[len(names)-1][1], ", "+quoted), nil - case len(bytes.TrimSpace(inner)) == 0: - return replace(data, start+open, start+open+closeOffset, quoted), nil - default: - return splice(data, start+open, quoted+","), nil + loc := envListRe.FindIndex(data[start:end]) + if loc == nil { + return replace(data, start, start, "\nenv = ["+quoted+"]"), nil + } + open := start + loc[1] + closeAt := bytes.IndexByte(data[open:end], ']') + if closeAt < 0 { + return nil, errors.New("could not parse the env list in the [[containers]] block") + } + names := quotedRe.FindAllIndex(data[open:open+closeAt], -1) + for _, n := range names { + if string(data[open+n[0]+1:open+n[1]-1]) == profile { + return data, nil } } - - at, indent := start, "" - if loc := typeLineRe.FindIndex(block); loc != nil { - at = start + loc[1] - line := block[loc[0]:loc[1]] - indent = string(line[:len(line)-len(bytes.TrimLeft(line, " \t"))]) - } - nl := bytes.IndexByte(data[at:], '\n') - if nl < 0 { - return append(append([]byte{}, data...), "\n"+indent+"env = ["+quoted+"]\n"...), nil + if len(names) == 0 { + return replace(data, open, open, quoted), nil } - return splice(data, at+nl+1, indent+"env = ["+quoted+"]\n"), nil + at := open + names[len(names)-1][1] + return replace(data, at, at, ", "+quoted), nil } -// withProfileVar sets key to value in the [env.] table, creating the -// table at the end of the file when it does not exist. -func withProfileVar(data []byte, profile, key, value string) []byte { - assignment := key + " = " + fmt.Sprintf("%q", value) - q := regexp.QuoteMeta(profile) - headerRe := regexp.MustCompile(`(?m)^[ \t]*\[env\.(?:` + q + `|"` + q + `")\][ \t]*(?:#.*)?$`) +// setProfileVar sets key in the [env.] table, adding the table at the +// end of the file when it is missing. +func setProfileVar(data []byte, profile, key, value string) []byte { + assignment := fmt.Sprintf("%s = %q", key, value) + headerRe := regexp.MustCompile(`(?m)^[ \t]*\[env\.` + regexp.QuoteMeta(profile) + `\][ \t]*$`) header := headerRe.FindIndex(data) if header == nil { - return append(bytes.TrimRight(data, "\n"), "\n\n[env."+profile+"]\n"+assignment+"\n"...) + trimmed := len(bytes.TrimRight(data, "\n")) + return replace(data, trimmed, len(data), "\n\n[env."+profile+"]\n"+assignment+"\n") } - start, end := header[1], len(data) if next := tableHeaderRe.FindIndex(data[start:]); next != nil { end = start + next[0] } - // Keys are matched case-insensitively because viper lowercases them. + // viper lowercases keys, so match the key in any case. keyRe := regexp.MustCompile(`(?mi)^[ \t]*` + regexp.QuoteMeta(key) + `[ \t]*=[ \t]*("[^"\n]*"|'[^'\n]*'|[^ \t\n#]+)`) if loc := keyRe.FindSubmatchIndex(data[start:end]); loc != nil { return replace(data, start+loc[2], start+loc[3], fmt.Sprintf("%q", value)) } - return splice(data, start, "\n"+assignment) -} - -func splice(data []byte, at int, s string) []byte { - return replace(data, at, at, s) + return replace(data, start, start, "\n"+assignment) } +// replace returns a new slice with data[from:to] replaced by s. func replace(data []byte, from, to int, s string) []byte { out := make([]byte, 0, len(data)-(to-from)+len(s)) out = append(out, data[:from]...) diff --git a/internal/config/env_profile_test.go b/internal/config/env_profile_test.go index eaf0c3e6f..078c10d8b 100644 --- a/internal/config/env_profile_test.go +++ b/internal/config/env_profile_test.go @@ -19,84 +19,40 @@ func writeAndLoad(t *testing.T, content string) string { return path } -func TestSetEnvProfileVar(t *testing.T) { - for _, tc := range []struct { - name string - in string - want string - }{ - { - name: "empty env list", - in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = []\n", - want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\"]\n\n[env.p]\nKEY = \"1\"\n", - }, - { - name: "env list holding only a comment", - in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [ # none yet\n]\n", - want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\", # none yet\n]\n\n[env.p]\nKEY = \"1\"\n", - }, - { - name: "indented block without a type line", - in: "[[containers]]\n port = \"4566\"\n", - want: "[[containers]]\nenv = [\"p\"]\n port = \"4566\"\n\n[env.p]\nKEY = \"1\"\n", - }, - { - name: "existing quoted table with the key in another case", - in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = ['p']\n\n[env.\"p\"] # mine\nkey = 0\nOTHER = \"x\"\n", - want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = ['p']\n\n[env.\"p\"] # mine\nkey = \"1\"\nOTHER = \"x\"\n", - }, - { - name: "existing table without the key", - in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\n\n[env.p]\nOTHER = \"x\"\n", - want: "[[containers]]\ntype = \"aws\"\nenv = [\"p\"]\nport = \"4566\"\n\n[env.p]\nKEY = \"1\"\nOTHER = \"x\"\n", - }, - { - name: "only the first containers block is edited", - in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\n\n[[containers]]\ntype = \"snowflake\"\nport = \"4567\"\nenv = [\"q\"]\n\n[env.q]\nA = \"b\"\n", - want: "[[containers]]\ntype = \"aws\"\nenv = [\"p\"]\nport = \"4566\"\n\n[[containers]]\ntype = \"snowflake\"\nport = \"4567\"\nenv = [\"q\"]\n\n[env.q]\nA = \"b\"\n\n[env.p]\nKEY = \"1\"\n", - }, - } { - t.Run(tc.name, func(t *testing.T) { - path := writeAndLoad(t, tc.in) +func readConfig(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + return string(data) +} - require.NoError(t, SetEnvProfileVar("p", "KEY", "1")) +func TestSetEnvProfileVar_MultiLineEnvList(t *testing.T) { + path := writeAndLoad(t, "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\n \"mine\",\n]\n\n[env.mine]\nDEBUG = \"1\"\n") - got, err := os.ReadFile(path) - require.NoError(t, err) - assert.Equal(t, tc.want, string(got)) - }) - } + changed, err := SetEnvProfileVar("p", "KEY", "1") + + require.NoError(t, err) + assert.True(t, changed) + assert.Equal(t, "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\n \"mine\", \"p\",\n]\n\n[env.mine]\nDEBUG = \"1\"\n\n[env.p]\nKEY = \"1\"\n", readConfig(t, path)) } -func TestSetEnvProfileVar_RollsBackWhenALaterProfileOverrides(t *testing.T) { +func TestSetEnvProfileVar_RestoresFileWhenALaterProfileOverrides(t *testing.T) { original := "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"p\", \"later\"]\n\n[env.p]\nKEY = \"0\"\n\n[env.later]\nKEY = \"0\"\n" path := writeAndLoad(t, original) - err := SetEnvProfileVar("p", "KEY", "1") + _, err := SetEnvProfileVar("p", "KEY", "1") require.Error(t, err) - assert.Contains(t, err.Error(), "[env.later]") - got, readErr := os.ReadFile(path) - require.NoError(t, readErr) - assert.Equal(t, original, string(got)) + assert.Equal(t, original, readConfig(t, path)) } -func TestSetEnvProfileVar_RollsBackWhenResultDoesNotParse(t *testing.T) { - // A dotted key already defines the profile, so appending a table header - // would define it twice. +func TestSetEnvProfileVar_RestoresFileWhenResultDoesNotParse(t *testing.T) { + // The dotted key already defines the table, so adding a header defines it twice. original := "env.p.OTHER = \"x\"\n\n[[containers]]\ntype = \"aws\"\nport = \"4566\"\n" path := writeAndLoad(t, original) - require.Error(t, SetEnvProfileVar("p", "KEY", "1")) + _, err := SetEnvProfileVar("p", "KEY", "1") - got, err := os.ReadFile(path) - require.NoError(t, err) - assert.Equal(t, original, string(got)) -} - -func TestSetEnvProfileVar_NoConfigFile(t *testing.T) { - viper.Reset() - t.Cleanup(viper.Reset) - - assert.Error(t, SetEnvProfileVar("p", "KEY", "1")) + require.Error(t, err) + assert.Equal(t, original, readConfig(t, path)) } diff --git a/internal/emulator/aws/appinspector.go b/internal/emulator/aws/appinspector.go index 366dfdaa4..98e5751fb 100644 --- a/internal/emulator/aws/appinspector.go +++ b/internal/emulator/aws/appinspector.go @@ -1,7 +1,6 @@ package aws import ( - "bytes" "context" "encoding/json" "fmt" @@ -12,8 +11,8 @@ import ( "github.com/localstack/lstk/internal/inspect" ) -// appInspectorStatusPath is App Inspector's runtime switch. The emulator does -// not keep the state across restarts; there APP_INSPECTOR decides. +// appInspectorStatusPath turns App Inspector on or off. The state resets on +// restart, where the APP_INSPECTOR env var decides. const appInspectorStatusPath = "/_localstack/appinspector/status" type appInspectorStatus struct { @@ -22,45 +21,26 @@ type appInspectorStatus struct { } func (c *Client) AppInspectorEnabled(ctx context.Context, baseURL string) (bool, error) { - s, err := c.appInspectorStatus(ctx, http.MethodGet, baseURL, nil) - if err != nil { - return false, err - } - return s.enabled() + s, err := c.appInspectorStatus(ctx, http.MethodGet, baseURL, "") + return strings.EqualFold(s.Status, "enabled"), err } func (c *Client) SetAppInspectorEnabled(ctx context.Context, baseURL string, enabled bool) (bool, error) { - status := "disabled" + body := `{"status": "disabled"}` if enabled { - status = "enabled" - } - body, err := json.Marshal(map[string]string{"status": status}) - if err != nil { - return false, err + body = `{"status": "enabled"}` } s, err := c.appInspectorStatus(ctx, http.MethodPut, baseURL, body) - if err != nil { - return false, err - } - got, err := s.enabled() - if err != nil { - return false, err - } - if got != enabled { - return false, fmt.Errorf("emulator reported App Inspector status %s after the change", s.Status) - } - return s.Changed, nil + return s.Changed, err } -func (c *Client) appInspectorStatus(ctx context.Context, method, baseURL string, body []byte) (appInspectorStatus, error) { +func (c *Client) appInspectorStatus(ctx context.Context, method, baseURL, body string) (appInspectorStatus, error) { url := strings.TrimRight(baseURL, "/") + appInspectorStatusPath - req, err := http.NewRequestWithContext(ctx, method, url, bytes.NewReader(body)) + req, err := http.NewRequestWithContext(ctx, method, url, strings.NewReader(body)) if err != nil { return appInspectorStatus{}, fmt.Errorf("failed to create App Inspector request: %w", err) } - if body != nil { - req.Header.Set("Content-Type", "application/json") - } + req.Header.Set("Content-Type", "application/json") resp, err := c.http.Do(req) if err != nil { @@ -68,38 +48,17 @@ func (c *Client) appInspectorStatus(ctx context.Context, method, baseURL string, } defer func() { _ = resp.Body.Close() }() - data, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) - if err != nil { - return appInspectorStatus{}, fmt.Errorf("failed to read App Inspector response: %w", err) - } - // Every licensed emulator that ships App Inspector registers this route, so - // a 404 means this one predates it (or is the community image). + // A 404 means the emulator predates App Inspector, or is the community image. if resp.StatusCode == http.StatusNotFound { return appInspectorStatus{}, inspect.ErrUnavailable } if resp.StatusCode != http.StatusOK { - var e struct { - Error string `json:"error"` - } - if json.Unmarshal(data, &e) == nil && e.Error != "" { - data = []byte(e.Error) - } + data, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) return appInspectorStatus{}, emulatorStatusError(fmt.Sprintf("LocalStack returned status %d", resp.StatusCode), data) } - var s appInspectorStatus - if err := json.Unmarshal(data, &s); err != nil { + if err := json.NewDecoder(resp.Body).Decode(&s); err != nil { return appInspectorStatus{}, fmt.Errorf("failed to decode App Inspector response: %w", err) } return s, nil } - -func (s appInspectorStatus) enabled() (bool, error) { - switch strings.ToUpper(s.Status) { - case "ENABLED": - return true, nil - case "DISABLED": - return false, nil - } - return false, fmt.Errorf("unexpected App Inspector status %q", s.Status) -} diff --git a/internal/emulator/aws/appinspector_test.go b/internal/emulator/aws/appinspector_test.go deleted file mode 100644 index 0f26eb223..000000000 --- a/internal/emulator/aws/appinspector_test.go +++ /dev/null @@ -1,102 +0,0 @@ -package aws - -import ( - "context" - "io" - "net/http" - "net/http/httptest" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/localstack/lstk/internal/inspect" -) - -func appInspectorServer(t *testing.T, status int, body string) *httptest.Server { - t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, appInspectorStatusPath, r.URL.Path) - w.WriteHeader(status) - _, _ = io.WriteString(w, body) - })) - t.Cleanup(srv.Close) - return srv -} - -func TestAppInspectorEnabled(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - body string - want bool - }{ - {`{"status": "ENABLED", "note": "", "epoch": "a3db29c4"}`, true}, - {`{"status": "DISABLED", "note": "", "epoch": null}`, false}, - } { - srv := appInspectorServer(t, http.StatusOK, tc.body) - - got, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) - - require.NoError(t, err) - assert.Equal(t, tc.want, got) - } -} - -func TestSetAppInspectorEnabled(t *testing.T) { - t.Parallel() - var gotBody, gotMethod, gotContentType string - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - b, _ := io.ReadAll(r.Body) - gotBody, gotMethod, gotContentType = string(b), r.Method, r.Header.Get("Content-Type") - _, _ = io.WriteString(w, `{"status": "DISABLED", "changed": true}`) - })) - t.Cleanup(srv.Close) - - changed, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL+"/", false) - - require.NoError(t, err) - assert.True(t, changed) - assert.Equal(t, http.MethodPut, gotMethod) - assert.Equal(t, "application/json", gotContentType) - assert.JSONEq(t, `{"status": "disabled"}`, gotBody) -} - -func TestAppInspectorErrors(t *testing.T) { - t.Parallel() - - t.Run("404 means the emulator has no App Inspector", func(t *testing.T) { - t.Parallel() - srv := appInspectorServer(t, http.StatusNotFound, "") - - _, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) - - assert.ErrorIs(t, err, inspect.ErrUnavailable) - }) - - t.Run("error body is shown without its JSON wrapper", func(t *testing.T) { - t.Parallel() - srv := appInspectorServer(t, http.StatusInternalServerError, `{"error": "Failed to change AppInspector status"}`) - - _, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL, true) - - assert.EqualError(t, err, "LocalStack returned status 500: Failed to change AppInspector status") - }) - - t.Run("state that does not match the request", func(t *testing.T) { - t.Parallel() - srv := appInspectorServer(t, http.StatusOK, `{"status": "DISABLED", "changed": false}`) - - _, err := NewClient().SetAppInspectorEnabled(context.Background(), srv.URL, true) - - assert.Error(t, err) - }) - - t.Run("unknown status value", func(t *testing.T) { - t.Parallel() - srv := appInspectorServer(t, http.StatusOK, `{"status": "PAUSED"}`) - - _, err := NewClient().AppInspectorEnabled(context.Background(), srv.URL) - - assert.Error(t, err) - }) -} diff --git a/internal/inspect/inspect.go b/internal/inspect/inspect.go index fbf8ee0b3..f4b60a62c 100644 --- a/internal/inspect/inspect.go +++ b/internal/inspect/inspect.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "strings" "github.com/localstack/lstk/internal/config" "github.com/localstack/lstk/internal/container" @@ -12,46 +11,27 @@ import ( "github.com/localstack/lstk/internal/runtime" ) -// ErrUnavailable means the emulator has no App Inspector, e.g. an image that -// predates it. +// ErrUnavailable means the emulator has no App Inspector, e.g. an older image. var ErrUnavailable = errors.New("this emulator has no App Inspector") -const ( - // profileName is the [env.*] profile lstk owns for the saved setting. - profileName = "app-inspector" - // envVar turns App Inspector on at emulator startup. The runtime switch - // does not survive a restart. - envVar = "APP_INSPECTOR" -) - -// Client reads and flips App Inspector on a running AWS emulator. +// Client reads and sets App Inspector on a running AWS emulator. type Client interface { AppInspectorEnabled(ctx context.Context, host string) (bool, error) SetAppInspectorEnabled(ctx context.Context, host string, enabled bool) (changed bool, err error) } -// Target is the emulator a command acts on. +// Target is the emulator to act on. ConfigPath is empty for an --endpoint-url +// target, which lstk does not start. type Target struct { Runtime runtime.Runtime Containers []config.ContainerConfig Client Client Host string - // Config is nil for an --endpoint-url target: lstk does not start that - // emulator, so its config says nothing about it. - Config *LocalConfig -} - -// LocalConfig is the part of the lstk config that decides App Inspector's -// state when lstk starts the emulator. -type LocalConfig struct { - Container config.ContainerConfig - Env map[string]map[string]string - Path string + ConfigPath string } -// Set enables or disables App Inspector on the running emulator. With persist -// it also saves the setting to the config file, which works while the -// emulator is stopped too. +// Set turns App Inspector on or off on the running emulator. With persist it +// also saves the setting to the config file, even when the emulator is stopped. func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) error { running, err := isRunning(ctx, sink, t, persist) if err != nil { @@ -73,20 +53,34 @@ func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) } } - if persist { - if err := save(sink, t.Config, enabled); err != nil { - return err - } - if !running { - sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "The emulator is not running. The setting applies on the next start."}) + if !persist { + if enabled && t.ConfigPath != "" { + sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "To keep it on after restarts, run: lstk inspect enable --persist"}) } return nil } - emitRestartNote(sink, t.Config, enabled) + + value := "0" + if enabled { + value = "1" + } + changed, err := config.SetEnvProfileVar("app-inspector", "APP_INSPECTOR", value) + if err != nil { + sink.Emit(output.ErrorEvent{Title: "Could not save the App Inspector setting to " + t.ConfigPath, Summary: err.Error()}) + return output.NewSilentError(err) + } + if changed { + sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("App Inspector will be %s on every start (saved to %s)", state(enabled), t.ConfigPath)}) + } else { + sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: fmt.Sprintf("App Inspector is already %s on every start (%s)", state(enabled), t.ConfigPath)}) + } + if !running { + sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "The emulator is not running. The setting applies on the next start."}) + } return nil } -// Status reports whether App Inspector is enabled on the running emulator. +// Status shows whether App Inspector is on for the running emulator. func Status(ctx context.Context, sink output.Sink, t Target) error { running, err := isRunning(ctx, sink, t, false) if err != nil { @@ -100,7 +94,6 @@ func Status(ctx context.Context, sink output.Sink, t Target) error { return emitRequestError(sink, err) } sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: "App Inspector is " + state(enabled)}) - emitRestartNote(sink, t.Config, enabled) return nil } @@ -121,68 +114,6 @@ func isRunning(ctx context.Context, sink output.Sink, t Target, persist bool) (b return len(running) > 0, nil } -func save(sink output.Sink, cfg *LocalConfig, enabled bool) error { - if cfg == nil { - return errors.New("no lstk config to save the App Inspector setting to") - } - fail := func(err error) error { - sink.Emit(output.ErrorEvent{Title: "Could not save the App Inspector setting to " + cfg.Path, Summary: err.Error()}) - return output.NewSilentError(err) - } - saved, err := enabledOnStart(cfg) - if err != nil { - return fail(err) - } - if saved == enabled { - sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: fmt.Sprintf("App Inspector is already %s on every start (%s)", state(enabled), cfg.Path)}) - return nil - } - value := "0" - if enabled { - value = "1" - } - if err := config.SetEnvProfileVar(profileName, envVar, value); err != nil { - return fail(err) - } - sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("App Inspector will be %s on every start (saved to %s)", state(enabled), cfg.Path)}) - return nil -} - -// emitRestartNote warns when the running state will not survive a restart, -// because the config says otherwise. -func emitRestartNote(sink output.Sink, cfg *LocalConfig, enabled bool) { - if cfg == nil { - return - } - onStart, err := enabledOnStart(cfg) - if err != nil || onStart == enabled { - return - } - text := "App Inspector turns off when the emulator restarts. To keep it on, run: lstk inspect enable --persist" - if !enabled { - text = fmt.Sprintf("App Inspector turns back on when the emulator restarts, as %s enables it. To keep it off, run: lstk inspect disable --persist", cfg.Path) - } - sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: text}) -} - -// enabledOnStart reports whether the config's env profiles turn App Inspector -// on at startup. The last profile setting the variable wins, and LocalStack -// reads "1" and "true" as on. -func enabledOnStart(cfg *LocalConfig) (bool, error) { - env, err := cfg.Container.ResolvedEnv(cfg.Env) - if err != nil { - return false, err - } - enabled := false - for _, kv := range env { - if k, v, _ := strings.Cut(kv, "="); k == envVar { - v = strings.ToLower(strings.TrimSpace(v)) - enabled = v == "1" || v == "true" - } - } - return enabled, nil -} - func emitNotRunning(sink output.Sink, extra ...output.ErrorAction) error { sink.Emit(output.ErrorEvent{ Title: "LocalStack is not running", @@ -194,13 +125,10 @@ func emitNotRunning(sink output.Sink, extra ...output.ErrorAction) error { func emitRequestError(sink output.Sink, err error) error { if errors.Is(err, ErrUnavailable) { - sink.Emit(output.ErrorEvent{ - Title: "App Inspector is not available in this emulator", - Summary: "This emulator version predates App Inspector. Use a recent LocalStack image.", - }) - return output.NewSilentError(err) + sink.Emit(output.ErrorEvent{Title: "App Inspector is not available in this emulator", Summary: "Use a recent LocalStack image."}) + } else { + sink.Emit(output.ErrorEvent{Title: "App Inspector request failed", Summary: err.Error()}) } - sink.Emit(output.ErrorEvent{Title: "App Inspector request failed", Summary: err.Error()}) return output.NewSilentError(err) } diff --git a/internal/inspect/inspect_test.go b/internal/inspect/inspect_test.go deleted file mode 100644 index 13009fb99..000000000 --- a/internal/inspect/inspect_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package inspect - -import ( - "testing" - - "github.com/localstack/lstk/internal/config" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestEnabledOnStart(t *testing.T) { - t.Parallel() - for _, tc := range []struct { - name string - profiles []string - env map[string]map[string]string - want bool - }{ - {"unset", nil, nil, false}, - {"one", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": "1"}}, true}, - {"true in any case with spaces", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": " TRUE "}}, true}, - {"other truthy-looking words are off", []string{"a"}, map[string]map[string]string{"a": {"app_inspector": "yes"}}, false}, - {"last profile wins", []string{"a", "b"}, map[string]map[string]string{"a": {"app_inspector": "1"}, "b": {"app_inspector": "0"}}, false}, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - cfg := &LocalConfig{Container: config.ContainerConfig{Type: config.EmulatorAWS, Env: tc.profiles}, Env: tc.env} - - got, err := enabledOnStart(cfg) - - require.NoError(t, err) - assert.Equal(t, tc.want, got) - }) - } -} diff --git a/openspec/changes/json-output-schema/tasks.md b/openspec/changes/json-output-schema/tasks.md index de9f10686..237494481 100644 --- a/openspec/changes/json-output-schema/tasks.md +++ b/openspec/changes/json-output-schema/tasks.md @@ -35,7 +35,6 @@ Implemented first, ahead of every other command: each depends only on the shared - [ ] 4.3 `status`: add annotation; change the domain loop (`internal/container/status.go`) to report every configured emulator (running or not) instead of failing fast on the first non-running one, gated so plain-text behavior is unchanged; `{"emulators": [...]}` data shape. - [ ] 4.4 `logs` (bounded): add annotation, `{"lines": [...]}` data shape. - [ ] 4.5 `logs --follow`: wire the NDJSON stream writer from 1.5. -- [ ] 4.5b `inspect enable`/`disable`/`status`: add annotation, `{"running", "enabled", "changed", "persisted", "enabledOnStart"}` data shape, `EMULATOR_NOT_RUNNING`/`EMULATOR_NOT_CONFIGURED`/`EMULATOR_WRONG_TYPE` codes. - [ ] 4.6 Integration tests for each command in this wave, covering both the success payload and its documented error codes. ## 5. Remaining emulator lifecycle diff --git a/test/integration/inspect_test.go b/test/integration/inspect_test.go index d3ede5286..0075c9ea3 100644 --- a/test/integration/inspect_test.go +++ b/test/integration/inspect_test.go @@ -2,7 +2,6 @@ package integration_test import ( "encoding/json" - "io" "net/http" "net/http/httptest" "os" @@ -10,74 +9,53 @@ import ( "strings" "sync" "testing" + "time" "github.com/localstack/lstk/test/integration/env" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// appInspectorServer mimics an AWS emulator's /_localstack/appinspector/status -// endpoint (GET reports the state, PUT {"status": "enabled"|"disabled"} sets it) -// and records the status sent by each PUT. -type appInspectorServer struct { - *httptest.Server - mu sync.Mutex - enabled bool - puts []string -} - -func newAppInspectorServer(t *testing.T, enabled bool) *appInspectorServer { +// appInspectorServer mimics the AWS emulator's App Inspector switch and +// records the status sent by each PUT. +func appInspectorServer(t *testing.T, enabled bool) (*httptest.Server, func() []string) { t.Helper() - s := &appInspectorServer{enabled: enabled} + var mu sync.Mutex + var puts []string health := awsHealthHandler() - s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/_localstack/appinspector/status" { health.ServeHTTP(w, r) return } - s.mu.Lock() - defer s.mu.Unlock() - w.Header().Set("Content-Type", "application/json") - switch r.Method { - case http.MethodGet: - _ = json.NewEncoder(w).Encode(map[string]any{"status": statusWord(s.enabled), "note": "", "epoch": nil}) - case http.MethodPut: - body, _ := io.ReadAll(r.Body) + mu.Lock() + defer mu.Unlock() + changed := false + if r.Method == http.MethodPut { var req struct { Status string `json:"status"` } - if err := json.Unmarshal(body, &req); err != nil { - w.WriteHeader(http.StatusBadRequest) - return - } - s.puts = append(s.puts, req.Status) - want := req.Status == "enabled" - changed := want != s.enabled - s.enabled = want - _ = json.NewEncoder(w).Encode(map[string]any{"status": statusWord(s.enabled), "changed": changed}) - default: - w.WriteHeader(http.StatusMethodNotAllowed) + _ = json.NewDecoder(r.Body).Decode(&req) + puts = append(puts, req.Status) + changed = enabled != (req.Status == "enabled") + enabled = req.Status == "enabled" + } + status := "DISABLED" + if enabled { + status = "ENABLED" } + _ = json.NewEncoder(w).Encode(map[string]any{"status": status, "changed": changed}) })) - t.Cleanup(s.Close) - return s -} - -func statusWord(enabled bool) string { - if enabled { - return "ENABLED" + t.Cleanup(srv.Close) + return srv, func() []string { + mu.Lock() + defer mu.Unlock() + return append([]string(nil), puts...) } - return "DISABLED" -} - -func (s *appInspectorServer) recordedPuts() []string { - s.mu.Lock() - defer s.mu.Unlock() - return append([]string(nil), s.puts...) } -// inspectEnv is an isolated environment whose Docker endpoint is unreachable, so -// any test using it proves the command never needed Docker. +// inspectEnv points Docker at a closed port, so tests using it prove the +// command never needed Docker. func inspectEnv(t *testing.T) env.Environ { t.Helper() return env.Environ(testEnvWithHome(t.TempDir(), "")). @@ -110,58 +88,31 @@ port = "4566" check_for_update_on_startup = false ` -func TestInspectEnableEndpointURL(t *testing.T) { - t.Parallel() - srv := newAppInspectorServer(t, false) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "App Inspector enabled") - assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) -} - -func TestInspectEnableWhenAlreadyEnabled(t *testing.T) { +func TestInspectEndpointURL(t *testing.T) { t.Parallel() - srv := newAppInspectorServer(t, true) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "App Inspector is already enabled") -} - -func TestInspectDisableEndpointURL(t *testing.T) { - t.Parallel() - srv := newAppInspectorServer(t, true) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--endpoint-url", srv.URL, "--non-interactive", "inspect", "disable") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "App Inspector disabled") - assert.Equal(t, []string{"disabled"}, srv.recordedPuts()) -} - -func TestInspectStatusEndpointURL(t *testing.T) { - t.Parallel() - for _, enabled := range []bool{true, false} { - want := "App Inspector is disabled" - if enabled { - want = "App Inspector is enabled" - } - t.Run(want, func(t *testing.T) { + for _, tc := range []struct { + name string + enabled bool + cmd string + want string + wantPuts []string + }{ + {"enable", false, "enable", "App Inspector enabled", []string{"enabled"}}, + {"enable when already on", true, "enable", "App Inspector is already enabled", []string{"enabled"}}, + {"disable", true, "disable", "App Inspector disabled", []string{"disabled"}}, + {"status on", true, "status", "App Inspector is enabled", nil}, + {"status off", false, "status", "App Inspector is disabled", nil}, + } { + t.Run(tc.name, func(t *testing.T) { t.Parallel() - srv := newAppInspectorServer(t, enabled) + srv, puts := appInspectorServer(t, tc.enabled) stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--endpoint-url", srv.URL, "--non-interactive", "inspect", "status") + "--endpoint-url", srv.URL, "--non-interactive", "inspect", tc.cmd) require.NoError(t, err, stderr) - assert.Contains(t, stdout, want) - assert.Empty(t, srv.recordedPuts(), "status must not change the state") + assert.Contains(t, stdout, tc.want) + assert.Equal(t, tc.wantPuts, puts()) }) } } @@ -181,31 +132,34 @@ func TestInspectFailsWhenEmulatorLacksAppInspector(t *testing.T) { func TestInspectPersistRejectsEndpointURL(t *testing.T) { t.Parallel() - srv := newAppInspectorServer(t, false) + srv, puts := appInspectorServer(t, false) stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), "--endpoint-url", srv.URL, "--non-interactive", "inspect", "enable", "--persist") requireExitCode(t, 1, err) assert.Contains(t, stdout, "--persist") - assert.Empty(t, srv.recordedPuts(), "a rejected invocation must not change the emulator") + assert.Empty(t, puts()) } -// With the emulator stopped, --persist saves the setting so the next start -// picks it up, without needing Docker at all. -func TestInspectEnablePersistWhileStoppedWritesConfig(t *testing.T) { +// With the emulator stopped, --persist only saves the setting. A second run +// finds nothing to change. +func TestInspectPersist(t *testing.T) { t.Parallel() - configPath := writeInspectConfig(t, inspectBaseConfig) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "next start") - assert.Equal(t, `# my settings + for _, tc := range []struct { + name string + cmd string + in string + want string + }{ + { + name: "enable adds a profile and keeps comments", + cmd: "enable", + in: inspectBaseConfig, + want: `# my settings [[containers]] -type = "aws" # emulator type env = ["app-inspector"] +type = "aws" # emulator type tag = "latest" port = "4566" # env = [] # Named environment profiles @@ -215,164 +169,38 @@ check_for_update_on_startup = false [env.app-inspector] APP_INSPECTOR = "1" -`, readFile(t, configPath)) -} - -func TestInspectEnablePersistKeepsExistingProfiles(t *testing.T) { - t.Parallel() - configPath := writeInspectConfig(t, `[[containers]] -type = "aws" -port = "4566" -env = ["debug"] - -[env.debug] -DEBUG = "1" -`) - - _, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - - require.NoError(t, err, stderr) - assert.Equal(t, `[[containers]] -type = "aws" -port = "4566" -env = ["debug", "app-inspector"] - -[env.debug] -DEBUG = "1" - -[env.app-inspector] -APP_INSPECTOR = "1" -`, readFile(t, configPath)) -} - -func TestInspectEnablePersistIsIdempotent(t *testing.T) { - t.Parallel() - configPath := writeInspectConfig(t, inspectBaseConfig) - e := inspectEnv(t) - - _, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - require.NoError(t, err, stderr) - afterFirst := readFile(t, configPath) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "already") - assert.Equal(t, afterFirst, readFile(t, configPath)) -} - -// A profile the user wrote themselves already enables App Inspector, so there -// is nothing to save. -func TestInspectEnablePersistRespectsUserProfile(t *testing.T) { - t.Parallel() - original := `[[containers]] -type = "aws" -port = "4566" -env = ["mine"] - -[env.mine] -APP_INSPECTOR = "true" -` - configPath := writeInspectConfig(t, original) - - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "already") - assert.Equal(t, original, readFile(t, configPath)) -} - -func TestInspectDisablePersistAfterEnable(t *testing.T) { - t.Parallel() - configPath := writeInspectConfig(t, inspectBaseConfig) - e := inspectEnv(t) - - _, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - require.NoError(t, err, stderr) - - _, stderr, err = runLstk(t, testContext(t), t.TempDir(), e, - "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") - require.NoError(t, err, stderr) - - got := readFile(t, configPath) - assert.Contains(t, got, "[env.app-inspector]\nAPP_INSPECTOR = \"0\"\n") - assert.Equal(t, 1, strings.Count(got, `"app-inspector"`), "profile must be referenced once") -} - -// A profile that turns App Inspector on is overridden by lstk's own profile, -// which is applied last. -func TestInspectDisablePersistOverridesUserProfile(t *testing.T) { - t.Parallel() - configPath := writeInspectConfig(t, `[[containers]] -type = "aws" -port = "4566" -env = [ - "mine", -] - -[env.mine] -APP_INSPECTOR = "1" -`) - - _, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") - - require.NoError(t, err, stderr) - assert.Equal(t, `[[containers]] -type = "aws" -port = "4566" -env = [ - "mine", "app-inspector", -] - -[env.mine] -APP_INSPECTOR = "1" - -[env.app-inspector] -APP_INSPECTOR = "0" -`, readFile(t, configPath)) -} - -// lstk's profile is listed before one that also sets APP_INSPECTOR, so saving -// would have no effect; the file must be left as it was. -func TestInspectPersistFailsWhenALaterProfileOverrides(t *testing.T) { - t.Parallel() - original := `[[containers]] -type = "aws" -port = "4566" -env = ["app-inspector", "mine"] - -[env.app-inspector] -APP_INSPECTOR = "0" - -[env.mine] -APP_INSPECTOR = "0" -` - configPath := writeInspectConfig(t, original) - - stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - - requireExitCode(t, 1, err) - assert.Contains(t, stdout, "Could not save the App Inspector setting") - assert.Contains(t, stdout, "[env.mine]") - assert.Equal(t, original, readFile(t, configPath)) -} - -func TestInspectDisablePersistWhenNeverEnabledLeavesConfigAlone(t *testing.T) { - t.Parallel() - configPath := writeInspectConfig(t, inspectBaseConfig) +`, + }, + { + name: "enable keeps existing profiles", + cmd: "enable", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"debug\"]\n\n[env.debug]\nDEBUG = \"1\"\n", + want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"debug\", \"app-inspector\"]\n\n[env.debug]\nDEBUG = \"1\"\n\n[env.app-inspector]\nAPP_INSPECTOR = \"1\"\n", + }, + { + name: "disable flips the saved value", + cmd: "disable", + in: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"app-inspector\"]\n\n[env.app-inspector]\nAPP_INSPECTOR = \"1\"\n", + want: "[[containers]]\ntype = \"aws\"\nport = \"4566\"\nenv = [\"app-inspector\"]\n\n[env.app-inspector]\nAPP_INSPECTOR = \"0\"\n", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, tc.in) + e := inspectEnv(t) + args := []string{"--config", configPath, "--non-interactive", "inspect", tc.cmd, "--persist"} - stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), - "--config", configPath, "--non-interactive", "inspect", "disable", "--persist") + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, args...) + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "next start") + assert.Equal(t, tc.want, readFile(t, configPath)) - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "already") - assert.Equal(t, inspectBaseConfig, readFile(t, configPath)) + stdout, stderr, err = runLstk(t, testContext(t), t.TempDir(), e, args...) + require.NoError(t, err, stderr) + assert.Contains(t, stdout, "already") + assert.Equal(t, tc.want, readFile(t, configPath)) + }) + } } func TestInspectPersistWithoutConfigFile(t *testing.T) { @@ -421,44 +249,24 @@ func TestInspectEnableLocalEmulator(t *testing.T) { ctx := testContext(t) startTestContainer(t, ctx) - srv := newAppInspectorServer(t, false) + srv, puts := appInspectorServer(t, false) analyticsSrv, events := mockAnalyticsServer(t) stdout, stderr, err := runLstk(t, ctx, t.TempDir(), env.Environ(testEnvWithHome(t.TempDir(), "")). - With(env.LocalStackHost, lsHost(srv.Server)). + With(env.LocalStackHost, lsHost(srv)). With(env.AnalyticsEndpoint, analyticsSrv.URL), "--non-interactive", "inspect", "enable") require.NoError(t, err, stderr) assert.Contains(t, stdout, "App Inspector enabled") - assert.Contains(t, stdout, "lstk inspect enable --persist", "should explain how to keep it on after a restart") - assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) + assert.Contains(t, stdout, "lstk inspect enable --persist") + assert.Equal(t, []string{"enabled"}, puts()) assertCommandTelemetry(t, events, "inspect enable", 0) } -func TestInspectEnablePersistLocalEmulator(t *testing.T) { - requireDocker(t) - cleanup() - t.Cleanup(cleanup) - - ctx := testContext(t) - startTestContainer(t, ctx) - srv := newAppInspectorServer(t, false) - configPath := writeInspectConfig(t, inspectBaseConfig) - - stdout, stderr, err := runLstk(t, ctx, t.TempDir(), - env.Environ(testEnvWithHome(t.TempDir(), "")).With(env.LocalStackHost, lsHost(srv.Server)), - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") - - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "App Inspector enabled") - assert.NotContains(t, stdout, "next start") - assert.Equal(t, []string{"enabled"}, srv.recordedPuts()) - assert.Contains(t, readFile(t, configPath), "[env.app-inspector]\nAPP_INSPECTOR = \"1\"\n") -} - -// The persisted setting must reach a real emulator on its next start. +// The saved setting must reach a real emulator on its next start. The emulator +// turns App Inspector on shortly after it reports ready, so the check waits. func TestInspectPersistAppliesOnStart(t *testing.T) { requireDocker(t) token := env.Require(t, env.AuthToken) @@ -469,14 +277,14 @@ func TestInspectPersistAppliesOnStart(t *testing.T) { configPath := writeInspectConfig(t, inspectBaseConfig) e := env.Environ(testEnvWithHome(t.TempDir(), "")).With(env.AuthToken, token) - _, stderr, err := runLstk(t, ctx, t.TempDir(), e, - "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") + _, stderr, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") require.NoError(t, err, stderr) _, stderr, err = runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "start") require.NoError(t, err, stderr) - stdout, stderr, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "inspect", "status") - require.NoError(t, err, stderr) - assert.Contains(t, stdout, "App Inspector is enabled") + require.Eventually(t, func() bool { + stdout, _, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "inspect", "status") + return err == nil && strings.Contains(stdout, "App Inspector is enabled") + }, 15*time.Second, 500*time.Millisecond) } From 841bd932b3dd5b421e5bdc71d352679a91723db1 Mon Sep 17 00:00:00 2001 From: Anisa Oshafi Date: Tue, 13 Oct 2026 19:54:27 +0200 Subject: [PATCH 3/3] Add --json to lstk inspect and fix volume cleanup in CI --- cmd/inspect.go | 31 ++++--- docs/structured-output.md | 19 +++- internal/config/env_profile.go | 17 +--- internal/config/env_profile_test.go | 2 +- internal/emulator/aws/appinspector.go | 4 +- internal/inspect/inspect.go | 35 ++------ internal/output/envelope_data.go | 6 ++ internal/output/envelope_sink.go | 5 ++ internal/output/events.go | 15 ++++ internal/output/plain_format.go | 25 ++++++ internal/output/plain_format_test.go | 24 ++++++ openspec/changes/json-output-schema/tasks.md | 1 + test/integration/inspect_test.go | 91 +++++++++++++++++--- 13 files changed, 203 insertions(+), 72 deletions(-) diff --git a/cmd/inspect.go b/cmd/inspect.go index 53d879ff3..bb326069e 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -39,10 +39,11 @@ func newInspectSetCmd(cfg *env.Env, enabled bool) *cobra.Command { Short: title + " App Inspector", Long: title + " App Inspector on the running AWS emulator.\n\n" + "The change lasts until the emulator restarts. With --persist, lstk also saves it to the config file (as APP_INSPECTOR in an [env.app-inspector] profile), so App Inspector is " + state + " on every start. --persist works while the emulator is stopped too.", - Args: cobra.NoArgs, - PreRunE: initConfigDeferCreate(nil), + Args: cobra.NoArgs, + PreRunE: initConfigDeferCreate(nil), + Annotations: map[string]string{jsonSupportedAnnotation: "true"}, RunE: func(cmd *cobra.Command, args []string) error { - sink := output.NewPlainSink(os.Stdout) + sink := jsonAwareSink(cmd, cfg, os.Stdout) t, err := resolveInspectTarget(cmd, cfg, sink, persist) if err != nil { return err @@ -59,12 +60,13 @@ func newInspectSetCmd(cfg *env.Env, enabled bool) *cobra.Command { func newInspectStatusCmd(cfg *env.Env) *cobra.Command { return &cobra.Command{ - Use: "status", - Short: "Show whether App Inspector is enabled", - Args: cobra.NoArgs, - PreRunE: initConfigDeferCreate(nil), + Use: "status", + Short: "Show whether App Inspector is enabled", + Args: cobra.NoArgs, + PreRunE: initConfigDeferCreate(nil), + Annotations: map[string]string{jsonSupportedAnnotation: "true"}, RunE: func(cmd *cobra.Command, args []string) error { - sink := output.NewPlainSink(os.Stdout) + sink := jsonAwareSink(cmd, cfg, os.Stdout) t, err := resolveInspectTarget(cmd, cfg, sink, false) if err != nil { return err @@ -77,8 +79,6 @@ func newInspectStatusCmd(cfg *env.Env) *cobra.Command { } } -// resolveInspectTarget returns the --endpoint-url emulator when one is set, -// otherwise the AWS emulator from the lstk config. func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, persist bool) (inspect.Target, error) { fail := func(e output.ErrorEvent) (inspect.Target, error) { sink.Emit(e) @@ -87,12 +87,16 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe remote, err := endpoint.Resolve(cmd.Context(), cmd) if err != nil { - return inspect.Target{}, err + return fail(output.ErrorEvent{Title: err.Error(), Code: output.ErrEmulatorNotRunning}) } if remote != nil { if persist { source, _, _ := endpoint.ResolvedSource(cmd) - return fail(output.ErrorEvent{Title: "--persist cannot be used with " + source, Summary: "--persist saves to the lstk config, which does not start the emulator at " + remote.URL + "."}) + return fail(output.ErrorEvent{ + Title: "--persist cannot be used with " + source, + Summary: "--persist saves to the lstk config, which does not start the emulator at " + remote.URL + ".", + Code: output.ErrValidationError, + }) } if remote.Type != config.EmulatorAWS { return fail(output.ErrorEvent{ @@ -106,7 +110,7 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe appConfig, err := config.Get() if err != nil { - return inspect.Target{}, fmt.Errorf("failed to get config: %w", err) + return inspect.Target{}, failGetConfig(sink, cfg, err) } var awsContainer *config.ContainerConfig for i := range appConfig.Containers { @@ -122,6 +126,7 @@ func resolveInspectTarget(cmd *cobra.Command, cfg *env.Env, sink output.Sink, pe return fail(output.ErrorEvent{ Title: "No config file to save the setting to yet", Actions: []output.ErrorAction{{Label: "Create it by starting the emulator once:", Value: "lstk start"}}, + Code: output.ErrConfigNotFound, }) } diff --git a/docs/structured-output.md b/docs/structured-output.md index 4b8ae6597..d7e8d5ef6 100644 --- a/docs/structured-output.md +++ b/docs/structured-output.md @@ -8,7 +8,7 @@ The contract described here (a shared envelope shape, a shared error-code vocabu `--json` support is being rolled out per command, not all at once. The [Command Catalog](#command-catalog) below is split into two parts: -- **[Implemented](#implemented)** — `stop`, `reset`, `update`, `start`, `status`. These accept `--json` today and produce exactly the shapes documented below. +- **[Implemented](#implemented)** — `stop`, `reset`, `update`, `start`, `status`, `inspect enable`/`disable`/`status`. These accept `--json` today and produce exactly the shapes documented below. - **[Proposed for future work](#proposed-for-future-work-draft)** — every other built-in command. Attempting `--json` on any of these today is rejected with `NOT_JSON_CAPABLE`. This part is a **first-draft proposal only** — see the warning at the top of that section before relying on any of it. ## The envelope @@ -318,6 +318,23 @@ Codes: `RUNTIME_UNAVAILABLE`, `AUTH_REQUIRED`, `LICENSE_INVALID` (bad/expired/re ``` Codes: `RUNTIME_UNAVAILABLE`, `EMULATOR_NOT_RUNNING`, `CONFIG_INVALID`, `CONFIG_NOT_FOUND` (bad or missing `--config` path). +**`lstk inspect enable`** / **`disable`** / **`status`**: App Inspector's state on the running emulator. `changed` is whether this command changed it (always `false` for `status`). With `--persist`, `saved` reports the setting in the config file. When `--persist` runs while the emulator is stopped, `enabled` and `changed` are omitted. +```json +{ + "schemaVersion": 1, + "command": "inspect enable", + "status": "ok", + "data": { + "enabled": true, + "changed": true, + "saved": {"enabled": true, "changed": true} + }, + "warnings": [], + "error": null +} +``` +Codes: `RUNTIME_UNAVAILABLE`, `EMULATOR_NOT_RUNNING`, `EMULATOR_NOT_CONFIGURED`, `EMULATOR_WRONG_TYPE`, `VALIDATION_ERROR` (`--persist` with an endpoint URL), `CONFIG_NOT_FOUND` (`--persist` with no config file yet), `CONFIG_INVALID`. + ### Proposed for future work (draft) > **This section is a first-draft proposal only, not a committed contract.** None of the commands below accept `--json` yet — every one of them is rejected with `NOT_JSON_CAPABLE` today. The shapes shown are a starting point for design discussion, included here in full so the whole intended surface can be reviewed at once rather than piecemeal across many small follow-up PRs. Expect fields, error codes, and possibly the overall approach for any of these to change based on human feedback before implementation — treat everything below as a proposal to critique, not a spec to build against. diff --git a/internal/config/env_profile.go b/internal/config/env_profile.go index af76fd85b..e5544b4f9 100644 --- a/internal/config/env_profile.go +++ b/internal/config/env_profile.go @@ -14,11 +14,9 @@ var ( quotedRe = regexp.MustCompile(`"[^"\n]*"|'[^'\n]*'`) ) -// SetEnvProfileVar sets key = value in [env.] and appends the profile -// to the first [[containers]] block's env list, so it wins over the others. -// It edits the file in place to keep comments, and restores it if the result -// does not load or the value does not take effect. It reports whether the -// file changed. +// SetEnvProfileVar sets key in [env.] and lists that profile last in +// the first [[containers]] block, so it wins. The file is restored if the value +// does not take effect. func SetEnvProfileVar(profile, key, value string) (bool, error) { path := resolvedConfigPath() if path == "" { @@ -48,8 +46,6 @@ func SetEnvProfileVar(profile, key, value string) (bool, error) { return true, nil } -// checkResolvedEnv reloads the config and checks that the first container -// gets key=value. func checkResolvedEnv(path, key, value string) error { if err := loadConfig(path); err != nil { return err @@ -74,8 +70,6 @@ func checkResolvedEnv(path, key, value string) error { return nil } -// addEnvReference appends profile to the env list of the first [[containers]] -// block, or adds an env line under the block header when there is none. func addEnvReference(data []byte, profile string) ([]byte, error) { header := containersHeaderRe.FindIndex(data) if header == nil { @@ -108,8 +102,6 @@ func addEnvReference(data []byte, profile string) ([]byte, error) { return replace(data, at, at, ", "+quoted), nil } -// setProfileVar sets key in the [env.] table, adding the table at the -// end of the file when it is missing. func setProfileVar(data []byte, profile, key, value string) []byte { assignment := fmt.Sprintf("%s = %q", key, value) headerRe := regexp.MustCompile(`(?m)^[ \t]*\[env\.` + regexp.QuoteMeta(profile) + `\][ \t]*$`) @@ -122,7 +114,7 @@ func setProfileVar(data []byte, profile, key, value string) []byte { if next := tableHeaderRe.FindIndex(data[start:]); next != nil { end = start + next[0] } - // viper lowercases keys, so match the key in any case. + // viper lowercases keys. keyRe := regexp.MustCompile(`(?mi)^[ \t]*` + regexp.QuoteMeta(key) + `[ \t]*=[ \t]*("[^"\n]*"|'[^'\n]*'|[^ \t\n#]+)`) if loc := keyRe.FindSubmatchIndex(data[start:end]); loc != nil { return replace(data, start+loc[2], start+loc[3], fmt.Sprintf("%q", value)) @@ -130,7 +122,6 @@ func setProfileVar(data []byte, profile, key, value string) []byte { return replace(data, start, start, "\n"+assignment) } -// replace returns a new slice with data[from:to] replaced by s. func replace(data []byte, from, to int, s string) []byte { out := make([]byte, 0, len(data)-(to-from)+len(s)) out = append(out, data[:from]...) diff --git a/internal/config/env_profile_test.go b/internal/config/env_profile_test.go index 078c10d8b..fa34b3e1e 100644 --- a/internal/config/env_profile_test.go +++ b/internal/config/env_profile_test.go @@ -47,7 +47,7 @@ func TestSetEnvProfileVar_RestoresFileWhenALaterProfileOverrides(t *testing.T) { } func TestSetEnvProfileVar_RestoresFileWhenResultDoesNotParse(t *testing.T) { - // The dotted key already defines the table, so adding a header defines it twice. + // The dotted key already defines [env.p], so a new header is a duplicate. original := "env.p.OTHER = \"x\"\n\n[[containers]]\ntype = \"aws\"\nport = \"4566\"\n" path := writeAndLoad(t, original) diff --git a/internal/emulator/aws/appinspector.go b/internal/emulator/aws/appinspector.go index 98e5751fb..af17507bb 100644 --- a/internal/emulator/aws/appinspector.go +++ b/internal/emulator/aws/appinspector.go @@ -11,8 +11,6 @@ import ( "github.com/localstack/lstk/internal/inspect" ) -// appInspectorStatusPath turns App Inspector on or off. The state resets on -// restart, where the APP_INSPECTOR env var decides. const appInspectorStatusPath = "/_localstack/appinspector/status" type appInspectorStatus struct { @@ -48,7 +46,7 @@ func (c *Client) appInspectorStatus(ctx context.Context, method, baseURL, body s } defer func() { _ = resp.Body.Close() }() - // A 404 means the emulator predates App Inspector, or is the community image. + // The route is missing on older images and the community image. if resp.StatusCode == http.StatusNotFound { return appInspectorStatus{}, inspect.ErrUnavailable } diff --git a/internal/inspect/inspect.go b/internal/inspect/inspect.go index f4b60a62c..0ca9a989e 100644 --- a/internal/inspect/inspect.go +++ b/internal/inspect/inspect.go @@ -11,27 +11,21 @@ import ( "github.com/localstack/lstk/internal/runtime" ) -// ErrUnavailable means the emulator has no App Inspector, e.g. an older image. var ErrUnavailable = errors.New("this emulator has no App Inspector") -// Client reads and sets App Inspector on a running AWS emulator. type Client interface { AppInspectorEnabled(ctx context.Context, host string) (bool, error) SetAppInspectorEnabled(ctx context.Context, host string, enabled bool) (changed bool, err error) } -// Target is the emulator to act on. ConfigPath is empty for an --endpoint-url -// target, which lstk does not start. type Target struct { Runtime runtime.Runtime Containers []config.ContainerConfig Client Client Host string - ConfigPath string + ConfigPath string // empty for an --endpoint-url target } -// Set turns App Inspector on or off on the running emulator. With persist it -// also saves the setting to the config file, even when the emulator is stopped. func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) error { running, err := isRunning(ctx, sink, t, persist) if err != nil { @@ -46,11 +40,7 @@ func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) if err != nil { return emitRequestError(sink, err) } - if changed { - sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: "App Inspector " + state(enabled)}) - } else { - sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: "App Inspector is already " + state(enabled)}) - } + sink.Emit(output.AppInspectorEvent{Enabled: enabled, Changed: changed}) } if !persist { @@ -66,21 +56,16 @@ func Set(ctx context.Context, sink output.Sink, t Target, enabled, persist bool) } changed, err := config.SetEnvProfileVar("app-inspector", "APP_INSPECTOR", value) if err != nil { - sink.Emit(output.ErrorEvent{Title: "Could not save the App Inspector setting to " + t.ConfigPath, Summary: err.Error()}) + sink.Emit(output.ErrorEvent{Title: "Could not save the App Inspector setting to " + t.ConfigPath, Summary: err.Error(), Code: output.ErrConfigInvalid}) return output.NewSilentError(err) } - if changed { - sink.Emit(output.MessageEvent{Severity: output.SeveritySuccess, Text: fmt.Sprintf("App Inspector will be %s on every start (saved to %s)", state(enabled), t.ConfigPath)}) - } else { - sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: fmt.Sprintf("App Inspector is already %s on every start (%s)", state(enabled), t.ConfigPath)}) - } + sink.Emit(output.AppInspectorSavedEvent{Enabled: enabled, Changed: changed, ConfigPath: t.ConfigPath}) if !running { sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "The emulator is not running. The setting applies on the next start."}) } return nil } -// Status shows whether App Inspector is on for the running emulator. func Status(ctx context.Context, sink output.Sink, t Target) error { running, err := isRunning(ctx, sink, t, false) if err != nil { @@ -93,12 +78,11 @@ func Status(ctx context.Context, sink output.Sink, t Target) error { if err != nil { return emitRequestError(sink, err) } - sink.Emit(output.MessageEvent{Severity: output.SeverityInfo, Text: "App Inspector is " + state(enabled)}) + sink.Emit(output.AppInspectorEvent{Enabled: enabled}) return nil } -// isRunning treats an unreachable Docker as "not running" when persisting, -// since saving the setting does not need the emulator. +// With persist, an unreachable Docker counts as "not running": saving needs no emulator. func isRunning(ctx context.Context, sink output.Sink, t Target, persist bool) (bool, error) { if err := t.Runtime.IsHealthy(ctx); err != nil { if persist { @@ -132,13 +116,6 @@ func emitRequestError(sink output.Sink, err error) error { return output.NewSilentError(err) } -func state(enabled bool) string { - if enabled { - return "enabled" - } - return "disabled" -} - func verb(enabled bool) string { if enabled { return "enable" diff --git a/internal/output/envelope_data.go b/internal/output/envelope_data.go index ec3cc7563..58da21070 100644 --- a/internal/output/envelope_data.go +++ b/internal/output/envelope_data.go @@ -57,3 +57,9 @@ type JsonStatusResource struct { Region string `json:"region"` Account string `json:"account"` } + +// JsonAppInspectorSaved is `inspect`'s data.saved. +type JsonAppInspectorSaved struct { + Enabled bool `json:"enabled"` + Changed bool `json:"changed"` +} diff --git a/internal/output/envelope_sink.go b/internal/output/envelope_sink.go index b669a22d6..d11777424 100644 --- a/internal/output/envelope_sink.go +++ b/internal/output/envelope_sink.go @@ -43,6 +43,11 @@ func (s *EnvelopeSink) Emit(event Event) { case EmulatorResetEvent: s.data["emulator"] = JsonEmulatorRef(e) s.data["reset"] = true + case AppInspectorEvent: + s.data["enabled"] = e.Enabled + s.data["changed"] = e.Changed + case AppInspectorSavedEvent: + s.data["saved"] = JsonAppInspectorSaved{Enabled: e.Enabled, Changed: e.Changed} case EmulatorStartedEvent: s.data["emulator"] = e.Type s.data["container"] = e.Container diff --git a/internal/output/events.go b/internal/output/events.go index 1ca11365b..e2348fe21 100644 --- a/internal/output/events.go +++ b/internal/output/events.go @@ -168,6 +168,19 @@ type EmulatorResetEvent struct { Name string } +// AppInspectorEvent reports App Inspector's state on the running emulator. +type AppInspectorEvent struct { + Enabled bool + Changed bool +} + +// AppInspectorSavedEvent reports the App Inspector setting saved in the config file. +type AppInspectorSavedEvent struct { + Enabled bool + Changed bool + ConfigPath string +} + // EmulatorStartedEvent reports the emulator is up and reachable, whether // freshly started or already running. JSON-only: no plain-text line, since // start's existing messages already cover that. @@ -262,6 +275,8 @@ func (PodSnapshotRemovedEvent) sealedEvent() {} func (SnapshotShownEvent) sealedEvent() {} func (EmulatorStoppedEvent) sealedEvent() {} func (EmulatorResetEvent) sealedEvent() {} +func (AppInspectorEvent) sealedEvent() {} +func (AppInspectorSavedEvent) sealedEvent() {} func (EmulatorStartedEvent) sealedEvent() {} func (EmulatorStatusEvent) sealedEvent() {} func (UpdateCheckedEvent) sealedEvent() {} diff --git a/internal/output/plain_format.go b/internal/output/plain_format.go index 9119d7838..347c30cef 100644 --- a/internal/output/plain_format.go +++ b/internal/output/plain_format.go @@ -71,6 +71,10 @@ func FormatEventLine(event Event) (string, bool) { return formatEmulatorStopped(e), true case EmulatorResetEvent: return formatEmulatorReset(e), true + case AppInspectorEvent: + return formatAppInspector(e), true + case AppInspectorSavedEvent: + return formatAppInspectorSaved(e), true case EmulatorStatusEvent: return formatEmulatorStatus(e), true case UpdateCheckedEvent: @@ -209,6 +213,27 @@ func formatEmulatorReset(e EmulatorResetEvent) string { return SuccessMarker() + " Emulator state reset" } +func formatAppInspector(e AppInspectorEvent) string { + if e.Changed { + return SuccessMarker() + " App Inspector " + enabledWord(e.Enabled) + } + return "App Inspector is " + enabledWord(e.Enabled) +} + +func formatAppInspectorSaved(e AppInspectorSavedEvent) string { + if e.Changed { + return fmt.Sprintf("%s App Inspector will be %s on every start (saved to %s)", SuccessMarker(), enabledWord(e.Enabled), e.ConfigPath) + } + return fmt.Sprintf("App Inspector is %s on every start (%s)", enabledWord(e.Enabled), e.ConfigPath) +} + +func enabledWord(enabled bool) string { + if enabled { + return "enabled" + } + return "disabled" +} + func formatEmulatorStatus(e EmulatorStatusEvent) string { if !e.Running { return e.Type + ": not running" diff --git a/internal/output/plain_format_test.go b/internal/output/plain_format_test.go index 79851c036..5ce3aff7a 100644 --- a/internal/output/plain_format_test.go +++ b/internal/output/plain_format_test.go @@ -202,6 +202,30 @@ func TestFormatEventLine(t *testing.T) { want: SuccessMarker() + " LocalStack AWS Emulator stopped", wantOK: true, }, + { + name: "app inspector changed", + event: AppInspectorEvent{Enabled: true, Changed: true}, + want: SuccessMarker() + " App Inspector enabled", + wantOK: true, + }, + { + name: "app inspector unchanged", + event: AppInspectorEvent{Enabled: false}, + want: "App Inspector is disabled", + wantOK: true, + }, + { + name: "app inspector saved", + event: AppInspectorSavedEvent{Enabled: true, Changed: true, ConfigPath: "~/.config/lstk/config.toml"}, + want: SuccessMarker() + " App Inspector will be enabled on every start (saved to ~/.config/lstk/config.toml)", + wantOK: true, + }, + { + name: "app inspector already saved", + event: AppInspectorSavedEvent{Enabled: false, ConfigPath: "~/.config/lstk/config.toml"}, + want: "App Inspector is disabled on every start (~/.config/lstk/config.toml)", + wantOK: true, + }, { name: "emulator reset event", event: EmulatorResetEvent{Type: "aws", Name: "localstack-aws"}, diff --git a/openspec/changes/json-output-schema/tasks.md b/openspec/changes/json-output-schema/tasks.md index 237494481..a154dd0df 100644 --- a/openspec/changes/json-output-schema/tasks.md +++ b/openspec/changes/json-output-schema/tasks.md @@ -35,6 +35,7 @@ Implemented first, ahead of every other command: each depends only on the shared - [ ] 4.3 `status`: add annotation; change the domain loop (`internal/container/status.go`) to report every configured emulator (running or not) instead of failing fast on the first non-running one, gated so plain-text behavior is unchanged; `{"emulators": [...]}` data shape. - [ ] 4.4 `logs` (bounded): add annotation, `{"lines": [...]}` data shape. - [ ] 4.5 `logs --follow`: wire the NDJSON stream writer from 1.5. +- [x] 4.5b `inspect enable`/`disable`/`status`: annotation, `{"enabled", "changed", "saved"}` data shape, with integration tests. - [ ] 4.6 Integration tests for each command in this wave, covering both the success payload and its documented error codes. ## 5. Remaining emulator lifecycle diff --git a/test/integration/inspect_test.go b/test/integration/inspect_test.go index 0075c9ea3..af326ba75 100644 --- a/test/integration/inspect_test.go +++ b/test/integration/inspect_test.go @@ -16,8 +16,6 @@ import ( "github.com/stretchr/testify/require" ) -// appInspectorServer mimics the AWS emulator's App Inspector switch and -// records the status sent by each PUT. func appInspectorServer(t *testing.T, enabled bool) (*httptest.Server, func() []string) { t.Helper() var mu sync.Mutex @@ -54,8 +52,6 @@ func appInspectorServer(t *testing.T, enabled bool) (*httptest.Server, func() [] } } -// inspectEnv points Docker at a closed port, so tests using it prove the -// command never needed Docker. func inspectEnv(t *testing.T) env.Environ { t.Helper() return env.Environ(testEnvWithHome(t.TempDir(), "")). @@ -98,7 +94,7 @@ func TestInspectEndpointURL(t *testing.T) { wantPuts []string }{ {"enable", false, "enable", "App Inspector enabled", []string{"enabled"}}, - {"enable when already on", true, "enable", "App Inspector is already enabled", []string{"enabled"}}, + {"enable when already on", true, "enable", "App Inspector is enabled", []string{"enabled"}}, {"disable", true, "disable", "App Inspector disabled", []string{"disabled"}}, {"status on", true, "status", "App Inspector is enabled", nil}, {"status off", false, "status", "App Inspector is disabled", nil}, @@ -117,7 +113,6 @@ func TestInspectEndpointURL(t *testing.T) { } } -// An emulator older than App Inspector has no status route and answers 404. func TestInspectFailsWhenEmulatorLacksAppInspector(t *testing.T) { t.Parallel() srv := awsHealthServer(t) @@ -142,8 +137,6 @@ func TestInspectPersistRejectsEndpointURL(t *testing.T) { assert.Empty(t, puts()) } -// With the emulator stopped, --persist only saves the setting. A second run -// finds nothing to change. func TestInspectPersist(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -197,7 +190,7 @@ APP_INSPECTOR = "1" stdout, stderr, err = runLstk(t, testContext(t), t.TempDir(), e, args...) require.NoError(t, err, stderr) - assert.Contains(t, stdout, "already") + assert.Contains(t, stdout, "on every start ("+configPath+")") assert.Equal(t, tc.want, readFile(t, configPath)) }) } @@ -229,6 +222,78 @@ func TestInspectRejectsNonAWSConfig(t *testing.T) { assert.Equal(t, original, readFile(t, configPath)) } +func TestInspectJSON(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + enabled bool + args []string + wantData string + }{ + {"status", true, []string{"inspect", "status"}, `{"enabled": true, "changed": false}`}, + {"enable", false, []string{"inspect", "enable"}, `{"enabled": true, "changed": true}`}, + {"disable", true, []string{"inspect", "disable"}, `{"enabled": false, "changed": true}`}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + srv, _ := appInspectorServer(t, tc.enabled) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + append([]string{"--endpoint-url", srv.URL, "--json"}, tc.args...)...) + + require.NoError(t, err, stderr) + envelope := decodeEnvelope(t, stdout) + assert.Equal(t, "ok", envelope.Status) + assert.Equal(t, strings.Join(tc.args, " "), envelope.Command) + assert.JSONEq(t, tc.wantData, string(envelope.Data)) + }) + } +} + +func TestInspectPersistJSON(t *testing.T) { + t.Parallel() + configPath := writeInspectConfig(t, inspectBaseConfig) + + stdout, stderr, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), + "--config", configPath, "--json", "inspect", "enable", "--persist") + + require.NoError(t, err, stderr) + envelope := decodeEnvelope(t, stdout) + assert.Equal(t, "ok", envelope.Status) + assert.JSONEq(t, `{"saved": {"enabled": true, "changed": true}}`, string(envelope.Data)) +} + +func TestInspectJSONErrors(t *testing.T) { + t.Parallel() + aws, _ := appInspectorServer(t, false) + snowflake := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"version": "1.0.0", "services": map[string]string{"snowflake": "running"}}) + })) + t.Cleanup(snowflake.Close) + snowflakeConfig := writeInspectConfig(t, "[[containers]]\ntype = \"snowflake\"\nport = \"4566\"\n") + + for _, tc := range []struct { + name string + args []string + wantCode string + }{ + {"persist with endpoint", []string{"--endpoint-url", aws.URL, "inspect", "enable", "--persist"}, "VALIDATION_ERROR"}, + {"non-AWS endpoint", []string{"--endpoint-url", snowflake.URL, "inspect", "status"}, "EMULATOR_WRONG_TYPE"}, + {"non-AWS config", []string{"--config", snowflakeConfig, "inspect", "status"}, "EMULATOR_NOT_CONFIGURED"}, + {"persist without config file", []string{"inspect", "enable", "--persist"}, "CONFIG_NOT_FOUND"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), inspectEnv(t), append([]string{"--json"}, tc.args...)...) + + requireExitCode(t, 1, err) + envelope := decodeEnvelope(t, stdout) + require.NotNil(t, envelope.Error) + assert.Equal(t, tc.wantCode, envelope.Error.Code) + }) + } +} + func TestInspectEnableNotRunning(t *testing.T) { requireDocker(t) cleanup() @@ -265,17 +330,19 @@ func TestInspectEnableLocalEmulator(t *testing.T) { assertCommandTelemetry(t, events, "inspect enable", 0) } -// The saved setting must reach a real emulator on its next start. The emulator -// turns App Inspector on shortly after it reports ready, so the check waits. +// The emulator turns App Inspector on shortly after it reports ready, so the check waits. func TestInspectPersistAppliesOnStart(t *testing.T) { requireDocker(t) token := env.Require(t, env.AuthToken) cleanup() + home := t.TempDir() + scheduleVolumeCleanup(t, home) + // Runs first (LIFO), so the emulator stops before its volume is removed. t.Cleanup(cleanup) ctx := testContext(t) configPath := writeInspectConfig(t, inspectBaseConfig) - e := env.Environ(testEnvWithHome(t.TempDir(), "")).With(env.AuthToken, token) + e := env.Environ(testEnvWithHome(home, "")).With(env.AuthToken, token) _, stderr, err := runLstk(t, ctx, t.TempDir(), e, "--config", configPath, "--non-interactive", "inspect", "enable", "--persist") require.NoError(t, err, stderr)