Repository navigation
[release/17.0] Harden Go SDK runtime and complete typed API export metadata - #22123
Jose Perez Rodriguez (joperezr) merged 1 commit into
Conversation
## Description Generated Go AppHosts can run callbacks concurrently with the main builder chain without racing the sticky error or lazy collection accessors. Invalid request arguments and unencodable callback results now fail their individual calls instead of disconnecting every pending operation. Callback decoding errors and `runtime.Goexit` produce explicit JSON-RPC errors; message headers and cancellation requests are bounded. The Go API export also includes structured `type` and `optional` metadata for every field and constant. DTO optionality comes from ATS metadata, not the generated `omitempty` tag or pointer spelling. This is additive to schema version 1 and includes options, runtime fields, enum constants, and exported value leaves. Export snapshots and the output-format documentation are updated. Related to #21204, #21212, and #21115. This does not close either follow-up issue. The remaining structured Kubernetes manifest work is separate in #21824. ### User-facing usage Collection serialization preserves its existing signature; failures are now observable through the additive `Err` methods: ```go payload := resource.GetTags().ToJSON() if err := resource.GetTags().Err(); err != nil { return err } _ = payload ``` `aspire sdk export --language go --package Name@Version` now emits fields such as: ```json { "name": "Mode", "type": "*TestPersistenceMode", "optional": true } ``` Cancellation watchers are armed only after the invocation carrying their IDs has entered the FIFO write queue. The cancellation RPC has a five-second timeout; a canceled call still returns through its own context without waiting for that RPC. ### Validation Commands ran on Windows with the restored repository SDK and an isolated Go 1.27.3 toolchain added to `PATH`. Linux race checks used Go 1.26.10 and GCC from the existing local `aspire-go-e2e-6972:latest` image. | Command | Result | | --- | --- | | `.\restore.cmd` | Passed; 17 dependency audit warnings, no errors. | | `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 130 tests, no failures or skips. Reviewed and accepted generated Go/JSON snapshot changes. | | `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-method "*.GeneratedRuntime_PassesGoRuntimeHardeningTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed after adding the final actual-getter-failure assertions. | | `go test -race -timeout 60s -v .` | Passed in a generated module extracted from the Go snapshot, with `runtime_test.go`, `transport_hardening_test.go`, and `decode_test.go` from the C# tests. Includes a callback and main chaining on the same captured generated builder, encoding failures, callback errors/Goexit, header bounds, and cancellation ordering/timeout cleanup. | | `go test -race -timeout 60s -run TestCallbackAndMainCanChainOnCapturedBuilder .` | Failed as expected against the original main snapshot with data races; the same regression passes with this branch. | | `dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 698 tests, 1 skipped. Includes integration scanning and Go export/codegen coverage. | | `dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.SdkExportCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` | Passed: 31 tests. | | `dotnet build src\Aspire.Cli\Aspire.Cli.csproj --configuration Release /p:SkipNativeBuild=true` | Passed, with the existing ILCompiler audit warning. Managed Release build, not native AOT. | | `.\.dotnet\dotnet.exe artifacts\bin\Aspire.Cli\Release\net11.0\aspire.dll sdk export --help` | Passed; help lists Go as an export language. | | `git diff --check` | Passed. | ### Remaining policy and execution gaps - Callback registry release still needs a host release signal and a cross-language ownership contract. This PR does not unregister callbacks when a registration capability returns, because the host can invoke them later. - Disconnect-driven `os.Exit(1)` is unchanged. Choosing an observable shutdown contract requires a cross-language decision; no GA lifecycle guarantee is introduced. - The reported required-token path is currently unreachable because `SplitParameters` places all cancellation tokens in Go options. Both token-emission paths select the supplied token's context. - A bundled current-main CLI export measuring Redis/Azure Storage XML-summary coverage was not run. No documentation-copy policy was changed based solely on project-built behavior. - No new grouped export view was introduced; consumers still have `capabilityId` for grouping. The aspire.dev consumer and its grouping decision are outside this product-only change. - Native AOT publishing, the full CLI suite, a many-integration live AppHost, polyglot E2E, macOS, and the complete CI matrix were not run. Go feature-flag-off validation is obsolete because #21183 removed that flag. ### Security considerations This changes handling of AppHost JSON-RPC messages and callback payloads. Headers are limited to 16 entries and 8 KiB before body allocation, in addition to the existing 64 MiB body cap. Invalid typed callback arguments are rejected before user code is called; decoding no longer silently substitutes invalid DTO fields. No authentication, listener, or permission policy changes are made. A formal threat model/security review has not been performed. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [x] Yes - If yes, did you have an API Review for it? - [ ] Yes - [x] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [x] No - [ ] No - Does the change make any security assumptions or guarantees? - [x] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [x] No - [ ] No The additive APIs are Go `List.Err`/`Dict.Err` and JSON export metadata; no public C# API was added. Go documentation comments describe the error contract. Triple-slash XML elements do not apply to these Go members. The fixes in this PR are complete; the explicitly deferred cross-language policy and bundle-validation items remain tracked in the related issues. --------- Co-authored-by: IEvangelist <7679720+IEvangelist@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> (cherry picked from commit e7ff435)
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 22123Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 22123" |
Tests selector10 / 101 PR test projects · 4 PR jobs, from 16 changed files. Selected PR test projects (10 / 101)
Selected PR jobs (4)
How these were chosen — grouped by what changed
🔧 🔧 📦 affected project 🔧 🔧 🔧 🔧 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
🟡 Changes recommended
DTO callback shims still lack token-aware decoding, causing both omitted CancellationToken.None arguments and nonempty token IDs to be rejected instead of invoking the callback.
1 open issue
What changed in this PR
Backports #21827 to release/17.0, hardening the generated Go SDK runtime and adding typed export metadata for Aspire’s Go AppHost Preview. This is the prerequisite for the subsequent Go error-propagation, API, and starter-template backports.
Changes:
- Synchronizes wrapper state and isolates callback and request-encoding failures.
- Improves cancellation ordering and cleanup, preserves early host cancellations, and bounds message headers.
- Adds field and constant type/optionality metadata, with regression tests, snapshots, and documentation.
| File | Description |
|---|---|
tests/Aspire.Hosting.RemoteHost.Tests/Snapshots/KubernetesManifestTests.WithField_GeneratedSdkAcceptsStructuredValues_language=Go.verified.go |
Updates generated error-access checks. |
tests/Aspire.Hosting.RemoteHost.Tests/CancellationTokenRegistryTests.cs |
Tests early cancellation and pending-marker limits. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/GoApiExportTests.GoOnlySurface.verified.json |
Records typed field and constant metadata. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/GoApiExportTests.FocusedApiExport.verified.json |
Updates focused export expectations. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/EscapedMetadataGeneratedAspire.verified.go |
Records synchronized generated accessors. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/Snapshots/AtsGeneratedAspire.verified.go |
Updates generated SDK and runtime expectations. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/GoApiExportTests.cs |
Verifies declared types and metadata-based optionality. |
tests/Aspire.Hosting.CodeGeneration.Go.Tests/AtsGoCodeGeneratorTests.cs |
Adds runtime regressions and Linux race detection. |
src/Aspire.Hosting.RemoteHost/CancellationTokenRegistry.cs |
Preserves early cancellations with bounded storage. |
src/Aspire.Hosting.CodeGeneration.Go/Resources/transport.go |
Hardens callbacks, encoding, cancellation, and headers. |
src/Aspire.Hosting.CodeGeneration.Go/Resources/base.go |
Synchronizes errors and tightens callback decoding. |
src/Aspire.Hosting.CodeGeneration.Go/GoApiModel.cs |
Adds field and constant metadata properties. |
src/Aspire.Hosting.CodeGeneration.Go/GoApiExportWriter.cs |
Serializes type and optionality metadata. |
src/Aspire.Hosting.CodeGeneration.Go/AtsGoCodeGenerator.cs |
Emits synchronized access and token-aware callbacks. |
src/Aspire.Hosting.CodeGeneration.Go/AtsGoCodeGenerator.ApiProjection.cs |
Projects declared types and ATS optionality. |
docs/specs/cli-output-formats.md |
Documents additive Go export metadata. |
Review effort: Balanced
| if (clientExpression is not null && IsCancellationTokenTypeId(type.TypeId)) | ||
| { | ||
| return $"callbackCancellationTokenArg({clientExpression}, args, {index})"; | ||
| } |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
9a97ab8
into
release/17.0
|
✅ No documentation update needed. Step 5 branch: excluded → base_branch_is_release, title_release_prefix, body_backport_marker. This is a backport PR (title prefixed |
## Description Consolidate the Go AppHost release backports into one PR against `release/17.0`, so shipping the complete Go changes requires one merge rather than three remaining dependent backport merges. David Pine's [November 11 release-order message](https://teams.microsoft.com/l/message/19:86b689da3ce444f8952d0b79157f7562@thread.tacv2/1794418391504?tenantId=72f988bf-86f1-41af-91ab-2d7cd011db47&groupId=f2e0b0b6-f130-46fa-8ec6-2ef4a7168ed2&parentMessageId=1794418391504&teamName=Aspire&channelName=Tactics%20And%20Shiproom&createdTime=1794418391504) opens with "These backports, in this order to get Go AppHost release ready:" and lists #22123, #22057, #22064, and #22121 in that order. The existing backport descriptions specify the same sequence. At preparation time, `release/17.0` points to `9a97ab889bfb731de74ead0280be279ffdd000bf`, the already-merged #22123. This branch first reverts that commit, then reapplies the four original source commits with `git cherry-pick -x` in Pine's order: 1. #21827 / #22123: `e7ff435302acb88f9cfe7c9a61e3633e0f553f08` - harden the Go SDK runtime and complete typed API export metadata. 2. #21969 / #22057: `031536bc9eea22ee9ce2cc1d31bfd30d0c14cfd2` - propagate child resource errors through Build. 3. #21976 / #22064: `cd9e0e8d159d20f3cf7e64eda2415dcc3d40c168` - make generated Go SDK APIs idiomatic. 4. #21874 / #22121: `209d9226e0463b638a5d68a034505e874cae9091` - add the React frontend to the Go starter. The revert exists only on this consolidation branch; the shared release branch was not changed. All four source patches are unchanged (verified with matching patch IDs), with their original source references retained in commit messages. Earlier release prerequisites remain untouched. This PR supersedes #22057, #22064, and #22121, which have been closed with comments linking here. Existing review discussions remain available on those PRs. ### User-facing usage The starter provides a Go API, React frontend, and optional Redis: ```console aspire new aspire-go-starter --non-interactive --use-redis-cache false ``` Use `--use-redis-cache true` to include Redis. Generated projects contain `apphost.go`, `api/`, and `frontend/`. ### Breaking changes The #21976 source change updates generated Go SDK signatures and names. Existing Go AppHosts need to regenerate their SDK and update affected calls. Shared ATS metadata changes are also included. ### Screenshots / Recordings > **This PR includes UI changes.** Please add screenshots or screen recordings so reviewers can evaluate the visual changes without running locally. > > - For before/after comparisons, place them side-by-side or label them clearly. > - For interactive changes (animations, transitions, new flows), prefer a short screen recording (GIF or video). > - If you cannot capture visuals now, note what scenario to test and mark this section as TODO. <!-- Add screenshots/recordings here --> TODO: Visual evidence was not captured. Run the generated Go starter and inspect its React frontend, weather data through the Vite proxy, and optional Redis scenario. ### Security considerations These are unchanged source backports, not new security fixes. They include Go transport/callback/cancellation handling and starter process, proxy, and deployment behavior. Existing review comments on #22057 still need disposition; consolidation does not resolve or invalidate those comments. No new threat-model or security-review result is claimed here. Fixes # (issue) ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - [ ] No. Follow-up changes expected. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - [ ] No - Did you add public API? - [x] Yes - If yes, did you have an API Review for it? - [ ] Yes - [ ] No - Did you add `<remarks />` and `<code />` elements on your triple slash comments? - [ ] Yes - [ ] No - [ ] No - Does the change make any security assumptions or guarantees? - [x] Yes - If yes, have you done a threat model and had a security review? - [ ] Yes - [ ] No - [ ] No ## Customer Impact Go AppHost users receive the compatible generated SDK, child-resource error propagation, runtime hardening, and a complete Go/React starter together, without having to assemble dependent backports. ## Testing Validation was performed on the consolidated release-based head `065867bf777dd1e755e2718e8ac979a83fd37613`, not inherited from the source PRs. Test processes used the repo-local `artifacts\tools\go\bin` on PATH (`go version go1.27.4 windows/amd64`). - `.\restore.cmd` - passed; pre-existing NuGet vulnerability warnings were reported. - `dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **137 tests, 0 skipped**, including Go build/vet/runtime and gofmt-dependent tests. An intermediate rerun found generated frontend `node_modules` copied into the scaffold test fixture; removing only generated `node_modules`/`dist` artifacts restored the clean run without source changes. - `dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-method "*.GoStarterTemplate*" --filter-method "*.GoStarterHonorsRedisChoice*" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **10 tests**. - `dotnet test --project tests\Aspire.Hosting.Go.Tests\Aspire.Hosting.Go.Tests.csproj --no-launch-profile -- --filter-method "*.WithModTidyIsIdempotent" --filter-method "*.WithModTidyCreatesChildResource" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **2 tests**. - `dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-class "*.CancellationTokenRegistryTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - passed, **14 tests**. An earlier attempted filter named `RemoteCancellationRegistryTests` selected zero tests; the corrected run above is the validation result. - `dotnet build tests\Aspire.Cli.EndToEnd.Tests\Aspire.Cli.EndToEnd.Tests.csproj --no-restore` - passed, **0 warnings/errors**. E2E execution was not run on this Windows host. - `npm ci` followed by `npm run build` in `src\Aspire.Cli\Templating\Templates\go-starter\frontend` - passed; TypeScript and Vite production build succeeded. Generated build artifacts were removed afterward. - `dotnet run --project tools\SelectTests -- --from origin/release/17.0 --to HEAD --explain` - passed; **0 unattributed changed files**. `.gitattributes` selects all PR test projects/jobs, including starter validation and polyglot. - `dotnet test --project tests\Infrastructure.Tests\Infrastructure.Tests.csproj --no-launch-profile -- --filter-namespace "Infrastructure.Tests.TestTriggerMap" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"` - **364 passed, 22 failed**. Failures are Windows test-harness limitations: Bash subprocesses cannot resolve backslash-delimited Windows script paths, and five git-fixture cleanup cases throw `UnauthorizedAccessException` deleting git objects. This suite is not claimed green; Linux/CI validation is still required. No test or script source was changed. - `git diff --check origin/release/17.0...HEAD` - passed. Starter runtime, Docker deployment, screenshots, full polyglot compatibility suites, and GitHub CI execution were not validated locally. ## Risk Medium. The source changes include shared ATS metadata, breaking generated Go SDK changes, transport/error handling, and starter/frontend deployment behavior. Consolidation preserves the source patches but does not replace review or CI validation. ## Regression? Unknown: the source material does not identify a regressed release. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Combines the stacked dashboard backports into one PR for `release/17.0`. The commits are cherry-picked in stack order, one per source PR, onto the current `release/17.0` tip: 1. #22125: Store dashboard tags, resource order and metric pins in browser local storage (#21840) 2. #22126: Add dashboard resource-selector dropdown (#21861) 3. #22127: Rename the WithTags trait from Aspire_Dashboard_Tag to Aspire_Tag (#21881) 4. #22128: Restore single-line compact rows in the dashboard resource list (#21880) 5. #22129: Allocate the next dashboard run ID when two dashboards start in the same millisecond (#21913) 6. #22130: Simplify the dashboard around its two main jobs (#21943) 7. #22131: Return 400 for OTLP JSON requests with malformed ids (#21436) 8. #22132: Address deferred review feedback for the modern dashboard (#21766) ## Conflict resolution The stack was cut from an older `release/17.0`. Since then, #22123 (Harden Go SDK runtime) landed and regenerated `TwoPassScanningGeneratedAspire.verified.go`. That conflicted with #21766, which replaces `WithDashboardPosition` with `WithDisplayOrder`. The resolution drops `WithDashboardPosition` and regenerates the snapshot with the current Go generator, so `WithDisplayOrder` now uses the hardened `s.Err()` pattern. The other seven commits applied cleanly. ## Validation All `Aspire.Hosting.CodeGeneration.*.Tests` projects pass locally (Go, Java, Python, Rust, TypeScript). --------- Co-authored-by: Sébastien Ros <1165805+sebastienros@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Fowler <davidfowl@gmail.com> Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Maddy Montaquila <maddyleger1@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Maddy Montaquila <maddy@MadBook-Pro-20.local> Co-authored-by: Taha Koçal <tahakocalgs@gmail.com> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>

Description
Backport of #21827 to
release/17.0, cherry-picked from e7ff435 without source modifications. Fixes Go callback concurrency, per-call transport error handling, cancellation ordering, and missing typed API-export metadata.This PR targets
release/17.0directly with a single commit and no abandoned-stack commits. It replaces #22122, whose head wasmain. Merge this first, then #22057, then #22064, then #22121. Those PRs also targetrelease/17.0directly and temporarily include their prerequisite commits; they are not an abandoned stack.Related issues: #21204, #21212, #21115. This does not close those broader follow-ups.
Checklist
<remarks />and<code />elements on your triple slash comments?Additive APIs are Go collection
Errmethods and API-export metadata, not public C# APIs. JSON-RPC payload/header handling changes; no authentication or permission-policy changes. Review statuses above follow the source PR.Customer Impact
Go AppHosts can race when callbacks and builder chains share wrappers. Invalid call payloads can disconnect unrelated pending operations. This backport isolates those failures, synchronizes shared state, and exposes structured field and constant metadata in Go API exports.
Testing
Validated on this release-based branch:
.\restore.cmd— passed.dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 128 passed; two scaffold newline checks initially failed because three unchanged starter files retained CRLF from an earlier checkout despiteeol=lf. Confirmed these inputs and the scaffold tests are identical to the release baseline; normalized only working-tree line endings, with no committed file changes.dotnet test --project tests\Aspire.Hosting.CodeGeneration.Go.Tests\Aspire.Hosting.CodeGeneration.Go.Tests.csproj --no-launch-profile -- --filter-method "*.GoStarterFiles_UseLfLineEndings" --filter-method "*.ScaffoldedAndStarterSources_AreGofmtClean" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— both previously failing checks passed after normalization. All 130 tests have passed across these runs.dotnet test --project tests\Aspire.Hosting.RemoteHost.Tests\Aspire.Hosting.RemoteHost.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 713 passed, 1 skipped, 0 failed.dotnet test --project tests\Aspire.Cli.Tests\Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.SdkExportCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"— 31 passed, 0 failed.git diff --check origin/release/17.0..HEAD— passed.Linux race checks, native AOT, and full cross-platform CI were not rerun for this backport; source-PR validation is not claimed as release-branch validation.
Risk
Medium: changes Go runtime concurrency, transport/cancellation behavior, and API-export metadata. Successful-call signatures and the existing export schema version are retained; focused runtime and export tests cover the changed behavior.
Regression?
Unknown: the source PR does not identify a regressing release or commit.