Harden endpoint agent command channel against request storms
A misconfigured BaseUrl (plaintext http:// against the TLS port) made the command long-poll fail at connect in milliseconds. The worker misread that fast failure as a normal idle hold-window close and re-polled with no delay, spinning into a request storm. Gate the idle-close path on the poll having stayed open at least MinIdlePollDuration, so a sub-second transport failure takes the transient backoff instead; add a MinPollInterval floor so no path can exceed a safe request rate. Also reject a non-loopback http:// BaseUrl at startup with a single clear error, turning a silent runtime flood into a fast fail. Loopback http stays valid for local dev.
This commit is contained in:
@@ -84,6 +84,39 @@ public sealed class AgentConfig
|
||||
=> !string.IsNullOrWhiteSpace(BaseUrl)
|
||||
&& !string.IsNullOrWhiteSpace(ApiKey)
|
||||
&& ApiKey != "REPLACE_WITH_COLLECTOR_KEY";
|
||||
|
||||
// Validates the BaseUrl scheme before any request is made. A plaintext
|
||||
// http:// URL aimed at a real (non-loopback) server is the classic
|
||||
// misconfiguration that hits the TLS port and used to spin the command
|
||||
// long-poll into a storm; refusing to start on it turns a silent, noisy
|
||||
// runtime failure into a single clear startup error. Loopback over http is
|
||||
// still allowed so a developer running the API on plain http locally works.
|
||||
// Returns a human-readable problem, or null when the BaseUrl is acceptable.
|
||||
public string? BaseUrlProblem()
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(BaseUrl))
|
||||
return null;
|
||||
|
||||
if (!Uri.TryCreate(BaseUrl.Trim(), UriKind.Absolute, out var uri))
|
||||
return $"BaseUrl '{BaseUrl}' is not a valid absolute URL.";
|
||||
|
||||
if (uri.Scheme == Uri.UriSchemeHttps)
|
||||
return null;
|
||||
|
||||
if (uri.Scheme == Uri.UriSchemeHttp)
|
||||
{
|
||||
if (uri.IsLoopback)
|
||||
return null;
|
||||
return $"BaseUrl '{BaseUrl}' uses plaintext http:// against a " +
|
||||
"non-loopback host. The server requires https://; an http:// " +
|
||||
"URL hits the TLS port and fails every request. Reprovision " +
|
||||
"BaseUrl in HKLM\\SOFTWARE\\GE\\ShopDB (or appsettings.json) " +
|
||||
"with an https:// URL.";
|
||||
}
|
||||
|
||||
return $"BaseUrl '{BaseUrl}' uses unsupported scheme '{uri.Scheme}'. " +
|
||||
"Use https:// (or http:// only for a loopback dev server).";
|
||||
}
|
||||
}
|
||||
|
||||
// Watched targets for this endpoint. Both lists carry entries with a wire name
|
||||
|
||||
@@ -24,6 +24,19 @@ public sealed class CommandWorker : BackgroundService
|
||||
// operator has to fix something, so re-polling fast changes nothing.
|
||||
private static readonly TimeSpan PermanentRetryDelay = TimeSpan.FromSeconds(300);
|
||||
|
||||
// A genuine idle long-poll close only happens after the server has held the
|
||||
// request open for most of its hold window (~25s). A connection that drops
|
||||
// in well under this did not idle out - it failed at connect/TLS (e.g. an
|
||||
// http:// BaseUrl hitting the TLS port), which must NOT be treated as a
|
||||
// normal close and fast re-polled. Below this threshold we take the
|
||||
// transient-failure backoff instead.
|
||||
private static readonly TimeSpan MinIdlePollDuration = TimeSpan.FromSeconds(5);
|
||||
|
||||
// Hard floor between poll starts. Even if some unforeseen path returns
|
||||
// immediately every iteration, this caps the loop at a safe request rate so
|
||||
// a misconfigured agent can never turn into a request storm.
|
||||
private static readonly TimeSpan MinPollInterval = TimeSpan.FromSeconds(2);
|
||||
|
||||
private readonly CommandClient client;
|
||||
private readonly CommandExecutor executor;
|
||||
private readonly ILogger<CommandWorker> logger;
|
||||
@@ -46,6 +59,7 @@ public sealed class CommandWorker : BackgroundService
|
||||
|
||||
while (!stoppingToken.IsCancellationRequested)
|
||||
{
|
||||
var pollStarted = System.Diagnostics.Stopwatch.StartNew();
|
||||
try
|
||||
{
|
||||
var claim = await client.ClaimNextAsync(hostname, stoppingToken);
|
||||
@@ -64,18 +78,32 @@ public sealed class CommandWorker : BackgroundService
|
||||
"Permanent command-channel error; backing off");
|
||||
await SafeDelay(PermanentRetryDelay, stoppingToken);
|
||||
}
|
||||
catch (Exception error) when (IsIdlePollClose(error))
|
||||
catch (Exception error) when (IsIdlePollClose(error)
|
||||
&& pollStarted.Elapsed >= MinIdlePollDuration)
|
||||
{
|
||||
// The server (or a proxy) closed the held-open long poll without
|
||||
// a command. That is the normal end of a hold window, not a
|
||||
// failure: log quietly at debug and re-poll immediately.
|
||||
// The server (or a proxy) held the poll open for most of the
|
||||
// hold window and then closed it without a command. That is the
|
||||
// normal end of a hold window, not a failure: log quietly at
|
||||
// debug and re-poll immediately.
|
||||
logger.LogDebug(error, "Command poll connection closed; re-polling");
|
||||
}
|
||||
catch (Exception error)
|
||||
{
|
||||
// Anything else - including a connection/TLS failure that drops
|
||||
// in under MinIdlePollDuration (e.g. an http:// BaseUrl hitting
|
||||
// the TLS port) - is a real transient failure. Back off so a
|
||||
// misconfiguration cannot spin into a request storm.
|
||||
logger.LogWarning(error, "Command poll failed; retrying shortly");
|
||||
await SafeDelay(TransientRetryDelay, stoppingToken);
|
||||
}
|
||||
|
||||
// Floor the loop rate. On the immediate re-poll paths (204 or a
|
||||
// genuine idle close) the elapsed time already exceeds this, so it
|
||||
// adds no latency; it only bites if an iteration returned unusually
|
||||
// fast, capping the worst case at a safe request rate.
|
||||
var remaining = MinPollInterval - pollStarted.Elapsed;
|
||||
if (remaining > TimeSpan.Zero)
|
||||
await SafeDelay(remaining, stoppingToken);
|
||||
}
|
||||
|
||||
logger.LogInformation("Command channel stopped");
|
||||
@@ -114,8 +142,10 @@ public sealed class CommandWorker : BackgroundService
|
||||
|
||||
// A long poll that the server or an intermediary closes without sending a
|
||||
// command surfaces as an HttpRequestException wrapping an HttpIOException
|
||||
// (ResponseEnded), or a bare IOException. That is the expected end of a hold
|
||||
// window, so it should not be treated as a transient failure.
|
||||
// (ResponseEnded), or a bare IOException. Whether that is a benign hold-
|
||||
// window close or a fast connect/TLS failure is decided by the caller using
|
||||
// how long the poll stayed open (see MinIdlePollDuration); this only matches
|
||||
// the exception shape.
|
||||
private static bool IsIdlePollClose(Exception error)
|
||||
{
|
||||
for (Exception? current = error; current is not null; current = current.InnerException)
|
||||
|
||||
@@ -46,6 +46,13 @@ if (!config.IsUsable())
|
||||
return 1;
|
||||
}
|
||||
|
||||
var baseUrlProblem = config.BaseUrlProblem();
|
||||
if (baseUrlProblem is not null)
|
||||
{
|
||||
bootstrapLogger.LogError("Config rejected: {Problem}", baseUrlProblem);
|
||||
return 1;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
if (runOnce)
|
||||
|
||||
Reference in New Issue
Block a user