mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 12:42:38 +00:00
fix(agent): V1 bails cleanly when TenantId missing instead of crashing
Customer gardnermedia (2026-04-30) hit a fatal V1 startup crash: "InvalidOperationException: Invalid agent configuration" thrown from MonitoringService..ctor. Root cause: LoadConfiguration returned a config with TenantId=null because HKLM\SOFTWARE\Microsoft\Enrollments did not yet carry an EnrollmentType=6\AADTenantID entry and no bootstrap-config .json was present. Without a guard, control reached MonitoringService construction and IsValid() threw an unhandled exception → crash log → "exception_crash" classification on the next start. V2 already bails cleanly in this case (AgentBootstrap.Run line 123-127). This commit gives V1 the same safety net plus a more diagnostic message so any future occurrence is immediately analyzable from the crash log. Changes: * Program.cs: pre-flight TenantId guard right after the --await-enrollment block. Logs an Error and returns cleanly so the ProcessExit handler writes clean-exit.marker — no spurious crash log, Scheduled Task simply retries on the next trigger. * MonitoringService..ctor: replace the opaque "Invalid agent configuration" throw with one that names the missing fields (ApiBaseUrl / SessionId / TenantId). * MonitoringServiceConfigValidationTests: 5 new xUnit cases covering each missing-field path plus the multi-missing case. * V1 buildcounter bump. V2 unchanged — its existing AgentBootstrap guard already covers this class of crash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+112
@@ -0,0 +1,112 @@
|
||||
using System;
|
||||
using System.IO;
|
||||
using AutopilotMonitor.Agent.Core.Configuration;
|
||||
using AutopilotMonitor.Agent.Core.Logging;
|
||||
using AutopilotMonitor.Agent.Core.Monitoring.Runtime;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Agent.Core.Tests.Core
|
||||
{
|
||||
/// <summary>
|
||||
/// Verifies that MonitoringService rejects an incomplete AgentConfiguration with a
|
||||
/// diagnostic message that names the missing fields. Customer-impact regression
|
||||
/// (gardnermedia, 2026-04-30): a TenantId-less startup crashed with an opaque
|
||||
/// "Invalid agent configuration" message that defied forensic analysis.
|
||||
/// </summary>
|
||||
public class MonitoringServiceConfigValidationTests : IDisposable
|
||||
{
|
||||
private readonly string _tempDir;
|
||||
|
||||
public MonitoringServiceConfigValidationTests()
|
||||
{
|
||||
_tempDir = Path.Combine(Path.GetTempPath(), "autopilot-cfg-validation-" + Guid.NewGuid().ToString("N"));
|
||||
Directory.CreateDirectory(_tempDir);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
try { Directory.Delete(_tempDir, true); } catch { }
|
||||
}
|
||||
|
||||
private AgentConfiguration BuildValidConfig() => new AgentConfiguration
|
||||
{
|
||||
ApiBaseUrl = "https://example.invalid",
|
||||
SessionId = Guid.NewGuid().ToString(),
|
||||
TenantId = Guid.NewGuid().ToString(),
|
||||
SpoolDirectory = Path.Combine(_tempDir, "spool"),
|
||||
LogDirectory = Path.Combine(_tempDir, "logs")
|
||||
};
|
||||
|
||||
private AgentLogger BuildLogger() => new AgentLogger(Path.Combine(_tempDir, "logs"));
|
||||
|
||||
[Fact]
|
||||
public void Constructor_throws_with_field_name_when_TenantId_missing()
|
||||
{
|
||||
var config = BuildValidConfig();
|
||||
config.TenantId = null;
|
||||
|
||||
var ex = Assert.Throws<InvalidOperationException>(
|
||||
() => new MonitoringService(config, BuildLogger(), agentVersion: "test"));
|
||||
|
||||
Assert.Contains("missing", ex.Message);
|
||||
Assert.Contains(nameof(AgentConfiguration.TenantId), ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Constructor_throws_with_field_name_when_SessionId_missing()
|
||||
{
|
||||
var config = BuildValidConfig();
|
||||
config.SessionId = "";
|
||||
|
||||
var ex = Assert.Throws<InvalidOperationException>(
|
||||
() => new MonitoringService(config, BuildLogger(), agentVersion: "test"));
|
||||
|
||||
Assert.Contains(nameof(AgentConfiguration.SessionId), ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Constructor_throws_with_field_name_when_ApiBaseUrl_missing()
|
||||
{
|
||||
var config = BuildValidConfig();
|
||||
config.ApiBaseUrl = null;
|
||||
|
||||
var ex = Assert.Throws<InvalidOperationException>(
|
||||
() => new MonitoringService(config, BuildLogger(), agentVersion: "test"));
|
||||
|
||||
Assert.Contains(nameof(AgentConfiguration.ApiBaseUrl), ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Constructor_throws_with_all_field_names_when_multiple_missing()
|
||||
{
|
||||
var config = new AgentConfiguration
|
||||
{
|
||||
ApiBaseUrl = null,
|
||||
SessionId = null,
|
||||
TenantId = null,
|
||||
SpoolDirectory = Path.Combine(_tempDir, "spool"),
|
||||
LogDirectory = Path.Combine(_tempDir, "logs")
|
||||
};
|
||||
|
||||
var ex = Assert.Throws<InvalidOperationException>(
|
||||
() => new MonitoringService(config, BuildLogger(), agentVersion: "test"));
|
||||
|
||||
Assert.Contains(nameof(AgentConfiguration.ApiBaseUrl), ex.Message);
|
||||
Assert.Contains(nameof(AgentConfiguration.SessionId), ex.Message);
|
||||
Assert.Contains(nameof(AgentConfiguration.TenantId), ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Constructor_message_does_not_change_for_valid_config()
|
||||
{
|
||||
// Sanity: a fully-populated config should NOT throw the validation exception.
|
||||
// It may throw later (cert load, spool init) — we only assert the validation
|
||||
// path itself does not fire on a complete config.
|
||||
var config = BuildValidConfig();
|
||||
|
||||
var ex = Record.Exception(() => new MonitoringService(config, BuildLogger(), agentVersion: "test"));
|
||||
if (ex is InvalidOperationException ioe)
|
||||
Assert.DoesNotContain("Invalid agent configuration:", ioe.Message);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -101,9 +101,14 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Runtime
|
||||
_previousCrashException = previousCrashException;
|
||||
_lastBootTimeUtc = lastBootTimeUtc;
|
||||
|
||||
if (!_configuration.IsValid())
|
||||
var missingFields = new List<string>();
|
||||
if (string.IsNullOrEmpty(_configuration.ApiBaseUrl)) missingFields.Add(nameof(_configuration.ApiBaseUrl));
|
||||
if (string.IsNullOrEmpty(_configuration.SessionId)) missingFields.Add(nameof(_configuration.SessionId));
|
||||
if (string.IsNullOrEmpty(_configuration.TenantId)) missingFields.Add(nameof(_configuration.TenantId));
|
||||
if (missingFields.Count > 0)
|
||||
{
|
||||
throw new InvalidOperationException("Invalid agent configuration");
|
||||
throw new InvalidOperationException(
|
||||
"Invalid agent configuration: missing " + string.Join(", ", missingFields));
|
||||
}
|
||||
|
||||
_spool = new EventSpool(_configuration.SpoolDirectory);
|
||||
|
||||
@@ -255,6 +255,20 @@ namespace AutopilotMonitor.Agent
|
||||
}
|
||||
}
|
||||
|
||||
// Pre-flight: bail cleanly if TenantId is missing instead of crashing inside
|
||||
// MonitoringService.IsValid(). Mirrors V2's AgentBootstrap guard. Common cause:
|
||||
// agent scheduled before MDM enrollment populated HKLM\SOFTWARE\Microsoft\Enrollments
|
||||
// and no bootstrap-config.json present. The Scheduled Task will retry on next trigger.
|
||||
if (string.IsNullOrEmpty(config.TenantId))
|
||||
{
|
||||
logger.Error("Agent cannot start: TenantId not available (registry empty + no bootstrap config). " +
|
||||
"This usually means the agent was scheduled before MDM enrollment populated the registry. " +
|
||||
"Will retry on next scheduled trigger.");
|
||||
if (consoleMode)
|
||||
Console.WriteLine("ERROR: TenantId not available. Agent will exit cleanly (will retry on next trigger).");
|
||||
return;
|
||||
}
|
||||
|
||||
// Evaluate previous exit signals before starting the monitoring service.
|
||||
// clean-exit.marker present → previous run exited cleanly (OS shutdown, reboot, normal exit)
|
||||
// crash_*.log present → unhandled exception crash (with stack trace)
|
||||
|
||||
@@ -1 +1 @@
|
||||
1219
|
||||
1222
|
||||
|
||||
Reference in New Issue
Block a user