diff --git a/CLAUDE.md b/CLAUDE.md index 0d416630..01d6ca53 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,7 +70,7 @@ Notes: - `terminal/` - Plain-mode terminal helpers (spinner, TTY detection) - `tracing/` - OpenTelemetry setup (`LSTK_OTEL=1`) - `ui/` - Bubble Tea views for interactive output - - `update/` - Self-update logic: version check via GitHub API, binary/Homebrew/npm update paths, archive extraction; the binary path verifies the downloaded archive's SHA-256 against the release's `checksums.txt` before replacing the executable (hard fail on missing/malformed manifest or mismatch) + - `update/` - Self-update logic: version check via GitHub API, binary/Homebrew/npm update paths, archive extraction; the binary path verifies the downloaded archive's SHA-256 against the release's `checksums.txt` before replacing the executable (hard fail on missing/malformed manifest or mismatch). `classifyPath` also detects installs owned by another package manager (mise, asdf, Nix, Scoop, Chocolatey) from the resolved binary path — those default to a one-line notice instead of the prompt, and `lstk update` refuses on them rather than replacing a file it does not own; the automatic check's policy is `update.CheckMode`, resolved at the command boundary (see Automatic Update Check below) - `validate/` - Reusable input validators for user-supplied CLI values (pod names, env var names, auth tokens) rejecting malformed/hostile input (control chars, path traversal, percent-encoding, shell metacharacters) - `version/` - Version info - `volume/` - `lstk volume` domain logic @@ -130,6 +130,14 @@ Each `[[containers]]` block may set an optional `container_name` (override the d `GATEWAY_LISTEN` (host exposure and published ports) is read from the container's resolved env, not hardcoded; parsing and derivation live in `internal/container/gateway.go`. +# Automatic Update Check + +`lstk start` (and the bare root) checks GitHub for a newer release. The policy is `[cli] update_check` — `prompt` (default), `notify` (one-line note, no input wait), or `off` (no request at all) — overridable by `LSTK_UPDATE_CHECK`. Precedence and fallback rules live in `resolveUpdateCheckMode` (`cmd/update_check.go`): an unparsable value is warned about and skipped rather than failing the command, and `prompt` is downgraded to `notify` off a TTY, since only the TUI answers a `UserInputRequestEvent`. + +`buildNotifyOptions` (same file) is the one place the policy is resolved, shared by both start paths — they previously built separate `NotifyOptions`, which is how the non-interactive path came to ignore `cli.update_skipped_version`. `internal/update` never reads config; everything is injected. + +Installs owned by another package manager default to `notify` and are never self-updated (`lstk update` refuses with `UPDATE_EXTERNALLY_MANAGED`); an explicit `update_check` overrides that in both directions. Explicit `lstk update` always checks. Detection rules and per-manager wording are documented on `classifyPath` and `ExternalManager` (`internal/update/install_method.go`). + # Offline / Enterprise Environments There is no `--offline` flag. Instead `container.Start` degrades gracefully when internet requests fail (Docker Hub unreachable, proxy/TLS interception, license server unreachable): local images are used when pulls fail, and the license pre-flight is skipped on transport-level failures, non-definitive server responses (5xx/407), or unsupported-tag rejections so the container validates its own bundled license. Definitive license rejections (HTTP 400/401/403) drop the cached license and offer an in-place re-login instead of requiring a manual `lstk logout` (DEVX-658). The exact fallback and retry rules live in `tryPrePullLicenseValidation`/`validateLicense`/`startWithLicenseRetry` (`internal/container/start.go`); pair them with a custom `image` in the config to point at a locally loaded image or an internal-registry mirror. @@ -154,6 +162,7 @@ Environment variables: - `LOCALSTACK_AUTH_TOKEN` - Auth token (skips browser login if set). It takes precedence over credentials stored in the keyring, so a per-invocation token overrides a previous `lstk login` without a `lstk logout` first; resolution order is env var → keyring → browser login (`auth.GetToken`, mirrored in `cmd/root.go`'s telemetry token resolution). - `LSTK_STARTUP_TIMEOUT` - Startup readiness deadline for `lstk start` (Go duration). Zero/unset uses the per-mode default resolved in `resolveStartupTimeout` (`internal/container/start.go`): 20s interactive (deadline only shows a recoverable keep-waiting/stop prompt, re-armed by "keep waiting"), 60s non-interactive (fatal; the container is left running for inspection). Container exits are detected separately — and instantly, with the exit code — via the exit wait `runtime.Runtime.Start` registers between create and start. `lstk start --timeout ` (also on the bare root) overrides this for a single run; the flag wins over the env var when explicitly set, and `--timeout 0` falls back to the per-mode default (`addTimeoutFlag`/`applyTimeoutFlag` in `cmd/root.go`). `restart` and the snapshot auto-start path do not expose the flag. - `LSTK_OTEL=1` - Enables OpenTelemetry trace export (disabled by default); when enabled, standard `OTEL_EXPORTER_OTLP_*` env vars are respected by the SDK. Requires an OTLP-compatible backend to receive and visualize telemetry — for local development, `make otel` starts one (UI at http://localhost:16686). +- `LSTK_UPDATE_CHECK` - Policy for the automatic update check on the start path: `prompt` (default), `notify` (one-line note, no input wait), or `off` (no check, no request). Overrides `[cli] update_check` in config.toml; see Automatic Update Check below. - `LSTK_MERGE_STRATEGY` - Default merge strategy for `snapshot load` / `load` (`account-region-merge`, `overwrite`, or `service-merge`) when `--merge` is not passed; an explicit `--merge` always wins. Resolved in `resolveMergeStrategy` (`cmd/snapshot.go`). # Infrastructure as Code Commands diff --git a/README.md b/README.md index 11e68b9c..579690a2 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Running `lstk` will automatically handle authentication, configuration, and cont - **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) -- **Self-update** — `lstk update` checks for and installs the latest release +- **Self-update** — `lstk update` checks for and installs the latest release. The automatic check on start is configurable via `[cli] update_check` (`prompt` / `notify` / `off`) or `LSTK_UPDATE_CHECK`; installs managed by mise, asdf, Nix, Scoop or Chocolatey are only ever notified about and left to their own manager - **Structured JSON output** — pass `--json` to a supported command for a machine-readable envelope instead of formatted text; see [structured output](https://github.com/localstack/lstk/blob/main/docs/structured-output.md) For the full command reference, configuration options, environment variables, and troubleshooting, see the **[lstk documentation](https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/)**. diff --git a/cmd/root.go b/cmd/root.go index 12dfbabd..97ed7c7c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -346,12 +346,14 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t logger.Info("could not resolve friendly config path: %v", err) } - // Apply the --type flag before resolving snapshot and start options so - // everything downstream reflects the selected emulator. Messages go to a plain - // sink even in interactive mode because the config mutation has to happen before - // the TUI starts (the auto-load loader and start options are built from it). + // Anything reported before the TUI can start goes through this sink, in + // interactive mode too. + plainSink := output.NewPlainSink(os.Stdout) + + // Apply --type before resolving snapshot and start options so everything + // downstream reflects the selected emulator. if emulatorType != "" { - newContainers, applyErr := container.ApplyEmulatorType(ctx, rt, output.NewPlainSink(os.Stdout), emulatorType, appConfig.Containers, firstRun, configPath) + newContainers, applyErr := container.ApplyEmulatorType(ctx, rt, plainSink, emulatorType, appConfig.Containers, firstRun, configPath) if applyErr != nil { return applyErr } @@ -373,14 +375,11 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t opts := buildStartOptions(cfg, appConfig, logger, tel, persist) - notifyOpts := update.NotifyOptions{ - GitHubToken: cfg.GitHubToken, - UpdatePrompt: true, - SkippedVersion: appConfig.CLI.UpdateSkippedVersion, - PersistSkipVersion: config.SetUpdateSkippedVersion, - } + // Resolved once so the two output paths cannot disagree about the policy. + interactive := isInteractiveMode(cfg) + notifyOpts := buildNotifyOptions(plainSink, cfg, appConfig, configPath, firstRun, interactive) - if isInteractiveMode(cfg) { + if interactive { return ui.Run(ctx, ui.RunOptions{ Runtime: rt, Version: version.Version(), @@ -393,16 +392,15 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t }) } - sink := output.NewPlainSink(os.Stdout) if firstRun && len(appConfig.Containers) > 0 { emName := appConfig.Containers[0].Type.ShortName() - sink.Emit(output.MessageEvent{ + plainSink.Emit(output.MessageEvent{ Severity: output.SeverityNote, Text: fmt.Sprintf("Configured with default emulator %s.", emName), }) } - update.NotifyUpdate(ctx, sink, update.NotifyOptions{GitHubToken: cfg.GitHubToken}) - resolvedVersion, err := container.Start(ctx, rt, sink, opts, false) + update.NotifyUpdate(ctx, plainSink, notifyOpts) + resolvedVersion, err := container.Start(ctx, rt, plainSink, opts, false) if err != nil { return err } @@ -410,7 +408,7 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t // this run (resolvedVersion is empty when it was already running). This mirrors // v1's AUTO_LOAD_POD: state is loaded as the emulator comes up, not on every invocation. if autoLoad != nil && resolvedVersion != "" { - if err := autoLoad(ctx, sink); err != nil { + if err := autoLoad(ctx, plainSink); err != nil { return err } } diff --git a/cmd/update.go b/cmd/update.go index 2ecfa878..e51c8f32 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -13,9 +13,10 @@ func newUpdateCmd(cfg *env.Env) *cobra.Command { var checkOnly bool cmd := &cobra.Command{ - Use: "update", - Short: "Update lstk to the latest version", - Long: "Check for and apply updates to the lstk CLI. Respects the original installation method (Homebrew, npm, or direct binary).", + Use: "update", + Short: "Update lstk to the latest version", + Long: "Check for and apply updates to the lstk CLI. Respects the original installation method (Homebrew, npm, or direct binary), and refuses to touch an install owned by another package manager (mise, asdf, Nix, Scoop, Chocolatey) — those report that manager's own upgrade command instead.\n\n" + + "Running this command always checks, regardless of the [cli] update_check setting, which governs only the automatic check on 'lstk start'.", PreRunE: initConfigDeferCreate(nil), Annotations: map[string]string{jsonSupportedAnnotation: "true"}, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cmd/update_check.go b/cmd/update_check.go new file mode 100644 index 00000000..dd483a7c --- /dev/null +++ b/cmd/update_check.go @@ -0,0 +1,99 @@ +package cmd + +import ( + "fmt" + + "github.com/localstack/lstk/internal/config" + "github.com/localstack/lstk/internal/env" + "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/update" +) + +// updateCheckContext is what resolveUpdateCheckMode needs to pick a policy, +// gathered at the boundary so the resolution stays a pure function. +type updateCheckContext struct { + EnvValue string // LSTK_UPDATE_CHECK, empty when unset + ConfigValue string // [cli] update_check, empty when unset + ExternallyManaged bool // another package manager owns the binary + Interactive bool // a prompt could actually be answered +} + +// resolveUpdateCheckMode resolves the policy in precedence order: +// LSTK_UPDATE_CHECK, [cli] update_check, then the install-implied default +// (notify when a package manager owns the binary, prompt otherwise). +// +// An unparsable value is reported and skipped, never fatal: the setting governs +// a best-effort background check, so a typo must not stop `lstk start` — and +// there is no `lstk config set` to fix it with. Falling through source by source +// keeps the documented precedence true even when one source is garbage. +// +// Off a terminal, prompt is downgraded to notify: only the TUI answers a +// UserInputRequestEvent, so a plain sink would block until context cancellation. +func resolveUpdateCheckMode(sink output.Sink, checkCtx updateCheckContext) update.CheckMode { + mode := resolveConfiguredCheckMode(sink, checkCtx) + if mode == update.CheckModePrompt && !checkCtx.Interactive { + return update.CheckModeNotify + } + return mode +} + +func resolveConfiguredCheckMode(sink output.Sink, checkCtx updateCheckContext) update.CheckMode { + sources := []struct { + label string + value string + }{ + {"LSTK_UPDATE_CHECK", checkCtx.EnvValue}, + {"update_check in [cli]", checkCtx.ConfigValue}, + } + + for _, source := range sources { + if source.value == "" { + continue + } + mode, err := update.ParseCheckMode(source.value) + if err != nil { + sink.Emit(output.MessageEvent{ + Severity: output.SeverityWarning, + Text: fmt.Sprintf("Ignoring %s: %v", source.label, err), + }) + continue + } + return mode + } + + if checkCtx.ExternallyManaged { + return update.CheckModeNotify + } + return update.CheckModePrompt +} + +// buildNotifyOptions resolves the one policy both start paths use. Building it +// once is what keeps them in sync: the non-interactive path used to construct +// its own NotifyOptions and so ignored the skipped version (DEVX-1029). +func buildNotifyOptions(sink output.Sink, cfg *env.Env, appConfig *config.Config, configPath string, firstRun, interactive bool) update.NotifyOptions { + info := update.DetectInstallMethod() + + opts := update.NotifyOptions{ + Mode: resolveUpdateCheckMode(sink, updateCheckContext{ + EnvValue: cfg.UpdateCheck, + ConfigValue: appConfig.CLI.UpdateCheck, + ExternallyManaged: info.ExternallyManaged(), + Interactive: interactive, + }), + GitHubToken: cfg.GitHubToken, + SkippedVersion: appConfig.CLI.UpdateSkippedVersion, + PersistSkipVersion: config.SetUpdateSkippedVersion, + Install: info, + ConfigPath: configPath, + } + + // No config file to write to on a first run, and creating one here would + // suppress the emulator picker — so withhold the option (see NotifyOptions). + if !firstRun { + opts.PersistCheckMode = func(mode update.CheckMode) error { + return config.SetUpdateCheck(string(mode)) + } + } + + return opts +} diff --git a/cmd/update_check_test.go b/cmd/update_check_test.go new file mode 100644 index 00000000..661dac4d --- /dev/null +++ b/cmd/update_check_test.go @@ -0,0 +1,98 @@ +package cmd + +import ( + "bytes" + "testing" + + "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/update" + "github.com/stretchr/testify/assert" +) + +// TestResolveUpdateCheckMode covers the seam between what the user configured +// (LSTK_UPDATE_CHECK, [cli] update_check, the install-implied default) and the +// policy handed to update.NotifyUpdate. +func TestResolveUpdateCheckMode(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + checkCtx updateCheckContext + want update.CheckMode + wantWarnings []string + }{ + { + name: "nothing set defaults to prompt", + checkCtx: updateCheckContext{Interactive: true}, + want: update.CheckModePrompt, + }, + { + name: "nothing set on an externally managed install defaults to notify", + checkCtx: updateCheckContext{ExternallyManaged: true, Interactive: true}, + want: update.CheckModeNotify, + }, + { + name: "config value is used when the env var is unset", + checkCtx: updateCheckContext{ConfigValue: "off", Interactive: true}, + want: update.CheckModeOff, + }, + { + name: "env var beats config", + checkCtx: updateCheckContext{EnvValue: "prompt", ConfigValue: "off", Interactive: true}, + want: update.CheckModePrompt, + }, + { + name: "explicit prompt overrides the externally managed default", + checkCtx: updateCheckContext{ConfigValue: "prompt", ExternallyManaged: true, Interactive: true}, + want: update.CheckModePrompt, + }, + { + name: "explicit off overrides the externally managed default", + checkCtx: updateCheckContext{EnvValue: "off", ExternallyManaged: true, Interactive: true}, + want: update.CheckModeOff, + }, + { + // Only the TUI answers a prompt, so a non-interactive run notifies. + name: "prompt is downgraded to notify when not interactive", + checkCtx: updateCheckContext{ConfigValue: "prompt"}, + want: update.CheckModeNotify, + }, + { + name: "off is honored when not interactive", + checkCtx: updateCheckContext{ConfigValue: "off"}, + want: update.CheckModeOff, + }, + { + name: "invalid env value warns and falls through to config", + checkCtx: updateCheckContext{EnvValue: "yes", ConfigValue: "notify", Interactive: true}, + want: update.CheckModeNotify, + wantWarnings: []string{`> Warning: Ignoring LSTK_UPDATE_CHECK: invalid update_check value "yes" (must be one of: prompt, notify, off)`}, + }, + { + name: "invalid values in both sources warn and fall through to the default", + checkCtx: updateCheckContext{EnvValue: "yes", ConfigValue: "disabled", ExternallyManaged: true, Interactive: true}, + want: update.CheckModeNotify, + wantWarnings: []string{ + `> Warning: Ignoring LSTK_UPDATE_CHECK: invalid update_check value "yes" (must be one of: prompt, notify, off)`, + `> Warning: Ignoring update_check in [cli]: invalid update_check value "disabled" (must be one of: prompt, notify, off)`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + got := resolveUpdateCheckMode(output.NewPlainSink(&buf), tt.checkCtx) + + assert.Equal(t, tt.want, got) + for _, warning := range tt.wantWarnings { + assert.Contains(t, buf.String(), warning) + } + if len(tt.wantWarnings) == 0 { + assert.Empty(t, buf.String(), "a valid configuration should print nothing") + } + }) + } +} diff --git a/docs/structured-output.md b/docs/structured-output.md index 4030305c..6d69a715 100644 --- a/docs/structured-output.md +++ b/docs/structured-output.md @@ -79,7 +79,7 @@ Within the main payload, the `error` field contains the following sub-fields: | Field | Type | Notes | |---|---|---| | `code` | string | One of the enumerated codes below. Never free text — see the error-codes table. The primary, stable identifier — branch on this for anything specific. | -| `category` | string | One of 7 coarse groupings of `code` (`RUNTIME`, `EMULATOR`, `AUTH`, `RESOURCE`, `CONFIG`, `USAGE`, `INTERNAL`) — see [Error categories](#error-categories) below. Additive alongside `code`, not a replacement for it: a caller that only wants broad handling can switch on `category`'s ~7 values instead of `code`'s ~28, while a caller that already keys off a specific `code` is unaffected. | +| `category` | string | One of 7 coarse groupings of `code` (`RUNTIME`, `EMULATOR`, `AUTH`, `RESOURCE`, `CONFIG`, `USAGE`, `INTERNAL`) — see [Error categories](#error-categories) below. Additive alongside `code`, not a replacement for it: a caller that only wants broad handling can switch on `category`'s ~7 values instead of `code`'s ~29, while a caller that already keys off a specific `code` is unaffected. | | `message` | string | Human-readable headline, informational only. **Not guaranteed stable across versions** — scripts must branch on `code`, not `message`. | | `retryable` | bool | A static property of `code` (not computed per failure) — see below. Note this is independent of `category`: a category can contain both retryable and non-retryable codes (e.g. `RUNTIME` contains both `NETWORK_ERROR` [retryable] and `DNS_RESOLUTION_REQUIRED` [not]), so `retryable` can't be inferred from `category` alone. | | `details` | object | Optional, code-specific structured context. Omitted when empty. Illustrative example, for a future `SNAPSHOT_BUCKET_NOT_FOUND`: `{"bucket": "my-terraform-state"}`. Also where the additional diagnostic depth plain text and the TUI show alongside the `message` headline lands, as `summary`/`detail` string keys, when available — e.g. `{"summary": "cannot connect to Docker daemon: ..."}` for `RUNTIME_UNAVAILABLE`. | @@ -118,6 +118,7 @@ Every `error.code` is one of the following fixed constants. A failure that doesn | `VALIDATION_ERROR` | A semantically invalid combination of flags/arguments was given | No | `USAGE` | | `USAGE_ERROR` | Cobra-level flag or argument parsing failed | No | `USAGE` | | `NOT_JSON_CAPABLE` | The requested command has not been annotated as JSON-capable yet | No | `USAGE` | +| `UPDATE_EXTERNALLY_MANAGED` | Another package manager owns the lstk binary, so lstk will not replace it | No | `USAGE` | | `NETWORK_ERROR` | An unclassified network/transport failure occurred | Yes | `RUNTIME` | | `CANCELLED` | The operation was interrupted (e.g. context cancellation via Ctrl+C) | Yes | `INTERNAL` | | `INTERNAL_ERROR` | Unclassified or unexpected failure; the universal fallback | No | `INTERNAL` | @@ -149,7 +150,7 @@ CONFIG CONFIG_INVALID, CONFIG_NOT_FOUND, INTEGRATION_NOT_SET_UP → lstk's own configuration is the problem USAGE CONFIRMATION_REQUIRED, VALIDATION_ERROR, USAGE_ERROR, - NOT_JSON_CAPABLE + NOT_JSON_CAPABLE, UPDATE_EXTERNALLY_MANAGED → the invocation itself needs to change INTERNAL CANCELLED, INTERNAL_ERROR @@ -256,7 +257,7 @@ Codes: `EMULATOR_NOT_CONFIGURED` (no AWS container configured), `EMULATOR_NOT_RU "error": null } ``` -Codes: `NETWORK_ERROR` (GitHub API unreachable), `INTERNAL_ERROR` (archive download verification, extraction, or replacement failure), `CONFIG_INVALID`, `CONFIG_NOT_FOUND` (bad or missing `--config` path). +Codes: `UPDATE_EXTERNALLY_MANAGED` (another package manager owns the binary — refused before any network request; `--check` is unaffected), `NETWORK_ERROR` (GitHub API unreachable), `INTERNAL_ERROR` (archive download verification, extraction, or replacement failure), `CONFIG_INVALID`, `CONFIG_NOT_FOUND` (bad or missing `--config` path). ### Proposed for future work (draft) diff --git a/internal/config/config.go b/internal/config/config.go index e95040b2..99d82f2f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -19,6 +19,10 @@ var defaultConfigTemplate string type CLIConfig struct { UpdateSkippedVersion string `mapstructure:"update_skipped_version"` + // UpdateCheck is the raw [cli] update_check value, left unvalidated on + // purpose: Get() is called by every command, so rejecting a typo here would + // make one unusable setting break the whole CLI. Parsed at the boundary. + UpdateCheck string `mapstructure:"update_check"` } type Config struct { @@ -37,6 +41,13 @@ func setDefaults() { }) } +// loadConfig reads the config file at path into the shared viper instance. +// +// The Reset discards the LSTK_* env binding env.Init() installed, so viper's own +// env-over-config precedence is unavailable: a setting an env var should override +// resolves both sources explicitly at the command boundary (see +// resolveUpdateCheckMode). Do not "fix" this with AutomaticEnv() here — that +// would create a second, competing precedence path. func loadConfig(path string) error { viper.Reset() setDefaults() @@ -175,6 +186,13 @@ func SetUpdateSkippedVersion(version string) error { return Set("cli.update_skipped_version", version) } +// SetUpdateCheck persists the update-check policy, preserving comments and +// formatting (see setInFile). It takes a string rather than update.CheckMode so +// this package need not import the update domain. +func SetUpdateCheck(mode string) error { + return Set("cli.update_check", mode) +} + func Get() (*Config, error) { var cfg Config if err := viper.Unmarshal(&cfg); err != nil { diff --git a/internal/config/default_config.toml b/internal/config/default_config.toml index 4c07d723..971d0ff5 100644 --- a/internal/config/default_config.toml +++ b/internal/config/default_config.toml @@ -30,6 +30,16 @@ port = "4566" # Host port the emulator will be accessible on # # volumes = ["./test.sf.sql:/etc/localstack/init/ready.d/test.sf.sql"] # snapshot = "pod:my-baseline" # Snapshot REF auto-loaded on start (AWS only); skip once with 'lstk start --no-snapshot' +# CLI behaviour, independent of any emulator. +# +# [cli] +# update_check = "prompt" # Automatic update check on 'lstk start': "prompt" asks +# # (interactive terminals only), "notify" prints one line and continues, +# # "off" disables it entirely. Overridden by LSTK_UPDATE_CHECK. +# # 'lstk update' always checks when you run it yourself. +# # Installs owned by mise, asdf, Nix, Scoop or Chocolatey default to +# # "notify" and are left to that manager's own upgrade command. + # Environment profiles let you group environment variables and reference # them by name in one or more containers via the 'env' field above. # diff --git a/internal/env/env.go b/internal/env/env.go index 71534ea1..e808f524 100644 --- a/internal/env/env.go +++ b/internal/env/env.go @@ -25,6 +25,7 @@ type Env struct { JSON bool GitHubToken string MergeStrategy string + UpdateCheck string } // Init initializes environment variable configuration and returns the result. @@ -51,6 +52,7 @@ func Init() *Env { AnalyticsEndpoint: viper.GetString("analytics_endpoint"), GitHubToken: viper.GetString("github_token"), MergeStrategy: viper.GetString("merge_strategy"), + UpdateCheck: viper.GetString("update_check"), } } diff --git a/internal/output/error_code.go b/internal/output/error_code.go index ee10d7cc..f981d4ff 100644 --- a/internal/output/error_code.go +++ b/internal/output/error_code.go @@ -31,9 +31,12 @@ const ( ErrValidationError ErrorCode = "VALIDATION_ERROR" ErrUsageError ErrorCode = "USAGE_ERROR" ErrNotJSONCapable ErrorCode = "NOT_JSON_CAPABLE" - ErrNetworkError ErrorCode = "NETWORK_ERROR" - ErrCancelled ErrorCode = "CANCELLED" - ErrInternal ErrorCode = "INTERNAL_ERROR" + // Another package manager owns lstk's binary. Usage, not runtime: the fix is + // that manager's own upgrade command, not a retry. + ErrUpdateExternallyManaged ErrorCode = "UPDATE_EXTERNALLY_MANAGED" + ErrNetworkError ErrorCode = "NETWORK_ERROR" + ErrCancelled ErrorCode = "CANCELLED" + ErrInternal ErrorCode = "INTERNAL_ERROR" ) // retryableCodes is the single source of truth for whether a given ErrorCode @@ -115,6 +118,7 @@ var allErrorCodes = []ErrorCode{ ErrValidationError, ErrUsageError, ErrNotJSONCapable, + ErrUpdateExternallyManaged, ErrNetworkError, ErrCancelled, ErrInternal, @@ -124,34 +128,35 @@ var allErrorCodes = []ErrorCode{ // static ErrorCategory, mirroring retryableCodes above. Every code in // allErrorCodes SHALL have an entry here. var categoryByCode = map[ErrorCode]ErrorCategory{ - ErrRuntimeUnavailable: CategoryRuntime, - ErrImagePullFailed: CategoryRuntime, - ErrDependencyMissing: CategoryRuntime, - ErrDNSResolutionRequired: CategoryRuntime, - ErrNetworkError: CategoryRuntime, - ErrEmulatorNotRunning: CategoryEmulator, - ErrEmulatorAlreadyRunning: CategoryEmulator, - ErrEmulatorWrongType: CategoryEmulator, - ErrEmulatorNotConfigured: CategoryEmulator, - ErrEmulatorStartFailed: CategoryEmulator, - ErrAuthRequired: CategoryAuth, - ErrAuthLoginFailed: CategoryAuth, - ErrCredentialsMissing: CategoryAuth, - ErrLicenseInvalid: CategoryAuth, - ErrLicenseUnsupportedTag: CategoryAuth, - ErrSnapshotNotFound: CategoryResource, - ErrSnapshotInvalidRef: CategoryResource, - ErrSnapshotRemoteError: CategoryResource, - ErrSnapshotBucketNotFound: CategoryResource, - ErrConfigInvalid: CategoryConfig, - ErrConfigNotFound: CategoryConfig, - ErrIntegrationNotSetUp: CategoryConfig, - ErrConfirmationRequired: CategoryUsage, - ErrValidationError: CategoryUsage, - ErrUsageError: CategoryUsage, - ErrNotJSONCapable: CategoryUsage, - ErrCancelled: CategoryInternal, - ErrInternal: CategoryInternal, + ErrRuntimeUnavailable: CategoryRuntime, + ErrImagePullFailed: CategoryRuntime, + ErrDependencyMissing: CategoryRuntime, + ErrDNSResolutionRequired: CategoryRuntime, + ErrNetworkError: CategoryRuntime, + ErrEmulatorNotRunning: CategoryEmulator, + ErrEmulatorAlreadyRunning: CategoryEmulator, + ErrEmulatorWrongType: CategoryEmulator, + ErrEmulatorNotConfigured: CategoryEmulator, + ErrEmulatorStartFailed: CategoryEmulator, + ErrAuthRequired: CategoryAuth, + ErrAuthLoginFailed: CategoryAuth, + ErrCredentialsMissing: CategoryAuth, + ErrLicenseInvalid: CategoryAuth, + ErrLicenseUnsupportedTag: CategoryAuth, + ErrSnapshotNotFound: CategoryResource, + ErrSnapshotInvalidRef: CategoryResource, + ErrSnapshotRemoteError: CategoryResource, + ErrSnapshotBucketNotFound: CategoryResource, + ErrConfigInvalid: CategoryConfig, + ErrConfigNotFound: CategoryConfig, + ErrIntegrationNotSetUp: CategoryConfig, + ErrConfirmationRequired: CategoryUsage, + ErrValidationError: CategoryUsage, + ErrUsageError: CategoryUsage, + ErrNotJSONCapable: CategoryUsage, + ErrUpdateExternallyManaged: CategoryUsage, + ErrCancelled: CategoryInternal, + ErrInternal: CategoryInternal, } // Category reports the code's static, coarse grouping. Every ErrorCode in diff --git a/internal/output/error_code_test.go b/internal/output/error_code_test.go index 0a0fd0ec..4e47deeb 100644 --- a/internal/output/error_code_test.go +++ b/internal/output/error_code_test.go @@ -31,8 +31,8 @@ func TestErrorCode_AllErrorCodesIsComplete(t *testing.T) { t.Errorf("ErrorCode %q appears %d times in allErrorCodes, want exactly once", code, count) } } - if len(allErrorCodes) != 28 { - t.Errorf("expected 28 documented error codes, got %d — update this test's expectation alongside error-codes/spec.md if a code was intentionally added or removed", len(allErrorCodes)) + if len(allErrorCodes) != 29 { + t.Errorf("expected 29 documented error codes, got %d — update this test's expectation alongside error-codes/spec.md if a code was intentionally added or removed", len(allErrorCodes)) } } diff --git a/internal/update/check_mode.go b/internal/update/check_mode.go new file mode 100644 index 00000000..f6f4c8fb --- /dev/null +++ b/internal/update/check_mode.go @@ -0,0 +1,30 @@ +package update + +import ( + "fmt" + "strings" +) + +// CheckMode is the policy for the automatic update check on the start path. It +// does not affect an explicit `lstk update`, which always checks. +type CheckMode string + +const ( + CheckModePrompt CheckMode = "prompt" // ask, blocking on the answer (default) + CheckModeNotify CheckMode = "notify" // one-line note, no waiting for input + CheckModeOff CheckMode = "off" // no check: no request, no output +) + +// ParseCheckMode validates a raw update_check value from config or +// LSTK_UPDATE_CHECK. Unlike config.ParseEmulatorType it trims and lowercases +// first: the value is hand-typed into a shell or CI env file, and three closed +// values leave no ambiguity. An empty string is invalid — callers treat "unset" +// as "no opinion" before calling here. +func ParseCheckMode(s string) (CheckMode, error) { + switch mode := CheckMode(strings.ToLower(strings.TrimSpace(s))); mode { + case CheckModePrompt, CheckModeNotify, CheckModeOff: + return mode, nil + default: + return "", fmt.Errorf("invalid update_check value %q (must be one of: %s, %s, %s)", s, CheckModePrompt, CheckModeNotify, CheckModeOff) + } +} diff --git a/internal/update/check_mode_test.go b/internal/update/check_mode_test.go new file mode 100644 index 00000000..495baf72 --- /dev/null +++ b/internal/update/check_mode_test.go @@ -0,0 +1,56 @@ +package update + +import "testing" + +func TestParseCheckMode(t *testing.T) { + t.Parallel() + + valid := []struct { + in string + want CheckMode + }{ + {"prompt", CheckModePrompt}, + {"notify", CheckModeNotify}, + {"off", CheckModeOff}, + // Hand-typed into a shell or CI env file, so case and space are tolerated. + {"OFF", CheckModeOff}, + {" off ", CheckModeOff}, + {"Notify", CheckModeNotify}, + } + for _, tt := range valid { + t.Run("valid/"+tt.in, func(t *testing.T) { + t.Parallel() + got, err := ParseCheckMode(tt.in) + if err != nil { + t.Fatalf("ParseCheckMode(%q) returned error: %v", tt.in, err) + } + if got != tt.want { + t.Fatalf("ParseCheckMode(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } + + invalid := []string{"", "yes", "1", "true", "false", "disabled", "pro mpt"} + for _, in := range invalid { + t.Run("invalid/"+in, func(t *testing.T) { + t.Parallel() + _, err := ParseCheckMode(in) + if err == nil { + t.Fatalf("ParseCheckMode(%q) succeeded, want an error", in) + } + }) + } +} + +func TestParseCheckModeErrorMessage(t *testing.T) { + t.Parallel() + + _, err := ParseCheckMode("yes") + if err == nil { + t.Fatal("expected an error") + } + const want = `invalid update_check value "yes" (must be one of: prompt, notify, off)` + if err.Error() != want { + t.Fatalf("error = %q, want %q", err.Error(), want) + } +} diff --git a/internal/update/install_method.go b/internal/update/install_method.go index f882411e..b4612b19 100644 --- a/internal/update/install_method.go +++ b/internal/update/install_method.go @@ -3,6 +3,7 @@ package update import ( "os" "path/filepath" + "slices" "strings" ) @@ -12,6 +13,7 @@ const ( InstallBinary InstallMethod = iota // standalone binary download InstallHomebrew // installed via Homebrew cask InstallNPM // installed via npm + InstallExternal // owned by a third-party package or version manager ) func (m InstallMethod) String() string { @@ -20,17 +22,79 @@ func (m InstallMethod) String() string { return "homebrew" case InstallNPM: return "npm" + case InstallExternal: + return "external" default: return "binary" } } +// ExternalManager identifies the package manager that owns the lstk binary, +// empty unless Method is InstallExternal. +// +// lstk must not update these itself: replacing the binary would leave the +// manager's registry pointing at a version it no longer installed, or fail +// outright against Nix's read-only store. Homebrew and npm are absent because +// lstk drives those through `brew upgrade` / `npm install -g`. +type ExternalManager string + +const ( + ManagerNix ExternalManager = "nix" + ManagerMise ExternalManager = "mise" + ManagerASDF ExternalManager = "asdf" + ManagerScoop ExternalManager = "scoop" + ManagerChocolatey ExternalManager = "chocolatey" +) + +// externalManagers holds the user-facing facts per manager, one row each. +// +// upgradeCommand is empty for Nix and asdf on purpose: a Nix install may be a +// profile, a nixos-rebuild generation or home-manager, and asdf has no `upgrade` +// verb. Printing a command that fails is worse than naming the manager. +var externalManagers = map[ExternalManager]struct { + displayName string + upgradeCommand string +}{ + ManagerNix: {"Nix", ""}, + ManagerMise: {"mise", "mise upgrade lstk"}, + ManagerASDF: {"asdf", ""}, + ManagerScoop: {"Scoop", "scoop update lstk"}, + ManagerChocolatey: {"Chocolatey", "choco upgrade lstk"}, +} + +// DisplayName is the manager's name as its own project capitalizes it. +func (m ExternalManager) DisplayName() string { + if entry, ok := externalManagers[m]; ok { + return entry.displayName + } + return string(m) +} + +func (m ExternalManager) UpgradeCommand() string { + return externalManagers[m].upgradeCommand +} + +// UpgradeAdvice is a clause for use inside a sentence: "run mise upgrade lstk", +// or "update it with Nix" when no single command applies. +func (m ExternalManager) UpgradeAdvice() string { + if cmd := m.UpgradeCommand(); cmd != "" { + return "run " + cmd + } + return "update it with " + m.DisplayName() +} + // InstallInfo holds the detected install method and the resolved binary path. type InstallInfo struct { Method InstallMethod + Manager ExternalManager // empty unless Method is InstallExternal ResolvedPath string } +// ExternallyManaged means lstk must never replace this binary. +func (i InstallInfo) ExternallyManaged() bool { + return i.Method == InstallExternal +} + // DetectInstallMethod determines how lstk was installed by inspecting the // resolved path of the running binary. func DetectInstallMethod() InstallInfo { @@ -42,25 +106,74 @@ func DetectInstallMethod() InstallInfo { if err != nil { resolved = exe } - return InstallInfo{ - Method: classifyPath(resolved), - ResolvedPath: resolved, - } + return classifyPath(resolved) } -func classifyPath(resolved string) InstallMethod { - cleaned := filepath.Clean(resolved) - segments := strings.Split(cleaned, string(os.PathSeparator)) +type externalManagerMarker struct { + manager ExternalManager + // followedBy are the segments that may come next. Empty means the segment + // name alone is conclusive. + followedBy []string +} + +// externalManagerBySegment maps a lowercased path segment to its manager. +// +// Most entries require a following segment: a bare "mise" or "scoop" directory +// is more likely a checkout of that tool than an lstk it installed, and a false +// positive means `lstk update` refuses and advises a command the user cannot +// run. The dot-prefixed names are specific enough alone. +var externalManagerBySegment = map[string]externalManagerMarker{ + "mise": {ManagerMise, []string{"installs", "shims", "tools"}}, + "asdf": {ManagerASDF, []string{"installs", "shims"}}, + "scoop": {ManagerScoop, []string{"apps", "shims"}}, + "chocolatey": {ManagerChocolatey, []string{"lib", "bin"}}, + "nix": {ManagerNix, []string{"store"}}, + ".asdf": {ManagerASDF, nil}, + ".nix-profile": {ManagerNix, nil}, +} + +// classifyPath derives the install method from the resolved executable path. +// +// The loop order is the contract: lstk's own install methods match before any +// manager segment, because an npm-installed lstk under a mise-managed Node.js is +// still an npm install and must keep updating through npm. +// +// Detection is path-only by design. A write-permission probe cannot tell a +// root-owned /usr/local (self-managed, needs sudo) from a manager-owned +// directory, and the two need different advice. Nix profiles resolve through +// EvalSymlinks into /nix/store, so matching the store covers them. +func classifyPath(resolved string) InstallInfo { + // Every marker is lowercase; ResolvedPath keeps the original casing. + segments := splitPathSegments(strings.ToLower(resolved)) for _, seg := range segments { - lower := strings.ToLower(seg) - if lower == "caskroom" { - return InstallHomebrew + switch seg { + case "caskroom": + return InstallInfo{Method: InstallHomebrew, ResolvedPath: resolved} + case "node_modules": + return InstallInfo{Method: InstallNPM, ResolvedPath: resolved} + } + } + + for i, seg := range segments { + marker, ok := externalManagerBySegment[seg] + if !ok { + continue } - if lower == "node_modules" { - return InstallNPM + if len(marker.followedBy) > 0 && (i+1 >= len(segments) || !slices.Contains(marker.followedBy, segments[i+1])) { + continue } + return InstallInfo{Method: InstallExternal, Manager: marker.manager, ResolvedPath: resolved} } - return InstallBinary + return InstallInfo{Method: InstallBinary, ResolvedPath: resolved} +} + +// splitPathSegments splits on both separators regardless of host OS, so a +// Windows path classifies (and tests) on Linux and vice versa. A Unix filename +// containing a backslash splits too; harmless, since no marker can result. +func splitPathSegments(path string) []string { + return strings.FieldsFunc(filepath.Clean(path), func(r rune) bool { + return r == '/' || r == '\\' + }) } diff --git a/internal/update/install_method_test.go b/internal/update/install_method_test.go index ca2087eb..09007e41 100644 --- a/internal/update/install_method_test.go +++ b/internal/update/install_method_test.go @@ -8,49 +8,122 @@ func TestClassifyPath(t *testing.T) { t.Parallel() tests := []struct { - name string - path string - want InstallMethod + name string + path string + wantMethod InstallMethod + wantManager ExternalManager }{ { - name: "homebrew cask on apple silicon", - path: "/opt/homebrew/Caskroom/lstk/0.3.0/lstk", - want: InstallHomebrew, + name: "homebrew cask on apple silicon", + path: "/opt/homebrew/Caskroom/lstk/0.3.0/lstk", + wantMethod: InstallHomebrew, }, { - name: "homebrew cask on intel mac", - path: "/usr/local/Caskroom/lstk/0.3.0/lstk", - want: InstallHomebrew, + name: "homebrew cask on intel mac", + path: "/usr/local/Caskroom/lstk/0.3.0/lstk", + wantMethod: InstallHomebrew, }, { - name: "npm global install", - path: "/Users/someone/.local/share/mise/installs/node/24.8.0/lib/node_modules/@localstack/lstk_darwin_arm64/lstk", - want: InstallNPM, + // An npm lstk under a mise-managed *node* is still an npm install and + // must keep updating through npm (see 273738e). + name: "npm global install under mise-managed node", + path: "/Users/someone/.local/share/mise/installs/node/24.8.0/lib/node_modules/@localstack/lstk_darwin_arm64/lstk", + wantMethod: InstallNPM, }, { - name: "npm global install default prefix", - path: "/usr/local/lib/node_modules/@localstack/lstk_darwin_amd64/lstk", - want: InstallNPM, + name: "npm global install default prefix", + path: "/usr/local/lib/node_modules/@localstack/lstk_darwin_amd64/lstk", + wantMethod: InstallNPM, }, { - name: "npm global install via asdf", - path: "/Users/geo/.asdf/installs/nodejs/22.12.0/lib/node_modules/@localstack/lstk_darwin_arm64/lstk", - want: InstallNPM, + name: "npm global install via asdf", + path: "/Users/someone/.asdf/installs/nodejs/22.12.0/lib/node_modules/@localstack/lstk_darwin_arm64/lstk", + wantMethod: InstallNPM, }, { - name: "standalone binary in usr local bin", - path: "/usr/local/bin/lstk", - want: InstallBinary, + name: "npm global install on windows", + path: `C:\Users\me\AppData\Roaming\npm\node_modules\@localstack\lstk_windows_amd64\lstk.exe`, + wantMethod: InstallNPM, }, { - name: "standalone binary in home dir", - path: "/home/user/bin/lstk", - want: InstallBinary, + name: "nix store", + path: "/nix/store/9k1qz3lstk-lstk-1.2.3/bin/lstk", + wantMethod: InstallExternal, + wantManager: ManagerNix, }, { - name: "dev build", - path: "/home/user/Projects/lstk/bin/lstk", - want: InstallBinary, + name: "nix profile", + path: "/Users/me/.nix-profile/bin/lstk", + wantMethod: InstallExternal, + wantManager: ManagerNix, + }, + // A manager's name alone is not rare enough: a checkout of the tool itself + // would otherwise get a refusal advising a command that does not apply. + { + name: "directory merely named nix is not a nix install", + path: "/home/user/projects/nix/bin/lstk", + wantMethod: InstallBinary, + }, + { + name: "directory merely named mise is not a mise install", + path: "/home/user/projects/mise/target/release/lstk", + wantMethod: InstallBinary, + }, + { + name: "directory merely named scoop is not a scoop install", + path: "/home/user/projects/scoop/bin/lstk", + wantMethod: InstallBinary, + }, + { + name: "mise shim", + path: "/Users/me/.local/share/mise/shims/lstk", + wantMethod: InstallExternal, + wantManager: ManagerMise, + }, + { + name: "scoop shim", + path: `C:\Users\me\scoop\shims\lstk.exe`, + wantMethod: InstallExternal, + wantManager: ManagerScoop, + }, + { + name: "mise managed lstk", + path: "/Users/me/.local/share/mise/installs/lstk/1.2.3/lstk", + wantMethod: InstallExternal, + wantManager: ManagerMise, + }, + { + name: "asdf managed lstk", + path: "/Users/me/.asdf/installs/lstk/1.2.3/bin/lstk", + wantMethod: InstallExternal, + wantManager: ManagerASDF, + }, + { + name: "scoop managed lstk", + path: `C:\Users\me\scoop\apps\lstk\current\lstk.exe`, + wantMethod: InstallExternal, + wantManager: ManagerScoop, + }, + { + name: "chocolatey managed lstk", + path: `C:\ProgramData\chocolatey\lib\lstk\tools\lstk.exe`, + wantMethod: InstallExternal, + wantManager: ManagerChocolatey, + }, + { + name: "standalone binary in usr local bin", + path: "/usr/local/bin/lstk", + wantMethod: InstallBinary, + }, + { + name: "standalone binary in home dir", + path: "/home/user/bin/lstk", + wantMethod: InstallBinary, + }, + { + name: "dev build", + path: "/home/user/Projects/lstk/bin/lstk", + wantMethod: InstallBinary, }, } @@ -58,9 +131,74 @@ func TestClassifyPath(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() got := classifyPath(tt.path) - if got != tt.want { - t.Fatalf("classifyPath(%q) = %v, want %v", tt.path, got, tt.want) + if got.Method != tt.wantMethod { + t.Fatalf("classifyPath(%q).Method = %v, want %v", tt.path, got.Method, tt.wantMethod) + } + if got.Manager != tt.wantManager { + t.Fatalf("classifyPath(%q).Manager = %q, want %q", tt.path, got.Manager, tt.wantManager) + } + if got.ResolvedPath != tt.path { + t.Fatalf("classifyPath(%q).ResolvedPath = %q, want the input path", tt.path, got.ResolvedPath) + } + if got.ExternallyManaged() != (tt.wantMethod == InstallExternal) { + t.Fatalf("classifyPath(%q).ExternallyManaged() = %v, want %v", tt.path, got.ExternallyManaged(), tt.wantMethod == InstallExternal) + } + }) + } +} + +// TestExternalManagerHints pins wording printed verbatim in the notify line and +// the `lstk update` refusal, so a change here changes the CLI's output. +func TestExternalManagerHints(t *testing.T) { + t.Parallel() + + tests := []struct { + manager ExternalManager + displayName string + upgradeCommand string + upgradeAdvice string + }{ + {ManagerMise, "mise", "mise upgrade lstk", "run mise upgrade lstk"}, + {ManagerScoop, "Scoop", "scoop update lstk", "run scoop update lstk"}, + {ManagerChocolatey, "Chocolatey", "choco upgrade lstk", "run choco upgrade lstk"}, + // Nix splits across profile / nixos-rebuild / home-manager and asdf has no + // upgrade verb, so neither names a command. + {ManagerNix, "Nix", "", "update it with Nix"}, + {ManagerASDF, "asdf", "", "update it with asdf"}, + } + + for _, tt := range tests { + t.Run(string(tt.manager), func(t *testing.T) { + t.Parallel() + if got := tt.manager.DisplayName(); got != tt.displayName { + t.Errorf("DisplayName() = %q, want %q", got, tt.displayName) + } + if got := tt.manager.UpgradeCommand(); got != tt.upgradeCommand { + t.Errorf("UpgradeCommand() = %q, want %q", got, tt.upgradeCommand) + } + if got := tt.manager.UpgradeAdvice(); got != tt.upgradeAdvice { + t.Errorf("UpgradeAdvice() = %q, want %q", got, tt.upgradeAdvice) } }) } } + +func TestInstallMethodString(t *testing.T) { + t.Parallel() + + tests := []struct { + method InstallMethod + want string + }{ + {InstallBinary, "binary"}, + {InstallHomebrew, "homebrew"}, + {InstallNPM, "npm"}, + {InstallExternal, "external"}, + } + + for _, tt := range tests { + if got := tt.method.String(); got != tt.want { + t.Errorf("InstallMethod(%d).String() = %q, want %q", tt.method, got, tt.want) + } + } +} diff --git a/internal/update/notify.go b/internal/update/notify.go index 244420df..507d371a 100644 --- a/internal/update/notify.go +++ b/internal/update/notify.go @@ -12,18 +12,31 @@ import ( type versionFetcher func(ctx context.Context, token string) (string, error) type NotifyOptions struct { - GitHubToken string - UpdatePrompt bool + // Mode is resolved at the command boundary (see cmd/update_check.go) rather + // than read from config here, keeping this package independent of Viper. + Mode CheckMode + GitHubToken string + SkippedVersion string PersistSkipVersion func(version string) error -} -const checkTimeout = 2 * time.Second + // Install is detected once at the command boundary, so the notify wording and + // any update applied from the prompt agree on who owns the binary. + Install InstallInfo -func CheckQuietly(ctx context.Context, githubToken string) (current, latest string, available bool) { - return checkQuietlyWithVersion(ctx, githubToken, version.Version(), fetchLatestVersion) + // ConfigPath is the file the "Don't ask again" choice writes to, named in its + // confirmation. + ConfigPath string + + // PersistCheckMode stores the policy chosen through the prompt. Nil hides the + // "Don't ask again" option: on a first run there is no config file yet, and + // creating one here would suppress the emulator picker, so the option is + // withheld rather than offered as a silent no-op. + PersistCheckMode func(mode CheckMode) error } +const checkTimeout = 2 * time.Second + func checkQuietlyWithVersion(ctx context.Context, githubToken string, currentVersion string, fetch versionFetcher) (current, latest string, available bool) { current = currentVersion // Skip update check for dev builds @@ -51,35 +64,57 @@ func NotifyUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions) (ex } func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyOptions, currentVersion string, fetch versionFetcher) (exitAfter bool) { + // Before anything else, so "off" costs no network request, not just no output. + if opts.Mode == CheckModeOff { + return false + } + current, latest, available := checkQuietlyWithVersion(ctx, opts.GitHubToken, currentVersion, fetch) if !available { return false } + // Before the mode branch, so a version skipped while prompting stays + // suppressed after switching to notify. if opts.SkippedVersion != "" && normalizeVersion(opts.SkippedVersion) == normalizeVersion(latest) { return false } - if !opts.UpdatePrompt { - sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: fmt.Sprintf("Update available: %s → %s (run lstk update)", current, latest)}) + // Anything but an explicit prompt notifies: the fallthrough must be the + // non-blocking branch so a zero-value Mode never waits on input. + if opts.Mode != CheckModePrompt { + sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: notifyLine(current, latest, opts.Install.Manager)}) return false } return promptAndUpdate(ctx, sink, opts, current, latest) } +// Manager-owned installs point at the manager, since `lstk update` refuses. +func notifyLine(current, latest string, manager ExternalManager) string { + if manager != "" { + return fmt.Sprintf("Update available: %s → %s (installed with %s — %s)", current, latest, manager.DisplayName(), manager.UpgradeAdvice()) + } + return fmt.Sprintf("Update available: %s → %s (run lstk update)", current, latest) +} + func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, current, latest string) (exitAfter bool) { releaseNotesURL := fmt.Sprintf("https://github.com/%s/releases/latest", githubRepo) sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: fmt.Sprintf("New lstk version available! %s → %s", current, latest)}) sink.Emit(output.MessageEvent{Severity: output.SeveritySecondary, Text: fmt.Sprintf("> Release notes: %s", releaseNotesURL)}) - responseCh := make(chan output.InputResponse, 1) - sink.Emit(output.ActionChoice("Update lstk to latest version?", []output.InputOption{ + options := []output.InputOption{ {Key: "u", Label: "Update now"}, {Key: "r", Label: "Remind me next time"}, {Key: "s", Label: "Skip this version"}, - }, responseCh)) + } + if opts.PersistCheckMode != nil { + options = append(options, output.InputOption{Key: "n", Label: "Don't ask again"}) + } + + responseCh := make(chan output.InputResponse, 1) + sink.Emit(output.ActionChoice("Update lstk to latest version?", options, responseCh)) var resp output.InputResponse select { @@ -94,7 +129,7 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, switch resp.SelectedKey { case "u": - if _, err := applyUpdate(ctx, sink, latest, opts.GitHubToken); err != nil { + if _, err := applyUpdate(ctx, sink, opts.Install, latest, opts.GitHubToken); err != nil { sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("Update failed: %v", err)}) return false } @@ -110,7 +145,32 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, } sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Skipping version " + latest}) return false + case "n": + persistCheckMode(sink, opts) + return false } return false } + +// persistCheckMode stores the "Don't ask again" choice. It saves notify, not off: +// the user asked not to be interrupted, not to stop hearing about releases. +func persistCheckMode(sink output.Sink, opts NotifyOptions) { + if err := opts.PersistCheckMode(CheckModeNotify); err != nil { + sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: fmt.Sprintf("Failed to save update check preference: %v", err)}) + return + } + + target := opts.ConfigPath + if target == "" { + target = "your lstk config file" + } + sink.Emit(output.MessageEvent{ + Severity: output.SeverityNote, + Text: fmt.Sprintf("Won't ask again — saved update_check = %q to %s", CheckModeNotify, target), + }) + sink.Emit(output.MessageEvent{ + Severity: output.SeveritySecondary, + Text: `> One-line update notes still appear; set update_check to "prompt" to be asked again, or "off" to disable the check`, + }) +} diff --git a/internal/update/notify_test.go b/internal/update/notify_test.go index 499b0916..c958f158 100644 --- a/internal/update/notify_test.go +++ b/internal/update/notify_test.go @@ -42,8 +42,18 @@ func testFetcher(serverURL string) versionFetcher { } } +// failingFetcher fails the test if called, so a caller can assert no version +// request was made rather than merely that nothing was printed. +func failingFetcher(t *testing.T) versionFetcher { + t.Helper() + return func(ctx context.Context, token string) (string, error) { + t.Error("version check performed a request when it should not have") + return "", nil + } +} + func TestCheckQuietlyDevBuild(t *testing.T) { - current, latest, available := CheckQuietly(context.Background(), "") + current, latest, available := checkQuietlyWithVersion(context.Background(), "", "dev", failingFetcher(t)) assert.Equal(t, "dev", current) assert.Empty(t, latest) assert.False(t, available) @@ -87,7 +97,7 @@ func TestNotifyUpdateNoUpdateAvailable(t *testing.T) { var events []output.Event sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "v1.0.0", testFetcher(server.URL)) + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModePrompt}, "v1.0.0", testFetcher(server.URL)) assert.False(t, exit) assert.Empty(t, events) } @@ -99,7 +109,7 @@ func TestNotifyUpdatePromptDisabled(t *testing.T) { var events []output.Event sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{}, "1.0.0", testFetcher(server.URL)) + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModeNotify}, "1.0.0", testFetcher(server.URL)) assert.False(t, exit) assert.Len(t, events, 1) msg, ok := events[0].(output.MessageEvent) @@ -122,7 +132,7 @@ func TestNotifyUpdatePromptSkip(t *testing.T) { }) exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ - UpdatePrompt: true, + Mode: CheckModePrompt, PersistSkipVersion: func(v string) error { skippedVersion = v return nil @@ -140,7 +150,7 @@ func TestNotifyUpdateSkippedVersionSuppressesPrompt(t *testing.T) { sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ - UpdatePrompt: true, + Mode: CheckModePrompt, SkippedVersion: "v2.0.0", }, "1.0.0", testFetcher(server.URL)) assert.False(t, exit) @@ -159,7 +169,7 @@ func TestNotifyUpdatePromptRemind(t *testing.T) { } }) - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModePrompt}, "1.0.0", testFetcher(server.URL)) assert.False(t, exit) } @@ -180,7 +190,142 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) { } }) - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModePrompt}, "1.0.0", testFetcher(server.URL)) + assert.False(t, exit) +} + +func TestNotifyUpdateOffMakesNoRequest(t *testing.T) { + var events []output.Event + sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) + + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModeOff}, "1.0.0", failingFetcher(t)) + assert.False(t, exit) + assert.Empty(t, events) +} + +// A zero-value Mode must never block: it falls back to the non-blocking note. +func TestNotifyUpdateZeroModeDoesNotPrompt(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + var events []output.Event + sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) + + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{}, "1.0.0", testFetcher(server.URL)) + assert.False(t, exit) + assert.Len(t, events, 1) + msg, ok := events[0].(output.MessageEvent) + assert.True(t, ok) + assert.Equal(t, "Update available: 1.0.0 → v2.0.0 (run lstk update)", msg.Text) +} + +func TestNotifyUpdateNotifyLineNamesExternalManager(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + tests := []struct { + manager ExternalManager + want string + }{ + {ManagerMise, "Update available: 1.0.0 → v2.0.0 (installed with mise — run mise upgrade lstk)"}, + {ManagerNix, "Update available: 1.0.0 → v2.0.0 (installed with Nix — update it with Nix)"}, + {ManagerScoop, "Update available: 1.0.0 → v2.0.0 (installed with Scoop — run scoop update lstk)"}, + } + + for _, tt := range tests { + t.Run(string(tt.manager), func(t *testing.T) { + var events []output.Event + sink := output.SinkFunc(func(event output.Event) { events = append(events, event) }) + + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ + Mode: CheckModeNotify, + Install: InstallInfo{Method: InstallExternal, Manager: tt.manager}, + }, "1.0.0", testFetcher(server.URL)) + assert.False(t, exit) + assert.Len(t, events, 1) + msg, ok := events[0].(output.MessageEvent) + assert.True(t, ok) + assert.Equal(t, tt.want, msg.Text) + }) + } +} + +// "Don't ask again" writes to the config file, so it is withheld when there is +// nothing to write to (a first run, where creating it would hide the picker). +func TestNotifyUpdateHidesDontAskAgainWithoutPersist(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + var options []output.InputOption + sink := output.SinkFunc(func(event output.Event) { + if req, ok := event.(output.UserInputRequestEvent); ok { + options = req.Options() + req.ResponseCh() <- output.InputResponse{SelectedKey: "r"} + } + }) + + notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{Mode: CheckModePrompt}, "1.0.0", testFetcher(server.URL)) + assert.Len(t, options, 3) + for _, opt := range options { + assert.NotEqual(t, "n", opt.Key) + } +} + +func TestNotifyUpdateDontAskAgainPersistsNotify(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + var persisted CheckMode + var events []output.Event + var options []output.InputOption + sink := output.SinkFunc(func(event output.Event) { + events = append(events, event) + if req, ok := event.(output.UserInputRequestEvent); ok { + options = req.Options() + req.ResponseCh() <- output.InputResponse{SelectedKey: "n"} + } + }) + + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ + Mode: CheckModePrompt, + ConfigPath: "/home/me/.config/lstk/config.toml", + PersistCheckMode: func(mode CheckMode) error { persisted = mode; return nil }, + }, "1.0.0", testFetcher(server.URL)) + assert.False(t, exit) + assert.Len(t, options, 4) + assert.Equal(t, "n", options[3].Key) + assert.Equal(t, "Don't ask again", options[3].Label) + assert.Equal(t, CheckModeNotify, persisted) + + var texts []string + for _, event := range events { + if msg, ok := event.(output.MessageEvent); ok { + texts = append(texts, msg.Text) + } + } + assert.Contains(t, texts, `Won't ask again — saved update_check = "notify" to /home/me/.config/lstk/config.toml`) } +func TestNotifyUpdateDontAskAgainPersistFailureWarns(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + var warnings []string + sink := output.SinkFunc(func(event output.Event) { + if msg, ok := event.(output.MessageEvent); ok && msg.Severity == output.SeverityWarning { + warnings = append(warnings, msg.Text) + } + if req, ok := event.(output.UserInputRequestEvent); ok { + req.ResponseCh() <- output.InputResponse{SelectedKey: "n"} + } + }) + + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ + Mode: CheckModePrompt, + PersistCheckMode: func(mode CheckMode) error { return fmt.Errorf("read-only file system") }, + }, "1.0.0", testFetcher(server.URL)) + + assert.False(t, exit) + assert.Contains(t, warnings, "Failed to save update check preference: read-only file system") +} diff --git a/internal/update/update.go b/internal/update/update.go index da399bdb..6ec85474 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -39,6 +39,16 @@ func Check(ctx context.Context, sink output.Sink, githubToken string) (string, b // Update checks for updates and applies the update if one is available. func Update(ctx context.Context, sink output.Sink, checkOnly bool, githubToken string) error { + info := DetectInstallMethod() + + // Refused before Check, so a binary lstk must not touch costs no request + // either. --check is read-only and stays allowed. + if !checkOnly { + if err := refuseExternalUpdate(sink, info); err != nil { + return err + } + } + current := version.Version() latest, available, err := Check(ctx, sink, githubToken) if err != nil { @@ -48,7 +58,7 @@ func Update(ctx context.Context, sink output.Sink, checkOnly bool, githubToken s return nil } - method, err := applyUpdate(ctx, sink, latest, githubToken) + method, err := applyUpdate(ctx, sink, info, latest, githubToken) if err != nil { sink.Emit(output.ErrorEvent{Title: err.Error(), Code: output.ErrInternal}) return output.NewSilentError(err) @@ -58,13 +68,54 @@ func Update(ctx context.Context, sink output.Sink, checkOnly bool, githubToken s return nil } -// applyUpdate detects the current install method and performs the update, -// returning its canonical name ("homebrew"/"npm"/"binary") on success. -func applyUpdate(ctx context.Context, sink output.Sink, latest, githubToken string) (string, error) { - info := DetectInstallMethod() +// refuseExternalUpdate reports that another package manager owns the binary and +// returns the silent error the command boundary propagates. Nil for installs +// lstk manages itself. +func refuseExternalUpdate(sink output.Sink, info InstallInfo) error { + if !info.ExternallyManaged() { + return nil + } + + manager := info.Manager.DisplayName() + err := externalInstallError(info) + + summary := fmt.Sprintf("%s owns this binary (%s); replacing it in place would leave %s out of sync.", manager, info.ResolvedPath, manager) + var actions []output.ErrorAction + if cmd := info.Manager.UpgradeCommand(); cmd != "" { + actions = append(actions, output.ErrorAction{Label: fmt.Sprintf("Update it with %s:", manager), Value: cmd}) + } else { + // No single correct command, so the advice goes in the summary rather than + // masquerading as something runnable. + summary += fmt.Sprintf(" Update it with %s instead.", manager) + } + actions = append(actions, output.ErrorAction{Label: "Or just check for a new version:", Value: "lstk update --check"}) + + sink.Emit(output.ErrorEvent{ + Title: err.Error(), + Summary: summary, + Actions: actions, + Code: output.ErrUpdateExternallyManaged, + }) + return output.NewSilentError(err) +} + +// externalInstallError is the shared sentence for both refusal paths, so they +// cannot describe the same situation differently. It carries no upgrade advice: +// refuseExternalUpdate offers that as an ErrorAction instead. +func externalInstallError(info InstallInfo) error { + return fmt.Errorf("lstk was installed with %s, so it cannot update itself", info.Manager.DisplayName()) +} +// applyUpdate returns the method's canonical name ("homebrew"/"npm"/"binary"). +func applyUpdate(ctx context.Context, sink output.Sink, info InstallInfo, latest, githubToken string) (string, error) { var err error switch info.Method { + case InstallExternal: + // Defense in depth: the prompt's "Update now" also routes here, so a user + // who forces update_check = "prompt" on a managed install must still never + // have their binary replaced. Renders as one warning line with no actions, + // hence the inline advice. + return "", fmt.Errorf("%w — %s to update it", externalInstallError(info), info.Manager.UpgradeAdvice()) case InstallHomebrew: sink.Emit(output.MessageEvent{Severity: output.SeverityNote, Text: "Installed through Homebrew, running brew upgrade"}) err = updateHomebrew(ctx, sink) diff --git a/test/integration/env/env.go b/test/integration/env/env.go index b5766988..7465e830 100644 --- a/test/integration/env/env.go +++ b/test/integration/env/env.go @@ -25,6 +25,9 @@ const ( Otel Key = "LSTK_OTEL" OtelEndpoint Key = "OTEL_EXPORTER_OTLP_ENDPOINT" StartupTimeout Key = "LSTK_STARTUP_TIMEOUT" + // UpdateCheck overrides the [cli] update_check policy: "prompt", "notify" or + // "off". + UpdateCheck Key = "LSTK_UPDATE_CHECK" // UpdateGitHubAPIEndpoint and UpdateGitHubDownloadEndpoint point the // updater's release-metadata API (api.github.com) and asset downloads // (github.com) at mock servers (undocumented, test-only). diff --git a/test/integration/main_test.go b/test/integration/main_test.go index f8b41d36..3fa1eeab 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -112,7 +112,9 @@ func TestMain(m *testing.M) { // TLS trust on non-Linux), so a full run on any one OS never visits the // other platforms' snapshots — Clean would flag them (or delete them under // UPDATE_SNAPS=true). Stale snapshots here are caught by review instead. - m.Run() + code := m.Run() + cleanupInstallPathBuilds() + os.Exit(code) } func requireDocker(t *testing.T) { diff --git a/test/integration/update_check_test.go b/test/integration/update_check_test.go new file mode 100644 index 00000000..525fd1cd --- /dev/null +++ b/test/integration/update_check_test.go @@ -0,0 +1,209 @@ +package integration_test + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/localstack/lstk/test/integration/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// updateCheckConfig carries distinctive comments, so a test that triggers a +// config write can prove the write preserved them. +const updateCheckConfig = `# User-maintained lstk config +[[containers]] +type = "aws" # Emulator type +tag = "latest" # Docker image tag +port = "4566" # Host port +` + +// writeUpdateCheckConfig appends extra lines (e.g. a [cli] section) and returns +// the file's path. +func writeUpdateCheckConfig(t *testing.T, extra string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(path, []byte(updateCheckConfig+extra), 0o644)) + return path +} + +func writeUpdateCheckConfigWithMode(t *testing.T, mode string) string { + t.Helper() + return writeUpdateCheckConfig(t, fmt.Sprintf("\n[cli]\nupdate_check = %q\n", mode)) +} + +func assertConfigCommentsPreserved(t *testing.T, configStr string) { + t.Helper() + assert.Contains(t, configStr, "# User-maintained lstk config", "file header comment should be preserved") + assert.Contains(t, configStr, "# Emulator type", "inline comments should be preserved") + assert.Contains(t, configStr, `port = "4566"`, "existing config values should be preserved") +} + +// updateCheckEnv returns the environment these tests run in, plus the mock's +// request counter: a mock GitHub advertising v0.0.2, and Docker unreachable so +// the run fails fast after the check instead of needing a daemon. +func updateCheckEnv(t *testing.T, extraEnv ...string) ([]string, *atomic.Int64) { + t.Helper() + srv, requests := mockGitHubReleaseServerCounting(t, "v0.0.2", nil) + environ := append(mockGitHubEnv(t, srv), unreachableDockerHost) + return append(environ, extraEnv...), requests +} + +func startUpdateCheckRun(t *testing.T, binPath, configFile string, extraEnv ...string) (*ptyProc, *atomic.Int64) { + t.Helper() + + environ, requests := updateCheckEnv(t, extraEnv...) + + // Long enough for the check and the Docker failure, short enough that a + // regression blocking on an unanswerable prompt fails rather than hangs. + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + t.Cleanup(cancel) + + cmd := exec.CommandContext(ctx, binPath, "--config", configFile) + cmd.Env = environ + return startCmdInPTY(t, ctx, cmd), requests +} + +func assertNoUpdateCheck(t *testing.T, out string, requests *atomic.Int64) { + t.Helper() + assert.NotContains(t, out, "Update available") + assert.NotContains(t, out, "New lstk version available") + assert.Zero(t, requests.Load(), "the check must not contact GitHub at all") +} + +// "off" must suppress the notice and the network request. +func TestUpdateCheckModeOff(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "bin") + configFile := writeUpdateCheckConfigWithMode(t, "off") + + p, requests := startUpdateCheckRun(t, binPath, configFile) + out, _ := p.wait() + + assertNoUpdateCheck(t, out, requests) +} + +// The middle ground the reporter asked for: a one-line hint that never waits for +// input. Reaching the Docker failure without a keypress proves it did not block. +func TestUpdateCheckModeNotify(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "bin") + configFile := writeUpdateCheckConfigWithMode(t, "notify") + + p, _ := startUpdateCheckRun(t, binPath, configFile) + out, _ := p.wait() + + assert.Contains(t, out, "Update available: 0.0.1 → v0.0.2 (run lstk update)") + assert.NotContains(t, out, "Update lstk to latest version?", "notify mode must not prompt") +} + +func TestUpdateCheckEnvOverridesConfig(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "bin") + + t.Run("env prompt beats config off", func(t *testing.T) { + t.Parallel() + configFile := writeUpdateCheckConfigWithMode(t, "off") + + p, _ := startUpdateCheckRun(t, binPath, configFile, string(env.UpdateCheck)+"=prompt") + p.waitForOutput("Update lstk to latest version?", "the env var should re-enable the prompt") + p.write("r") + _, _ = p.wait() + }) + + t.Run("env off beats config prompt", func(t *testing.T) { + t.Parallel() + configFile := writeUpdateCheckConfigWithMode(t, "prompt") + + p, requests := startUpdateCheckRun(t, binPath, configFile, string(env.UpdateCheck)+"=off") + out, _ := p.wait() + + assertNoUpdateCheck(t, out, requests) + }) +} + +func TestUpdateCheckInvalidValueWarnsAndStarts(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "bin") + configFile := writeUpdateCheckConfigWithMode(t, "yes") + + p, _ := startUpdateCheckRun(t, binPath, configFile) + p.waitForOutput(`Ignoring update_check in [cli]: invalid update_check value "yes" (must be one of: prompt, notify, off)`, + "an unparsable value should be reported, not silently ignored") + // Falls back to the default policy, so the prompt still has to be answered. + p.waitForOutput("New lstk version available", "an invalid value must not disable the check") + p.write("r") + _, _ = p.wait() +} + +// The reporter's setup: an lstk mise owns, nothing configured. Must not block, +// and must point at mise rather than `lstk update`, which refuses. +func TestUpdateNotifiesExternallyManagedInstallByDefault(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "mise", "installs", "lstk", "0.0.1") + configFile := writeUpdateCheckConfig(t, "") + + p, _ := startUpdateCheckRun(t, binPath, configFile) + out, _ := p.wait() + + assert.Contains(t, out, "Update available: 0.0.1 → v0.0.2 (installed with mise — run mise upgrade lstk)") + assert.NotContains(t, out, "Update lstk to latest version?", "an externally managed install must not be prompted") +} + +// The in-flow opt-out: turn the prompt off without reading docs, and have it +// take effect on the next run. +func TestUpdateCheckDontAskAgain(t *testing.T) { + t.Parallel() + + binPath := lstkAtInstallPath(t, testContext(t), "0.0.1", "bin") + configFile := writeUpdateCheckConfig(t, "") + + p, _ := startUpdateCheckRun(t, binPath, configFile) + p.waitForOutput("New lstk version available", "the prompt should appear with nothing configured") + require.Contains(t, p.output(), "Don't ask again", "the prompt should offer a durable opt-out") + p.write("n") + out, _ := p.wait() + + assert.Contains(t, out, "Won't ask again") + + configData, err := os.ReadFile(configFile) + require.NoError(t, err) + configStr := string(configData) + assert.Contains(t, configStr, "update_check", "the choice should be persisted") + assert.Contains(t, configStr, "notify", `the choice should persist "notify", not "off"`) + assertConfigCommentsPreserved(t, configStr) + + // The point of persisting: the next run notifies instead of asking. + second, _ := startUpdateCheckRun(t, binPath, configFile) + secondOut, _ := second.wait() + assert.Contains(t, secondOut, "Update available: 0.0.1 → v0.0.2 (run lstk update)") + assert.NotContains(t, secondOut, "Update lstk to latest version?") +} + +// A bug the shared-policy refactor fixes: the non-interactive path built its own +// NotifyOptions and so ignored a skipped version. +func TestUpdateNotificationHonorsSkippedVersionNonInteractive(t *testing.T) { + t.Parallel() + ctx := testContext(t) + + binPath := lstkAtInstallPath(t, ctx, "0.0.1", "bin") + configFile := writeUpdateCheckConfig(t, "\n[cli]\nupdate_skipped_version = \"v0.0.2\"\n") + + environ, _ := updateCheckEnv(t) + stdout, _, _ := runBinary(t, "", environ, binPath, "--config", configFile, "--non-interactive") + + // Exits non-zero because Docker is unreachable; what matters is the skipped + // version was honored on the way there. + assert.NotContains(t, stdout, "Update available", "a skipped version must stay suppressed non-interactively too") +} diff --git a/test/integration/update_test.go b/test/integration/update_test.go index b85722aa..c8b801ba 100644 --- a/test/integration/update_test.go +++ b/test/integration/update_test.go @@ -18,6 +18,8 @@ import ( "regexp" "runtime" "strings" + "sync" + "sync/atomic" "testing" "time" @@ -318,19 +320,10 @@ func TestUpdateNotification(t *testing.T) { ctx := testContext(t) - // Build a fake old version to a temp location + // Not shared via lstkAtInstallPath: the "update" subtest replaces this binary + // in place, so no other test may reuse it. tmpBinary := filepath.Join(t.TempDir(), execName("lstk")) - repoRoot, err := filepath.Abs("../..") - require.NoError(t, err) - - buildCmd := exec.CommandContext(ctx, "go", "build", - "-ldflags", "-X github.com/localstack/lstk/internal/version.version=0.0.1", - "-o", tmpBinary, - ".", - ) - buildCmd.Dir = repoRoot - out, err := buildCmd.CombinedOutput() - require.NoError(t, err, "go build failed: %s", string(out)) + buildLstkWithVersion(t, ctx, "0.0.1", tmpBinary) // Mock API server so license validation fails fast after the notification mockServer := createMockLicenseServer(false) @@ -338,14 +331,7 @@ func TestUpdateNotification(t *testing.T) { t.Run("skip", func(t *testing.T) { t.Parallel() - configFile := filepath.Join(t.TempDir(), "config.toml") - originalConfig := `# User-maintained lstk config -[[containers]] -type = "aws" # Emulator type -tag = "latest" # Docker image tag -port = "4566" # Host port -` - require.NoError(t, os.WriteFile(configFile, []byte(originalConfig), 0o644)) + configFile := writeUpdateCheckConfig(t, "") ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -364,9 +350,7 @@ port = "4566" # Host port require.NoError(t, err) configStr := string(configData) assert.Contains(t, configStr, "update_skipped_version", "skipped version should be persisted") - assert.Contains(t, configStr, "# User-maintained lstk config", "file header comment should be preserved") - assert.Contains(t, configStr, "# Emulator type", "inline comments should be preserved") - assert.Contains(t, configStr, `port = "4566"`, "existing config values should be preserved") + assertConfigCommentsPreserved(t, configStr) }) t.Run("update", func(t *testing.T) { @@ -470,8 +454,19 @@ func packageReleaseArchive(t *testing.T, binaryName string, binary []byte) []byt // mockGitHubEnv. func mockGitHubReleaseServer(t *testing.T, tag string, assets map[string][]byte) *httptest.Server { t.Helper() + srv, _ := mockGitHubReleaseServerCounting(t, tag, assets) + return srv +} + +// mockGitHubReleaseServerCounting is mockGitHubReleaseServer plus a request +// counter, so a test can prove no request was made rather than merely that +// nothing was printed. +func mockGitHubReleaseServerCounting(t *testing.T, tag string, assets map[string][]byte) (*httptest.Server, *atomic.Int64) { + t.Helper() + var requests atomic.Int64 downloadPrefix := "/localstack/lstk/releases/download/" + tag + "/" srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) switch { case r.URL.Path == "/repos/localstack/lstk/releases/latest": w.Header().Set("Content-Type", "application/json") @@ -488,7 +483,65 @@ func mockGitHubReleaseServer(t *testing.T, tag string, assets map[string][]byte) } })) t.Cleanup(srv.Close) - return srv + return srv, &requests +} + +var installPathBuilds sync.Map // layout key -> *installPathBuild + +type installPathBuild struct { + once sync.Once + root string + path string + err error +} + +// cleanupInstallPathBuilds removes the shared binaries lstkAtInstallPath built. +// They live outside t.TempDir() (they outlive the test that built them), so +// nothing else reclaims them. Called from TestMain. +func cleanupInstallPathBuilds() { + installPathBuilds.Range(func(_, value any) bool { + if build, ok := value.(*installPathBuild); ok && build.root != "" { + _ = os.RemoveAll(build.root) + } + return true + }) +} + +// lstkAtInstallPath returns an lstk binary with the given version stamped in, +// placed under . Detection reads the running binary's resolved path, +// so laying it out like a package manager is the only way to exercise it e2e. +// +// Each (version, layout) is built once per package and shared, since the build is +// the expensive part. Callers must treat the binary as read-only — a test that +// lets lstk replace it needs its own copy. +func lstkAtInstallPath(t *testing.T, ctx context.Context, version string, segments ...string) string { + t.Helper() + + key := version + "\x00" + strings.Join(segments, "\x00") + entry, _ := installPathBuilds.LoadOrStore(key, &installPathBuild{}) + build := entry.(*installPathBuild) + + build.once.Do(func() { + // Not t.TempDir(): the binary outlives the test that built it. + root, err := os.MkdirTemp("", "lstk-install-path-*") + if err != nil { + build.err = err + return + } + build.root = root + dir := filepath.Join(append([]string{root}, segments...)...) + if err := os.MkdirAll(dir, 0o755); err != nil { + build.err = err + return + } + path := filepath.Join(dir, execName("lstk")) + buildLstkWithVersion(t, ctx, version, path) + build.path = path + }) + + require.NoError(t, build.err) + require.NotEmpty(t, build.path, "shared build failed in another test") + return build.path } // mockGitHubEnv builds an isolated test environment whose updater GitHub @@ -641,3 +694,60 @@ func TestUpdateBinaryMockGitHubMissingChecksums(t *testing.T) { require.NoError(t, err) assert.Empty(t, leftovers, "aborted update must not leave temp files behind") } + +// TestUpdateRefusesExternallyManagedInstall covers the case a mise or Nix user +// hits: lstk must not replace a binary another package manager owns. The refusal +// has to happen before any network request, so the test also asserts the mock +// GitHub was never contacted. +func TestUpdateRefusesExternallyManagedInstall(t *testing.T) { + t.Parallel() + ctx := testContext(t) + + srv, requests := mockGitHubReleaseServerCounting(t, "v0.0.2", nil) + misePath := lstkAtInstallPath(t, ctx, "0.0.1", "mise", "installs", "lstk", "0.0.1") + + // Subtests share srv and run sequentially: the later ones do contact it, so + // the no-request assertion below is a delta rather than an absolute count. + t.Run("refuses to update", func(t *testing.T) { + requestsBefore := requests.Load() + stdout, stderr, err := runBinary(t, "", mockGitHubEnv(t, srv), misePath, "update", "--non-interactive") + requireExitCode(t, 1, err) + + assert.Contains(t, stdout, "installed with mise") + assert.Contains(t, stdout, "mise upgrade lstk") + assert.Empty(t, stderr, "the error is rendered through the sink, not re-printed on stderr") + assert.Equal(t, requestsBefore, requests.Load(), "an externally managed install must not even check for a version") + + version, _, err := runBinary(t, "", mockGitHubEnv(t, srv), misePath, "--version") + require.NoError(t, err) + assert.Contains(t, version, "0.0.1", "the binary must be left untouched") + }) + + t.Run("refusal reports a machine-readable code", func(t *testing.T) { + stdout, _, err := runBinary(t, "", mockGitHubEnv(t, srv), misePath, "update", "--json") + requireExitCode(t, 1, err) + + envelope := decodeEnvelope(t, stdout) + require.NotNil(t, envelope.Error) + assert.Equal(t, "UPDATE_EXTERNALLY_MANAGED", envelope.Error.Code) + assert.Equal(t, "USAGE", envelope.Error.Category) + assert.False(t, envelope.Error.Retryable) + }) + + t.Run("check still works", func(t *testing.T) { + stdout, stderr, err := runBinary(t, "", mockGitHubEnv(t, srv), misePath, "update", "--check", "--non-interactive") + require.NoError(t, err, "stderr: %s", stderr) + requireExitCode(t, 0, err) + assert.Contains(t, stdout, "Update available: 0.0.1 → v0.0.2") + }) + + t.Run("check ignores update_check off", func(t *testing.T) { + // update_check governs only the automatic check on start; asking + // explicitly always checks. + environ := append(mockGitHubEnv(t, srv), string(env.UpdateCheck)+"=off") + stdout, stderr, err := runBinary(t, "", environ, misePath, "update", "--check", "--non-interactive") + require.NoError(t, err, "stderr: %s", stderr) + requireExitCode(t, 0, err) + assert.Contains(t, stdout, "Update available: 0.0.1 → v0.0.2") + }) +}