add resource specific property bag to graph#12138
Open
nithyatsu wants to merge 7 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12138 +/- ##
==========================================
+ Coverage 52.30% 52.32% +0.01%
==========================================
Files 738 738
Lines 47301 47313 +12
==========================================
+ Hits 24743 24755 +12
Misses 20192 20192
Partials 2366 2366 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
nithyatsu
force-pushed
the
addpropertybag
branch
2 times, most recently
from
b15e66d to
6a8353d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Application Graph response model by adding an optional properties map on each ApplicationGraphResource, intended to expose resource-type-specific property bags (with duplicates omitted) across Applications.Core and Radius.Core API surfaces.
Changes:
- Add
properties?: Record<unknown>toApplicationGraphResourceTypeSpec models and propagate to Swagger/OpenAPI. - Populate
ApplicationGraphResource.Propertiesin graph computation, with deduplication logic to omit fields already surfaced elsewhere. - Update graph JSON fixtures and add/extend unit + functional tests to validate the new
propertiesoutput.
Doc impact (advisory):
docs/architecture/application-graph.mddocuments the graph data model and currently omitsdiffHash/properties; it likely needs an update to reflect the expanded response shape.
Reviewed changes
Copilot reviewed 9 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| typespec/Radius.Core/applications.tsp | Adds properties field (free-form map) to the Radius.Core graph resource model. |
| typespec/Applications.Core/applications.tsp | Adds properties field (free-form map) to the Applications.Core graph resource model. |
| swagger/specification/radius/resource-manager/Radius.Core/preview/2025-08-01-preview/openapi.json | Reflects the new properties field in the Radius.Core OpenAPI schema. |
| swagger/specification/applications/resource-manager/Applications.Core/preview/2023-10-01-preview/openapi.json | Reflects the new properties field in the Applications.Core OpenAPI schema. |
| pkg/corerp/api/v20250801preview/zz_generated_models.go | Adds Properties map[string]any to the generated Radius.Core graph resource Go model. |
| pkg/corerp/api/v20250801preview/zz_generated_models_serde.go | Updates JSON marshal/unmarshal to include properties for Radius.Core graph resources. |
| pkg/corerp/api/v20231001preview/zz_generated_models.go | Adds Properties map[string]any to the generated Applications.Core graph resource Go model. |
| pkg/corerp/api/v20231001preview/zz_generated_models_serde.go | Updates JSON marshal/unmarshal to include properties for Applications.Core graph resources. |
| pkg/corerp/frontend/controller/applications/graph_util.go | Populates graph resource Properties and introduces filtering/deduplication helper. |
| pkg/corerp/frontend/controller/applications/graph_util_test.go | Adds unit tests for property projection/deduplication behavior. |
| pkg/corerp/frontend/controller/applications/testdata/graph-app-gw-out.json | Updates expected graph JSON output to include properties. |
| pkg/corerp/frontend/controller/applications/testdata/graph-app-directroute-out.json | Updates expected graph JSON output to include properties. |
| test/functional-portable/corerp/noncloud/resources/application_test.go | Extends functional test to validate properties presence/shape for container resources. |
Files not reviewed (4)
- pkg/corerp/api/v20231001preview/zz_generated_models.go: Generated file
- pkg/corerp/api/v20231001preview/zz_generated_models_serde.go: Generated file
- pkg/corerp/api/v20250801preview/zz_generated_models.go: Generated file
- pkg/corerp/api/v20250801preview/zz_generated_models_serde.go: Generated file
nithyatsu
force-pushed
the
addpropertybag
branch
from
3421888 to
30f1675
Compare
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated 7 comments.
Files not reviewed (4)
- pkg/corerp/api/v20231001preview/zz_generated_models.go: Generated file
- pkg/corerp/api/v20231001preview/zz_generated_models_serde.go: Generated file
- pkg/corerp/api/v20250801preview/zz_generated_models.go: Generated file
- pkg/corerp/api/v20250801preview/zz_generated_models_serde.go: Generated file
Comment on lines
+119
to
+120
| @doc("Resource-type-specific properties of the resource as returned by its resource provider, with secrets redacted. The shape of this map varies by `type` (e.g. an `Applications.Core/containers` resource exposes different keys than `Applications.Datastores/redisCaches` or any user-defined resource type) and matches the `properties` returned by that resource's GET response. Fields already represented elsewhere in this model (id, type, name, provisioningState, status.outputResources, connections, routes) are omitted. Use `diffHash` rather than a byte-wise compare of this map to detect change.") | ||
| properties?: Record<unknown>; |
Comment on lines
+113
to
+114
| @doc("Resource-type-specific properties of the resource as returned by its resource provider, with secrets redacted. The shape of this map varies by `type` (e.g. a `Radius.Compute/containers` resource exposes different keys than `Radius.Datastores/redisCaches` or any user-defined resource type) and matches the `properties` returned by that resource's GET response. Fields already represented elsewhere in this model (id, type, name, provisioningState, status.outputResources, connections, routes) are omitted. Use `diffHash` rather than a byte-wise compare of this map to detect change.") | ||
| properties?: Record<unknown>; |
Comment on lines
+450
to
+452
| // existingKeys lists property keys whose value is surfaced as a | ||
| // first-class field on ApplicationGraphResource and therefore omitted from the projected | ||
| // Properties bag to avoid duplication. |
| @@ -260,3 +260,67 @@ func Test_getAPIVersionForResourceType_Validation(t *testing.T) { | |||
| }) | |||
| } | |||
| } | |||
|
|
|||
| func Test_projectGraphProperties(t *testing.T) { | |||
Comment on lines
+142
to
+148
| // Graft the actual Properties onto the expected resource | ||
| // so the struct-level equality check below succeeds for | ||
| // the rest of the fields without having to encode the | ||
| // environment-specific property bag in the fixture. | ||
| if i < len(expected) { | ||
| expected[i].Properties = r.Properties | ||
| } |
Comment on lines
+1341
to
1345
| "properties": { | ||
| "type": "object", | ||
| "description": "Resource-type-specific properties of the resource as returned by its resource provider, with secrets redacted. The shape of this map varies by `type` (e.g. a `Radius.Compute/containers` resource exposes different keys than `Radius.Datastores/redisCaches` or any user-defined resource type) and matches the `properties` returned by that resource's GET response. Fields already represented elsewhere in this model (id, type, name, provisioningState, status.outputResources, connections, routes) are omitted. Use `diffHash` rather than a byte-wise compare of this map to detect change.", | ||
| "additionalProperties": {} | ||
| } |
Comment on lines
+2330
to
2334
| "properties": { | ||
| "type": "object", | ||
| "description": "Resource-type-specific properties of the resource as returned by its resource provider, with secrets redacted. The shape of this map varies by `type` (e.g. an `Applications.Core/containers` resource exposes different keys than `Applications.Datastores/redisCaches` or any user-defined resource type) and matches the `properties` returned by that resource's GET response. Fields already represented elsewhere in this model (id, type, name, provisioningState, status.outputResources, connections, routes) are omitted. Use `diffHash` rather than a byte-wise compare of this map to detect change.", | ||
| "additionalProperties": {} | ||
| } |
brooke-hamilton
requested changes
Jun 19, 2026
brooke-hamilton
left a comment
brooke-hamilton
left a comment
Member
There was a problem hiding this comment.
This is looking good. These two things are in addition to the Copilot findings above.
| // Properties bag to avoid duplication. | ||
| var existingKeys = map[string]struct{}{ | ||
| "provisioningState": {}, | ||
| "connections": {}, |
Member
There was a problem hiding this comment.
The routes and status doc mismatches flagged above share one root cause: existingKeys and the documented "omitted" list were never reconciled. Please make a single, consistent decision and apply it everywhere:
statusis dropped entirely here (confirmed intentional for sensitivity), but every doc says onlystatus.outputResourcesis omitted.routesis documented as omitted but is not inexistingKeys, so it stays in the bag.
Reconcile existingKeys with the doc text and fix all six occurrences of the "omitted fields" sentence together — both Go struct comments (v20231001preview, v20250801preview), both .tsp files, and both openapi.json files — so the API contract is internally consistent.
|
|
||
| applicationGraphResource.Connections = connections | ||
| applicationGraphResource.OutputResources = outputResourcesFromAPIData(resource) | ||
| applicationGraphResource.Properties = getResourceTypeSpecificProperties(resource.Properties) |
Member
There was a problem hiding this comment.
The docs for the new properties field say "with secrets redacted," but getResourceTypeSpecificProperties performs no redaction here — it relies on upstream LIST redaction (e.g. ListResourcesWithRedaction for dynamic-rp). For user-defined types this holds, and the graph caller can already GET these same properties, so this is not a new disclosure. Could you confirm the core-RP container/gateway LIST payloads carry no inline secret values? If so, no code change is needed — just a note so the "secrets redacted" claim is backed by where redaction actually happens.
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
This pull request introduces a new
propertiesfield to theApplicationGraphResourcemodel across multiple API versions and layers (Go structs, OpenAPI specs, TypeSpec definitions, and JSON output). Thepropertiesfield contains resource-type-specific properties as returned by the resource provider, with sensitive information redacted and redundant fields omitted. The implementation includes logic to deduplicate these properties, updates serialization and deserialization, and adds comprehensive tests and documentation updates.Type of change
Fixes: #12124