add type, name and id to properties accessible. #10932

nithyatsu · 2025-12-11T23:42:12Z

Description

Populate Name, ID and Type of the child resource in addition to existing Properties, in the data accessible to a parent resource's recipe.

Type of change

This pull request fixes a bug in Radius and has an approved issue (issue link required).
This pull request adds or changes features of Radius and has an approved issue (issue link required).
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).

Fixes: #issue_number

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
- Not applicable
A design document PR is created in the design-notes repository, if new APIs are being introduced.
- Yes
- Not applicable
The design document has been reviewed and approved by Radius maintainers/approvers.
- Yes
- Not applicable
A PR for the samples repository is created, if existing samples are affected by the changes in this PR.
- Yes
- Not applicable
A PR for the documentation repository is created, if the changes in this PR affect the documentation or any user facing updates are made.
- Yes
- Not applicable
A PR for the recipes repository is created, if existing recipes are affected by the changes in this PR.
- Yes
- Not applicable

Signed-off-by: nithyatsu <[email protected]>

codecov · 2025-12-11T23:55:58Z

Codecov Report

❌ Patch coverage is 66.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.46%. Comparing base (a45fc2f) to head (bae4f72).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...urces/backend/controller/createorupdateresource.go	12.50%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10932      +/-   ##
==========================================
+ Coverage   50.43%   50.46%   +0.02%     
==========================================
  Files         672      672              
  Lines       42057    42072      +15     
==========================================
+ Hits        21213    21230      +17     
- Misses      18788    18789       +1     
+ Partials     2056     2053       -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: nithyatsu <[email protected]>

radius-functional-tests · 2025-12-12T00:13:25Z

Radius functional test overview

🔍 Go to test action run

Click here to see the test run details

Name	Value
Repository	nithyatsu/radius
Commit ref	`bae4f72`
Unique ID	func8381428923
Image tag	pr-func8381428923

gotestsum 1.13.0
KinD: v0.29.0
Dapr: 1.14.4
Azure KeyVault CSI driver: 1.4.2
Azure Workload identity webhook: 1.3.0
Bicep recipe location ghcr.io/radius-project/dev/test/testrecipes/test-bicep-recipes/<name>:pr-func8381428923
Terraform recipe location http://tf-module-server.radius-test-tf-module-server.svc.cluster.local/<name>.zip (in cluster)
applications-rp test image location: ghcr.io/radius-project/dev/applications-rp:pr-func8381428923
dynamic-rp test image location: ghcr.io/radius-project/dev/dynamic-rp:pr-func8381428923
controller test image location: ghcr.io/radius-project/dev/controller:pr-func8381428923
ucp test image location: ghcr.io/radius-project/dev/ucpd:pr-func8381428923
deployment-engine test image location: ghcr.io/radius-project/deployment-engine:latest

Test Status

⌛ Building Radius and pushing container images for functional tests...
✅ Container images build succeeded
⌛ Publishing Bicep Recipes for functional tests...
✅ Recipe publishing succeeded
⌛ Starting corerp-cloud functional tests...
✅ corerp-cloud functional tests succeeded
✅ ucp-cloud functional tests succeeded
✅ corerp-cloud functional tests succeeded

nithyatsu · 2025-12-12T00:31:49Z

pkg/recipes/recipecontext/types.go

+	// context.resource.connections.[connection-name].id
+	// context.resource.connections.[connection-name].name
+	// context.resource.connections.[connection-name].type
+	Connections map[string]recipes.ConnectedResource `json:"connections,omitempty"`


@zachcasper @sk593 , the comment captures the change to connections. please review.

Copilot

Pull request overview

This PR enhances the recipe context by exposing connected resource metadata (ID, Name, Type) in addition to the existing Properties field. Previously, recipes could only access connected resource properties directly; now they must access them through a properties field while also having access to resource metadata.

Key changes:

Introduced ConnectedResource struct containing ID, Name, Type, and Properties fields
Changed ConnectedResourcesProperties from map[string]map[string]any to map[string]ConnectedResource throughout the codebase
Updated recipe templates (Bicep and Terraform) to access connected resource properties via .properties. path

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/recipes/types.go	Added new `ConnectedResource` struct to encapsulate connected resource metadata and properties; updated `ResourceMetadata.ConnectedResourcesProperties` field type
pkg/resourceutil/utils.go	Added `ResourceMetadata` struct and `GetAllPropertiesFromResource` function to extract resource ID, name, type, and properties from resources
pkg/resourceutil/utils_test.go	Added comprehensive tests for `GetAllPropertiesFromResource` including edge cases, error scenarios, and nested properties
pkg/recipes/recipecontext/types.go	Updated `Resource.Connections` field type from `map[string]map[string]any` to `map[string]recipes.ConnectedResource` and updated documentation
pkg/recipes/recipecontext/context_test.go	Added test `TestNewContext_WithConnectedResources` to verify connected resource metadata is properly populated in recipe context
pkg/recipes/engine/engine_test.go	Updated test data to use new `ConnectedResource` struct format
pkg/recipes/terraform/execute_test.go	Updated test data to use new `ConnectedResource` struct format with full metadata
pkg/portableresources/backend/controller/createorupdateresource.go	Changed to call `GetAllPropertiesFromResource` instead of `GetPropertiesFromResource` and populate `ConnectedResource` struct with metadata
pkg/portableresources/backend/controller/createorupdateresource_test.go	Updated test data and added integration test for `GetAllPropertiesFromResource`
test/testrecipes/test-bicep-recipes/dynamicrp_recipe.bicep	Updated recipe to access connected resource properties via new `.properties.configMap` path
test/testrecipes/test-terraform-recipes/parent-udt/main.tf	Updated recipe to access connected resource properties via new `.properties.configMap` path

Copilot · 2025-12-12T15:41:33Z

pkg/resourceutil/utils.go

+// ResourceMetadata represents resource metadata including ID, Name, Type and Properties
+type ResourceMetadata struct {
+	ID         string         `json:"id"`
+	Name       string         `json:"name"`
+	Type       string         `json:"type"`
+	Properties map[string]any `json:"properties,omitempty"`
+}


There are now two different ResourceMetadata types in the codebase with the same name but different purposes:

resourceutil.ResourceMetadata (this struct) - represents resource metadata with ID, Name, Type, and Properties

recipes.ResourceMetadata - represents recipe metadata including resource ID, properties, connected resources, and parameters

This creates naming confusion and ambiguity. Consider renaming this struct to something more specific like ResourceInfo or ExtractedResourceMetadata to clearly differentiate it from the recipes package type and indicate its purpose is for extracting resource information.

This may be an important feedback.

Copilot · 2025-12-12T15:41:33Z

pkg/recipes/types.go

 	rpv1 "github.com/radius-project/radius/pkg/rp/v1"
 )

+// ConnectedResource represents a connected resource's metadata and properties


The comment should follow Go documentation conventions by starting with the type name. It should read: "ConnectedResource represents a connected resource's metadata and properties."

Suggested change

// ConnectedResource represents a connected resource's metadata and properties

// ConnectedResource represents a connected resource's metadata and properties.

Copilot · 2025-12-12T15:41:33Z

pkg/portableresources/backend/controller/createorupdateresource_test.go

+
+func TestGetAllPropertiesFromResource_Integration(t *testing.T) {
+	// Simple integration test for resourceutil.GetAllPropertiesFromResource
+	connectedResourceID := "/planes/radius/local/resourceGroups/radius-test-rg/providers/Applications.Datastores/sqlDatabases/test-db"


Trailing whitespace detected after the resource ID string. Remove the trailing whitespace for cleaner code.

Copilot · 2025-12-12T15:41:34Z

pkg/portableresources/backend/controller/createorupdateresource_test.go

+
+	// Test the GetAllPropertiesFromResource function
+	metadata, err := resourceutil.GetAllPropertiesFromResource(testResource, connectedResourceID)
+


Trailing whitespace detected after the function call. Remove the trailing whitespace for cleaner code.

Suggested change

Copilot · 2025-12-12T15:41:34Z

pkg/recipes/recipecontext/types.go

 	// Properties represents the properties of the resource.
 	Properties map[string]any `json:"properties,omitempty"`

 	// Connections represent a map of connections to other resources.


The comment has inconsistent capitalization. "Connections" should start with a lowercase letter to follow Go documentation conventions for field comments. Change "Connections represent" to "connections represent".

ytimocin · 2025-12-12T19:05:52Z

pkg/portableresources/backend/controller/createorupdateresource.go

 		return nil, fmt.Errorf("failed to get connected resource IDs: %w", err)
 	}
-	connectedResourcesProperties := make(map[string]map[string]any)
+	connectedResourcesProperties := make(map[string]recipes.ConnectedResource)


Can this be renamed to just connectedResources? Do we need to have Properties postfix?

ytimocin · 2025-12-12T19:07:08Z

pkg/portableresources/backend/controller/createorupdateresource.go

 		}

-		connectedResourceProperties, err := resourceutil.GetPropertiesFromResource(connectedResource.Data)
+		connectedResourceMetadata, err := resourceutil.GetAllPropertiesFromResource(connectedResource.Data, connectedResourceID)


What is the connectedResourceID here? Is it the same as connectedResource.ID?

ytimocin · 2025-12-12T19:07:44Z

pkg/portableresources/backend/controller/createorupdateresource.go

+		connectedResourceMetadata, err := resourceutil.GetAllPropertiesFromResource(connectedResource.Data, connectedResourceID)
 		if err != nil {
-			return nil, fmt.Errorf("failed to get properties from connected resource %s: %w", connectedResourceID, err)
+			return nil, fmt.Errorf("failed to get metadata from connected resource %s: %w", connectedResourceID, err)


The function is named GetAllPropertiesFromResource, why did we change the log to say metadata instead of properties?

ytimocin · 2025-12-12T19:09:13Z

pkg/portableresources/backend/controller/createorupdateresource_test.go

 	}
 }
+
+func TestGetAllPropertiesFromResource_Integration(t *testing.T) {


What does the _Integration mean here? What is our naming convention for tests like this? Should we have an underscore right after Test prefix?

ytimocin · 2025-12-12T19:09:48Z

pkg/recipes/engine/engine_test.go

 			"resourceName": "resource1",
 		},
-		ConnectedResourcesProperties: map[string]map[string]any{
+		ConnectedResourcesProperties: map[string]recipes.ConnectedResource{


Do we need to do some assertions here?

ytimocin · 2025-12-12T19:12:11Z

pkg/recipes/recipecontext/types.go

-	Connections map[string]map[string]any `json:"connections,omitempty"`
+	// The key is the connection name, and the value contains the connected resource's metadata and properties.
+	// We enrich the recipe context with this, allowing the recipe to access connected resource info using:
+	// context.resource.connections.[connection-name].properties.[property-name]


Should this be as follows:

Suggested change

// context.resource.connections.[connection-name].properties.[property-name]

// context.resource.connections.[connection-name].properties[property-name]

?

ytimocin · 2025-12-12T19:12:28Z

pkg/recipes/terraform/execute_test.go

 				},
 				ResourceRecipe: &recipes.ResourceMetadata{
-					ConnectedResourcesProperties: map[string]map[string]any{
+					ConnectedResourcesProperties: map[string]recipes.ConnectedResource{


Do we need to do some assertions?

ytimocin · 2025-12-12T19:13:13Z

pkg/resourceutil/utils.go

+// ResourceMetadata represents resource metadata including ID, Name, Type and Properties
+type ResourceMetadata struct {
+	ID         string         `json:"id"`
+	Name       string         `json:"name"`
+	Type       string         `json:"type"`
+	Properties map[string]any `json:"properties,omitempty"`
+}


This may be an important feedback.

ytimocin · 2025-12-12T19:14:33Z

pkg/resourceutil/utils.go

+
+// GetAllPropertiesFromResource extracts the resource metadata including ID, Name, Type and properties
+// by parsing the resource ID and extracting properties.
+func GetAllPropertiesFromResource[P any](resource P, resourceID string) (*ResourceMetadata, error) {


Does the resourceID belong to the resource? If so, why do we need to pass in the resourceID? Can't we just do resource.ID?

If not, is it the target resource or somethin like that?

We need clearer argument names, I believe.

ytimocin · 2025-12-12T19:15:24Z

pkg/resourceutil/utils.go

+	return &ResourceMetadata{
+		ID:         resourceID,
+		Name:       parsedResourceID.Name(),
+		Type:       parsedResourceID.Type(),
+		Properties: properties,
+	}, nil


What is the difference between resourceID and parsedResourceID?

add type, name and id to properties accessible.

9097aa2

Signed-off-by: nithyatsu <[email protected]>

nithyatsu temporarily deployed to functional-tests December 11, 2025 23:42 — with GitHub Actions Inactive

nithyatsu temporarily deployed to publish-bicep December 11, 2025 23:42 — with GitHub Actions Inactive

wip

bae4f72

Signed-off-by: nithyatsu <[email protected]>

nithyatsu temporarily deployed to functional-tests December 12, 2025 00:06 — with GitHub Actions Inactive

nithyatsu temporarily deployed to publish-bicep December 12, 2025 00:06 — with GitHub Actions Inactive

nithyatsu marked this pull request as ready for review December 12, 2025 00:20

nithyatsu requested review from a team as code owners December 12, 2025 00:20

nithyatsu commented Dec 12, 2025

View reviewed changes

brooke-hamilton requested a review from Copilot December 12, 2025 15:30

Copilot started reviewing on behalf of brooke-hamilton December 12, 2025 15:30 View session

Copilot AI reviewed Dec 12, 2025

View reviewed changes

ytimocin requested changes Dec 12, 2025

View reviewed changes

	// ConnectedResource represents a connected resource's metadata and properties
	// ConnectedResource represents a connected resource's metadata and properties.


		// Test the GetAllPropertiesFromResource function
		metadata, err := resourceutil.GetAllPropertiesFromResource(testResource, connectedResourceID)

	// context.resource.connections.[connection-name].properties.[property-name]
	// context.resource.connections.[connection-name].properties[property-name]

add type, name and id to properties accessible. #10932

Are you sure you want to change the base?

add type, name and id to properties accessible. #10932

Conversation

nithyatsu commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Contributor checklist

Uh oh!

codecov bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

radius-functional-tests bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Radius functional test overview

Test Status

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nithyatsu commented Dec 11, 2025 •

edited

Loading

codecov bot commented Dec 11, 2025 •

edited

Loading

radius-functional-tests bot commented Dec 12, 2025 •

edited

Loading