Remove unused direct Azure CLI wrapper - #12942
Merged
Merged
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The removed package has no remaining in-repo references and the doc update cleanly removes an obsolete statement without changing runtime behavior.
Pull request overview
This PR removes an unused Azure CLI wrapper package (pkg/azure/azcli) and updates the CLI architecture documentation to stop describing that wrapper as an active rad subprocess path, aligning the docs with current behavior and avoiding implying a maintained execution surface.
Changes:
- Delete
pkg/azure/azcli/azcli.go(and the now-nonexistentazclipackage) which previously exposedRunCLICommand. - Update
docs/architecture/rad-cli.mdto remove the “direct Azure CLI wrapper” mention while keeping the Azure Identity SDK guidance intact.
File summaries
| File | Description |
|---|---|
| pkg/azure/azcli/azcli.go | Removes the unused exported RunCLICommand wrapper and deletes the package implementation. |
| docs/architecture/rad-cli.md | Updates Windows automation documentation to remove the obsolete direct Azure CLI wrapper reference while preserving the Azure Identity SDK note. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
brooke-hamilton
marked this pull request as ready for review
September 9, 2026 14:07
brooke-hamilton
enabled auto-merge
September 9, 2026 14:07
This was referenced Sep 9, 2026
willdavsmith
previously approved these changes
Sep 10, 2026
lakshmimsft
previously approved these changes
Sep 10, 2026
pull Bot
pushed a commit
to TheTechOddBug/radius
that referenced
this pull request
Sep 10, 2026
…radius-project#12944) ## Summary Remove native-Git orphan-branch archival and use OCI as the only durable archive backend. Rename the backend-neutral graph adapter to `pkg/graph/persistence/archive`, require explicit archive injection, and retain JSON CRUD, traversal validation, missing-item behavior, archive names, and encoded source-branch keys. **Intentional compatibility change:** `rad startup` and `rad shutdown` require `RADIUS_STATE_REGISTRY`; modeled graph archival in GitHub Actions requires `RADIUS_GRAPH_REGISTRY`. Configure OCI repositories without tags and authenticate using Docker credentials. `RADIUS_STATE_BACKEND` may be unset or `oci`; explicit `git` now returns actionable OCI configuration instructions, and unknown values remain invalid. Configuration errors surface when `Archive.Open` is called, not during unrelated CLI initialization. Local modeled graph output remains `app-graph.json` with no registry requirement or archive access. OCI artifact format, authentication, HTTP configuration, and configured startup/shutdown behavior are unchanged. Archive errors are returned to the caller without a filesystem fallback. The documentation describes the supported OCI configuration and local-output behavior. ## Reason for change Fixes radius-project#12943. Parent: radius-project#12887 (left open). This removes the archive-related native Git subprocess family rather than adding prompt suppression, execution timeouts, or another Git implementation. The existing `go-git` dependency and unrelated Git functionality remain unchanged. This PR is based independently on `main` and does not include the Azure-wrapper removal in radius-project#12942 or broader Bicep/kubectl/process-policy work. ## How to test Run the targeted tests and CLI compilation: ```sh go test ./pkg/statearchive/... ./pkg/graph/persistence/... ./pkg/cli/cmd/app/graph/... ./pkg/cli/cmd/startup ./pkg/cli/cmd/shutdown ./cmd/rad/cmd go test -short ./pkg/statearchive/... ./pkg/graph/persistence/... ./pkg/cli/cmd/app/graph/... ./pkg/cli/cmd/startup ./pkg/cli/cmd/shutdown ./cmd/rad/cmd ./cmd/rad ``` Both passed on Windows with Go 1.27. Coverage includes OCI-only configuration, removed/unknown backend errors, missing state/graph registries, nil archive rejection, injected graph JSON save/load/list/delete and commit errors, encoded source-branch separation, OCI round trips, local output without registry configuration, and root initialization followed by `rad version --cli` under missing/invalid archive configuration. Graph adapter tests no longer initialize Git repositories or skip in short mode. CSpell passed for all six changed Markdown files. Markdown lint reports 66 existing violations in three files, identical to the untouched baseline; the same three files have pre-existing table-formatting warnings. Unrelated formatting was left unchanged. The pinned pnpm bootstrap could not download, so the existing Markdown tools were run through the npm cache. No cluster/GHCR end-to-end run is claimed. ## File change summary | File | Summary of change | | ---- | ----------------- | | `pkg/statearchive/git/*` | Delete the native Git archive implementation and implementation-specific tests. | | `pkg/statearchive/factory/*` | Configure OCI only and defer actionable configuration errors until archive use. | | `pkg/statearchive/statearchive.go`, `pkg/statearchive/oci/oci.go` | Update obsolete Git comments; preserve shared contracts and OCI implementation behavior. | | `pkg/graph/persistence/archive/*`, `pkg/graph/persistence/store.go` | Rename the graph adapter, require archive injection, replace Git fixtures, and retain/extend behavior coverage. | | `pkg/statearchive/oci/oci_test.go` | Update the graph adapter import and extend persisted graph lifecycle assertions. | | `cmd/rad/cmd/root*.go`, `pkg/cli/framework/framework.go` | Wire the renamed adapter without silently dropping persistence; cover non-archival initialization. | | `pkg/cli/cmd/app/graph/graph*.go` | Preserve local output and source-branch encoding, explain OCI requirements, and replace real-Git fixtures. | | `pkg/cli/cmd/startup/startup_test.go`, `pkg/cli/cmd/shutdown/shutdown_test.go` | Cover missing registry errors before restore/backup touches the control plane. | | `docs/architecture/{README,application-graph,rad-cli,state-archive,state-persistence}.md`, contributor CLI guide | Document OCI configuration, archive behavior, and local graph output. | --------- Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
brooke-hamilton
dismissed stale reviews from lakshmimsft and willdavsmith
via
September 11, 2026 13:19
b177e2a
brooke-hamilton
force-pushed
the
brooke-hamilton-azure-cli-wrapper-removal
branch
from
September 11, 2026 13:19
7c3f531 to
b177e2a
Compare
lakshmimsft
approved these changes
Sep 11, 2026
pull Bot
pushed a commit
to TheTechOddBug/radius
that referenced
this pull request
Sep 11, 2026
## Summary Expose `process.IsWindowless() bool` as the shared, live Windows no-console policy query for future tool adapters. Both existing `*exec.Cmd` constructors use the same query through command configuration. Console detection remains based on `GetConsoleCP`, preserving classic consoles and Windows Terminal/ConPTY; non-Windows platforms return false. In windowless mode, configuration assigns an explicit EOF reader only when `Cmd.Stdin` is nil. Existing readers are preserved, and callers can replace stdin after construction, including `bytes.NewReader(sqlData)` for PostgreSQL restore. Go already connects nil stdin to the null device, so this makes the default explicit rather than introducing a universal anti-hang mechanism. Existing `SysProcAttr` fields and flags are preserved while applying `HideWindow` and `CREATE_NO_WINDOW`. No detachment or Job Object breakaway flags, production timeouts, environment switches, global environment mutation, or package-initialization console probes are added. Attached-console/non-Windows behavior, output, exit status, and context cancellation remain unchanged. ## Reason for change Fixes radius-project#12945. Implements only section 1 of parent radius-project#12887. Tool-specific prompt handling, Bicep context/pipe changes, kubeconfig validation, PostgreSQL flags, Azure SDK changes, and the separate removals in radius-project#12942 and radius-project#12944 are out of scope. This is an independent draft based on `main` at `a0654b16470228271daa8875889b189bb8951c21`, not stacked on either removal PR or the parent branch. ## How to test Run the focused Windows suite, including the unchanged native Job Object/process-tree and Bicep stderr-inheritance regressions: ```powershell go test .\pkg\process .\pkg\cli\bicep .\test\windowless -run '^(TestCommands_|TestConfigure_|TestProcessHelper$|TestVersion_ClosesCallerStderrPipePromptly$|TestRadVersion_)' -count=1 -timeout=2m ``` The process cases cover the shared query, both constructors, explicit EOF, real finite input delivery before configuration and after construction, preserved caller settings, nonzero exits, stdout/stderr, and context cancellation. These run with simulated attached and windowless Windows policy states. The process suite also passed on Linux with race detection, and the Windows arm64 process tests compile successfully. The architecture guide passes spellcheck. Markdown checks report pre-existing table-format findings on unchanged lines 20, 27, and 35; unrelated table reformatting is intentionally excluded. ## File change summary | File | Summary of change | | ---- | ----------------- | | `pkg/process/process.go` | Document and expose the shared query and stdin contract without changing constructor signatures. | | `pkg/process/process_windows.go` | Reuse the shared query and supply explicit EOF only for unset windowless stdin. | | `pkg/process/process_other.go` | Return false for the platform policy query; retain no-op configuration. | | `pkg/process/process_test.go` | Add bounded helper-process coverage for stdin delivery, output, exit status, and context cancellation. | | `pkg/process/process_windows_test.go` | Cover attached/windowless policy, both constructors, and caller flag/field preservation. | | `pkg/process/process_other_test.go` | Cover both constructors, the false query, and unchanged configuration/input on non-Windows platforms. | | `docs/architecture/rad-cli.md` | Add a focused paragraph documenting the query, EOF default, caller input, and prompt-control limitations. | --------- Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
brooke-hamilton
deleted the
brooke-hamilton-azure-cli-wrapper-removal
branch
September 11, 2026 16:10
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.
Summary
Remove the unused
pkg/azure/azclipackage and its obsolete reference in the CLI architecture documentation. Searches of tracked files at the latestorigin/main(a0654b16470228271daa8875889b189bb8951c21) found no in-repository callers or imports, and no explicit build/test commands referencing the package. This establishes lack of in-repository use, not absence of every possible external consumer of the exported symbol.Azure Identity's SDK-owned
AzureCLICredentialsubprocess is a separate path and remains unchanged. The documentation retains its windowless-execution limitation and non-CLI authentication recommendations. No shared process policy, Git, Bicep, kubectl, authentication, or dependencies change.Reason for change
The unreferenced wrapper should be removed rather than hardened as an active Radius subprocess path.
Fixes #12941
Parent: #12887. This PR addresses only the dead-code removal sub-issue and does not close or implement the remaining parent work.
How to test
go test ./pkg/azure/... ./pkg/process— passed on Windows.go list -deps -test ./cmd/rad/...— passed.RunCLICommand,pkg/azure/azcli, andazcli— no remaining references.git diff --checkand targeted CSpell — passed.npm exec --offlinebecause Corepack's pnpm download failed with a TLS handshake error. Markdownlint reports the same 12 pre-existing MD060 table-spacing errors in the base and modified document; table formatting also reports existing unformatted tables. These unrelated tables are intentionally unchanged.File change summary
pkg/azure/azcli/azcli.goRunCLICommandand the now-empty package.docs/architecture/rad-cli.md