Resolve recipe pack references by name in Radius.Core/environments#12134
Open
willdavsmith wants to merge 1 commit into
Open
Resolve recipe pack references by name in Radius.Core/environments#12134willdavsmith wants to merge 1 commit into
willdavsmith wants to merge 1 commit into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Unit Tests 2 files ± 0 439 suites ±0 7m 38s ⏱️ -1s Results for commit f2e847c. ± Comparison against base commit 76fd2e4. This pull request removes 2 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12134 +/- ##
=======================================
Coverage 52.35% 52.36%
=======================================
Files 737 737
Lines 47240 47263 +23
=======================================
+ Hits 24733 24747 +14
- Misses 20141 20146 +5
- Partials 2366 2370 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
willdavsmith
force-pushed
the
willdavsmith/recipe-pack-by-name-design
branch
3 times, most recently
from
3d1d996 to
8d0dfff
Compare
Comment on lines
+215
to
218
| // Re-parse to reject structurally invalid names (e.g. empty or containing '/'). | ||
| if parsed, err := resources.ParseResource(packID.String()); err != nil || !parsed.IsResource() { | ||
| return "", rest.NewBadRequestResponse(fmt.Sprintf("Invalid recipe pack reference %q: provide a recipe pack name or a %s resource ID.", ref, datamodel.RecipePackResourceType)) | ||
| } |
Comment on lines
+173
to
+183
| func Test_RecipePacks_ByName_Deployment(t *testing.T) { | ||
| template := "testdata/recipepacks-test-by-name.bicep" | ||
| appName := "recipepacks-byname-app" | ||
| appNamespace := "recipepacks-byname-ns" | ||
| parentResourceTypeName := "Test.Resources/userTypeAlpha" | ||
| parentResourceTypeParam := strings.Split(parentResourceTypeName, "/")[1] | ||
| filepath := "testdata/testresourcetypes.yaml" | ||
| options := rp.NewRPTestOptions(t) | ||
| cli := radcli.NewCLI(t, options.ConfigFilePath) | ||
|
|
||
| test := rp.NewRPTest(t, appName, []rp.TestStep{ |
Allow the recipePacks array on Radius.Core/environments to contain bare names (e.g. 'postgresPack') in addition to full resource IDs. The CreateOrUpdate controller resolves each name against the environment's own plane and resource group, fail-fast validates the pack exists (HTTP 400 if not), and persists canonical full resource IDs so every downstream reader (config loader, app graph, rad env show, referencedBy maintenance) is unchanged. - Add resolveAndValidateRecipePacks / resolveRecipePackRef and normalize stored references in the controller - Unit tests for name resolution, mixed name/ID, not-found, wrong-type, invalid name, and stored-ID normalization - Functional test variant that references a pack by name (explicit dependsOn for deterministic ordering) Signed-off-by: willdavsmith <willdavsmith@gmail.com>
willdavsmith
force-pushed
the
willdavsmith/recipe-pack-by-name-design
branch
from
8d0dfff to
f2e847c
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
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.
Description
Today an environment's
recipePacksarray only accepts fully-qualified resource IDs, so a simplerecipePacks: ['postgresPack']in Bicep fails. This implements issue #12117 by letting each entry be either a bare pack name or a full resource ID.Resolution and validation happen server-side in the
Radius.Core/environmentsCreateOrUpdate controller, which is the single write choke point for Bicep deploy, direct API, and CLI create/update. Canonical full resource IDs are persisted, so every downstream reader (backend config loader, app graph,rad env show,referencedBymaintenance) is unchanged. No API schema change is required sincerecipePacksis alreadystring[].Resolution flow
Every entry in
recipePacks[]is processed inRun()viaresolveAndValidateRecipePacks->resolveRecipePackRef, one entry at a time. Each entry is classified by whether it parses as a full resource ID:resources.ParseResource(ref)succeeds) - treated as explicit. Its type must beRadius.Core/recipePacks; the canonicalid.String()is returned as-is.postgresPack) - resolved against the environment's own scope:scopeID = ParseScope(envID.RootScope())packID = scopeID.Append(TypeSegment{Type: "Radius.Core/recipePacks", Name: ref})The normalized canonical IDs are written back to
newResource.Properties.RecipePacksbefore save, so downstream readers only ever see full IDs.Which environment / resource group
envIDis the environment being created or updated, taken from the request URL.envID.RootScope()is its plane + resource group, so a bare name resolves into the same plane and resource group as the environment itself. Cross-scope packs require a full ID.Example: env
/planes/radius/local/resourceGroups/myGroup/providers/Radius.Core/environments/myEnv+postgresPackresolves to/planes/radius/local/resourceGroups/myGroup/providers/Radius.Core/recipePacks/postgresPack.Error cases
Checked per-entry, in order; the first failure short-circuits the whole request. Empty or nil
recipePacksis a no-op....has type %q; expected "Radius.Core/recipePacks"./)Invalid recipe pack reference %q: provide a recipe pack name or a ... resource ID.DatabaseClient().Getfails)Referenced recipe pack %q could not be found (resolved to %q): %vFailed to parse recipe pack %q: %vResource type '%s' is defined in multiple recipe packs: %s and %sCould not resolve recipe pack %q within scope %q.Two design points: the not-found error is fail-fast (matching the issue's "previously deployed pack" model - the pack must already exist), and it echoes both the original reference and the resolved ID, so a mistyped name or wrong-resource-group mistake is obvious.
Type of change
Fixes: #12117
Contributor checklist
Please verify that the PR meets the following requirements, where applicable:
An overview of proposed schema changes is included in a linked GitHub issue.
A design document is added or updated under
eng/design-notes/in this repository, if new APIs are being introduced.The design document has been reviewed and approved by Radius maintainers/approvers.
A PR for resource-types-contrib is created, if resource types or recipes are affected by the changes in this PR.
A PR for dashboard is created, if the Radius Dashboard is affected by the changes in this PR.
A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
This is a user-facing change; a follow-up docs PR is needed before this leaves draft.