feat: migrate CargoAPI to .NET 10 and add reproducible Docker stack - #4
Conversation
Reviewer's GuideThe PR upgrades the backend and EF Core stack to .NET 10, pins the toolchain and CI, introduces a health-gated Docker Compose environment with SQL Server and opt-in migrations, adds fail-fast configuration and health endpoints, and updates documentation without changing carrier-selection domain behavior. Sequence diagram for Compose startup and migrationsequenceDiagram
participant Compose
participant SQL as SQL Server
participant API as CargoAPI API
participant DB as CargoDb
Compose->>SQL: Start container
SQL-->>Compose: healthcheck succeeds
Compose->>API: Start with Database__ApplyMigrations=true
API->>DB: MigrateAsync()
DB-->>API: Migration completes
API-->>Compose: Serve HTTP on port 8080
Sequence diagram for API readiness checkingsequenceDiagram
participant Client
participant API as CargoAPI API
participant DB as SQL Server
Client->>API: GET /health/live
API-->>Client: 200 OK
Client->>API: GET /health/ready
API->>DB: CanConnectAsync()
alt database reachable
DB-->>API: Connection succeeds
API-->>Client: 200 OK ready
else database unavailable
DB-->>API: Connection fails
API-->>Client: 503 Service Unavailable
end
Flow diagram for connection configuration and startup validationflowchart TD
Config[ConnectionStrings:DefaultConnection] --> Validate{Configured?}
Validate -->|no| Fail[Startup fails]
Validate -->|yes| Services[Configure EF Core and Hangfire]
Services --> Migration{Database:ApplyMigrations}
Migration -->|true| Apply[MigrateAsync]
Migration -->|false| Run[Start API]
Apply --> Run
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="docker-compose.yml" line_range="29-30" />
<code_context>
+ environment:
+ ASPNETCORE_ENVIRONMENT: Development
+ ASPNETCORE_HTTP_PORTS: 8080
+ ConnectionStrings__DefaultConnection: >-
+ Server=sqlserver,1433;Database=CargoDb;User Id=sa;Password=${MSSQL_SA_PASSWORD};Encrypt=True;TrustServerCertificate=True;
+ Database__ApplyMigrations: "true"
+ depends_on:
</code_context>
<issue_to_address>
**issue (bug_risk):** The Compose-generated connection string is invalid when `MSSQL_SA_PASSWORD` contains a semicolon or another connection-string delimiter, because the interpolated password is inserted unquoted as `Password=${MSSQL_SA_PASSWORD}`. SQL Server accepts such passwords, so the API and migration step fail to connect for valid local passwords.
**Triggers:** When the local SQL password contains a semicolon or other characters requiring connection-string escaping.
**Suggested fix:** Constrain the documented password character set or generate the connection string with proper ADO.NET escaping/quoting instead of interpolating the raw password.
```suggestion
ConnectionStrings__DefaultConnection: >-
Server=sqlserver,1433;Database=CargoDb;User Id=sa;Password="${MSSQL_SA_PASSWORD}";Encrypt=True;TrustServerCertificate=True;
```
</issue_to_address>
### Comment 2
<location path="README.md" line_range="251-255" />
<code_context>
+
+### Requirements
+
+- .NET 10 SDK
+- SQL Server LocalDB, Express or another SQL Server instance
+- `dotnet-ef` 10.x for migration commands
+
+The repository includes `global.json` to keep local and CI SDK selection in the .NET 10 toolchain.
+
+Install EF tooling if needed:
```bash
-dotnet run --project CargoAPI.API
+dotnet tool install --global dotnet-ef --version 10.*
```
</code_context>
<issue_to_address>
**nitpick:** The SDK-only quick-start command runs with the committed LocalDB connection string unless the user separately overrides it, so the documented flow fails on Linux and fails for users whose SQL Server is Express or another instance rather than `(localdb)\MSSQLLocalDB`.
**Triggers:** When following the SDK-only instructions without configuring `ConnectionStrings__DefaultConnection` first.
**Suggested fix:** Make the SDK instructions require an explicit connection-string override, or provide platform-specific configuration and commands before `dotnet run`.
````suggestion
The committed `appsettings.json` contains only a LocalDB development connection string and does not contain a database password. Before applying migrations or running the API, explicitly override it for your SQL Server instance without editing tracked files:
```bash
export ConnectionStrings__DefaultConnection='Server=localhost,1433;Database=CargoDb;User Id=sa;Password=ChangeThis_LocalOnly_2026!;Encrypt=True;TrustServerCertificate=True'
```
````
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the opt-in startup migration can change and persist the SQL Server schema, so reverting the application does not necessarily restore the database state, although the Compose database is local and can be recreated or repaired. The runtime and container changes otherwise introduce operational behavior that should be validated through normal deployment and startup checks.
Blocking findings: docker-compose.yml:30
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ConnectionStrings__DefaultConnection: >- | ||
| Server=sqlserver,1433;Database=CargoDb;User Id=sa;Password=${MSSQL_SA_PASSWORD};Encrypt=True;TrustServerCertificate=True; |
There was a problem hiding this comment.
issue (bug_risk): The Compose-generated connection string is invalid when MSSQL_SA_PASSWORD contains a semicolon or another connection-string delimiter, because the interpolated password is inserted unquoted as Password=${MSSQL_SA_PASSWORD}. SQL Server accepts such passwords, so the API and migration step fail to connect for valid local passwords.
Triggers: When the local SQL password contains a semicolon or other characters requiring connection-string escaping.
Suggested fix: Constrain the documented password character set or generate the connection string with proper ADO.NET escaping/quoting instead of interpolating the raw password.
| ConnectionStrings__DefaultConnection: >- | |
| Server=sqlserver,1433;Database=CargoDb;User Id=sa;Password=${MSSQL_SA_PASSWORD};Encrypt=True;TrustServerCertificate=True; | |
| ConnectionStrings__DefaultConnection: >- | |
| Server=sqlserver,1433;Database=CargoDb;User Id=sa;Password="${MSSQL_SA_PASSWORD}";Encrypt=True;TrustServerCertificate=True; |
| The committed `appsettings.json` contains only a LocalDB development connection string and does not contain a database password. You can override it without editing tracked files: | ||
|
|
||
| ```text | ||
| Swagger: http://localhost:5246/swagger | ||
| Hangfire: http://localhost:5246/hangfire | ||
| ConnectionStrings__DefaultConnection=<your connection string> | ||
| ``` |
There was a problem hiding this comment.
nitpick: The SDK-only quick-start command runs with the committed LocalDB connection string unless the user separately overrides it, so the documented flow fails on Linux and fails for users whose SQL Server is Express or another instance rather than (localdb)\MSSQLLocalDB.
Triggers: When following the SDK-only instructions without configuring ConnectionStrings__DefaultConnection first.
Suggested fix: Make the SDK instructions require an explicit connection-string override, or provide platform-specific configuration and commands before dotnet run.
| The committed `appsettings.json` contains only a LocalDB development connection string and does not contain a database password. You can override it without editing tracked files: | |
| ```text | |
| Swagger: http://localhost:5246/swagger | |
| Hangfire: http://localhost:5246/hangfire | |
| ConnectionStrings__DefaultConnection=<your connection string> | |
| ``` | |
| The committed `appsettings.json` contains only a LocalDB development connection string and does not contain a database password. Before applying migrations or running the API, explicitly override it for your SQL Server instance without editing tracked files: | |
| ```bash | |
| export ConnectionStrings__DefaultConnection='Server=localhost,1433;Database=CargoDb;User Id=sa;Password=ChangeThis_LocalOnly_2026!;Encrypt=True;TrustServerCertificate=True' | |
| ``` |
There was a problem hiding this comment.
🟡 Changes recommended
A few changes undermine the stated “reproducible” intent (SDK roll-forward and floating container image tags) and there is at least one concrete cleanup needed (unused health-check registration).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR upgrades CargoAPI from .NET 6 to .NET 10, updates EF Core accordingly, and introduces a Docker-based local development stack (API + SQL Server) with startup migration control and new health endpoints.
Changes:
- Migrate solution projects and CI to .NET 10; upgrade EF Core packages to
10.0.11and addglobal.json. - Add Dockerfile + Docker Compose stack (SQL Server + API) with
.env-driven password and health-gated startup. - Add liveness/readiness endpoints and opt-in “migrate on startup” behavior controlled by configuration.
File summaries
| File | Description |
|---|---|
| README.md | Updates docs for .NET 10, Docker Compose quick start, health endpoints, and migration behavior. |
| global.json | Pins SDK selection to .NET 10 toolchain (with roll-forward behavior). |
| Dockerfile | Adds multi-stage .NET 10 build/runtime container image. |
| docker-compose.yml | Adds SQL Server + API local dev stack with healthcheck gating and env-based configuration. |
| .env.example | Adds template for required local SQL password used by Compose. |
| .dockerignore | Reduces Docker build context and avoids copying local secrets/build outputs. |
| .gitignore | Ignores .env to prevent accidental secret commits. |
| .github/workflows/ci.yml | Updates CI to use .NET 10 and simplifies restore/build/test steps. |
| CargoAPI.API/Program.cs | Adds fail-fast connection string requirement, migration-on-start toggle, and health endpoints. |
| CargoAPI.API/appsettings.json | Adds Database:ApplyMigrations default configuration. |
| CargoAPI.API/CargoAPI.API.csproj | Targets net10.0 and upgrades EF Core design package. |
| CargoAPI.Business/CargoAPI.Business.csproj | Targets net10.0. |
| CargoAPI.DataAccess/CargoAPI.DataAccess.csproj | Targets net10.0 and upgrades EF Core packages. |
| CargoAPI.Entities/CargoAPI.Entities.csproj | Targets net10.0. |
| CargoAPI.Tests/CargoAPI.Tests.csproj | Targets net10.0. |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| FROM mcr.microsoft.com/dotnet/sdk:10.0 AS build | ||
| WORKDIR /src |
| sqlserver: | ||
| image: mcr.microsoft.com/mssql/server:2022-latest | ||
| environment: |
| { | ||
| "sdk": { | ||
| "version": "10.0.400", | ||
| "rollForward": "latestFeature" | ||
| } | ||
| } |
| builder.Services.AddEndpointsApiExplorer(); | ||
| builder.Services.AddSwaggerGen(); | ||
| builder.Services.AddHealthChecks(); |
| COPY CargoAPI.sln ./ | ||
| COPY CargoAPI.API/CargoAPI.API.csproj CargoAPI.API/ | ||
| COPY CargoAPI.Business/CargoAPI.Business.csproj CargoAPI.Business/ | ||
| COPY CargoAPI.DataAccess/CargoAPI.DataAccess.csproj CargoAPI.DataAccess/ | ||
| COPY CargoAPI.Entities/CargoAPI.Entities.csproj CargoAPI.Entities/ | ||
| COPY CargoAPI.Tests/CargoAPI.Tests.csproj CargoAPI.Tests/ | ||
| RUN dotnet restore CargoAPI.API/CargoAPI.API.csproj |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f1362f1f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Run unit tests | ||
| run: dotnet test CargoAPI.Tests/CargoAPI.Tests.csproj --configuration Release --no-restore --verbosity normal | ||
| - name: Test | ||
| run: dotnet test CargoAPI.Tests/CargoAPI.Tests.csproj --configuration Release --no-build --verbosity normal |
There was a problem hiding this comment.
On a clean GitHub Actions runner, this test step fails because CargoAPI.sln does not include CargoAPI.Tests, so the preceding restore and build steps neither restore nor compile the test project. The documented --no-build behavior also implies --no-restore, leaving no test assembly or assets to execute; add the test project to the solution, build it explicitly, or remove --no-build.
Useful? React with 👍 / 👎.
Summary
Modernize the backend case study from out-of-support .NET 6 to .NET 10 LTS and make the full API + SQL Server development environment reproducible with Docker Compose.
Runtime and persistence
net10.0.10.0.11.global.json.Containerized development
Dockerfileusing .NET 10 SDK/runtime images.docker-compose.yml..env, with only a template committed..envin Git.Operational readiness
/health/livefor process liveness./health/readybacked byDatabase.CanConnectAsyncand HTTP 503 on database unavailability.ConnectionStrings:DefaultConnectionfail fast at startup.Documentation
docker compose up --build.Review intent
The existing carrier-selection domain behavior is intentionally unchanged by this PR. Pricing semantics for below-range/gap values remain tracked separately in issue #3.
Merge only after the .NET 10 CI build and unit tests are green.
Summary by Sourcery
Modernize CargoAPI for .NET 10 and provide a reproducible, health-aware Docker development environment.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Chores: