Skip to content

Containerize Notification service (hardened image + health/ready probes) #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
devin-ai-integration wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Containerize Notification service (hardened image + health/ready probes) #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/.dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
**/bin/
**/obj/
**/out/
**/publish/
**/.vs/
**/.vscode/
**/.idea/
**/*.user
**/.git/
**/.gitignore
**/Dockerfile*
**/.dockerignore
**/docker-compose*.yml
**/.env
**/.env.*
**/appsettings.*.local.json
**/secrets.json
**/*.log
**/.DS_Store
28 changes: 18 additions & 10 deletions src/Services/Notification/Notification.API/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,32 @@
FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS base
WORKDIR /app
EXPOSE 5005
# syntax=docker/dockerfile:1

# ---- Build stage ----
FROM mcr.microsoft.com/dotnet/sdk:10.0 AS build
WORKDIR /src
# Copy ONLY .csproj files first (one COPY per referenced project) for layer-cached restore
COPY ["Services/Notification/Notification.API/Notification.API.csproj", "Services/Notification/Notification.API/"]
COPY ["Services/Notification/Notification.Domain/Notification.Domain.csproj", "Services/Notification/Notification.Domain/"]
COPY ["Services/Notification/Notification.Infrastructure/Notification.Infrastructure.csproj", "Services/Notification/Notification.Infrastructure/"]
COPY ["Shared/Shared.Contracts/Shared.Contracts.csproj", "Shared/Shared.Contracts/"]
COPY ["Shared/Shared.Infrastructure/Shared.Infrastructure.csproj", "Shared/Shared.Infrastructure/"]
RUN dotnet restore "Services/Notification/Notification.API/Notification.API.csproj"
COPY . .
WORKDIR "/src/Services/Notification/Notification.API"
RUN dotnet build -c Release -o /app/build
RUN dotnet publish "Services/Notification/Notification.API/Notification.API.csproj" \
-c Release -o /app/publish /p:UseAppHost=false

FROM build AS publish
RUN dotnet publish -c Release -o /app/publish /p:UseAppHost=false

FROM base AS final
# ---- Runtime stage ----
FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS final
WORKDIR /app
COPY --from=publish /app/publish .
# curl is required by the HEALTHCHECK below and is NOT in the aspnet image
RUN apt-get update \
&& apt-get install -y --no-install-recommends curl \
&& rm -rf /var/lib/apt/lists/*
# Fixed container-internal port; nothing host/secret specific is baked in.
# Remap externally with `docker run -p <host>:5005`.
ENV ASPNETCORE_URLS=http://+:5005
EXPOSE 5005
COPY --from=build /app/publish .
USER $APP_UID
HEALTHCHECK --interval=30s --timeout=3s --start-period=15s --retries=3 \
CMD curl -fsS "http://localhost:5005/health" || exit 1
ENTRYPOINT ["dotnet", "Notification.API.dll"]
5 changes: 3 additions & 2 deletions src/Services/Notification/Notification.API/Notification.API.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
<ItemGroup>
<ProjectReference Include="..\Notification.Domain\Notification.Domain.csproj" />
<ProjectReference Include="..\Notification.Infrastructure\Notification.Infrastructure.csproj" />
<ProjectReference Include="..\..\Shared\Shared.Contracts\Shared.Contracts.csproj" />
<ProjectReference Include="..\..\Shared\Shared.Infrastructure\Shared.Infrastructure.csproj" />
<ProjectReference Include="..\..\..\Shared\Shared.Contracts\Shared.Contracts.csproj" />
<ProjectReference Include="..\..\..\Shared\Shared.Infrastructure\Shared.Infrastructure.csproj" />
Comment on lines +10 to +11

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

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.

🔍 Shared project reference path fix is correct but other services still have the broken path

The change from ..\..\Shared\... to ..\..\..\Shared\... is a genuine fix. From src/Services/Notification/Notification.API/, the old path ../../Shared resolves to src/Services/Shared/ which does not exist — the actual location is src/Shared/. The new three-level-up path correctly resolves to src/Shared/. However, all other services (Order, Customer, Product, Identity) still use the two-level ..\..\Shared\... path in their .csproj files (e.g. src/Services/Order/Order.API/Order.API.csproj:10-11). Those references are equally broken. It's possible the solution file or a Directory.Build.props was masking this, but the paths are objectively wrong for both local and Docker builds.

Open in Devin Review

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

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

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.

Correct, and intentional. This PR is deliberately scoped to containerizing Notification only, so I fixed only Notification.API.csproj — that fix is required because the Notification image can't build otherwise. The other services (Order, Customer, Product, Identity) have the same broken ..\..\Shared\... path but were already containerized in prior PRs and building, so I left them untouched to keep this PR scoped. Fixing them belongs in a separate change.

</ItemGroup>
<ItemGroup>
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="10.*" />
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks.EntityFrameworkCore" Version="10.*" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="7.*" />
</ItemGroup>
</Project>
21 changes: 18 additions & 3 deletions src/Services/Notification/Notification.API/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
using Notification.Infrastructure.Data;
using Notification.Infrastructure.Repositories;
using Microsoft.EntityFrameworkCore;
using Microsoft.AspNetCore.Diagnostics.HealthChecks;

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddControllers();
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
builder.Services.AddHealthChecks();
builder.Services.AddHealthChecks()
.AddDbContextCheck<NotificationDbContext>("database", tags: new[] { "ready" });

builder.Services.AddDbContext<NotificationDbContext>(options =>
options.UseNpgsql(builder.Configuration.GetConnectionString("DefaultConnection")));
Expand All @@ -23,7 +25,16 @@
using (var scope = app.Services.CreateScope())
{
var db = scope.ServiceProvider.GetRequiredService<NotificationDbContext>();
db.Database.EnsureCreated();
try
{
db.Database.EnsureCreated();
}
catch (Exception ex)
{
// Liveness must not depend on the database being reachable at startup;
// readiness (/ready) reflects DB availability instead.
app.Logger.LogWarning(ex, "Database initialization skipped; database unavailable at startup.");
}
}

if (app.Environment.IsDevelopment())
Expand All @@ -33,6 +44,10 @@
}

app.MapControllers();
app.MapHealthChecks("/healthz");
// Liveness: process is up; no dependency checks.
app.MapHealthChecks("/health", new HealthCheckOptions { Predicate = _ => false });
app.MapHealthChecks("/healthz", new HealthCheckOptions { Predicate = _ => false });
// Readiness: dependencies (DB) reachable -> 200, else 503.
app.MapHealthChecks("/ready", new HealthCheckOptions { Predicate = c => c.Tags.Contains("ready") });

app.Run();
3 changes: 0 additions & 3 deletions src/Services/Notification/Notification.API/appsettings.json
View file
Open in desktop

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

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.

🔍 Default connection string removed only for Notification service, inconsistent with other services

The ConnectionStrings.DefaultConnection was removed from appsettings.json, but all other services (Order, Customer, Identity, Product) still have their connection strings in their respective appsettings.json files (e.g. src/Services/Order/Order.API/appsettings.json:9). In Docker, this works because docker-compose.yml provides the connection string via environment variable. For local development without Docker, GetConnectionString("DefaultConnection") returns null, which is passed to UseNpgsql(). The EnsureCreated() failure is caught by the new try/catch, and the /ready health check will report 503 — so the app starts but is non-functional for DB operations. This may be intentional (avoiding credentials in config), but the inconsistency with other services and lack of an appsettings.Development.json fallback is worth confirming.

Open in Devin Review

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

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

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.

Intentional — the goal here is externalizing config so no credentials are baked into the image; the connection string comes from ConnectionStrings__DefaultConnection, which compose supplies. The startup EnsureCreated() is wrapped in try/catch so liveness (/health) never depends on the DB, and /ready correctly reports 503 when the DB is unreachable (e.g. local run without Docker/env var). The inconsistency with the other services' appsettings.json is expected: this PR is scoped to Notification, and those services will be aligned separately.

Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,5 @@
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
},
"ConnectionStrings": {
"DefaultConnection": "Host=localhost;Database=notificationdb;Username=postgres;Password=postgres"
}
}