Repository navigation
Fix rad deploy resolving Radius.Core environment ID in the wrong group - #12599
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes rad deploy -g <group> incorrectly resolving a workspace default Radius.Core environment ID against the deploy-time --group scope instead of the scope encoded in the full environment resource ID, preventing reuse of environments across resource groups.
Changes:
- Use
envID.RootScope()when fetching Radius.Core environments referenced by full resource ID, aligning behavior with the existing Applications.Core lookup path. - Improve the “environment does not exist” validation error to report the scope that was actually checked (ID-encoded scope when a full ID is provided).
- Add a regression test covering Radius.Core environment resolution across differing environment/deploy scopes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/cli/cmd/deploy/deploy.go | Fixes Radius.Core environment lookup to use ID-encoded scope and improves the related validation error message. |
| pkg/cli/cmd/deploy/deploy_test.go | Updates existing Radius.Core env tests for the new signature and adds a regression test for cross-group environment resolution. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12599 +/- ##
==========================================
+ Coverage 54.36% 54.37% +0.01%
==========================================
Files 770 770
Lines 51299 51304 +5
==========================================
+ Hits 27890 27899 +9
+ Misses 20795 20792 -3
+ Partials 2614 2613 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thank you for the submission. This issue is pending a discussion with the PMs. @willtsai , @zachcasper |
|
Thanks for the update, @lakshmimsft — understood that this is pending discussion with the PMs, so I'll hold off on further changes here until that's resolved. In the meantime I addressed Copilot's review comment on |
c83f2ee to
7f942ce
Compare
4e2f56e to
a8d7a38
Compare
4bc37aa to
48f55a2
Compare
56b7250 to
c85f733
Compare
c85f733 to
6733235
Compare
|
Working as promised. Before: After: |
|
Hi @pujitha24, thank you for contributing to Radius. The following commits do not currently show a GitHub Verified signature: Please follow our commit-signing guide to configure GPG, SSH, or S/MIME signing. Then re-sign the affected commits and force-push the rewritten branch with Cryptographic commit signing is separate from the DCO |
|
@brooke-hamilton good catch — d1a8154, 6733235, cbd79fe, and 318d590 went in unsigned. Commit signing is configured on my side now (ssh format), but it wasn't in effect when these landed, so there's nothing to verify on them as-is. Fixing that means rebasing to re-sign each commit and force-pushing the branch, which isn't something I can do in this pass — I'll follow up on that separately rather than leave it broken. @zachcasper thanks for confirming it works as expected — good to have that verified end-to-end. |
Motivation: When a workspace's default environment is stored as a full Radius.Core resource ID (e.g. created via `rad init` in preview mode), running `rad deploy <template> -g <group>` against a resource group other than the environment's own group failed with "The environment ... does not exist in scope ...", even though `rad deploy --help` and the `rad group` command's own documentation state that an application's resources do not have to live in the same resource group as the environment they reference, and that passing a full environment ID is the documented way to use an environment from a different group. Approach: `FetchEnvironment` in pkg/cli/cmd/deploy/deploy.go already handled this correctly for Applications.Core environments (it passes the full ID through to GetEnvironment, which extracts scope from the ID). The Radius.Core branch instead discarded the full ID's own resource group and re-queried using only the environment name against `r.Workspace.Scope` (the `--group`-overridden scope), so a Radius.Core environment could never be reused across resource groups. This change brings the Radius.Core branch in line with the Applications.Core branch: when the environment reference is a full resource ID, the lookup now uses the scope encoded in that ID (`envID.RootScope()`) instead of the deploy-time `--group` scope. The "environment does not exist" error message in Validate() is also updated to report the scope that was actually checked, instead of always reporting the `--group` scope, which was misleading in this case. Note: user-visible behavior for the common case (deploying into the same group as the environment) is unchanged; this only affects deployments that use `-g`/`--group` with an environment stored/passed as a full Radius.Core resource ID belonging to a different group. Validation: - go build ./... - go test ./pkg/cli/cmd/deploy/... ./pkg/cli/cmd/run/... -count=1 (both ok) - CGO_ENABLED=1 go test ./pkg/cli/cmd/... ./cmd/rad/... -timeout 600s (the command run by the documented `make test-validate-cli` target per docs/contributing/contributing-code/contributing-code-tests/README.md) — all packages pass - Added Test_FetchEnvironment_RadiusCoreEnvironmentUsesOwnGroup, which reproduces the reported failure with a fake Radius.Core environment server that 404s unless queried with the environment ID's own scope; confirmed this test fails against the pre-fix code and passes with the fix by temporarily reverting the scope-selection change and rerunning it. Report: radius-project#12573 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
…etRadiusCoreEnvironment Test_getRadiusCoreEnvironment was passing a full Radius.Core resource ID directly as the environmentName argument to getRadiusCoreEnvironment, which doesn't reflect how the real EnvironmentsClient.Get API or the production caller (FetchEnvironment) use it. Extract scope/name from the ID before calling, matching FetchEnvironment's own envID.RootScope() / envID.Name() usage, and update the expected environment name to the bare name. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
…t_RadiusCoreEnvironmentUsesOwnGroup Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
5e368e7 to
9215a87
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
2ba622f
Motivation:
When a workspace's default environment is stored as a full Radius.Core
resource ID (e.g. created via
rad initin preview mode), runningrad deploy <template> -g <group>against a resource group other thanthe environment's own group failed with "The environment ... does not
exist in scope ...", even though
rad deploy --helpand therad groupcommand's own documentation state that an application's resources do
not have to live in the same resource group as the environment they
reference, and that passing a full environment ID is the documented way
to use an environment from a different group.
Approach:
FetchEnvironmentin pkg/cli/cmd/deploy/deploy.go already handled thiscorrectly for Applications.Core environments (it passes the full ID
through to GetEnvironment, which extracts scope from the ID). The
Radius.Core branch instead discarded the full ID's own resource group
and re-queried using only the environment name against
r.Workspace.Scope(the--group-overridden scope), so a Radius.Coreenvironment could never be reused across resource groups. This change
brings the Radius.Core branch in line with the Applications.Core branch:
when the environment reference is a full resource ID, the lookup now
uses the scope encoded in that ID (
envID.RootScope()) instead of thedeploy-time
--groupscope. The "environment does not exist" errormessage in Validate() is also updated to report the scope that was
actually checked, instead of always reporting the
--groupscope,which was misleading in this case.
Note: user-visible behavior for the common case (deploying into the
same group as the environment) is unchanged; this only affects
deployments that use
-g/--groupwith an environment stored/passedas a full Radius.Core resource ID belonging to a different group.
Validation:
(the command run by the documented
make test-validate-clitargetper docs/contributing/contributing-code/contributing-code-tests/README.md)
— all packages pass
reproduces the reported failure with a fake Radius.Core environment
server that 404s unless queried with the environment ID's own scope;
confirmed this test fails against the pre-fix code and passes with
the fix by temporarily reverting the scope-selection change and
rerunning it.
Report: #12573
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Fixes #12573