Skip to content

Containerize Identity service (hardened image + /health & /ready probes)#82

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

Containerize Identity service (hardened image + /health & /ready probes)#82
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1783565681-containerize-identity

Conversation

@devin-ai-integration

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

Copy link
Copy Markdown
Contributor

Summary

Containerizes the Identity service into a hardened, config-externalized image with liveness/readiness probes, wired into the existing compose stack. Port stays hardcoded at 5001 (container-internal); remap externally via -p <host>:5001.

Dockerfile (Services/Identity/Identity.API/Dockerfile) — replaces the prior basic Dockerfile with the playbook's hardened multi-stage pattern:

  • sdk:10.0 build stage with per-project .csproj COPY for layer-cached dotnet restore, then dotnet publish -c Release /p:UseAppHost=false.
  • aspnet:10.0 runtime stage installs curl (not in the aspnet image), runs as non-root USER $APP_UID, and adds a HEALTHCHECK probing /health (liveness — not /ready, per playbook G3 to avoid restart loops on transient DB outages).

Health endpoints (Program.cs):

AddHealthChecks().AddDbContextCheck<IdentityDbContext>("database", tags: new[] { "ready" });
...
MapHealthChecks("/health",  Predicate = _ => false);          // liveness
MapHealthChecks("/healthz", Predicate = _ => false);          // liveness alias (preserves existing fleet convention)
MapHealthChecks("/ready",   Predicate = c => c.Tags.Contains("ready")); // readiness (DB) → 200/503

The pre-existing /healthz is kept as a liveness alias so anything already probing it still gets 200.

Config externalization (appsettings.json): removed the hardcoded ConnectionStrings:DefaultConnection (Host=localhost;...;Password=postgres). The connection string comes solely from ConnectionStrings__DefaultConnection, which compose already supplies.

Packages (Identity.API.csproj): added Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore (10.*) for the DbContext readiness check.

.dockerignore (new, at build-context root src/): excludes bin//obj//publish/, IDE files, local secrets, and docker/env files so stale local artifacts never poison the image.

Scoped to Identity only. Per playbook G5 the ..\..\Shared\* references are left as-is (they build with a warning only). The compose identity-service block already existed and needs no change.

Verification

Built and run with real Docker from src/:

  • docker build → image builds successfully.
  • No DB: GET /health200 Healthy, GET /healthz200, GET /ready503.
  • Real postgres (identitydb): GET /ready200 Healthy.
  • docker inspect health status → healthy.
  • docker exec ... iduid=1654(app) (non-root).
  • dotnet build ... -c Release → succeeds (only the expected Shared reference warning).
  • docker compose config → valid.

Test containers/network cleaned up afterward.

Link to Devin session: https://partner-workshops.devinenterprise.com/sessions/ed508d65169b4444b74a40ecc4a40d50
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

Comment on lines +30 to +31
HEALTHCHECK --interval=30s --timeout=3s --start-period=15s --retries=3 \
CMD curl -fsS "http://localhost:5001/health" || exit 1

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.

🔍 Docker HEALTHCHECK uses liveness endpoint, not readiness — won't detect database outages

The Dockerfile HEALTHCHECK (src/Services/Identity/Identity.API/Dockerfile:31) hits /health, which is mapped with Predicate = _ => false (src/Services/Identity/Identity.API/Program.cs:27), meaning it runs zero health checks and always returns 200. This means the Docker daemon will never mark the container as unhealthy even if the database is completely unreachable. If the intent is for Docker (or Compose depends_on with condition: service_healthy) to gate on actual readiness, the HEALTHCHECK should hit /ready instead. If the intent is purely liveness (process is up), the current setup is correct but the 3-second timeout may be unnecessarily tight for a curl-based probe.

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 is intentional. The HEALTHCHECK targets liveness (/health) rather than readiness (/ready) by design: gating container health on DB reachability causes restart loops during transient DB outages. /ready (which does check the DB via the ready-tagged AddDbContextCheck) is provided separately for orchestrators/LBs that perform readiness gating. The accepted tradeoff in plain docker-compose is that a DB outage won't be reflected in container health.

On the 3s timeout: it's the standard interval used across the service fleet's healthchecks for consistency and is sufficient for a localhost curl probe (with retries=3 and a 15s start period before failures count). Leaving as-is.

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