Edges phase2#12474
Closed
nithyatsu wants to merge 7 commits into
Closed
Conversation
…004, Phase 1) Phase 1 of spec 004 (specs/004-graph-dependency-edges/) — two behavior changes to the CLI static graph 'rad app graph app.bicep': 1. Edges from dependsOn — the static graph now surfaces implicit dependency edges from Bicep's compiled dependsOn list alongside the existing properties.connections edges. Same-pair overlap de-dupes with Connection wins (SC-001). Templates that reference another resource only via secretKeyRef now produce a Kind: Dependency edge (SC-002). 2. Radius.Core control-plane types (applications, environments, recipePacks) remain excluded from graph nodes and edge targets — upstream already lands this exclusion under the isExcludedResourceType helper; this branch relies on it rather than duplicating. Wire change on Radius.Core/2025-08-01-preview only: ApplicationGraphConnection gains a required kind: ConnectionKind field (Connection or Dependency). Applications.Core is untouched (FR-016) — its TypeSpec, generated code, handlers, and tests remain byte-identical. Implementation: - typespec/Radius.Core/applications.tsp: add ConnectionKind enum and required kind field. - Regenerated Go models, swagger, and Bicep type extensions via 'make generate'. - New pkg/graph/edges/ package: Resource, Edge types, and ExtractEdges primitive (connection + dependsOn + exclusion + Connection-wins de-dup + reciprocal mirroring + deterministic sort). Zero imports from pkg/cli/, pkg/corerp/, or net/http (verified via 'go list -deps'), keeping Phase 2 runtime reuse a wiring change. - pkg/cli/graph/modeled.go: BuildModeledGraph builds []edges.Resource in its resource loop and calls edges.ExtractEdges(_, excludeResourceTypes). A new applyEdges helper attaches each Edge to the owning resource's Connections slice via Edge.Owner()/Peer(). A new resolveConnectionSources helper pre-resolves properties.connections[*].source into canonical IDs. The former inline outboundConnections and addInboundConnections helpers are removed. All existing security work (secureString/secureObject parameter tracing, sensitiveKeyBlocklist, resolveGraphProperties, dropOnGraph) and icon work (collectStaticGraphIcons, resolveIconHash, includeIcons parameter) is preserved verbatim. - pkg/corerp/frontend/controller/applications/v20250801preview/graph_util.go: sets Kind: Connection on every emitted ApplicationGraphConnection (two sites — outbound converter and inbound mirror). Phase 2 will start emitting Kind: Dependency here via caller-supplied dependsOnEdges on GetGraphRequest. - pkg/cli/graph/modeled_test.go: adds SC-001 (ConnectionWinsOverDependencyOnRabbitMQShape) and SC-002 (DependencyEdgeSurfacesWithoutConnectionsBlock). - docs/architecture/application-graph.md: new 'Edge Sources and Kind' subsection; class diagram gains kind + ConnectionKind. - specs/004-graph-dependency-edges/: full Spec Kit folder with spec.md, plan.md, tasks.md, research.md, data-model.md, contracts/, quickstart.md, checklists/. Phase 2 (out of scope for this PR): server-side dependency edges via a new caller-supplied dependsOnEdges field on GetGraphRequest — the client (post 'bicep build') sends the compiled list to the server, which merges it through the same pkg/graph/edges/ primitive. Related follow-up: #12467 (evaluate simple ARM name expressions in the static graph so composed resource names render as literals rather than [format(...)] expressions). Success criteria verified: SC-001 (TestBuildModeledGraph_ConnectionWinsOverDependencyOnRabbitMQShape), SC-002 (TestBuildModeledGraph_DependencyEdgeSurfacesWithoutConnectionsBlock + end-to-end fixture), SC-003 (Applications.Core byte-identical; Radius.Core preview handler tests green), SC-004 ('go list -deps ./pkg/graph/edges/...' has no pkg/cli, pkg/corerp, or net/http imports), SC-005 (excludeResourceTypes in pkg/cli/graph/modeled.go is a single source location). Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
The Phase 1 pkg/graph/edges package introduced parallel Resource / Edge / kind / direction types that shadowed the wire model (ApplicationGraphConnection, ConnectionKind, Direction). That parallel data model earned little in exchange for the extra cognitive load — both callers convert to and from the wire type anyway. This commit collapses the primitive to one function that operates directly on the wire type: MergeDependencyEdges(graph, incoming map[string][]*ApplicationGraphConnection, excluded). The incoming shape matches the Radius.Core/2025-08-01-preview GetGraphRequest.dependsOnEdges wire field (added in the next commit) so the server can hand the request input straight through. The CLI static-graph builder constructs an equivalent map from Bicep's dependsOn list via a new exported ExtractDependsOnEdges helper. Filtering rules preserved from Phase 1: source and target must be in the graph, source and target types must not be excluded, and any (source, target) pair that already has a Kind: Connection outbound edge on source is dropped (Connection wins). Layering: pkg/graph/edges now imports pkg/corerp/api/v20250801preview. This is fine — pkg/graph/edges was neutral before to avoid pulling in CLI packages; the wire package is not a CLI package. Server code importing pkg/graph/edges is allowed (no cli imports transitively). - pkg/graph/edges/doc.go, edges.go: rewritten. No more Resource, Edge, Kind*/Direction* constants, or Owner/Peer helpers. Only MergeDependencyEdges + a small internal sort helper. - pkg/graph/edges/edges_test.go: rewritten. Eight table-driven cases covering nil/empty inputs, happy path with deterministic sort, excluded source, excluded target, unknown endpoints, Connection wins, duplicate incoming pairs, and malformed input entries. - pkg/cli/graph/modeled.go: BuildModeledGraph reworked to use the new primitive. outboundConnections and addInboundConnections restored inline (now setting Kind: Connection). New exported ExtractDependsOnEdges helper builds the map[sourceID]->[]edge shape from a compiled ARM template so the CLI enriched-graph mode (later commit on this branch) can reuse it. Removed applyEdges and resolveConnectionSources. All existing pkg/cli/graph tests still pass — Phase 1's SC-001 (Connection wins) and SC-002 (Dependency-only edge) integration cases exercise the new BuildModeledGraph flow end to end. Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Add an optional dependsOnEdges field on the Radius.Core 2025-08-01-preview GetGraphRequest so callers (initially `rad app graph -a <name> <app.bicep>`) can supply Kind: Dependency edges the server cannot otherwise see. The field is a map keyed by source resource ID whose values are ApplicationGraphConnection arrays; entries are expected to have direction: Outbound and kind: Dependency. Wire prep only — no server or CLI behavior change in this commit. The Applications.Core stable API is untouched. Regenerated Go models/serde, OpenAPI, and Bicep types from the TypeSpec edit. Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Refactor the getGraph request reader to parse the entire GetGraphRequest body once (readGraphRequest) instead of only pulling out IncludeIcons, then plumb the new DependsOnEdges map through computeGraphPayload into computeGraph. Add a server-side dependsOnExclusionSet that mirrors excludeResourceTypes in pkg/cli/graph/modeled.go (Applications.Core and Radius.Core applications/environments plus Radius.Core recipePacks), and call edges.MergeDependencyEdges as the tail step of computeGraph.
Callers that do not send a body — or that omit dependsOnEdges — get the same connection-only graph as before. When a caller supplies edges, the merge helper's existing rules apply: excluded types drop, unknown endpoints drop, and any (source, target) pair already present as Kind: Connection wins over the caller-supplied Dependency.
The parallel Applications.Core pipeline in pkg/corerp/frontend/controller/applications/{getgraph,graph_util}.go is not touched — the stable API keeps its unchanged behavior.
Tests: includeicons_test.go retargets Test_readGraphRequest and adds a DependsOnEdges parsing case. graph_util_test.go (new) covers plumbing end-to-end through computeGraph and pins dependsOnExclusionSet so drift with the CLI-side excludeResourceTypes is caught in CI.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Extend the preview `rad app graph` command to accept an optional positional path to the application's app.bicep alongside -a/--application. When both are provided, the CLI compiles the template locally, runs cligraph.ExtractDependsOnEdges over the resulting ARM JSON, and attaches the resulting map to GetGraphRequest.DependsOnEdges. The server merges those edges into the deployed graph as Kind: Dependency. Existing invocation shapes are unchanged: `-a name` alone and a single name-only positional argument still return the connection-only deployed graph. A bicep-file positional argument without -a is rejected because the enriched mode still needs to know which deployed application to fetch. The stable `rad app graph` command (v20231001preview) is not touched — the new mode only makes sense against the preview API surface where DependsOnEdges exists. Tests: validation cases cover the two enriched-mode shapes (accepted with -a name plus a .bicep path, rejected without -a). A run-time test uses a gomock'd Bicep to feed a minimal dependsOn template through the runner and asserts the resulting DependsOnEdges reach the request body. A second test covers the clierrors wrapping of Bicep compile failures. Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Update docs/architecture/application-graph.md to reflect the Phase 2 runtime merge and CLI enriched mode: - Edge Sources and Kind: the server-side runtime handler now merges an optional dependsOnEdges map from GetGraphRequest through edges.MergeDependencyEdges, and a third producer (CLI enriched deployed mode via rad app graph -a <name> --preview <path>.bicep) joins the static graph builder. - Shared exclusion set: note that dependsOnExclusionSet in the preview graph_util.go is kept in sync with excludeResourceTypes in pkg/cli/graph/modeled.go by a pinning test. - Radius.Core preview wire-model table: extend the Request Body row to cover dependsOnEdges alongside includeIcons. - Icon-enrichment mermaid: insert the MergeDependencyEdges step between computeGraphPayload and namespace collection. - CLI examples: add the enriched invocation `rad app graph -a my-app --preview ./app.bicep`. - Class diagram: add the dependsOnEdges field on GetGraphRequest. Key Components row for the preview runner also notes the new enriched-mode acceptance of an app.bicep path. Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
|
This PR requires exactly 1 of the following labels: pr:standard, pr:important. Label descriptions:
@nithyatsu, please add the appropriate label to this PR before merging. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12474 +/- ##
==========================================
+ Coverage 53.42% 53.74% +0.31%
==========================================
Files 762 763 +1
Lines 50246 50381 +135
==========================================
+ Hits 26843 27076 +233
+ Misses 20879 20756 -123
- Partials 2524 2549 +25 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Summary
Reason for change
Fixes #
How to test
File change summary