Repository navigation
Add --recipe-pack-group flag to rad env create/update --preview - #12634
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a --recipe-pack-group flag to the rad env create --preview and rad env update --preview commands so that bare recipe pack names provided via --recipe-packs can be resolved against a different resource group than the environment’s own scope (addressing #12425).
Changes:
- Added
--recipe-pack-groupflag and validation guard requiring it be used with--recipe-packs. - Updated recipe pack ID resolution logic to use the specified group for bare pack names.
- Added targeted unit tests covering parsing, validation, and run-time resolution behavior for both create/update.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cli/cmd/env/create/preview/create.go | Adds flag and uses it to scope bare recipe-pack resolution during create. |
| pkg/cli/cmd/env/create/preview/create_test.go | Adds coverage for validation and run-time resolution when --recipe-pack-group is set. |
| pkg/cli/cmd/env/update/preview/update.go | Adds flag and uses it to scope bare recipe-pack resolution during update. |
| pkg/cli/cmd/env/update/preview/update_test.go | Adds coverage for validation and run-time resolution when --recipe-pack-group is set. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
zachcasper
left a comment
There was a problem hiding this comment.
Tested. Working as advertised. Thanks for the contribution!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12634 +/- ##
==========================================
+ Coverage 54.36% 54.37% +0.01%
==========================================
Files 770 770
Lines 51263 51299 +36
==========================================
+ Hits 27867 27895 +28
- Misses 20788 20792 +4
- Partials 2608 2612 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@pujitha24 Thanks again for contributing. Pls update pr for the comments logged by copilot and myself above and then we will be able to merge it in. |
c903ee1 to
086515c
Compare
|
086515c addresses all of these:
Added test coverage for the empty The one CI failure ( |
Head branch was pushed to by a user without write access
20943b0 to
023f1f6
Compare
|
Heads up @zachcasper — rebasing this onto the current base to clear a merge conflict dismissed your approval. The branch is now at |
Head branch was pushed to by a user without write access
6f39e3e to
56b36b4
Compare
|
Hi @pujitha24, thank you for the update. This PR is ready to merge, but every commit must have a GitHub Verified signature. Please follow our commit-signing guide to configure GPG, SSH, or S/MIME signing. Afterward, re-sign all existing commits in this PR and force-push the rewritten branch with |
|
Thanks @brooke-hamilton. I don't have GPG/SSH commit signing configured in this environment, so I can't re-sign the history and force-push from here right now. I'll get signing set up on my account and push a rewritten, signed branch once that's done. Separately — the earlier comments from Copilot and @lakshmimsft (empty |
Motivation: rad env create --preview and rad env update --preview resolve a bare recipe pack name (passed via --recipe-packs) against the environment's own resource group only. There was no way to reference a Recipe Pack that lives in a different resource group other than passing its full Radius resource ID, an undocumented workaround. A bare name in a different resource group fails with: Recipe pack "test-recipes" does not exist. Please provide a valid recipe pack to set on the environment. Approach: Add a --recipe-pack-group string flag to both commands. When set, bare recipe pack names passed via --recipe-packs are resolved against this resource group's scope instead of the environment's own workspace scope. Full recipe pack resource IDs are unaffected, since they are already fully scoped. The flag is rejected when passed without --recipe-packs. Validation: Ran `go build ./pkg/cli/...` and `make test-validate-cli` (this repo's documented test tier for rad CLI command changes, covering ./pkg/cli/cmd/... and ./cmd/rad/...) — all 1271 tests pass, including new targeted tests in create_test.go and update_test.go asserting that a bare recipe pack name resolves against --recipe-pack-group's scope instead of the workspace's own scope, and that --recipe-pack-group without --recipe-packs is rejected. This is a unit-test-provable code path (scope string construction feeding resource ID resolution); no live cluster was used or is required to validate it. Report: radius-project#12425 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
- Reject an explicitly-provided-but-empty --recipe-packs value in rad env update --preview, matching the existing rad env create --preview behavior (Copilot). - Derive the plane portion of recipePackScope from the workspace's own scope instead of hard-coding /planes/radius/local, in both create and update (Copilot). - Validate --recipe-pack-group with the existing common.ValidateResourceGroupName before it's interpolated into a resource ID scope, in both create and update (lakshmimsft). Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Head branch was pushed to by a user without write access
6f9da00 to
71e64d9
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
40c013a
Summary
Adds a
--recipe-pack-groupflag torad env create --previewandrad env update --preview. When a bare recipe pack name is passed via--recipe-packs, it is now resolved against the resource group named by--recipe-pack-group(if set) instead of always being resolved against theenvironment's own resource group.
Reason for change
Fixes #12425
Before this change,
rad env create/rad env updatehad no way toreference a Recipe Pack that lives in a different resource group than the
environment, other than passing the pack's full Radius resource ID (an
undocumented workaround). A bare recipe pack name was always resolved
against the environment's own workspace scope, so:
fails with
Recipe pack "test-recipes" does not exist...whenevertest-recipeslives in a different resource group.How to test
Both bare
pack1references now resolve againstother-groupinstead ofmyenv's own resource group.Automated coverage:
make test-validate-cliis this repo's documented test tier forradCLIcommand changes (runs
./pkg/cli/cmd/...and./cmd/rad/...); all 1271tests pass, including new targeted tests in
create_test.goandupdate_test.gothat assert a bare recipe pack name resolves to the--recipe-pack-groupscope instead of the workspace's own scope, and that--recipe-pack-groupis rejected when passed without--recipe-packs.File change summary
pkg/cli/cmd/env/create/preview/create.go--recipe-pack-groupflag, parse it inValidate, and use it (when set) to scope bare recipe pack name resolution inresolveRecipePacks.pkg/cli/cmd/env/create/preview/create_test.goRun()-level resolution against the specified group.pkg/cli/cmd/env/update/preview/update.go--recipe-pack-groupflag, parse it inValidate, and use it (when set) to scope bare recipe pack name resolution inRun.pkg/cli/cmd/env/update/preview/update_test.goRun()-level resolution against the specified group.