Remove deprecated lstk config profile command#360
Open
gtsiolis wants to merge 2 commits into
Open
Conversation
`config profile` was introduced already-deprecated in #146 as a backward-compat alias for `lstk setup aws`, but emitted no runtime deprecation warning — the notice lived only in `--help`. It is interactive-only (errors in non-interactive mode, so it can't appear in scripts or CI) and a strict subset of `setup aws`; both delegate to the same handler. Rather than add a louder warning (Cobra's Deprecated field bypasses output.Sink and hides the command from --help and generated docs), remove the alias. - Drop newConfigProfileCmd; simplify newConfigCmd wiring. - Rename ui.RunConfigProfile -> ui.RunSetupAWS (mirrors ui.RunSetupAzure). - Migrate the decline-path integration test to `setup aws`; drop the confirm-path duplicate; assert `config profile` now exits non-zero.
TestAzStopInterceptionNoOpWhenNotIntercepting flaked on windows-latest with "TempDir RemoveAll cleanup ... being used by another process". The interception flow deliberately leaves the global Azure config's telemetry prefs untouched, so `az` spawns a detached background telemetry-upload process that outlives the command (CREATE_NEW_CONSOLE, no wait) and keeps a handle inside the test's temp HOME, breaking t.TempDir() cleanup on Windows. Set AZURE_CORE_COLLECT_TELEMETRY=false for the test's az invocations so the CLI never records telemetry and never spawns the uploader. Pre-existing flake on main, unrelated to the config-profile removal.
anisaoshafi
approved these changes
Jul 3, 2026
anisaoshafi
left a comment
Collaborator
There was a problem hiding this comment.
Two small comments, otherwise LGTM!
| // config key. Tests that run `az` against a temp HOME set it to "false" so the CLI | ||
| // does not spawn its detached background telemetry-upload process, which on Windows | ||
| // outlives `az` and keeps a handle inside the temp dir — breaking t.TempDir() cleanup. | ||
| AzureCollectTelemetry Key = "AZURE_CORE_COLLECT_TELEMETRY" |
Collaborator
There was a problem hiding this comment.
| - `lstk az start-interception` / `lstk az stop-interception` — Opt-in second mode: instead of the isolated dir, these mutate the user's **global** `~/.azure` so plain `az` (any terminal/script) targets LocalStack, then switch back. `start-interception` runs the same register → activate → `instance_discovery=false` → dummy-login flow against the global config (but does not touch global telemetry/survey prefs) and is independent of `lstk setup azure`. `stop-interception` switches the active cloud back to `AzureCloud` (override with `--cloud <name>`, validated against the live `az cloud list`) and re-enables instance discovery — but only if `LocalStack` is still the active cloud, to avoid clobbering an unrelated selection. | ||
|
|
||
| This naming avoids AWS-specific "profile" terminology and uses a clear verb for mutation operations. | ||
| The deprecated `lstk config profile` command still works but points users to `lstk setup aws`. |
Collaborator
There was a problem hiding this comment.
Do you think it's a good idea to add in CLAUDE.md the decision around not using Cobra's deprecated field, as explained in the PR description already?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes
lstk config profile, a deprecated backward-compat alias forlstk setup aws.Why
config profilewas introduced already-deprecated in #146, but the "Deprecated: use 'lstk setup aws' instead" notice lived only in--help— running it gave no runtime nudge (flagged by DevRel). It is interactive-only (hard-errors in non-interactive mode, so it can't appear in scripts or CI) and a strict subset ofsetup aws; both delegate to the same handler.We considered DevRel's suggestion of Cobra's built-in
Deprecatedfield but rejected it: it prints raw toos.Stderr(bypassing ouroutput.Sinklayer) and silently strips the command from--helpand generatedlstk docs. Removing the alias outright is cleaner.Changes
newConfigProfileCmd; simplifynewConfigCmdwiring (drop the now-unusedenv/uideps).ui.RunConfigProfile→ui.RunSetupAWS(onlysetup awscalls it now; mirrorsui.RunSetupAzure).config profilementions ininternal/awsconfigandCLAUDE.md.setup aws(no equivalent existed); drop the confirm-path duplicate (identical toTestSetupAWSCreatesAWSProfileWhenConfirmed); add an exit-code assertion thatconfig profilenow exits non-zero withunknown command "profile".Testing
make build,go vet -C test/integration ./..., and unit tests forcmd/,internal/ui/,internal/awsconfig/— all pass.TestSetupAWSDoesNotCreateAWSProfileWhenDeclined,TestSetupAWSCreatesAWSProfileWhenConfirmed,TestInvalidUsageExitsNonZero(incl.removed_config_profile_subcommand),TestBareParentCommandExitsZero— 18/18 pass.Closes PRO-360