Update radius to setup default recipepack during rad init and env operations.#11294
Update radius to setup default recipepack during rad init and env operations.#11294nithyatsu wants to merge 17 commits intoradius-project:mainfrom
Conversation
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>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
nithyatsu
requested a deployment
to
external-contributor-approval
GitHub Actions
Waiting
There was a problem hiding this comment.
Pull request overview
Updates the Radius CLI to ensure a default Radius.Core recipe pack is created (in the fixed default scope) and attached to environments during rad init, rad env create --preview, and rad deploy, aligning environment setup and deployments with the newer Radius.Core API surface.
Changes:
- Introduce a
pkg/cli/recipepackpackage to define/create the default Kubernetes core recipe pack and IDs. - Update
rad initandrad env create --previewto create the default recipe pack and attach it to newly created Radius.Core environments. - Update
rad deployto scan compiled templates for Radius.Core environments and inject the default recipe pack when none are specified, with expanded fake servers/tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/functional-portable/cli/noncloud/testdata/corerp-recipe-pack-test.bicep | Adjusts recipe pack test template contents. |
| test/functional-portable/cli/noncloud/cli_test.go | Removes DevRecipes functional test that created env directly. |
| pkg/cli/test_client_factory/radius_core.go | Expands Radius.Core fake servers (404-on-get env/packs, create/update handlers, type-mapping helpers). |
| pkg/cli/recipepack/recipepack.go | Adds default recipe pack definition, IDs, and default RG scope helpers. |
| pkg/cli/recipepack/recipepack_test.go | Adds unit tests for default recipe pack definitions and resource construction. |
| pkg/cli/cmd/utils.go | Adds helper to build RecipePacksClients per referenced scope. |
| pkg/cli/cmd/utils_test.go | Adds tests for the new PopulateRecipePackClients helper. |
| pkg/cli/cmd/radinit/options.go | Switches computed workspace environment ID to Radius.Core provider namespace. |
| pkg/cli/cmd/radinit/options_test.go | Updates expectations to match Radius.Core environment IDs. |
| pkg/cli/cmd/radinit/init.go | Extends runner with Radius.Core client factories (workspace + default scope). |
| pkg/cli/cmd/radinit/init_test.go | Updates init tests to use Radius.Core test client factory and reflect extra RG creation call. |
| pkg/cli/cmd/radinit/environment.go | Creates Radius.Core environment, ensures default RG exists, creates default recipe pack, and attaches it. |
| pkg/cli/cmd/env/create/preview/create.go | Creates Radius.Core environment and attaches default recipe pack during preview env creation. |
| pkg/cli/cmd/env/create/preview/create_test.go | Updates preview env-create tests for default recipe pack behavior and output. |
| pkg/cli/cmd/deploy/deploy.go | Adds deploy-time default recipe pack injection for Radius.Core environments in compiled templates. |
| pkg/cli/cmd/deploy/deploy_test.go | Adds tests covering recipe pack injection/no-op paths and default-pack creation behavior. |
| // This is the bicep reference for id, and cannot be invalid. | ||
| packID, _ := resources.Parse(packIDStr) |
There was a problem hiding this comment.
Don't ignore the error from resources.Parse(packIDStr). If packIDStr is malformed (or an ARM/Bicep expression), Parse will fail and RootScope() will produce an incorrect scope (often "/"), leading to clients being created against the wrong scope. Return a clear error (or skip/handle expression IDs explicitly) when Parse fails.
| // This is the bicep reference for id, and cannot be invalid. | |
| packID, _ := resources.Parse(packIDStr) | |
| // This is expected to be a valid bicep reference for id, but we still | |
| // validate and fail fast if parsing does not succeed to avoid using | |
| // an incorrect default scope. | |
| packID, err := resources.Parse(packIDStr) | |
| if err != nil { | |
| return fmt.Errorf("failed to parse recipe pack ID %q: %w", packIDStr, err) | |
| } |
| defaultPack := recipepack.NewDefaultRecipePackResource() | ||
| _, err = r.DefaultScopeClientFactory.NewRecipePacksClient().CreateOrUpdate(ctx, recipepack.DefaultRecipePackResourceName, defaultPack, nil) | ||
| if err != nil { | ||
| return clierrors.MessageWithCause(err, "Failed to create default recipe pack.") | ||
| } |
There was a problem hiding this comment.
Creating the default recipe pack via CreateOrUpdate on every init run can overwrite user modifications to an existing "default" pack in the default scope. Prefer a GET-first flow (create only on 404) or use an ETag/If-Match strategy so init doesn't clobber an existing pack unexpectedly.
| defaultPack := recipepack.NewDefaultRecipePackResource() | ||
| _, err = r.DefaultScopeClientFactory.NewRecipePacksClient().CreateOrUpdate(ctx, recipepack.DefaultRecipePackResourceName, defaultPack, nil) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Creating the default recipe pack with CreateOrUpdate unconditionally can overwrite an existing user-customized "default" pack in the default scope. Prefer checking for existence (GET + create on 404) before writing, consistent with the deploy path's get-or-create behavior.
There was a problem hiding this comment.
@nithyatsu this function includes updates as well, so we should check for existing packs. Also, can a user not provide their own recipe packs as part of rad env create command?
| envProperties := corerpv20250801.EnvironmentProperties{ | ||
| Providers: providers, | ||
| } | ||
|
|
||
| var recipes map[string]map[string]corerp.RecipePropertiesClassification | ||
| if r.Options.Recipes.DevRecipes { | ||
| // Note: To use custom registry for recipes, users need to manually configure | ||
| // their environment after initialization or use custom recipe definitions | ||
| recipes, err = r.DevRecipeClient.GetDevRecipes(ctx) | ||
| // Initialize the Radius.Core client factory if not already set | ||
| if r.RadiusCoreClientFactory == nil { | ||
| clientFactory, err := cmd.InitializeRadiusCoreClientFactory(ctx, r.Workspace, r.Workspace.Scope) | ||
| if err != nil { | ||
| return err | ||
| return clierrors.MessageWithCause(err, "Failed to initialize Radius Core client.") | ||
| } | ||
| r.RadiusCoreClientFactory = clientFactory | ||
| } |
There was a problem hiding this comment.
rad init still sets/prints options.Recipes.DevRecipes (and Runner still carries DevRecipeClient), but CreateEnvironment no longer uses either to influence the created environment. Either remove the dev-recipes option/UI/fields or reintroduce equivalent behavior using the new Radius.Core recipe pack model so the option isn't a no-op.
| // DevRecipeClient is the interface for the dev recipe client. | ||
| DevRecipeClient DevRecipeClient | ||
|
|
||
| // RadiusCoreClientFactory is the client factory for Radius.Core resources. | ||
| // If nil, it will be initialized during Run. | ||
| RadiusCoreClientFactory *corerpv20250801.ClientFactory | ||
|
|
||
| // DefaultScopeClientFactory is the client factory scoped to the default resource group. | ||
| // Singleton recipe packs are always created/queried in the default scope. | ||
| DefaultScopeClientFactory *corerpv20250801.ClientFactory | ||
|
|
There was a problem hiding this comment.
Runner.DevRecipeClient appears to be unused now that environment creation no longer applies dev recipes. Consider removing this field (and related wiring) or updating the new recipe-pack-based flow to use it, to avoid dead code and misleading configuration surface.
| // DevRecipeClient is the interface for the dev recipe client. | |
| DevRecipeClient DevRecipeClient | |
| // RadiusCoreClientFactory is the client factory for Radius.Core resources. | |
| // If nil, it will be initialized during Run. | |
| RadiusCoreClientFactory *corerpv20250801.ClientFactory | |
| // DefaultScopeClientFactory is the client factory scoped to the default resource group. | |
| // Singleton recipe packs are always created/queried in the default scope. | |
| DefaultScopeClientFactory *corerpv20250801.ClientFactory | |
| // RadiusCoreClientFactory is the client factory for Radius.Core resources. | |
| // If nil, it will be initialized during Run. | |
| RadiusCoreClientFactory *corerpv20250801.ClientFactory |
Signed-off-by: nithyatsu <nithyasu@microsoft.com>
nithyatsu
requested a deployment
to
external-contributor-approval
GitHub Actions
Waiting
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11294 +/- ##
==========================================
- Coverage 51.00% 50.79% -0.22%
==========================================
Files 679 680 +1
Lines 43174 43508 +334
==========================================
+ Hits 22023 22098 +75
- Misses 19033 19267 +234
- Partials 2118 2143 +25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| // findRadiusCoreEnvironmentResources walks the template's resources and returns | ||
| // all Radius.Core/environments resources found (as mutable maps). | ||
| func findRadiusCoreEnvironmentResources(template map[string]any) []map[string]any { |
There was a problem hiding this comment.
We have an existing function for this https://github.com/kachawla/radius/blob/main/pkg/cli/bicep/resources.go#L107
| // no changes are made. Otherwise, it fetches or creates the default recipe pack from | ||
| // the default scope and injects their IDs into the template. |
There was a problem hiding this comment.
nit: we only expect one default recipe pack, right?
| // no changes are made. Otherwise, it fetches or creates the default recipe pack from | |
| // the default scope and injects their IDs into the template. | |
| // no changes are made. Otherwise, it fetches or creates the default recipe pack from | |
| // the default scope and injects its ID into the template. |
| // The compiled ARM template has a double-nested properties structure: | ||
| // envResource["properties"]["properties"] is where resource-level fields live. | ||
| // Navigate to the inner (resource) properties map. | ||
| outerProps, ok := envResource["properties"].(map[string]any) | ||
| if !ok { | ||
| outerProps = map[string]any{} | ||
| envResource["properties"] = outerProps | ||
| } | ||
|
|
||
| properties, ok := outerProps["properties"].(map[string]any) | ||
| if !ok { | ||
| properties = map[string]any{} | ||
| outerProps["properties"] = properties | ||
| } | ||
|
|
||
| // If the environment already has any recipe packs configured (literal IDs or | ||
| // Bicep expression references), leave it as-is — the user is managing packs explicitly. | ||
| if hasAnyRecipePacks(properties) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
What do you think about moving this as a property on https://github.com/kachawla/radius/blob/main/pkg/cli/bicep/resources.go#L38? Just provider better readability and abstraction.
| // getOrCreateDefaultRecipePack attempts to GET the default recipe pack from | ||
| // the default scope. If it doesn't exist (404), it creates it with all core | ||
| // resource type recipes. Returns the full resource ID. | ||
| func getOrCreateDefaultRecipePack(ctx context.Context, client *v20250801preview.RecipePacksClient) (string, error) { |
There was a problem hiding this comment.
Should we move recipe pack specific functionality like this to https://github.com/nithyatsu/radius/blob/e204475b2e5c24030995ac45fd9b4ce4f1441920/pkg/cli/recipepack/recipepack.go?
| test.Test(t) | ||
| } | ||
|
|
||
| // This test creates an environment by directly calling the CreateEnvironment function to test dev recipes. |
There was a problem hiding this comment.
if we're deleting these tests, shouldn't we be adding new ones? the functionality still exists for existing dev-recipes.
| } | ||
|
|
||
| // SingletonRecipePackDefinition defines a singleton recipe pack for a single resource type. | ||
| type SingletonRecipePackDefinition struct { |
There was a problem hiding this comment.
I thought we weren’t going to create singleton recipe packs. Is this and the other singleton references something that carried over from previous implementation?
| envRes := template["resources"].(map[string]any)["env"].(map[string]any) | ||
| outerProps := envRes["properties"].(map[string]any) | ||
| innerProps := outerProps["properties"].(map[string]any) | ||
| packs, ok := innerProps["recipePacks"].([]any) |
There was a problem hiding this comment.
This code is repeated multiple times, could we abstract it out in a helper method?
| template := map[string]any{ | ||
| "resources": map[string]any{ | ||
| "env": map[string]any{ | ||
| "type": "Radius.Core/environments@2025-08-01-preview", | ||
| "properties": map[string]any{ | ||
| "name": "myenv", | ||
| "properties": map[string]any{ | ||
| "recipePacks": []any{existingPackID}, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Same here, this is repeated code. Could we create a template builder helper function?
| template := map[string]any{ | ||
| "resources": map[string]any{ | ||
| "env": map[string]any{ | ||
| "type": "Radius.Core/environments@2025-08-01-preview", |
There was a problem hiding this comment.
Would be good to extract Radius.Core/environments@2025-08-01-preview into a constant, so that if the version changes, we won't have to update it in multiple places in this file.
There was a problem hiding this comment.
Do we need a test for if CreateOrUpdateResourceGroup returns an error?
| Return(nil). | ||
| Times(1) | ||
|
|
||
| // Default scope factory returns 404 on GET (packs don't exist yet) |
There was a problem hiding this comment.
Should we add another test for any error (not just 404)?
| defaultPack := recipepack.NewDefaultRecipePackResource() | ||
| _, err = r.DefaultScopeClientFactory.NewRecipePacksClient().CreateOrUpdate(ctx, recipepack.DefaultRecipePackResourceName, defaultPack, nil) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
@nithyatsu this function includes updates as well, so we should check for existing packs. Also, can a user not provide their own recipe packs as part of rad env create command?
| cmd.Flags().Bool(commonflags.ClearEnvAWSFlag, false, "Specify if aws provider needs to be cleared on env") | ||
| cmd.Flags().Bool(commonflags.ClearEnvKubernetesFlag, false, "Specify if kubernetes provider needs to be cleared on env (--preview)") | ||
| cmd.Flags().StringArrayP("recipe-packs", "", []string{}, "Specify recipe packs to be added to the environment (--preview)") | ||
| cmd.Flags().StringSliceP("recipe-packs", "", []string{}, "Specify recipe packs to replace the environment's recipe pack list (--preview). Accepts comma-separated values.") |
There was a problem hiding this comment.
This is not only used for replace though.. it can still be used to add new recipe packs if the user uses default in the list as well, right?
| // replace recipe packs if any are specified | ||
| if len(r.recipePacks) > 0 { | ||
| if env.Properties.RecipePacks == nil { | ||
| env.Properties.RecipePacks = []*string{} | ||
| } | ||
| // Create a new list to replace the existing recipe packs | ||
| newRecipePacks := []*string{} |
There was a problem hiding this comment.
Should we add a prompt/warning for users mentioning that the existing list will be replaced? I don't think this is very intuitive
@nithyatsu @willtsai
There was a problem hiding this comment.
Do we have a test to explicitly validate that the list is replaced?
|
|
||
| if r.Options.CloudProviders.Azure != nil { | ||
| providerList = append(providerList, r.Options.CloudProviders.Azure) | ||
| providers.Azure = &corerpv20250801.ProvidersAzure{ |
There was a problem hiding this comment.
Just curious - why do these need updating?
| // Note: To use custom registry for recipes, users need to manually configure | ||
| // their environment after initialization or use custom recipe definitions | ||
| recipes, err = r.DevRecipeClient.GetDevRecipes(ctx) |
There was a problem hiding this comment.
I think we should make sure capture this in release highlights that default dev recipes are removed?
@nithyatsu @willtsai
| const ( | ||
| // DefaultRecipePackResourceName is the name of the default recipe pack | ||
| // resource that contains recipes for all core resource types. | ||
| DefaultRecipePackResourceName = "default" |
There was a problem hiding this comment.
@willtsai Do we want to restrict users from using default as the name for their recipe packs or we don't care if they override it by using the same name?
@nithyatsu will it cause any issues in the assumptions we have made in the implementation if allow it to be used?
| { | ||
| Name: "containers", | ||
| ResourceType: "Radius.Compute/containers", | ||
| RecipeLocation: "ghcr.io/radius-project/kube-recipes/containers:" + tag, |
There was a problem hiding this comment.
I only see latest on ghcr. How are we planning to publish different versions?
|
|
||
| // This test creates an environment by directly calling the CreateEnvironment function to test dev recipes. | ||
| // After dev recipes are confirmed, the environment is deleted. | ||
| func Test_DevRecipes(t *testing.T) { |
There was a problem hiding this comment.
Should we remove rest of the code related to dev recipes?
| 'Radius.Networking/gateways': { | ||
| recipeKind: 'terraform' | ||
| recipeLocation: 'https://github.com/project-radius/resource-types-contrib.git//recipes/compute/containers/kubernetes?ref=v0.48' | ||
| parameters: { | ||
| allowPlatformOptions: true | ||
| } | ||
| recipeLocation: 'https://github.com/project-radius/resource-types-contrib.git//recipes/networking/gateways/kubernetes?ref=v0.48' | ||
| } | ||
| 'Radius.Security/secrets': { | ||
| 'Radius.Messaging/queues': { |
Description
rad init
rad env create --preview
rad deploy
For each environment found, inspects existing recipe packs, if none, adds default pack.
Type of change
Fixes: #11091
radius-project/resource-types-contrib#100 and this PR must be merged together.
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: