Report actionable errors when DotnetProjectResource is published - #19190
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19190Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19190" |
github-actions
Bot
added
the
needs-area-label
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Adds actionable publish/deploy failures for unsupported DotnetProjectResource publishing.
Changes:
- Adds pipeline and manifest validation.
- Documents supported publishing alternatives.
- Adds publish/deploy regression tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Aspire.Hosting.Dotnet/DotnetProjectResource.cs |
Adds publishing validation and guidance. |
src/Aspire.Hosting.Dotnet/README.md |
Documents publishing limitations and alternatives. |
tests/Aspire.Hosting.Dotnet.Tests/DotnetProjectResourceTests.cs |
Tests publishing failures and opt-outs. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Karol Zadora-Przylecki (karolz-ms)
left a comment
Karol Zadora-Przylecki (karolz-ms)
left a comment
There was a problem hiding this comment.
Found 2 correctness issues: publish/deploy validation is not ordered before all pipeline work, and explicit custom manifest callbacks are rejected.
PR Testing ReportPR Information
The repo container runner was selected initially, but Docker is not installed on the host. Testing continued locally after explicit approval. Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR artifact and targeted unit validationObjective: Verify the downloaded artifact and changed source both match the current PR head, then execute the focused unit coverage. Coverage Type: Artifact integrity and unit validation Status: ✅ Passed Steps:
Evidence:
Observations:
Scenario 2: Run-mode regressionObjective: Confirm Coverage Type: Happy path Status: ✅ Passed Steps:
Evidence:
Observations:
Scenario 3: Plain resource publish failureObjective: Verify Coverage Type: Unhappy path Status: ✅ Passed Expected Unhappy-Path Outcome: Non-zero exit, actionable validation error, and no manifest. Evidence:
Observations:
Scenario 4: Plain resource deploy failureObjective: Verify Coverage Type: Unhappy path Status: ✅ Passed Expected Unhappy-Path Outcome: Non-zero exit, the same actionable validation error, and no deployment manifest. Evidence:
Observations:
Scenario 5: ExcludeFromManifest publishingObjective: Verify an intentionally run-only resource remains a valid publish configuration. Coverage Type: Supported alternative Status: ✅ Passed Steps:
Evidence:
Observations:
Scenario 6: PublishAsDockerFile publishingObjective: Verify explicit container publishing remains supported and produces container metadata. Coverage Type: Supported alternative Status: ✅ Passed Steps:
Evidence:
Observations:
Scenario 7: Mixed-resource aggregationObjective: Verify multiple unsupported resources are aggregated while explicitly excluded and containerized resources are filtered out. Coverage Type: Boundary and unhappy path Status: ✅ Passed Expected Unhappy-Path Outcome: One actionable failure naming only the unsupported resources. Evidence:
Observations:
Summary
Overall Result✅ PR VERIFIED No blocking issues were found in the tested hosting behavior. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
PR Testing ReportPR Information
Docker was not installed on the host, so the approved local execution target was used. Artifact Version Verification
Changes AnalyzedFiles Changed
Change Categories
Test Scenarios ExecutedScenario 1: PR artifact and targeted source validationObjective: Verify the dogfood artifact and source checkout match the latest PR head, then execute the focused unit coverage. Coverage Type: Artifact integrity and unit validation Status: Passed Steps:
Evidence:
Observations:
Scenario 2: Run mode and before-start regressionObjective: Confirm a plain Coverage Type: Happy path and regression boundary Status: Passed Steps:
Evidence:
Observations:
Scenario 3: Direct build and push boundariesObjective: Verify directly selected build and push roots execute without triggering publish validation. Coverage Type: Boundary Status: Passed Steps:
Evidence:
Observations:
Scenario 4: Plain resource publish failureObjective: Verify Coverage Type: Unhappy path Status: Passed Expected Unhappy-Path Outcome: Non-zero exit, actionable validation error, no sibling publish side effect, and no manifest. Evidence:
Observations:
Scenario 5: Plain resource deploy failureObjective: Verify Coverage Type: Unhappy path Status: Passed Expected Unhappy-Path Outcome: Non-zero exit, the same actionable validation error, no sibling deploy side effect, and no manifest. Evidence:
Observations:
Scenario 6: ExcludeFromManifest publishingObjective: Verify an intentionally run-only resource remains a valid publish configuration. Coverage Type: Supported alternative Status: Passed Steps:
Evidence:
Observations:
Scenario 7: PublishAsDockerFile publishingObjective: Verify explicit container publishing remains supported and emits correct Dockerfile metadata. Coverage Type: Supported alternative Status: Passed Steps:
Evidence:
Observations:
Scenario 8: Custom manifest callback publishingObjective: Verify replacing the default manifest callback remains a supported explicit publishing choice. Coverage Type: Supported alternative Status: Passed Steps:
Evidence:
Observations:
Scenario 9: Mixed-resource aggregationObjective: Verify multiple unsupported resources are aggregated while excluded, containerized, and custom-published resources are filtered out. Coverage Type: Boundary and unhappy path Status: Passed Expected Unhappy-Path Outcome: One actionable failure naming only the unsupported resources, with plural grammar and no manifest. Evidence:
Observations:
Summary
Overall ResultPR VERIFIED No blocking issues were found in the tested hosting behavior. |
92c497a to
1877b81
Compare
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 3 / 100 test projects · 2 jobs, from 2 changed files. Selected test projects (3 / 100)
Selected jobs (2)
How these were chosen — grouped by what changed🔧 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Aspire.Hosting.Dotnet/DotnetProjectResource.cs:150
beforeStartStepNamesis computed before later resources' pipeline-configuration callbacks run. If a later callback wires a publish/deploy step intoBeforeStart, this callback has already added the publish validation as that step's dependency; normal Run startup will then execute the validation and fail for a plainDotnetProjectResource. Defer this reachability decision until all configuration callbacks have completed (or add a post-configuration validation phase), and cover the late-wiring case by adding a later resource callback that makesBeforeStartdepend on a publish/deploy preparation step.
var beforeStartStepNames = GetStepsRequiredByRoots(
context.Steps,
[WellKnownPipelineSteps.BeforeStart])
.Select(step => step.Name)
.ToHashSet(StringComparer.Ordinal);
return GetStepsRequiredByRoots(context.Steps, s_publishAndDeployWorkRootStepNames, beforeStartStepNames);
tests/Aspire.Hosting.Dotnet.Tests/DotnetProjectResourceTests.cs:302
- Setting
PipelineOptions.SteptoBeforeStartmakesGetPublishAndDeployStepsreturn immediately at its selected-step guard, becauseBeforeStartdoes not reach a publish/deploy root at that point. The test therefore never exercises the shared-step exclusion on lines 141-150. Real startup callsExecuteStepSequentiallyAsync(BeforeStart, ...)whilePipelineOptions.Stepremains unset, so this test can pass even when Run startup is broken. Exercise the actual before-start execution path with an unset pipeline option.
builder.Services.Configure<PipelineOptions>(options => options.Step = WellKnownPipelineSteps.BeforeStart);
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
065001e
into
main
|
/backport to release/13.5 |
|
Started backporting to |
|
Pull request created: #1501
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1501 targeting Added a
File modified: Note This draft PR needs human review before merging. |
Description
The new
DotnetProjectResourcedoes not support publishing yet, but the current implementation just silently fails when someone tries publishing. This PR replaces that with an actionable error.Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?