fix(health): report configured application version
/health read the APP_VERSION environment variable directly, but docker-compose passes App__Version, which binds to the App:Version configuration key. The variable under that name never existed in the container, so the endpoint always reported "unknown". Read App:Version through IConfiguration, the approach AdminSystemController already used for the same value. The resolution rule (configured version, else assembly version) moves to a shared BuildMetadata helper rather than being written twice; AdminSystemController now calls it, so the admin page and /health cannot drift apart. Local development is unaffected: nothing sets App:Version there, and the assembly-version fallback still applies. Tests pin the configuration KEY, not just the behaviour, including that an App__Version environment variable binds to App:Version. The original bug failed silently, so a behavioural test alone would not have caught it. Verified against a running backend: App__Version=9.9.9-test reports 9.9.9-test; unset reports the assembly version rather than "unknown". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,91 @@
|
|||||||
|
using JobTrackerApi.Services;
|
||||||
|
using Microsoft.Extensions.Configuration;
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
namespace JobTrackerApi.Tests;
|
||||||
|
|
||||||
|
// Pins the configuration KEY, not just the behaviour. /health previously read the
|
||||||
|
// APP_VERSION environment variable while docker-compose passed App__Version, so the
|
||||||
|
// endpoint silently reported "unknown" in every container. Nothing failed; the value
|
||||||
|
// was just wrong. These tests fail if the key drifts again.
|
||||||
|
public sealed class BuildMetadataTests
|
||||||
|
{
|
||||||
|
private static IConfiguration Config(params (string Key, string Value)[] values) =>
|
||||||
|
new ConfigurationBuilder()
|
||||||
|
.AddInMemoryCollection(values.Select(v => new KeyValuePair<string, string?>(v.Key, v.Value)))
|
||||||
|
.Build();
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Reads_the_App_Version_configuration_key()
|
||||||
|
{
|
||||||
|
var version = BuildMetadata.ResolveVersion(Config(("App:Version", "1.2.3")));
|
||||||
|
|
||||||
|
Assert.Equal("1.2.3", version);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void App__Version_environment_variable_binds_to_the_same_key()
|
||||||
|
{
|
||||||
|
// This is the exact path docker-compose uses: App__Version in the container
|
||||||
|
// environment must reach App:Version in configuration.
|
||||||
|
var cfg = new ConfigurationBuilder()
|
||||||
|
.AddInMemoryCollection(new Dictionary<string, string?>())
|
||||||
|
.Add(new FakeEnvironmentSource(("App__Version", "42")))
|
||||||
|
.Build();
|
||||||
|
|
||||||
|
Assert.Equal("42", BuildMetadata.ResolveVersion(cfg));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Falls_back_to_the_assembly_version_when_unconfigured()
|
||||||
|
{
|
||||||
|
// Local development sets nothing. "unknown" would be a regression, not a default.
|
||||||
|
var version = BuildMetadata.ResolveVersion(Config());
|
||||||
|
|
||||||
|
Assert.False(string.IsNullOrWhiteSpace(version));
|
||||||
|
Assert.NotEqual("unknown", version);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Theory]
|
||||||
|
[InlineData("$(git rev-parse --short HEAD)")]
|
||||||
|
[InlineData("${APP_COMMIT_SHA}")]
|
||||||
|
[InlineData(" ")]
|
||||||
|
[InlineData(null)]
|
||||||
|
public void Unresolved_placeholders_and_blanks_are_rejected(string? raw)
|
||||||
|
{
|
||||||
|
Assert.Null(BuildMetadata.Normalize(raw));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void A_real_value_survives_normalization_trimmed()
|
||||||
|
{
|
||||||
|
Assert.Equal("abc1234", BuildMetadata.Normalize(" abc1234 "));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Minimal stand-in for the environment-variable provider, so the test does not
|
||||||
|
// mutate the real process environment and race other tests.
|
||||||
|
private sealed class FakeEnvironmentSource : IConfigurationSource
|
||||||
|
{
|
||||||
|
private readonly (string Key, string Value)[] _values;
|
||||||
|
|
||||||
|
public FakeEnvironmentSource(params (string Key, string Value)[] values) => _values = values;
|
||||||
|
|
||||||
|
public IConfigurationProvider Build(IConfigurationBuilder builder) => new Provider(_values);
|
||||||
|
|
||||||
|
private sealed class Provider : ConfigurationProvider
|
||||||
|
{
|
||||||
|
private readonly (string Key, string Value)[] _values;
|
||||||
|
|
||||||
|
public Provider((string Key, string Value)[] values) => _values = values;
|
||||||
|
|
||||||
|
public override void Load()
|
||||||
|
{
|
||||||
|
foreach (var (key, value) in _values)
|
||||||
|
{
|
||||||
|
// "__" is the environment-variable spelling of the ":" section separator.
|
||||||
|
Data[key.Replace("__", ":")] = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -1,4 +1,3 @@
|
|||||||
using System.Reflection;
|
|
||||||
using System.Runtime.InteropServices;
|
using System.Runtime.InteropServices;
|
||||||
using JobTrackerApi.Data;
|
using JobTrackerApi.Data;
|
||||||
using JobTrackerApi.Services;
|
using JobTrackerApi.Services;
|
||||||
@@ -50,20 +49,8 @@ public sealed class AdminSystemController : ControllerBase
|
|||||||
AiServiceMetrics Ai
|
AiServiceMetrics Ai
|
||||||
);
|
);
|
||||||
|
|
||||||
private static string? NormalizeBuildMetadata(string? value)
|
// Shared with the /health endpoint so both report the same version from the same key.
|
||||||
{
|
private static string? NormalizeBuildMetadata(string? value) => BuildMetadata.Normalize(value);
|
||||||
var trimmed = (value ?? string.Empty).Trim();
|
|
||||||
if (string.IsNullOrWhiteSpace(trimmed)) return null;
|
|
||||||
|
|
||||||
// Ignore unresolved shell/compose placeholders that would otherwise leak
|
|
||||||
// directly into the admin UI, e.g. $(git rev-parse --short HEAD) or ${APP_COMMIT_SHA}.
|
|
||||||
if ((trimmed.StartsWith("$(") && trimmed.EndsWith(")")) || (trimmed.StartsWith("${") && trimmed.EndsWith("}")))
|
|
||||||
{
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
return trimmed;
|
|
||||||
}
|
|
||||||
|
|
||||||
private EmailSettingsSnapshot BuildFallbackEmailSettingsSnapshot()
|
private EmailSettingsSnapshot BuildFallbackEmailSettingsSnapshot()
|
||||||
{
|
{
|
||||||
@@ -221,11 +208,7 @@ public sealed class AdminSystemController : ControllerBase
|
|||||||
LastError: ex.Message);
|
LastError: ex.Message);
|
||||||
}
|
}
|
||||||
|
|
||||||
var version = NormalizeBuildMetadata(_cfg["App:Version"]);
|
var version = BuildMetadata.ResolveVersion(_cfg);
|
||||||
if (string.IsNullOrWhiteSpace(version))
|
|
||||||
{
|
|
||||||
version = Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "unknown";
|
|
||||||
}
|
|
||||||
|
|
||||||
var commitSha = NormalizeBuildMetadata(_cfg["App:CommitSha"]);
|
var commitSha = NormalizeBuildMetadata(_cfg["App:CommitSha"]);
|
||||||
var buildStamp = NormalizeBuildMetadata(_cfg["App:BuildStamp"]);
|
var buildStamp = NormalizeBuildMetadata(_cfg["App:BuildStamp"]);
|
||||||
|
|||||||
@@ -525,10 +525,13 @@ app.MapControllers();
|
|||||||
// that queried MariaDB would restart a perfectly healthy backend whenever the database blipped, and
|
// that queried MariaDB would restart a perfectly healthy backend whenever the database blipped, and
|
||||||
// would also be a free unauthenticated way to probe database availability.
|
// would also be a free unauthenticated way to probe database availability.
|
||||||
// docs/production-readiness-review.md.
|
// docs/production-readiness-review.md.
|
||||||
app.MapGet("/health", () => Results.Ok(new
|
// Version comes from configuration (App:Version), which is how docker-compose supplies it as
|
||||||
|
// App__Version. Reading the APP_VERSION environment variable directly meant /health always
|
||||||
|
// reported "unknown" in a container. Falls back to the assembly version for local development.
|
||||||
|
app.MapGet("/health", (IConfiguration cfg) => Results.Ok(new
|
||||||
{
|
{
|
||||||
status = "ok",
|
status = "ok",
|
||||||
version = Environment.GetEnvironmentVariable("APP_VERSION") ?? "unknown",
|
version = BuildMetadata.ResolveVersion(cfg),
|
||||||
})).AllowAnonymous();
|
})).AllowAnonymous();
|
||||||
|
|
||||||
// API schema for tooling/docs. Dev-only: not exposed in production deployments.
|
// API schema for tooling/docs. Dev-only: not exposed in production deployments.
|
||||||
|
|||||||
@@ -0,0 +1,39 @@
|
|||||||
|
using System.Reflection;
|
||||||
|
|
||||||
|
namespace JobTrackerApi.Services;
|
||||||
|
|
||||||
|
// Build metadata (version, commit, stamp) comes from configuration, populated by
|
||||||
|
// docker-compose as App__Version / App__CommitSha / App__BuildStamp.
|
||||||
|
//
|
||||||
|
// This lives here because two callers need the same answer: the admin system page and
|
||||||
|
// the anonymous /health endpoint. /health used to read the APP_VERSION *environment
|
||||||
|
// variable* directly, which compose never sets under that name — it passes App__Version,
|
||||||
|
// binding to the App:Version configuration key. So /health always reported "unknown".
|
||||||
|
// See docs/release-candidate-review.md (N2).
|
||||||
|
public static class BuildMetadata
|
||||||
|
{
|
||||||
|
// Ignore unresolved shell/compose placeholders that would otherwise leak into the
|
||||||
|
// admin UI or a health response, e.g. $(git rev-parse --short HEAD) or ${APP_COMMIT_SHA}.
|
||||||
|
public static string? Normalize(string? value)
|
||||||
|
{
|
||||||
|
var trimmed = (value ?? string.Empty).Trim();
|
||||||
|
if (string.IsNullOrWhiteSpace(trimmed)) return null;
|
||||||
|
|
||||||
|
if ((trimmed.StartsWith("$(") && trimmed.EndsWith(")")) || (trimmed.StartsWith("${") && trimmed.EndsWith("}")))
|
||||||
|
{
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return trimmed;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Configured version, else the assembly version, else "unknown". The assembly fallback
|
||||||
|
// is what makes local development useful: nothing sets App:Version there.
|
||||||
|
public static string ResolveVersion(IConfiguration cfg)
|
||||||
|
{
|
||||||
|
var version = Normalize(cfg["App:Version"]);
|
||||||
|
if (!string.IsNullOrWhiteSpace(version)) return version;
|
||||||
|
|
||||||
|
return Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "unknown";
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -144,7 +144,12 @@ upstream cannot be resolved.
|
|||||||
```bash
|
```bash
|
||||||
# Health endpoint — anonymous, does not touch the database
|
# Health endpoint — anonymous, does not touch the database
|
||||||
curl -fsS https://<host>/health
|
curl -fsS https://<host>/health
|
||||||
# expect: {"status":"ok","version":"..."}
|
# expect: {"status":"ok","version":"<the APP_VERSION you deployed>"}
|
||||||
|
#
|
||||||
|
# The version comes from App:Version, which compose passes as App__Version from APP_VERSION.
|
||||||
|
# CI sets it to the workflow run number. A version of "1.0.0.0" (the assembly fallback) means
|
||||||
|
# APP_VERSION did not reach the container — harmless in itself, but it tells you the build
|
||||||
|
# metadata is not flowing, so the admin system page will be vague about what is deployed.
|
||||||
|
|
||||||
# Auth still enforced (this is the check that proves the API is not open)
|
# Auth still enforced (this is the check that proves the API is not open)
|
||||||
curl -s -o /dev/null -w '%{http_code}\n' https://<host>/api/jobapplications
|
curl -s -o /dev/null -w '%{http_code}\n' https://<host>/api/jobapplications
|
||||||
|
|||||||
Reference in New Issue
Block a user