Skip to content

Containerize Customer service: hardened Dockerfile + /health & /ready probes#81

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783565560-containerize-customer
Open

Containerize Customer service: hardened Dockerfile + /health & /ready probes#81
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783565560-containerize-customer

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Containerizes the Customer service into a hardened, config-externalized image with liveness/readiness probes, wired into the existing compose stack. Scoped to Customer only.

Dockerfile (Services/Customer/Customer.API/Dockerfile) — replaces the previous basic single-stage file with a hardened multi-stage build:

  • sdk:10.0 build stage restores from per-project .csproj COPYs (layer-cached) then dotnet publish.
  • aspnet:10.0 runtime stage installs curl (not in the aspnet image, needed by HEALTHCHECK), runs as non-root USER $APP_UID, and adds a HEALTHCHECK probing /health.
  • Internal port hardcoded to 5002 (ENV ASPNETCORE_URLS=http://+:5002); remap externally via -p. Not build-time interpolated (avoids the healthcheck-probes-wrong-port pitfall).

Health endpoints (Program.cs):

builder.Services.AddHealthChecks()
    .AddDbContextCheck<CustomerDbContext>("database", tags: new[] { "ready" });
...
app.MapHealthChecks("/health",  new HealthCheckOptions { Predicate = _ => false });          // liveness
app.MapHealthChecks("/healthz", new HealthCheckOptions { Predicate = _ => false });          // liveness alias (fleet convention)
app.MapHealthChecks("/ready",   new HealthCheckOptions { Predicate = c => c.Tags.Contains("ready") }); // readiness (DB)

/healthz is preserved as a liveness alias. HEALTHCHECK targets /health (liveness), not /ready, to avoid restart loops on transient DB outages.

csproj — adds Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore (10.*) for AddDbContextCheck.

Config — removes the hardcoded ConnectionStrings:DefaultConnection (Host=localhost;...;Password=postgres) from appsettings.json; the connection string comes only from the ConnectionStrings__DefaultConnection env var that compose already supplies.

.dockerignore — added at the build-context root (src/) to keep local bin//obj//secrets out of the image.

composecustomer-service block already present and correct (port 5002, depends_on postgres+rabbitmq, connection string via env); verified, no edit needed.

The pre-existing ..\..\Shared\* project references are left as-is (they build with only an MSB9008 warning) to keep this PR scoped.

Verification (real Docker)

  • docker build — succeeds, image built.
  • No DB: GET /health200 Healthy, GET /healthz200, GET /ready503 Unhealthy.
  • With postgres: GET /ready200 Healthy.
  • docker inspect health status → healthy.
  • docker exec ... iduid=1654(app) (non-root).
  • Test containers/network cleaned up afterward.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/0e1be058efc4489ba39fc83d84d720e3
Requested by: @mbatchelor81


Open in Devin Review

@mbatchelor81 mbatchelor81 self-assigned this Jul 9, 2026
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

},
"ConnectionStrings": {
"DefaultConnection": "Host=localhost;Database=customerdb;Username=postgres;Password=postgres"
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Connection string removed only from Customer service, breaking local-dev parity with other services

The DefaultConnection string was removed from appsettings.json, but all four other services (Identity, Order, Product, Notification) still have theirs in their respective appsettings.json files. Running the Customer service locally without Docker (e.g. dotnet run) will pass null to UseNpgsql() at src/Services/Customer/Customer.API/Program.cs:14, causing a runtime exception on first DB access. The docker-compose.yml at src/docker-compose.yml:43 provides the value for containerized runs, so this only affects local development. If the intent is to externalize config, consider doing it consistently across all services or adding an appsettings.Development.json with a local fallback.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal is intentional: the containerization playbook this PR follows externalizes config, so the connection string comes only from the ConnectionStrings__DefaultConnection env var (which docker-compose.yml already supplies). The trade-off you note — bare dotnet run without Docker now needs the env var or an appsettings.Development.json fallback — is accepted, and applying the same treatment to the other four services is deliberately out of scope to keep this PR to Customer only. That consistency cleanup would be a good follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant