mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 20:53:06 +00:00
fix(agent): merge bootstrap-config.json on --install instead of clobbering
Three follow-ups from the TenantIdAwaiter PR review: * HIGH: A redeploy that passed only --tenant-id-wait (PS1's default invocation today) was overwriting bootstrap-config.json with BootstrapToken=null / TenantId=null, pushing devices to cert-auth before their MDM cert was usable. The install path now reads the existing file, merges only fields explicitly given on this invocation, and writes the merged result back. Field-given booleans drive the merge so that prior values survive an absent CLI flag. * MEDIUM: --tenant-id-wait 0 is now a real opt-out. Previously the persist block was gated on tenantIdWaitSeconds > 0, so an admin who flipped the PS1 default to 0 and redeployed kept the old non-zero wait persisted. The gate is now "any flag explicitly set" and the merge writes 0 over the prior value when 0 is what was given. * LOW: --tenant-id-wait <SEC> is now in --help under a new "TenantId resolution" section so the start-reliability switch is discoverable. The merge logic is extracted as Program.MergeBootstrapConfig — a pure function — and covered by 7 InstallModeMergeTests cases (wait-only preserve, explicit-zero override, tenant-only preserve, token-only preserve, first-install path, PS1 default invocation, legacy existing-config without the new field). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,158 @@
|
||||
using AutopilotMonitor.Agent.V2;
|
||||
using AutopilotMonitor.Agent.V2.Core.Configuration;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Agent.V2.Core.Tests.Program
|
||||
{
|
||||
/// <summary>
|
||||
/// Pinpoints the <c>--install</c> bootstrap-config merge semantics. The earlier
|
||||
/// implementation overwrote the file from CLI args alone, so a redeploy with only
|
||||
/// <c>--tenant-id-wait</c> wiped a previously-persisted BootstrapToken / TenantId
|
||||
/// and pushed devices to cert-auth before their cert was usable. The merge now
|
||||
/// preserves prior fields when their corresponding CLI flag is absent on this
|
||||
/// invocation, and honours an explicit value (including empty / 0) when present.
|
||||
/// </summary>
|
||||
public sealed class InstallModeMergeTests
|
||||
{
|
||||
[Fact]
|
||||
public void Wait_only_redeploy_preserves_existing_token_and_tenantid()
|
||||
{
|
||||
var existing = new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = "tok-abc",
|
||||
TenantId = "tenant-xyz",
|
||||
TenantIdWaitSeconds = 0,
|
||||
};
|
||||
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg: null!, bootstrapTokenGiven: false,
|
||||
tenantIdArg: null!, tenantIdGiven: false,
|
||||
tenantIdWaitSeconds: 600, tenantIdWaitGiven: true);
|
||||
|
||||
Assert.Equal("tok-abc", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-xyz", merged.TenantId);
|
||||
Assert.Equal(600, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Explicit_zero_wait_overrides_existing_nonzero()
|
||||
{
|
||||
var existing = new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = "tok-abc",
|
||||
TenantId = "tenant-xyz",
|
||||
TenantIdWaitSeconds = 600,
|
||||
};
|
||||
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg: null!, bootstrapTokenGiven: false,
|
||||
tenantIdArg: null!, tenantIdGiven: false,
|
||||
tenantIdWaitSeconds: 0, tenantIdWaitGiven: true);
|
||||
|
||||
// Token + tenantId untouched, wait explicitly disabled.
|
||||
Assert.Equal("tok-abc", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-xyz", merged.TenantId);
|
||||
Assert.Equal(0, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Tenant_only_redeploy_preserves_existing_wait()
|
||||
{
|
||||
var existing = new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = "tok-abc",
|
||||
TenantId = "tenant-old",
|
||||
TenantIdWaitSeconds = 1500,
|
||||
};
|
||||
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg: null!, bootstrapTokenGiven: false,
|
||||
tenantIdArg: "tenant-new", tenantIdGiven: true,
|
||||
tenantIdWaitSeconds: 0, tenantIdWaitGiven: false);
|
||||
|
||||
Assert.Equal("tok-abc", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-new", merged.TenantId);
|
||||
Assert.Equal(1500, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Token_only_redeploy_preserves_existing_wait_and_tenant()
|
||||
{
|
||||
var existing = new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = "tok-old",
|
||||
TenantId = "tenant-xyz",
|
||||
TenantIdWaitSeconds = 300,
|
||||
};
|
||||
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg: "tok-new", bootstrapTokenGiven: true,
|
||||
tenantIdArg: null!, tenantIdGiven: false,
|
||||
tenantIdWaitSeconds: 0, tenantIdWaitGiven: false);
|
||||
|
||||
Assert.Equal("tok-new", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-xyz", merged.TenantId);
|
||||
Assert.Equal(300, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void First_install_with_no_existing_config_uses_args_only()
|
||||
{
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing: null,
|
||||
bootstrapTokenArg: "tok-fresh", bootstrapTokenGiven: true,
|
||||
tenantIdArg: "tenant-fresh", tenantIdGiven: true,
|
||||
tenantIdWaitSeconds: 600, tenantIdWaitGiven: true);
|
||||
|
||||
Assert.Equal("tok-fresh", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-fresh", merged.TenantId);
|
||||
Assert.Equal(600, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void First_install_with_only_wait_arg_writes_null_token_and_tenant()
|
||||
{
|
||||
// PS1's default invocation: agent --install --tenant-id-wait 600
|
||||
// No existing config, no token, no tenantId. Result: nullable fields stay null,
|
||||
// wait gets the new value. Subsequent runtime config-load falls back to the
|
||||
// registry resolver for TenantId — that's the OOBE happy path.
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing: null,
|
||||
bootstrapTokenArg: null!, bootstrapTokenGiven: false,
|
||||
tenantIdArg: null!, tenantIdGiven: false,
|
||||
tenantIdWaitSeconds: 600, tenantIdWaitGiven: true);
|
||||
|
||||
Assert.Null(merged.BootstrapToken);
|
||||
Assert.Null(merged.TenantId);
|
||||
Assert.Equal(600, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Existing_config_without_wait_field_treated_as_zero()
|
||||
{
|
||||
// BackCompat: pre-fix bootstrap-config.json has no TenantIdWaitSeconds field;
|
||||
// Newtonsoft deserialises that to 0. New install with --tenant-id-wait 600
|
||||
// overrides cleanly; no extra migration logic required.
|
||||
var existing = new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = "tok-legacy",
|
||||
TenantId = "tenant-legacy",
|
||||
TenantIdWaitSeconds = 0,
|
||||
};
|
||||
|
||||
var merged = AutopilotMonitor.Agent.V2.Program.MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg: null!, bootstrapTokenGiven: false,
|
||||
tenantIdArg: null!, tenantIdGiven: false,
|
||||
tenantIdWaitSeconds: 600, tenantIdWaitGiven: true);
|
||||
|
||||
Assert.Equal("tok-legacy", merged.BootstrapToken);
|
||||
Assert.Equal("tenant-legacy", merged.TenantId);
|
||||
Assert.Equal(600, merged.TenantIdWaitSeconds);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -65,35 +65,66 @@ namespace AutopilotMonitor.Agent.V2
|
||||
// Persist bootstrap config if any of --bootstrap-token / --tenant-id /
|
||||
// --tenant-id-wait was provided. The Scheduled Task command line has no args —
|
||||
// the agent picks the persisted values up on the first post-install run.
|
||||
//
|
||||
// Read-merge-write: only fields explicitly set on this install invocation
|
||||
// overwrite the persisted file. This protects against a redeploy that passes
|
||||
// only --tenant-id-wait clobbering an existing BootstrapToken / TenantId, and
|
||||
// lets an admin opt out of the wait by passing --tenant-id-wait 0 explicitly
|
||||
// (which writes 0 over an old non-zero value).
|
||||
var bootstrapTokenArg = GetArgValue(args, "--bootstrap-token");
|
||||
var tenantIdArg = GetArgValue(args, "--tenant-id");
|
||||
var tenantIdWaitArg = GetArgValue(args, "--tenant-id-wait");
|
||||
|
||||
bool bootstrapTokenGiven = !string.IsNullOrEmpty(bootstrapTokenArg);
|
||||
bool tenantIdGiven = !string.IsNullOrEmpty(tenantIdArg);
|
||||
bool tenantIdWaitGiven = !string.IsNullOrEmpty(tenantIdWaitArg);
|
||||
|
||||
int tenantIdWaitSeconds = 0;
|
||||
if (!string.IsNullOrEmpty(tenantIdWaitArg) && !int.TryParse(tenantIdWaitArg, out tenantIdWaitSeconds))
|
||||
if (tenantIdWaitGiven && !int.TryParse(tenantIdWaitArg, out tenantIdWaitSeconds))
|
||||
{
|
||||
logger.Warning($"Install: --tenant-id-wait '{tenantIdWaitArg}' is not a valid integer — ignoring.");
|
||||
tenantIdWaitGiven = false;
|
||||
tenantIdWaitSeconds = 0;
|
||||
}
|
||||
|
||||
if (!string.IsNullOrEmpty(bootstrapTokenArg) ||
|
||||
!string.IsNullOrEmpty(tenantIdArg) ||
|
||||
tenantIdWaitSeconds > 0)
|
||||
if (bootstrapTokenGiven || tenantIdGiven || tenantIdWaitGiven)
|
||||
{
|
||||
var bootstrapConfigPath = Path.Combine(
|
||||
Environment.ExpandEnvironmentVariables(Constants.AgentDataDirectory),
|
||||
BootstrapConfigFileName);
|
||||
var bootstrapConfig = new BootstrapConfigFile
|
||||
|
||||
BootstrapConfigFile existing = null;
|
||||
if (File.Exists(bootstrapConfigPath))
|
||||
{
|
||||
BootstrapToken = bootstrapTokenArg,
|
||||
TenantId = tenantIdArg,
|
||||
TenantIdWaitSeconds = tenantIdWaitSeconds,
|
||||
};
|
||||
File.WriteAllText(bootstrapConfigPath, JsonConvert.SerializeObject(bootstrapConfig));
|
||||
try
|
||||
{
|
||||
existing = JsonConvert.DeserializeObject<BootstrapConfigFile>(
|
||||
File.ReadAllText(bootstrapConfigPath));
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
logger.Warning(
|
||||
$"Existing bootstrap-config.json could not be parsed " +
|
||||
$"({ex.GetType().Name}: {ex.Message}) — overwriting fresh.");
|
||||
}
|
||||
}
|
||||
|
||||
var merged = MergeBootstrapConfig(
|
||||
existing,
|
||||
bootstrapTokenArg, bootstrapTokenGiven,
|
||||
tenantIdArg, tenantIdGiven,
|
||||
tenantIdWaitSeconds, tenantIdWaitGiven);
|
||||
|
||||
File.WriteAllText(bootstrapConfigPath, JsonConvert.SerializeObject(merged));
|
||||
logger.Info(
|
||||
$"Bootstrap config persisted for Scheduled Task " +
|
||||
$"(token={(string.IsNullOrEmpty(bootstrapTokenArg) ? "no" : "yes")}, " +
|
||||
$"tenantId={(string.IsNullOrEmpty(tenantIdArg) ? "no" : "yes")}, " +
|
||||
$"tenantIdWait={tenantIdWaitSeconds}s).");
|
||||
$"(token={(string.IsNullOrEmpty(merged.BootstrapToken) ? "no" : "yes")}" +
|
||||
$"{(bootstrapTokenGiven ? "*" : "")}, " +
|
||||
$"tenantId={(string.IsNullOrEmpty(merged.TenantId) ? "no" : "yes")}" +
|
||||
$"{(tenantIdGiven ? "*" : "")}, " +
|
||||
$"tenantIdWait={merged.TenantIdWaitSeconds}s" +
|
||||
$"{(tenantIdWaitGiven ? "*" : "")})." +
|
||||
" Star = explicitly set on this --install invocation, others were preserved from prior config.");
|
||||
}
|
||||
|
||||
// Persist await-enrollment config if requested.
|
||||
@@ -260,6 +291,30 @@ namespace AutopilotMonitor.Agent.V2
|
||||
logger.Info($"Payload deployment completed: '{sourceDir}' -> '{targetDir}'");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Pure-function merge for <c>bootstrap-config.json</c>. Each <c>...Given</c> flag tells
|
||||
/// us whether the corresponding CLI arg was explicitly set on this <c>--install</c>
|
||||
/// invocation. When given, the new value wins (including explicit zero / empty) — this
|
||||
/// is what makes <c>--tenant-id-wait 0</c> a real opt-out. When not given, the prior
|
||||
/// persisted value is preserved — this is what stops a redeploy that only carries
|
||||
/// <c>--tenant-id-wait</c> from clobbering a previously-set BootstrapToken / TenantId.
|
||||
/// </summary>
|
||||
internal static BootstrapConfigFile MergeBootstrapConfig(
|
||||
BootstrapConfigFile existing,
|
||||
string bootstrapTokenArg, bool bootstrapTokenGiven,
|
||||
string tenantIdArg, bool tenantIdGiven,
|
||||
int tenantIdWaitSeconds, bool tenantIdWaitGiven)
|
||||
{
|
||||
return new BootstrapConfigFile
|
||||
{
|
||||
BootstrapToken = bootstrapTokenGiven ? bootstrapTokenArg : existing?.BootstrapToken,
|
||||
TenantId = tenantIdGiven ? tenantIdArg : existing?.TenantId,
|
||||
TenantIdWaitSeconds = tenantIdWaitGiven
|
||||
? tenantIdWaitSeconds
|
||||
: (existing?.TenantIdWaitSeconds ?? 0),
|
||||
};
|
||||
}
|
||||
|
||||
private static void TryWriteDeploymentMarker(AgentLogger logger)
|
||||
{
|
||||
try
|
||||
|
||||
@@ -415,6 +415,13 @@ namespace AutopilotMonitor.Agent.V2
|
||||
Console.Out.WriteLine(" --await-enrollment Wait for MDM certificate before starting monitoring");
|
||||
Console.Out.WriteLine(" --await-enrollment-timeout <MIN> Timeout in minutes (default: 480)");
|
||||
Console.Out.WriteLine();
|
||||
Console.Out.WriteLine("TenantId resolution:");
|
||||
Console.Out.WriteLine(" --tenant-id-wait <SEC> On miss, wait up to N seconds for the registry to");
|
||||
Console.Out.WriteLine(" populate (RegistryWatcher on Enrollments + CloudDomainJoin).");
|
||||
Console.Out.WriteLine(" 0 = disabled (legacy fast-fail). At --install time the value");
|
||||
Console.Out.WriteLine(" is persisted to bootstrap-config.json so the Scheduled Task");
|
||||
Console.Out.WriteLine(" picks it up on each run.");
|
||||
Console.Out.WriteLine();
|
||||
Console.Out.WriteLine("Overrides:");
|
||||
Console.Out.WriteLine(" --api-url <URL> Override backend API base URL (alias: --backend-api)");
|
||||
Console.Out.WriteLine(" --ime-log-path <PATH> Override IME logs directory");
|
||||
|
||||
@@ -1 +1 @@
|
||||
516
|
||||
518
|
||||
|
||||
@@ -1 +1 @@
|
||||
1233
|
||||
1234
|
||||
|
||||
Reference in New Issue
Block a user