Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Keep the Darling image build context lean (#1804): the Dockerfile copies source and publishes
# inside the build stage, so nothing prebuilt, versioned-binary, or platform-foreign belongs in it.
.git
.github
**/bin
**/obj
**/publish
releases
Darling/artifacts
deprecated
docs
*.md

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since the build context is the raw filesystem (not git), .dockerignore doesn't inherit .gitignore's exclusions — and the root .gitignore explicitly calls out appsettings.json as "Sensitive configuration (connection strings)" that's never committed but commonly present in a dev's working tree. The Dockerfile's COPY . . (repo root context) would pick up any such locally-present, git-ignored secret-bearing file on a plain local docker build, and since the service project is Sdk="Microsoft.NET.Sdk.Worker", a same-directory appsettings*.json is copied into the publish output too.

Worth mirroring the sensitive-file exclusions from .gitignore here (appsettings.json at minimum) as defense-in-depth, even though CI's checkout is always clean.

61 changes: 61 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -728,3 +728,64 @@ jobs:
${{ runner.temp }}/darling-pg.log
TestResults/
if-no-files-found: ignore

# ── Linux service build + container image (#1804) ────────────────────────────────────────────────
# The Darling service is cross-platform .NET on purpose, but until this job nothing PROVED it on
# every PR — the linux-x64 publish and the container image both built for the first time at release
# time or never. Same path-filter shape as darling-pg above: only runs the expensive work when
# Darling/service code (or this workflow, or the Dockerfile) changed, always reports a result so it
# can be a required check. No tests run here — the test projects are net10.0-windows (they reference
# the WPF apps); the cross-platform behavior they pin is exercised by the Windows jobs, and the
# container smoke lives in the compose quickstart. This job answers exactly two questions: does the
# service still publish for linux-x64, and does the image still build.
darling-linux:
name: Darling Linux build
runs-on: ubuntu-latest
timeout-minutes: 30
permissions:
contents: read

steps:
- uses: actions/checkout@v7

- name: Detect changed paths
id: filter
if: github.event_name != 'release'
uses: dorny/paths-filter@v4
with:
base: ${{ github.event_name == 'push' && github.event.before || '' }}
filters: |
darling:
- 'Darling/**/!(*.md)'
- 'PerformanceMonitor.Common/**'
- 'PerformanceMonitor.Collectors/**'
- 'PerformanceMonitor.Analysis/**'
Comment on lines +760 to +762

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These three lines are missing the /!(*.md) extglob carve-out that every other area filter in this file uses — including the core filter's identical PerformanceMonitor.Common/**/!(*.md) etc. at lines 92-94, and Darling/**/!(*.md) two lines above this hunk.

The comment block above the core filter (lines ~74-88) documents exactly why this matters, with a cited incident: a bare dir/** pattern (no markdown carve-out) makes the filter true for any file under that directory, including docs-only edits — proven in PR #1714 (a .gitignore-only change triggered the full Darling PG suite). Here, a markdown-only edit under PerformanceMonitor.Common/, PerformanceMonitor.Collectors/, or PerformanceMonitor.Analysis/ will now unnecessarily trigger darling-linux's dotnet publish + docker build on every PR touching those dirs' docs.

Suggest matching the established convention:

Suggested change
- 'PerformanceMonitor.Common/**'
- 'PerformanceMonitor.Collectors/**'
- 'PerformanceMonitor.Analysis/**'
- 'PerformanceMonitor.Common/**/!(*.md)'
- 'PerformanceMonitor.Collectors/**/!(*.md)'
- 'PerformanceMonitor.Analysis/**/!(*.md)'

- '.github/workflows/build.yml'

- name: Report the Linux gate decision
shell: bash
run: |
set -euo pipefail
if [ "${{ github.event_name }}" = "release" ]; then
echo "::notice title=Darling Linux build skipped::Release event - the dev push that produced this commit already ran it."
elif [ "${{ steps.filter.outputs.darling }}" = "true" ]; then
echo "::notice title=Darling Linux build running::Darling/service code (or this workflow) changed."
else
echo "::notice title=Darling Linux build skipped::No Darling/service code changed."
fi

- name: Setup .NET 10.0
if: steps.filter.outputs.darling == 'true'
uses: actions/setup-dotnet@v6
with:
global-json-file: global.json
cache: true
cache-dependency-path: '**/packages.lock.json'

- name: Publish service (linux-x64)
if: steps.filter.outputs.darling == 'true'
run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -r linux-x64 --self-contained false -o publish/DarlingService-linux

- name: Build container image
if: steps.filter.outputs.darling == 'true'
run: docker build -f Darling/Dockerfile -t performancemonitor-darling:pr .
65 changes: 65 additions & 0 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ on:

permissions:
contents: write
# #1804: the linux job pushes the nightly container image to ghcr.
packages: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This adds packages: write at the workflow level, which grants it to every job in the file (redispatch, build/windows, darling-pg) even though only the new linux job pushes to ghcr. Contrast with build.yml's darling-linux job in this same PR, which scopes down to permissions: contents: read at the job level for least privilege. Consider moving packages: write (and contents: write, if not otherwise needed by the other jobs) to a job-level permissions: block under linux instead of broadening the workflow default — same reasoning darling-pg/darling-linux already apply elsewhere in this repo.


jobs:
# Scheduled workflows always execute the DEFAULT branch's copy of this file, while nightly
Expand Down Expand Up @@ -389,3 +391,66 @@ jobs:
${{ runner.temp }}/darling-pg.log
TestResults/
if-no-files-found: ignore

# ── Linux artifact + container image (#1804) ─────────────────────────────────────────────────────
# Runs AFTER the windows build job so the nightly release exists to upload into. Publishes the
# linux-x64 service tar.gz with its own checksum file (the windows job owns SHA256SUMS.txt; a
# cross-job rewrite of one file is a race), and pushes the service image to ghcr tagged :nightly.
# The bundled pg-runtime is deliberately absent from the linux artifact — the compose distribution
# pairs the service with the official timescale/timescaledb image, and managed mode stays Windows.
linux:
needs: build
runs-on: ubuntu-latest
timeout-minutes: 30

steps:
- uses: actions/checkout@v7
with:
ref: dev

- name: Setup .NET 10.0
uses: actions/setup-dotnet@v6
with:
global-json-file: global.json
cache: true
cache-dependency-path: '**/packages.lock.json'

- name: Set nightly version
id: version
shell: bash
run: |
set -euo pipefail
base=$(grep -oPm1 '(?<=<Version>)[^<]+' Lite/PerformanceMonitorLite.csproj)
date=$(date +%Y%m%d)
echo "VERSION=${base}-nightly.${date}" >> "$GITHUB_OUTPUT"
echo "Nightly version: ${base}-nightly.${date}"

- name: Publish service (linux-x64)
run: dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -c Release -r linux-x64 --self-contained false -o publish/DarlingService-linux

- name: Package linux artifact + checksum
shell: bash
run: |
set -euo pipefail
version="${{ steps.version.outputs.VERSION }}"
mkdir -p releases
tar -C publish/DarlingService-linux -czf "releases/PerformanceMonitorDarling-linux-x64-${version}.tar.gz" .
(cd releases && sha256sum "PerformanceMonitorDarling-linux-x64-${version}.tar.gz" > SHA256SUMS-linux.txt && cat SHA256SUMS-linux.txt)

- name: Upload linux artifact to the nightly release
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: gh release upload nightly releases/PerformanceMonitorDarling-linux-x64-*.tar.gz releases/SHA256SUMS-linux.txt --clobber

- name: Build and push container image (ghcr, :nightly)
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
shell: bash
run: |
set -euo pipefail
version="${{ steps.version.outputs.VERSION }}"
image="ghcr.io/${{ github.repository_owner }}/performancemonitor-darling"
echo "$GH_TOKEN" | docker login ghcr.io -u "${{ github.actor }}" --password-stdin
docker build -f Darling/Dockerfile -t "${image}:nightly" -t "${image}:${version}" .
docker push "${image}:nightly"
docker push "${image}:${version}"
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **The Darling service builds for Linux on every PR and ships a nightly container image** ([#1804] stage 3) - the collector service is cross-platform .NET on purpose, but nothing PROVED it: the linux-x64 publish and the container image built at release time or never. A checked-in \`Darling/Dockerfile\` builds from source (sdk stage publish into the official aspnet runtime image - which sets the \`DOTNET_RUNNING_IN_CONTAINER\` marker the stage-2 bind gate keys on, and gains \`libgssapi-krb5-2\`, which Microsoft.Data.SqlClient probes at connect time on Linux even for SQL auth: found the hard way when the container smoke's first SQL connect failed on the missing library). CI gains a path-filtered \`darling-linux\` job answering exactly two questions on every Darling PR - does the service still publish for linux-x64, and does the image still build - and the nightly gains a linux job that uploads \`PerformanceMonitorDarling-linux-x64-*.tar.gz\` (its own checksum file; the Windows job owns SHA256SUMS.txt) and pushes \`ghcr.io/erikdarlingdata/performancemonitor-darling:nightly\`. The whole path was validated end-to-end locally first: the containerized service against dockerized TimescaleDB and SQL Server 2022 ran 25 collectors green with an \`env:\`-referenced SQL password, the web login/token/cookie gate and the MCP bearer gate answering correctly through mapped ports - a run that also caught a config-validation warning still claiming the network block is IGNORED in containers while the host correctly exposed it (fixed), and proved the store-authoritative control plane overrides the file on a reused store exactly as documented. The bundled pg-runtime is deliberately absent from the linux artifact: compose pairs the service with the official timescale/timescaledb image, and managed mode stays Windows.

- **Network exposure works in a container: the bind ladder's managed-mode gate extends to \`managed OR containerized\`** ([#1804] stage 2) - under \`postgres.managed = false\` the shared bind ladder refuses web/MCP network exposure and degrades to loopback-only ('your own reverse proxy governs BYO exposure') - the right rule on a host, and dead-on-arrival inside a container, where compose port mapping cannot reach a loopback bind and the mapping itself IS the boundary the reverse-proxy rule was standing in for. The pure decision table gains one input: \`inContainer\` (the official .NET images' \`DOTNET_RUNNING_IN_CONTAINER\` marker, read by the callers and passed in so the ladder stays pure), and the managed requirement relaxes for it - NOTHING ELSE does: the bearer token and the valid same-family \`allowFrom\` CIDR are required identically, a containerized not-exposed block skips the now-false 'network.* is ignored' notice, and the uncontained BYO rule is byte-for-byte unchanged, all pinned by new decision-table tests.

- **Secrets without DPAPI: every plaintext secret slot also takes an \`env:NAME\` or \`file:/path\` reference** ([#1804] stage 1 - the largest piece of the Linux/compose distribution, and useful to Windows BYO-store shops today) - the DPAPI fields (\`encryptedPassword\`, \`encryptedToken\`) are Windows-only by nature, and the only alternative was a literal secret sitting in \`darling.json\`, warned on every use. A monitored server's \`password\`, the new \`smtp.password\` (before this, SMTP had ONLY the DPAPI field - non-Windows hosts had no email-alerting path at all), and the mcp/web \`network.token\` slots now all accept a reference: \`env:\` reads the named environment variable, \`file:\` reads the file's trimmed contents (compose \`secrets:\` mounts end with a newline, and a newline inside a password is never what the operator meant). A reference is not a secret in the config file, which is the whole point - it does not trip the plaintext warnings - and a missing or empty target is a configuration error naming both the setting and the target, never a silent empty secret. The prefixes match case-sensitively and only at the start, so an exotic literal is expressible via a file. DPAPI stays preferred and byte-for-byte unchanged on Windows.
Expand Down
6 changes: 5 additions & 1 deletion Darling/Darling.Tests/DarlingWorkerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,11 @@ public void GetNetworkStartupWarnings_ByoWithNetwork_WarnsBothSectionsIgnored()

var warnings = DarlingWorker.GetNetworkStartupWarnings(config);
Assert.Contains(warnings, w => w.Contains("postgres.network", StringComparison.Ordinal) && w.Contains("bring-your-own", StringComparison.Ordinal));
Assert.Contains(warnings, w => w.Contains("mcp.network", StringComparison.Ordinal) && w.Contains("managed-mode only", StringComparison.Ordinal));
/* #1804: the mcp notice names the container path now — in a container the block is HONORED, so
the notice is suppressed there (see the bind ladder's container gate); outside one (this test
process) it still fires. The postgres.network notice is container-independent: the bundled
store never runs in BYO mode. */
Assert.Contains(warnings, w => w.Contains("mcp.network", StringComparison.Ordinal) && w.Contains("managed-mode (or container, #1804) only", StringComparison.Ordinal));
/* BYO never emits the admin-pivot warning — the network config is ignored anyway. */
Assert.DoesNotContain(warnings, w => w.Contains("pivot", StringComparison.OrdinalIgnoreCase) || w.Contains("config_command", StringComparison.Ordinal));
}
Expand Down
37 changes: 37 additions & 0 deletions Darling/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# The Darling service container (#1804) — builds from source so `docker build` works from any checkout:
#
# docker build -f Darling/Dockerfile -t performancemonitor-darling .
#
# Build context is the REPO ROOT (the service references PerformanceMonitor.Common / .Collectors /
# .Analysis / Darling.Storage by project). The runtime stage is the official aspnet image (the service
# hosts the web dashboard + MCP endpoint on Kestrel) and sets DOTNET_RUNNING_IN_CONTAINER, which is what
# the bind ladder's container gate keys on — network exposure with the mandatory token/CIDR works here
# under postgres.managed = false, because the compose port mapping is the boundary.
#
# The bundled zero-admin store is Windows-only BY DESIGN and never runs in this image: pair the container
# with the official timescale/timescaledb image (see the compose file) and keep postgres.managed = false.
# Secrets never land in darling.json — use env:/file: references (#1804 stage 1), which are compose
# `secrets:`-friendly.

FROM mcr.microsoft.com/dotnet/sdk:10.0 AS build
WORKDIR /src
COPY . .
RUN dotnet publish Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj \
-c Release -o /app/publish

FROM mcr.microsoft.com/dotnet/aspnet:10.0
# Microsoft.Data.SqlClient probes the Kerberos GSSAPI library at connect time on Linux — even for SQL
# auth — and the aspnet base image does not carry it. Found the hard way in the #1804 container smoke.
RUN apt-get update \
&& apt-get install -y --no-install-recommends libgssapi-krb5-2 \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /app
COPY --from=build /app/publish .

# The config mount point the compose file uses; override DARLING_CONFIG to relocate.
ENV DARLING_CONFIG=/config/darling.json

# Web dashboard + MCP (both opt-in via darling.json; exposed here only behind their token gates).
EXPOSE 5153 5152

ENTRYPOINT ["dotnet", "PerformanceMonitor.Darling.Service.dll"]
8 changes: 6 additions & 2 deletions Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,14 @@ internal static IReadOnlyList<string> GetNetworkStartupWarnings(DarlingConfig co
"postgres.network.* is set but postgres.managed is false — it is IGNORED in bring-your-own mode; your own PostgreSQL governs its network exposure (pg_hba / listen_addresses / TLS).");
}

if (config.Mcp.Network?.IsConfigured == true)
/* #1804: in a container the mcp/web network blocks ARE honored (the bind ladder's container
gate), so this notice would be a lie there — the smoke test caught it warning IGNORED in
the same breath as 'Starting MCP server on 0.0.0.0'. The postgres.network notice above
stays: the bundled store never runs in BYO mode, container or not. */
if (config.Mcp.Network?.IsConfigured == true && !Hosting.DarlingHostBinding.IsRunningInContainer)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GetNetworkStartupWarnings now calls the impure DarlingHostBinding.IsRunningInContainer (an environment-variable read) directly inside what its own doc comment (line 220) still calls a pure function ("Pure so a unit test asserts the returned strings..."). That claim is no longer true, and there's a real consequence: none of the three existing tests in DarlingWorkerTests.cs exercise the container branch, so this PR's actual fix — suppressing the false "IGNORED" notice in a container — has zero test coverage. GetNetworkStartupWarnings_ByoWithNetwork_WarnsBothSectionsIgnored still passes today only because DOTNET_RUNNING_IN_CONTAINER happens to be unset on the Windows test runners; if that ever changes, the test's mcp-warning assertion silently starts failing for the wrong reason.

Compare with the established pattern for exactly this problem elsewhere in the same PR/area: DarlingHostBinding.ResolveBind and DarlingWebHostService.ResolveWebBind(WebConfig, bool, bool? inContainer = null) both keep the impure container read at the call site and thread an inContainer parameter into the pure logic so it stays unit-testable. GetNetworkStartupWarnings should follow the same shape — take an inContainer (or bool isRunningInContainer) parameter, with the single caller at line 471 passing Hosting.DarlingHostBinding.IsRunningInContainer — and gain a test asserting the mcp warning is suppressed when inContainer: true.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This new suppression branch reads DarlingHostBinding.IsRunningInContainer (an impure environment read) directly inside GetNetworkStartupWarnings, and no test in DarlingWorkerTests.cs sets DOTNET_RUNNING_IN_CONTAINER to exercise the suppressed-in-a-container path — only the pre-existing "outside a container" branch is pinned (the modified test at DarlingWorkerTests.cs:182).

This is also a departure from the pattern DarlingHostBinding.ResolveBind itself documents for exactly this reason (see its inContainer parameter and the doc comment on IsRunningInContainer: "IMPURE ... which is why it lives beside — never inside — the pure ... ladder: the callers read it once and pass it in, so the decision table stays testable without environment games"). GetNetworkStartupWarnings could take inContainer as a parameter the same way, letting a test assert the warning is suppressed without mutating process environment state.

Right now the container-suppression behavior is only verified by the manual live-container smoke described in the PR body, not by CI.

{
warnings.Add(
"mcp.network.* is set but postgres.managed is false — the MCP network endpoint is managed-mode only, so it is IGNORED; the MCP server stays loopback-only.");
"mcp.network.* is set but postgres.managed is false — the MCP network endpoint is managed-mode (or container, #1804) only, so it is IGNORED; the MCP server stays loopback-only.");
}

return warnings;
Expand Down
Loading