setup default recipe pack for rad init, rad env commands and rad deploy env.bicep#11212
setup default recipe pack for rad init, rad env commands and rad deploy env.bicep#11212nithyatsu wants to merge 12 commits intoradius-project:mainfrom
Conversation
4a5c293 to
336a085
Compare
caf2a8f to
2d515c0
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
There was a problem hiding this comment.
Pull request overview
Updates the Radius CLI to ensure default (singleton) recipe packs exist in the fixed default scope and are linked automatically during rad init, rad env create/update (preview), and rad deploy when deploying Radius.Core/environments.
Changes:
- Introduces
pkg/cli/recipepackutilities for creating/inspecting singleton recipe packs and detecting conflicts. - Updates CLI flows (
rad init,rad env create/update --preview,rad deploy) to create missing singleton packs in/planes/radius/local/resourceGroups/defaultand attach/inject their IDs. - Extends Radius.Core test client fakes and adds unit tests around recipe pack inspection/injection behavior.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/functional-portable/cli/noncloud/testdata/corerp-recipe-pack-test.bicep | Updates functional test recipe pack contents. |
| test/functional-portable/cli/noncloud/cli_test.go | Removes dev-recipe functional test that directly called radinit internals. |
| pkg/cli/test_client_factory/radius_core.go | Adds fake server behaviors for env/recipe pack create/update and conflict simulation. |
| pkg/cli/recipepack/recipepack.go | New shared recipe pack/singleton utilities used across commands. |
| pkg/cli/recipepack/recipepack_test.go | New unit tests for recipe pack utilities (coverage/conflicts/extraction). |
| pkg/cli/cmd/utils.go | Adds PopulateRecipePackClients helper for multi-scope recipe pack inspection. |
| pkg/cli/cmd/utils_test.go | Adds tests for PopulateRecipePackClients. |
| pkg/cli/cmd/radinit/options.go / options_test.go | Switches workspace environment IDs to Radius.Core/environments. |
| pkg/cli/cmd/radinit/init.go / init_test.go | Wires Radius.Core client factories into rad init tests and expectations. |
| pkg/cli/cmd/radinit/environment.go | Creates Radius.Core environments and links singleton recipe packs in default scope. |
| pkg/cli/cmd/env/create/preview/create.go / create_test.go | Creates environments with default-scope singleton recipe packs (preview). |
| pkg/cli/cmd/env/update/preview/update.go / update_test.go | Inspects packs, detects conflicts, ensures missing singletons (preview). |
| pkg/cli/cmd/deploy/deploy.go / deploy_test.go | Injects missing singleton recipe pack IDs into templates before deploy. |
| // if err != nil { | ||
| // return nil, fmt.Errorf("failed to get recipe pack %q: %w", name, err) | ||
| // } | ||
| // if resp.Properties != nil && resp.Properties.Recipes != nil { | ||
| // for resourceType := range resp.Properties.Recipes { | ||
| // typeToPackNames[resourceType] = append(typeToPackNames[resourceType], name) | ||
| // } | ||
| // } | ||
| // } | ||
| // conflicts := make(map[string][]string) | ||
| // for resourceType, packs := range typeToPackNames { | ||
| // if len(packs) > 1 { | ||
| // conflicts[resourceType] = packs | ||
| // } | ||
| // } | ||
| // return conflicts, nil | ||
| // } | ||
|
|
There was a problem hiding this comment.
pkg/cli/recipepack/recipepack.go contains large blocks of commented-out, duplicate implementations (CollectResourceTypesFromRecipePacks / DetectResourceTypeConflicts). This dead code makes the new package harder to maintain and risks divergence from the real implementations; delete these blocks if they’re no longer needed or move them to version control history/docs.
| // if err != nil { | |
| // return nil, fmt.Errorf("failed to get recipe pack %q: %w", name, err) | |
| // } | |
| // if resp.Properties != nil && resp.Properties.Recipes != nil { | |
| // for resourceType := range resp.Properties.Recipes { | |
| // typeToPackNames[resourceType] = append(typeToPackNames[resourceType], name) | |
| // } | |
| // } | |
| // } | |
| // conflicts := make(map[string][]string) | |
| // for resourceType, packs := range typeToPackNames { | |
| // if len(packs) > 1 { | |
| // conflicts[resourceType] = packs | |
| // } | |
| // } | |
| // return conflicts, nil | |
| // } |
pkg/cli/cmd/deploy/deploy.go
Outdated
|
|
||
| // Extract symbolic name from patterns like: | ||
| // [reference('mypack').id] | ||
| // [resourceId('Radius.Core/recipePacks', 'name')] |
There was a problem hiding this comment.
findReferencedRecipePackNames()’s comment mentions parsing [resourceId('Radius.Core/recipePacks', 'name')], but extractSymbolicNameFromExpression() only handles reference('...') expressions. Either implement resourceId parsing or remove it from the comment to avoid misleading future changes.
| // [resourceId('Radius.Core/recipePacks', 'name')] |
| for _, packIDStr := range packIDs { | ||
| // This is the bicep reference for id, and cannot be invalid. | ||
| packID, _ := resources.Parse(packIDStr) | ||
| scope := packID.RootScope() | ||
| if _, exists := clientsByScope[scope]; exists { | ||
| continue | ||
| } |
There was a problem hiding this comment.
PopulateRecipePackClients ignores errors from resources.Parse. If packIDs contains an invalid ID (or an ARM expression that slipped through), Parse will return an error and packID.RootScope() will default to "/", causing an incorrect client to be created (or overwriting clientsByScope["/"]). Handle the error (skip invalid IDs like InspectRecipePacks does, or return a validation error) instead of discarding it.
| t.Run("no-op for empty packIDs", func(t *testing.T) { | ||
| clientsByScope := map[string]*v20250801preview.RecipePacksClient{ | ||
| scope: {}, // placeholder client | ||
| } | ||
|
|
||
| err := PopulateRecipePackClients(context.Background(), &workspaces.Workspace{Scope: scope}, clientsByScope, nil) | ||
| require.NoError(t, err) | ||
| require.Len(t, clientsByScope, 1) | ||
| }) | ||
|
|
||
| t.Run("skips packs whose scope is already in the map", func(t *testing.T) { | ||
| clientsByScope := map[string]*v20250801preview.RecipePacksClient{ | ||
| scope: {}, | ||
| } |
There was a problem hiding this comment.
The test initializes a map[string]*RecipePacksClient with scope: {}. {} is not a valid value for a pointer type and will not compile; use &v20250801preview.RecipePacksClient{} (or another non-nil placeholder) instead.
pkg/cli/cmd/deploy/deploy.go
Outdated
| if _, exists := coveredTypes[resourceType]; !exists { | ||
| coveredTypes[resourceType] = packName | ||
| } |
There was a problem hiding this comment.
When merging resource-type coverage from template-defined recipe packs (ARM expression references), conflicts are not detected if a resource type is already covered by a literal pack (or by another template-defined pack). This can allow environments to be deployed with two packs providing the same resource type without raising a conflict. Track pack lists per resource type for both sources and surface conflicts consistently.
| if _, exists := coveredTypes[resourceType]; !exists { | |
| coveredTypes[resourceType] = packName | |
| } | |
| if existingPack, exists := coveredTypes[resourceType]; exists { | |
| if existingPack != packName { | |
| return fmt.Errorf("resource type %q is provided by multiple recipe packs: %q and %q", resourceType, existingPack, packName) | |
| } | |
| } | |
| coveredTypes[resourceType] = packName |
| // Run implements create-or-update semantics. If the environment does not exist, it creates | ||
| // a new one with all singleton recipe packs for core resource types. If the environment | ||
| // already exists, it checks the existing recipe packs and fills in any missing singletons | ||
| // for core resource types, detecting conflicts along the way. |
There was a problem hiding this comment.
The Run() doc comment says this command “checks the existing recipe packs … detecting conflicts”, but the implementation always calls runCreate(), which unconditionally sets RecipePacks to the singleton set and does not inspect existing environment state. Update the comment to match current behavior or implement the described create-or-update/conflict logic.
| // Run implements create-or-update semantics. If the environment does not exist, it creates | |
| // a new one with all singleton recipe packs for core resource types. If the environment | |
| // already exists, it checks the existing recipe packs and fills in any missing singletons | |
| // for core resource types, detecting conflicts along the way. | |
| // Run creates a new environment with all singleton recipe packs for core resource types. | |
| // It does not inspect existing environment state or perform conflict detection. |
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com> rad init implementation Signed-off-by: nithyatsu <nithyasu@microsoft.com> name default pack Signed-off-by: nithyatsu <nithyasu@microsoft.com> add tests Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> rad init cahnges Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> renames Signed-off-by: nithyatsu <nithyasu@microsoft.com> create upadtes Signed-off-by: nithyatsu <nithyasu@microsoft.com> update Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> deploy changes Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> refactor + comments Signed-off-by: nithyatsu <nithyasu@microsoft.com> deploy fix Signed-off-by: nithyatsu <nithyasu@microsoft.com> fix Signed-off-by: nithyatsu <nithyasu@microsoft.com> remove dev recipe test Signed-off-by: nithyatsu <nithyasu@microsoft.com> wip Signed-off-by: nithyatsu <nithyasu@microsoft.com> ci: retrigger ci: retrigger ci: retrigger
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
b684d7a to
0124690
Compare
Description
rad init
rad env create --preview
rad env update
rad deploy
For each environment found, inspects existing recipe packs, checks for conflicts, creates (as needed) missing singletons in the default scope, and injects their IDs into the template's .properties.recipePacks so the environment is deployed with full recipe pack coverage.
Type of change
Fixes: #11091
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: