-
Notifications
You must be signed in to change notification settings - Fork 120
Usability: rad recipe show does not show parameter values #11005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
actual configured parameter values like 'eastus' and 'my-rg'. Signed-off-by: Alec13355 <aah1@mac.com>
…cccb1a2b8 to 082deb6cee063d5b8ce740fbee614460d2c2211b in the github-actions group (radius-project#11001) Bumps the github-actions group with 1 update: [securego/gosec](https://github.com/securego/gosec). Updates `securego/gosec` from 538a05cc5d6eb7bb41624e48f6e5019cccb1a2b8 to 082deb6cee063d5b8ce740fbee614460d2c2211b <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/securego/gosec/commit/082deb6cee063d5b8ce740fbee614460d2c2211b"><code>082deb6</code></a> whitelist crypto/rand Read from error checks (<a href="https://redirect.github.com/securego/gosec/issues/1446">#1446</a>)</li> <li><a href="https://github.com/securego/gosec/commit/095d529a906cabaf1adbea5e85fc13acce092a53"><code>095d529</code></a> chore(deps): update all dependencies (<a href="https://redirect.github.com/securego/gosec/issues/1443">#1443</a>)</li> <li><a href="https://github.com/securego/gosec/commit/c073629009897d89e03229bc81232c7375892086"><code>c073629</code></a> Improve slice bound check (<a href="https://redirect.github.com/securego/gosec/issues/1442">#1442</a>)</li> <li>See full diff in <a href="https://github.com/securego/gosec/compare/538a05cc5d6eb7bb41624e48f6e5019cccb1a2b8...082deb6cee063d5b8ce740fbee614460d2c2211b">compare view</a></li> </ul> </details> <br /> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Alec13355 <aah1@mac.com>
…0999) # Description This pull request adds support for a new `x-radius-sensitive` annotation in schema definitions, allowing string and object types to be marked as sensitive for downstream processing. Updates for manifest schema validation to follow in separate PR. ref: Design Doc: [2025-11-11-secrets-redactdata](https://github.com/radius-project/design-notes/blob/main/resources/2025-11-11-secrets-redactdata.md) - This pull request adds or changes features of Radius and has an approved issue (issue link radius-project#10421). Fixes: radius-project#10421 ## 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. - [] Yes <!-- TaskRadio schema --> - [x] Not applicable <!-- TaskRadio schema --> - A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [x] Yes <!-- TaskRadio design-pr --> - [] Not applicable <!-- TaskRadio design-pr --> - The design document has been reviewed and approved by Radius maintainers/approvers. - [x] Yes <!-- TaskRadio design-review --> - [] Not applicable <!-- TaskRadio design-review --> - A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [] Yes <!-- TaskRadio samples-pr --> - [x] Not applicable <!-- TaskRadio samples-pr --> - A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [] Yes <!-- TaskRadio docs-pr --> - [x] Not applicable <!-- TaskRadio docs-pr --> - A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. - [] Yes <!-- TaskRadio recipes-pr --> - [x] Not applicable <!-- TaskRadio recipes-pr --> Signed-off-by: lakshmimsft <ljavadekar@microsoft.com> Signed-off-by: Alec13355 <aah1@mac.com>
Signed-off-by: Alec13355 <aah1@mac.com>
radius-project#10963) # Description * Update Radius DE sync: radius side (https://github.com/azure-octo/deployment-engine/pull/514) ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). ## Contributor checklist Please verify that the PR meets the following requirements, where applicable: <!-- This checklist uses "TaskRadio" comments to make certain options mutually exclusive. See: https://github.com/mheap/require-checklist-action?tab=readme-ov-file#radio-groups For details on how this works and why it's required. --> - An overview of proposed schema changes is included in a linked GitHub issue. - [ ] Yes <!-- TaskRadio schema --> - [x] Not applicable <!-- TaskRadio schema --> - A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [ ] Yes <!-- TaskRadio design-pr --> - [x] Not applicable <!-- TaskRadio design-pr --> - The design document has been reviewed and approved by Radius maintainers/approvers. - [ ] Yes <!-- TaskRadio design-review --> - [x] Not applicable <!-- TaskRadio design-review --> - A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio samples-pr --> - [x] Not applicable <!-- TaskRadio samples-pr --> - A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [ ] Yes <!-- TaskRadio docs-pr --> - [x] Not applicable <!-- TaskRadio docs-pr --> - A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio recipes-pr --> - [x] Not applicable <!-- TaskRadio recipes-pr --> --------- Signed-off-by: willdavsmith <willdavsmith@gmail.com> Signed-off-by: Alec13355 <aah1@mac.com>
ytimocin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to try to add a functional test for this addition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where rad recipe show displayed 'null' instead of actual configured parameter values (like 'eastus' and 'my-rg') when parameters were set in the environment configuration.
Key Changes:
- Added environment resource retrieval to fetch configured parameter values
- Modified parameter display logic to prefer environment values over template defaults
- Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/cli/cmd/recipe/show/show.go | Added logic to fetch environment resource and override template parameter defaults with environment-specific values |
| pkg/cli/cmd/recipe/show/show_test.go | Added helper function setupTestClient, two new test cases for environment parameter scenarios, and fixed typo in existing test name |
|
|
||
| // Get environment-specific parameter values for this recipe (if environment is available) | ||
| var environmentParameters map[string]any | ||
| if environment != nil && environment.Properties.Recipes != nil { |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing nil check for environment.Properties before accessing environment.Properties.Recipes. According to the EnvironmentResource struct definition, Properties is a pointer field that could be nil, which would cause a panic. Add a nil check for Properties before checking Recipes.
| if environment != nil && environment.Properties.Recipes != nil { | |
| if environment != nil && environment.Properties != nil && environment.Properties.Recipes != nil { |
| t.Run("Show recipe details when GetEnvironment fails - Success", func(t *testing.T) { | ||
| ctrl := gomock.NewController(t) | ||
| envRecipe := v20231001preview.RecipeGetMetadataResponse{ | ||
| TemplateKind: to.Ptr(recipes.TemplateKindBicep), | ||
| TemplatePath: to.Ptr("ghcr.io/testpublicrecipe/bicep/modules/openai:v1"), | ||
| Parameters: map[string]any{ | ||
| "location": map[string]any{ | ||
| "type": "string", | ||
| "defaultValue": "westus", | ||
| }, | ||
| "resource_group_name": map[string]any{ | ||
| "type": "string", | ||
| "defaultValue": "null", | ||
| }, | ||
| }, | ||
| } | ||
| recipe := types.EnvironmentRecipe{ | ||
| Name: "default", | ||
| ResourceType: "Radius.Resources/openAI", | ||
| TemplateKind: recipes.TemplateKindBicep, | ||
| TemplatePath: "ghcr.io/testpublicrecipe/bicep/modules/openai:v1", | ||
| } | ||
|
|
||
| // Expected parameters should show template defaults since environment call fails | ||
| recipeParams := []types.RecipeParameter{ | ||
| { | ||
| Name: "resource_group_name", | ||
| Type: "string", | ||
| MaxValue: "-", | ||
| MinValue: "-", | ||
| DefaultValue: "null", // Template default since environment fails | ||
| }, | ||
| { | ||
| Name: "location", | ||
| Type: "string", | ||
| MaxValue: "-", | ||
| MinValue: "-", | ||
| DefaultValue: "westus", // Template default since environment fails | ||
| }, | ||
| } | ||
|
|
||
| appManagementClient := setupTestClient(ctrl) | ||
| appManagementClient.EXPECT(). | ||
| GetRecipeMetadata(gomock.Any(), gomock.Any(), gomock.Any()). | ||
| Return(envRecipe, nil).Times(1) | ||
|
|
||
| outputSink := &output.MockOutput{} | ||
|
|
||
| runner := &Runner{ | ||
| ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: appManagementClient}, | ||
| Output: outputSink, | ||
| Workspace: &workspaces.Workspace{}, | ||
| Format: "table", | ||
| RecipeName: "default", | ||
| ResourceType: "Radius.Resources/openAI", | ||
| } | ||
|
|
||
| err := runner.Run(context.Background()) | ||
| require.NoError(t, err) | ||
|
|
||
| expected := []any{ | ||
| output.FormattedOutput{ | ||
| Format: "table", | ||
| Obj: recipe, | ||
| Options: common.RecipeFormat(), | ||
| }, | ||
| output.LogOutput{ | ||
| Format: "", | ||
| }, | ||
| output.FormattedOutput{ | ||
| Format: "table", | ||
| Obj: recipeParams, | ||
| Options: common.RecipeParametersFormat(), | ||
| }, | ||
| } | ||
| require.Equal(t, expected, outputSink.Writes) | ||
| }) |
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the case where GetEnvironment returns an EnvironmentResource with nil Properties. This is an important edge case to test since the code needs to handle it gracefully without panicking.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11005 +/- ##
==========================================
+ Coverage 50.60% 50.62% +0.01%
==========================================
Files 673 673
Lines 42271 42283 +12
==========================================
+ Hits 21393 21407 +14
+ Misses 18818 18816 -2
Partials 2060 2060 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🥇 Using the code on this branch I was able to register the persistent volumes recipe with a default parameter value of "banana" (obvious fake value). After registering the recipe, looking at the registered recipe shows the default value: $ rad recipe show default --resource-type Radius.Compute/persistentVolumes
RECIPE TYPE TEMPLATE KIND TEMPLATE VERSION TEMPLATE
default Radius.Compute/persistentVolumes terraform http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/persistentVolumes-kubernetes.zip
PARAMETER TYPE DEFAULT VALUE MIN MAX
storage_class string banana - -@Alec13355 do you think it makes sense to address the copilot comments on this PR? |
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Fixes issue where 'rad recipe show' displayed 'null' instead of actual configured parameter values like 'eastus' and 'my-rg'.
Description
Please explain the changes you've made.
Type of change
Usability:
rad recipe showdoes not show parameter values #9454Fixes: #9454
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: