mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-08-11 10:03:34 +00:00
fix(install): stop the agent version warning firing on correct installs
The installer compares the agent binary it downloaded against the server that served it, stripping a leading "v" so "v6.0.4" and "6.0.4" match. It did not strip semver build metadata, so a server built from a working tree reporting "6.2.0-rc.8+git.46.g98a638e00.dirty" never matched the "v6.2.0-rc.8" agent it had just served, and the mismatch warning fired on every correct development install. This is the warning's whole job, so a false positive is expensive. It is the only client-side signal that a stale agent was downloaded, and because it always fired it read as background noise. That is exactly how a genuinely stale v6.0.5 agent was installed on a live host earlier today: the warning was there, above the install output, and looked like the one that always appears. Strip build metadata from both sides before comparing, keeping the prerelease suffix because 6.2.0-rc.8 and 6.2.0 are genuinely different releases. This is the same release-identity reduction the server applies when deciding whether a local agent artifact is fresh enough to serve; the contracts now state that one definition governs both ends rather than leaving each side to invent its own. Guarded by a test that pins both normalisation steps and exercises the comparison across the cases that matter: the dev-server shape that used to warn wrongly, the stale-download shape that must still warn, and a prerelease against its release. Verified to fail when either strip is removed.
This commit is contained in:
parent
1f45db6e54
commit
9d4f6ae923
4 changed files with 82 additions and 1 deletions
|
|
@ -110,6 +110,16 @@ server's own agent version, and resolves that expected version from the same
|
|||
source the agent build stamps in rather than from a compiled-in build
|
||||
placeholder, because development builds carry placeholders no version parser
|
||||
accepts and those are precisely the builds whose local artifacts go stale.
|
||||
One definition of "same agent version" governs both ends of that exchange. It
|
||||
is the release identity: a leading `v` and semver build metadata are not part
|
||||
of it, the prerelease suffix is. The server applies it when deciding whether a
|
||||
local artifact is fresh enough to serve, and the installer applies it when
|
||||
reporting whether the agent it downloaded matches the server that served it.
|
||||
Letting the two ends disagree costs more than a redundant check, because the
|
||||
installer's report is the only signal a human sees: when it compared raw
|
||||
strings it contradicted a correct server on every development install, and a
|
||||
contradiction that appears every time is read as noise rather than as the one
|
||||
warning that mattered.
|
||||
|
||||
Mock mode is a clean room on the report-admission boundary. Mock mode already
|
||||
suspends pull-based PVE/PBS/PMG collection by never building those clients, and
|
||||
|
|
|
|||
|
|
@ -2809,7 +2809,16 @@ placeholders that no version parser accepts and those are exactly the builds
|
|||
whose artifacts go stale. Refusal is loud rather than silent: a development
|
||||
server answers 404 naming the stale path and the build command, and a published
|
||||
release falls through to the release-asset proxy and fetches the matching
|
||||
version. Token-bearing
|
||||
version.
|
||||
The installer's own version diagnostics answer to the same identity. When it
|
||||
compares the agent it downloaded against the server that served it, it compares
|
||||
release identity, stripping a leading `v` and semver build metadata while
|
||||
keeping the prerelease suffix, because a server built from a working tree
|
||||
reports metadata the agent never carries. Comparing raw strings made the
|
||||
mismatch warning fire on every correct development install, and a warning that
|
||||
fires when nothing is wrong is worse than no warning: it is the only
|
||||
client-side signal that a stale agent was downloaded, and one that cries wolf
|
||||
gets skipped the time it is real. Token-bearing
|
||||
copy-paste commands must pass credentials through ephemeral `--token-file`
|
||||
transport and leave the installed service configured with the persistent
|
||||
runtime token file, never a raw `--token` process argument.
|
||||
|
|
|
|||
|
|
@ -3628,8 +3628,18 @@ NEW_VERSION=$("$TMP_BIN" --version 2>/dev/null | head -1 || echo "unknown")
|
|||
# Compare versions with any leading "v" stripped so the agent binary's "v6.0.4"
|
||||
# and the server /api/version "6.0.4" are treated as equal. Only a genuine
|
||||
# version difference (e.g. 6.0.3 vs 6.0.4) should raise the mismatch warning.
|
||||
#
|
||||
# Semver build metadata is stripped for the same reason. A server built from a
|
||||
# working tree reports "6.2.0-rc.8+git.46.g98a638e00.dirty" while the agent it
|
||||
# serves carries the release identity "v6.2.0-rc.8"; those are the same release,
|
||||
# and comparing them raw made this warning fire on every correct development
|
||||
# install. A warning that fires when nothing is wrong is worse than no warning,
|
||||
# because it trains the reader to skip the one time it is real. The prerelease
|
||||
# suffix is deliberately kept: 6.2.0-rc.8 and 6.2.0 are genuinely different.
|
||||
NEW_VERSION_NORMALIZED="${NEW_VERSION#v}"
|
||||
NEW_VERSION_NORMALIZED="${NEW_VERSION_NORMALIZED%%+*}"
|
||||
SERVER_VERSION_NORMALIZED="${SERVER_VERSION#v}"
|
||||
SERVER_VERSION_NORMALIZED="${SERVER_VERSION_NORMALIZED%%+*}"
|
||||
|
||||
if [[ -n "$SERVER_VERSION" && -n "$NEW_VERSION" && "$NEW_VERSION" != "unknown" && "$NEW_VERSION_NORMALIZED" != "$SERVER_VERSION_NORMALIZED" ]]; then
|
||||
log_warn "Downloaded agent version (${NEW_VERSION}) does not match Pulse server version (${SERVER_VERSION}). Check that Pulse is upgraded and that any reverse proxy is not serving a stale cached binary."
|
||||
|
|
|
|||
|
|
@ -5288,3 +5288,55 @@ func TestInstallSHStopsWrapperBeforeAgentInEveryBranch(t *testing.T) {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestInstallSHVersionMismatchWarningIgnoresBuildMetadata pins the comparison
|
||||
// behind the "Downloaded agent version does not match" warning. A server built
|
||||
// from a working tree reports build metadata the agent it serves never carries,
|
||||
// so a raw comparison fired on every correct development install. That is not a
|
||||
// cosmetic annoyance: the warning is the only client-side signal that a stale
|
||||
// agent was downloaded, and one that cries wolf gets skipped the time it counts.
|
||||
func TestInstallSHVersionMismatchWarningIgnoresBuildMetadata(t *testing.T) {
|
||||
content, err := os.ReadFile(repoFile("scripts", "install.sh"))
|
||||
if err != nil {
|
||||
t.Fatalf("read install.sh: %v", err)
|
||||
}
|
||||
script := string(content)
|
||||
|
||||
for _, required := range []string{
|
||||
`NEW_VERSION_NORMALIZED="${NEW_VERSION_NORMALIZED%%+*}"`,
|
||||
`SERVER_VERSION_NORMALIZED="${SERVER_VERSION_NORMALIZED%%+*}"`,
|
||||
} {
|
||||
if !strings.Contains(script, required) {
|
||||
t.Errorf("version comparison must strip semver build metadata, missing: %s", required)
|
||||
}
|
||||
}
|
||||
|
||||
// Exercise the same normalisation the installer performs.
|
||||
normalize := func(v string) string {
|
||||
v = strings.TrimPrefix(v, "v")
|
||||
if idx := strings.Index(v, "+"); idx >= 0 {
|
||||
v = v[:idx]
|
||||
}
|
||||
return v
|
||||
}
|
||||
cases := []struct {
|
||||
agent string
|
||||
server string
|
||||
warns bool
|
||||
}{
|
||||
// The shape that fired on every correct dev install.
|
||||
{"v6.2.0-rc.8", "6.2.0-rc.8+git.46.g98a638e00.dirty", false},
|
||||
{"v6.2.0-rc.8", "6.2.0-rc.8", false},
|
||||
// The stale download this warning exists to catch.
|
||||
{"v6.0.5-54-gc862fb0ca0", "6.2.0-rc.8+git.46.g98a638e00.dirty", true},
|
||||
// A prerelease is genuinely not its release.
|
||||
{"v6.2.0", "6.2.0-rc.8+git.46.gabc.dirty", true},
|
||||
{"v6.1.2", "6.2.0-rc.8", true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
got := normalize(tc.agent) != normalize(tc.server)
|
||||
if got != tc.warns {
|
||||
t.Errorf("agent %q vs server %q: warns=%v, want %v", tc.agent, tc.server, got, tc.warns)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue