mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Merge pull request #2166 from rcourtman/maintainer/20260922T173907Z
Keep copied installs usable and prevent duplicate disk rows
This commit is contained in:
pulse-triage[bot]
@@ -8193,3 +8193,18 @@ Focused proof lives in `internal/hostagent/agent_new_test.go`
|
||||
`TestDefaultCollectorHostInfoKeepsErrorWhenUnrecovered` and
|
||||
`TestGetReliableMachineIDNormalizesWindowsBraces`); the Windows recovery file is
|
||||
cross-compiled with `GOOS=windows go build ./internal/hostagent/`.
|
||||
|
||||
### Unix bootstrap survives single-line command fields
|
||||
|
||||
The shared frontend Unix installer command uses explicit shell statement
|
||||
separators rather than literal newlines or backslash-newline continuations.
|
||||
This preserves its grammar when copied into a single-line command field (#2123).
|
||||
The builder rejects embedded CR/LF in its normalized inputs rather than silently
|
||||
altering a quoted URL, credential, CA path or option. Shell quoting is unchanged.
|
||||
Download and unprivileged preflight still precede token-file creation and sudo
|
||||
installation. Root and sudo paths retain private token directories, file modes,
|
||||
TLS options, non-interactive operation and cleanup on success or failure.
|
||||
`agentInstallCommand.test.ts` covers normalized-input syntax and
|
||||
synthetic root/sudo execution under sh and Bash, including download, preflight
|
||||
and install failures. A clipboard fixture proves input compatibility, not native
|
||||
FreeBSD installation, offline dependency availability or service persistence.
|
||||
|
||||
@@ -10891,3 +10891,17 @@ setup and checks persisted non-auth preferences, rather than substituting a
|
||||
status-only or unrelated test for the setup contract. The registry routes only `internal/api/security_setup_fix.go` to this narrow
|
||||
setup proof policy; unrelated API runtime paths retain their existing verification
|
||||
requirements. No runtime paths or required contract updates are exempted.
|
||||
|
||||
### Single-line Unix client install transport
|
||||
|
||||
`frontend-modern/src/utils/agentInstallCommand.ts` emits a single-line Unix
|
||||
bootstrap with explicit shell separators so text-input paste normalization
|
||||
cannot join shell statements (#2123). It retains the canonical URL, quoted
|
||||
credential-to-private-file transport, custom CA/insecure continuity, and
|
||||
preflight-before-install ordering; no API request, response or auth scope changes.
|
||||
Normalized values containing CR/LF are rejected, not rewritten. Regression
|
||||
coverage in `agentInstallCommand.test.ts` checks shell syntax after
|
||||
text-input normalization, literal quoted token bytes, root/sudo private-file
|
||||
permissions, cleanup and early exit when download or preflight fails. This
|
||||
client transport repair does not establish the reporter's native pfSense abort
|
||||
cause or successful offline installation.
|
||||
|
||||
@@ -5245,3 +5245,24 @@ provider keys, canonical IDs, JSON projection, temperature/size/cadence, separat
|
||||
nodes/controller members, and confirmed inventory removal.
|
||||
`TestPhysicalDiskReadbackSourceIDFallback` covers missing source metadata.
|
||||
This is synthetic runtime evidence, not USB hardware or reporter acceptance.
|
||||
|
||||
### Linked direct-disk aliases with missing hardware identity (#2076)
|
||||
|
||||
Agent SMART and PVE physical-disk observations may share a canonical resource
|
||||
when they have the same already-correlated parent, the same normalized device
|
||||
path, compatible controller/target metadata and exactly one opposite-source
|
||||
candidate. The direct-device fallback applies only when at least one observation
|
||||
has neither a usable serial nor WWN. Two differing populated hardware identities
|
||||
are not missing identity; matching capacity or model is never identity evidence.
|
||||
Controller-member targets cannot use this fallback because a kernel path can
|
||||
represent several members. Existing explicit hardware and SAS correlation remain
|
||||
separate rules. Source-native IDs and per-source status are retained on the merged
|
||||
resource, as are hardware details supplied by the richer collector.
|
||||
|
||||
`TestIssue2076USBMixedSourceSnapshot` covers serial-present/absent in either
|
||||
collector, both absent and placeholder identity through repeated ordinary snapshot
|
||||
replacement, physical-disk views and JSON projection. Negative cases retain
|
||||
separate conflicting IDs, devices, hosts and controller members; the linked-disk
|
||||
ambiguity test rejects multiple opposite-source candidates in both directions.
|
||||
These are synthetic source proofs, not appliance acceptance or a claim about
|
||||
which collector produced a reporter's row.
|
||||
|
||||
@@ -1,54 +1,36 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "4b1fb511d1f1d2d363ba732914e6678f1111a9b2",
|
||||
"verified_at": "2026-09-22T13:31:09.864671Z",
|
||||
"base_sha": "702278cd8cca6bfbd65747c72fbb7afdc7f7a48e",
|
||||
"verified_at": "2026-09-22T16:26:06.944232Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx",
|
||||
"frontend-modern/src/components/shared/DetailSectionTable.tsx",
|
||||
"frontend-modern/src/components/shared/detailSectionModel.ts"
|
||||
"frontend-modern/src/utils/agentInstallCommand.ts"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx": "713c26cde7852d7499138684a142e623b99f591d6b1f60c5eb639efef25376ff",
|
||||
"frontend-modern/src/components/shared/DetailSectionTable.tsx": "f14935b93d3062e22215f55dcf8725589265b69a60a03dae10949b374a1e2cb8",
|
||||
"frontend-modern/src/components/shared/detailSectionModel.ts": "0669d3afc5bc3d0809c3c2ee4204913c28aa87703c218ee3110b626e5792d086"
|
||||
"frontend-modern/src/utils/agentInstallCommand.ts": "5c456b9b8cf2d114d5be62bb46c27b265d30fc49c3df88d2ec42631414f65c64"
|
||||
},
|
||||
"routes": [
|
||||
"GuestDrawerOverview production-component fixture /proof.html"
|
||||
"Production buildUnixAgentInstallCommand in synthetic clipboard/text-input fixture"
|
||||
],
|
||||
"viewports": [
|
||||
{
|
||||
"width": 1600,
|
||||
"height": 1100
|
||||
},
|
||||
{
|
||||
"width": 1280,
|
||||
"height": 1100
|
||||
},
|
||||
{
|
||||
"width": 768,
|
||||
"height": 1100
|
||||
"height": 850
|
||||
},
|
||||
{
|
||||
"width": 390,
|
||||
"height": 1100
|
||||
},
|
||||
{
|
||||
"width": 320,
|
||||
"height": 1100
|
||||
"height": 850
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"Guest with System, Memory, Filesystems and Tags sections",
|
||||
"Shared-prefix mount paths ending in Films and Series",
|
||||
"Long path with spaces and 140-character unbroken path segment",
|
||||
"1280px desktop with a constrained 650px pane; tablet/phone touch emulation"
|
||||
"Tokenless HTTPS, token HTTP, quoted URL/token, insecure HTTPS, custom CA with extra flags"
|
||||
],
|
||||
"interactions": [
|
||||
"Read all paths without hovering or scrolling horizontally",
|
||||
"Assert text ranges fit all five label boxes at all five layouts, no page overflow or uncaught errors, and five progress bars",
|
||||
"Inspect before/after desktop, constrained desktop and smallest-phone screenshots"
|
||||
"Copy generated command with browser clipboard API and paste with Control+V into input type=text",
|
||||
"Assert five variants preserve exact bytes at both viewports; old source changes all five",
|
||||
"Check pasted output syntax with sh and bash: old five fail, repaired five pass",
|
||||
"Inspect desktop-before and narrow-after screenshots"
|
||||
],
|
||||
"command": "pulse-worker-browser .proof-filesystems/after.cjs",
|
||||
"notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. Synthetic guest, real GuestDrawerOverview, shared renderer and application CSS. Before repair, desktop labels are 112px with ellipsis; after repair complete paths wrap above usage. No live backend, published-build qualification or reporter confirmation. Other detail rows retain their compact layout."
|
||||
"command": "pulse-worker-browser .proof-install/browser.cjs",
|
||||
"notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. TypeScript-transpiled production utility, synthetic pulse.invalid and invented tokens. Generic text field, not pfSense DOM or installation. No backend/network/service proof. Retained proof was rebound after the test-only assertion correction and exact candidate integration; production utility bytes are unchanged from the browser run."
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { execFileSync } from 'node:child_process';
|
||||
import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
buildUnixAgentInstallCommand,
|
||||
@@ -330,7 +330,7 @@ fi
|
||||
});
|
||||
|
||||
expect(command).toContain('--token-file "$token_file"');
|
||||
expect(command).toContain('--enable-docker \\\n --disable-host \\\n --enable-commands');
|
||||
expect(command).toContain('--enable-docker --disable-host --enable-commands');
|
||||
});
|
||||
|
||||
it('preserves extra env assignments for shared Windows install transport', () => {
|
||||
@@ -394,3 +394,129 @@ describe('resolveAgentCommandPlatform', () => {
|
||||
expect(resolveAgentCommandPlatform(null)).toBe('linux');
|
||||
});
|
||||
});
|
||||
|
||||
const variants = [
|
||||
{ baseUrl: 'https://pulse.invalid' },
|
||||
{ baseUrl: 'http://pulse.invalid/base/', token: 'invented-token' },
|
||||
{ baseUrl: "https://pulse.invalid/agent's path", token: "tok'en;$(false)`false`\\value" },
|
||||
{ baseUrl: 'https://pulse.invalid', insecure: true, token: 'invented-token' },
|
||||
{
|
||||
baseUrl: 'https://pulse.invalid',
|
||||
caCertPath: "/tmp/agent's ca.pem",
|
||||
extraArgs: ['--enable-docker', '--disable-host'],
|
||||
},
|
||||
];
|
||||
|
||||
describe('single-line Unix install commands', () => {
|
||||
it.each(variants)('survives text-input normalization: %j', (options) => {
|
||||
const command = buildUnixAgentInstallCommand(options);
|
||||
const input = document.createElement('input');
|
||||
input.type = 'text';
|
||||
input.value = command;
|
||||
expect(command).not.toMatch(/[\r\n]/);
|
||||
expect(input.value).toBe(command);
|
||||
for (const shell of ['sh', 'bash']) {
|
||||
expect(() => execFileSync(shell, ['-n', '-c', input.value])).not.toThrow();
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ baseUrl: 'https://pulse.invalid/a\nb' },
|
||||
{ baseUrl: 'https://pulse.invalid', token: 'a\rb' },
|
||||
{ baseUrl: 'https://pulse.invalid', caCertPath: '/tmp/a\nb' },
|
||||
{ baseUrl: 'https://pulse.invalid', extraArgs: ['--a\n--b'] },
|
||||
])('rejects embedded line breaks without silently changing values: %j', (options) => {
|
||||
expect(() => buildUnixAgentInstallCommand(options)).toThrow('must not contain line breaks');
|
||||
});
|
||||
|
||||
it.each(['sh', 'bash'])(
|
||||
'preserves execution and failure ordering under %s after paste',
|
||||
(shell) => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'pulse-single-line-'));
|
||||
const trace = join(dir, 'trace');
|
||||
const tokenPath = join(dir, 'token-path');
|
||||
const scriptPath = join(dir, 'script-path');
|
||||
const token = "tok'en;$(false)`false`\\value";
|
||||
const installer = join(dir, 'fixture-installer');
|
||||
const fake = (name: string, body: string) => {
|
||||
writeFileSync(join(dir, name), `#!/bin/sh\n${body}\n`);
|
||||
chmodSync(join(dir, name), 0o700);
|
||||
};
|
||||
try {
|
||||
fake('id', 'echo "$FAKE_UID"');
|
||||
fake('sudo', 'echo sudo >> "$TRACE"; exec "$@"');
|
||||
fake(
|
||||
'curl',
|
||||
`echo fetch >> "$TRACE"
|
||||
while [ "$#" -gt 0 ]; do
|
||||
if [ "$1" = -o ]; then output="$2"; shift; fi
|
||||
shift
|
||||
done
|
||||
printf %s "$output" > "$SCRIPT_PATH"
|
||||
[ "$FAIL_AT" != fetch ] || exit 22
|
||||
cp "$INSTALLER" "$output"`,
|
||||
);
|
||||
writeFileSync(
|
||||
installer,
|
||||
`#!/bin/sh
|
||||
set -e
|
||||
case " $* " in
|
||||
*' --preflight-only '*) echo preflight >> "$TRACE"; [ "$FAIL_AT" != preflight ]; exit $? ;;
|
||||
esac
|
||||
echo install >> "$TRACE"
|
||||
while [ "$#" -gt 0 ]; do
|
||||
if [ "$1" = --token-file ]; then token_file="$2"; shift; fi
|
||||
shift
|
||||
done
|
||||
printf %s "$token_file" > "$TOKEN_PATH"
|
||||
[ "$(stat -c %a "$token_file")" = 600 ]
|
||||
[ "$(stat -c %a "$(dirname "$token_file")")" = 700 ]
|
||||
[ "$(cat "$token_file")" = "$EXPECTED_TOKEN" ]
|
||||
[ "$FAIL_AT" != install ]
|
||||
`,
|
||||
);
|
||||
for (const uid of ['0', '1000']) {
|
||||
for (const failAt of ['', 'fetch', 'preflight', 'install']) {
|
||||
writeFileSync(trace, '');
|
||||
rmSync(tokenPath, { force: true });
|
||||
const command = buildUnixAgentInstallCommand({
|
||||
baseUrl: 'https://pulse.invalid',
|
||||
token,
|
||||
});
|
||||
const input = document.createElement('input');
|
||||
input.value = command;
|
||||
const run = () =>
|
||||
execFileSync(shell, ['-c', input.value], {
|
||||
env: {
|
||||
...process.env,
|
||||
PATH: `${dir}:${process.env.PATH}`,
|
||||
FAKE_UID: uid,
|
||||
FAIL_AT: failAt,
|
||||
TRACE: trace,
|
||||
TOKEN_PATH: tokenPath,
|
||||
SCRIPT_PATH: scriptPath,
|
||||
INSTALLER: installer,
|
||||
EXPECTED_TOKEN: token,
|
||||
},
|
||||
});
|
||||
if (failAt) expect(run).toThrow();
|
||||
else expect(run).not.toThrow();
|
||||
const events = readFileSync(trace, 'utf8').trim().split('\n');
|
||||
expect(events[0]).toBe('fetch');
|
||||
expect(existsSync(dirname(readFileSync(scriptPath, 'utf8')))).toBe(false);
|
||||
if (failAt === 'fetch' || failAt === 'preflight') {
|
||||
expect(events).toEqual(failAt === 'fetch' ? ['fetch'] : ['fetch', 'preflight']);
|
||||
expect(existsSync(tokenPath)).toBe(false);
|
||||
} else {
|
||||
expect(events[1]).toBe('preflight');
|
||||
expect(events).toContain('install');
|
||||
expect(existsSync(dirname(readFileSync(tokenPath, 'utf8')))).toBe(false);
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -148,65 +148,78 @@ export const buildUnixAgentInstallCommand = ({
|
||||
const normalizedExtraArgs = extraArgs.map((arg) => arg.trim()).filter((arg) => arg.length > 0);
|
||||
const installRequiresInsecure = insecure || normalizedBaseUrl.startsWith('http://');
|
||||
const curlFlags = insecure ? '-kfsSL' : '-fsSL';
|
||||
const caCertArg = normalizedCaCertPath
|
||||
? ` \\\n --cacert ${shellQuoteArg(normalizedCaCertPath)}`
|
||||
: '';
|
||||
const insecureArg = installRequiresInsecure ? ` \\\n --insecure` : '';
|
||||
// Build shell grammar explicitly: command fields such as pfSense's remove
|
||||
// pasted newlines. Never flatten a multiline script (or quoted user data).
|
||||
if (
|
||||
[normalizedBaseUrl, normalizedToken, normalizedCaCertPath, ...normalizedExtraArgs].some(
|
||||
(value) => /[\r\n]/.test(value),
|
||||
)
|
||||
) {
|
||||
throw new Error('Pulse install command values must not contain line breaks.');
|
||||
}
|
||||
const caCertArg = normalizedCaCertPath ? ` --cacert ${shellQuoteArg(normalizedCaCertPath)}` : '';
|
||||
const insecureArg = installRequiresInsecure ? ' --insecure' : '';
|
||||
const preflightArgs = [
|
||||
`--url ${shellQuoteArg(normalizedBaseUrl)}`,
|
||||
'--preflight-only',
|
||||
'--output json',
|
||||
'--non-interactive',
|
||||
].join(' \\\n ');
|
||||
].join(' ');
|
||||
const installArgs = [
|
||||
`--url ${shellQuoteArg(normalizedBaseUrl)}`,
|
||||
...(normalizedToken ? ['--token-file "$token_file"'] : []),
|
||||
...normalizedExtraArgs,
|
||||
'--non-interactive',
|
||||
].join(' \\\n ');
|
||||
].join(' ');
|
||||
const rootTokenSetup = normalizedToken
|
||||
? ` token_dir=$(mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX)
|
||||
token_file="$token_dir/token"
|
||||
umask 077
|
||||
printf %s ${shellQuoteArg(normalizedToken)} > "$token_file"
|
||||
`
|
||||
: '';
|
||||
? [
|
||||
'token_dir=$(mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX);',
|
||||
'token_file="$token_dir/token";',
|
||||
'umask 077;',
|
||||
`printf %s ${shellQuoteArg(normalizedToken)} > "$token_file";`,
|
||||
]
|
||||
: [];
|
||||
const sudoTokenSetup = normalizedToken
|
||||
? ` token_dir=$(sudo mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX)
|
||||
token_file="$token_dir/token"
|
||||
printf %s ${shellQuoteArg(normalizedToken)} | sudo tee "$token_file" >/dev/null
|
||||
sudo chmod 0600 "$token_file"
|
||||
`
|
||||
: '';
|
||||
? [
|
||||
'token_dir=$(sudo mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX);',
|
||||
'token_file="$token_dir/token";',
|
||||
`printf %s ${shellQuoteArg(normalizedToken)} | sudo tee "$token_file" >/dev/null;`,
|
||||
'sudo chmod 0600 "$token_file";',
|
||||
]
|
||||
: [];
|
||||
|
||||
return `(
|
||||
set -e
|
||||
tmp_dir=$(mktemp -d)
|
||||
token_dir=""
|
||||
install_script="$tmp_dir/install.sh"
|
||||
cleanup() {
|
||||
rm -rf -- "$tmp_dir"
|
||||
if [ -n "${'${token_dir:-}'}" ]; then
|
||||
if [ "$(id -u)" -eq 0 ]; then
|
||||
rm -rf -- "$token_dir"
|
||||
elif command -v sudo >/dev/null 2>&1; then
|
||||
sudo rm -rf -- "$token_dir" >/dev/null 2>&1 || true
|
||||
fi
|
||||
fi
|
||||
}
|
||||
trap cleanup EXIT HUP INT TERM
|
||||
curl ${curlFlags}${normalizedCaCertPath ? ` --cacert ${shellQuoteArg(normalizedCaCertPath)}` : ''} ${shellQuoteArg(`${normalizedBaseUrl}/install.sh`)} -o "$install_script"
|
||||
chmod +x "$install_script"
|
||||
bash "$install_script" ${preflightArgs}${caCertArg}${insecureArg}
|
||||
if [ "$(id -u)" -eq 0 ]; then
|
||||
${rootTokenSetup} bash "$install_script" ${installArgs}${caCertArg}${insecureArg}
|
||||
elif command -v sudo >/dev/null 2>&1; then
|
||||
${sudoTokenSetup} sudo bash "$install_script" ${installArgs}${caCertArg}${insecureArg}
|
||||
else
|
||||
echo "Root privileges required. Run as root (su -) and retry." >&2
|
||||
exit 1
|
||||
fi
|
||||
)`;
|
||||
return [
|
||||
'(',
|
||||
'set -e;',
|
||||
'tmp_dir=$(mktemp -d);',
|
||||
'token_dir="";',
|
||||
'install_script="$tmp_dir/install.sh";',
|
||||
'cleanup() {',
|
||||
'rm -rf -- "$tmp_dir";',
|
||||
'if [ -n "${token_dir:-}" ]; then',
|
||||
'if [ "$(id -u)" -eq 0 ]; then',
|
||||
'rm -rf -- "$token_dir";',
|
||||
'elif command -v sudo >/dev/null 2>&1; then',
|
||||
'sudo rm -rf -- "$token_dir" >/dev/null 2>&1 || true;',
|
||||
'fi;',
|
||||
'fi;',
|
||||
'};',
|
||||
'trap cleanup EXIT HUP INT TERM;',
|
||||
`curl ${curlFlags}${caCertArg} ${shellQuoteArg(`${normalizedBaseUrl}/install.sh`)} -o "$install_script";`,
|
||||
'chmod +x "$install_script";',
|
||||
`bash "$install_script" ${preflightArgs}${caCertArg}${insecureArg};`,
|
||||
'if [ "$(id -u)" -eq 0 ]; then',
|
||||
...rootTokenSetup,
|
||||
`bash "$install_script" ${installArgs}${caCertArg}${insecureArg};`,
|
||||
'elif command -v sudo >/dev/null 2>&1; then',
|
||||
...sudoTokenSetup,
|
||||
`sudo bash "$install_script" ${installArgs}${caCertArg}${insecureArg};`,
|
||||
'else',
|
||||
'echo "Root privileges required. Run as root (su -) and retry." >&2;',
|
||||
'exit 1;',
|
||||
'fi;',
|
||||
')',
|
||||
].join(' ');
|
||||
};
|
||||
|
||||
export const buildPowerShellInstallScriptBootstrap = (baseUrl: string) => {
|
||||
|
||||
@@ -3053,7 +3053,8 @@ func (rr *ResourceRegistry) resolveLinkedResource(source DataSource, sourceID st
|
||||
// one already-linked host boundary. This is the durable join for cases where
|
||||
// Proxmox reports a SAS address in its serial field while smartctl reports the
|
||||
// drive's real serial. Device paths are safe only inside the common parent and
|
||||
// only when the topology match is unique.
|
||||
// only when the topology match is unique. Serial-less direct devices also use
|
||||
// this join: a USB bridge may expose hardware identity to only one collector.
|
||||
func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incoming Resource) string {
|
||||
if incoming.PhysicalDisk == nil || incoming.ParentID == nil {
|
||||
return ""
|
||||
@@ -3090,7 +3091,7 @@ func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incomin
|
||||
existingDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(existing.PhysicalDisk.DevPath))
|
||||
agentReportsSAS := (source == SourceProxmox && strings.EqualFold(existing.PhysicalDisk.DiskType, "sas")) ||
|
||||
(source == SourceAgent && strings.EqualFold(incoming.PhysicalDisk.DiskType, "sas"))
|
||||
deviceMatch := agentReportsSAS &&
|
||||
deviceMatch := (agentReportsSAS || physicalDiskMissingIdentityPathCompatible(incoming.PhysicalDisk, existing.PhysicalDisk)) &&
|
||||
incomingDevice != "" &&
|
||||
incomingDevice == existingDevice &&
|
||||
physicalDiskTopologyCompatible(incoming.PhysicalDisk, existing.PhysicalDisk)
|
||||
@@ -3105,6 +3106,21 @@ func (rr *ResourceRegistry) resolveLinkedPhysicalDisk(source DataSource, incomin
|
||||
return matchID
|
||||
}
|
||||
|
||||
// A kernel path identifies a direct device only within its already-correlated
|
||||
// host. Missing identity is not conflicting identity. Never use this fallback
|
||||
// for controller members: the block path can represent several physical disks.
|
||||
func physicalDiskMissingIdentityPathCompatible(left, right *PhysicalDiskMeta) bool {
|
||||
if left == nil || right == nil {
|
||||
return false
|
||||
}
|
||||
if diskinventory.IsControllerMemberTarget(left.Target) || diskinventory.IsControllerMemberTarget(right.Target) {
|
||||
return false
|
||||
}
|
||||
leftHasID := diskinventory.IsUsableHardwareID(left.Serial) || diskinventory.IsUsableHardwareID(left.WWN)
|
||||
rightHasID := diskinventory.IsUsableHardwareID(right.Serial) || diskinventory.IsUsableHardwareID(right.WWN)
|
||||
return !leftHasID || !rightHasID
|
||||
}
|
||||
|
||||
func physicalDiskTopologyCompatible(left, right *PhysicalDiskMeta) bool {
|
||||
if left == nil || right == nil {
|
||||
return false
|
||||
@@ -3503,13 +3519,13 @@ func (rr *ResourceRegistry) mergeInto(existing *Resource, incoming Resource, sou
|
||||
previous := existing.PhysicalDisk
|
||||
existing.PhysicalDisk = mergePhysicalDiskData(existing.PhysicalDisk, incoming.PhysicalDisk)
|
||||
if source == SourceProxmox && previous != nil && hasDataSource(existing.Sources, SourceAgent) {
|
||||
if previous.Serial != "" &&
|
||||
if diskinventory.IsUsableHardwareID(previous.Serial) &&
|
||||
(previous.Collection == nil ||
|
||||
(previous.Collection.Serial.State == diskinventory.FieldAvailable &&
|
||||
strings.HasPrefix(strings.ToLower(previous.Collection.Serial.Source), "smartctl"))) {
|
||||
existing.PhysicalDisk.Serial = previous.Serial
|
||||
}
|
||||
if previous.WWN != "" {
|
||||
if diskinventory.IsUsableHardwareID(previous.WWN) {
|
||||
existing.PhysicalDisk.WWN = previous.WWN
|
||||
}
|
||||
if previous.DiskType != "" {
|
||||
@@ -3830,10 +3846,10 @@ func mergePhysicalDiskData(existing *PhysicalDiskMeta, incoming *PhysicalDiskMet
|
||||
if incoming.Vendor != "" {
|
||||
merged.Vendor = incoming.Vendor
|
||||
}
|
||||
if incoming.Serial != "" {
|
||||
if diskinventory.IsUsableHardwareID(incoming.Serial) {
|
||||
merged.Serial = incoming.Serial
|
||||
}
|
||||
if incoming.WWN != "" {
|
||||
if diskinventory.IsUsableHardwareID(incoming.WWN) {
|
||||
merged.WWN = incoming.WWN
|
||||
}
|
||||
if incoming.DiskType != "" {
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package unifiedresources
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -6482,3 +6484,123 @@ func TestPlatformOnlyResourceStatusRecoversWithoutOverridingAgent(t *testing.T)
|
||||
t.Fatalf("platform overwrote higher-priority agent status: %s", got.Status)
|
||||
}
|
||||
}
|
||||
|
||||
func usbAliasSnapshot(agentSerial, pveSerial string) models.StateSnapshot {
|
||||
now := time.Now()
|
||||
return models.StateSnapshot{
|
||||
Nodes: []models.Node{{ID: "pve-node", Name: "node", Instance: "pve", LinkedAgentID: "agent", Status: "online", LastSeen: now}},
|
||||
Hosts: []models.Host{{ID: "agent", Hostname: "node", LinkedNodeID: "pve-node", Status: "online", LastSeen: now, Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{Device: "sdx", Serial: agentSerial, Type: "usb", SizeBytes: 239_000_000_000, Health: "PASSED", Temperature: 31}}}}},
|
||||
PhysicalDisks: []models.PhysicalDisk{{ID: ProxmoxPhysicalDiskSourceID("pve", "node", "/dev/sdx", "", ""), Instance: "pve", Node: "node", DevPath: "/dev/sdx", Serial: pveSerial, Type: "usb", Size: 239_000_000_000, Health: "PASSED", LastChecked: now}},
|
||||
}
|
||||
}
|
||||
|
||||
// Exercise ordinary host/PVE snapshot ingestion, canonical views and their JSON
|
||||
// projection repeatedly, not only a correlation helper or an invented identity.
|
||||
func TestIssue2076USBMixedSourceSnapshot(t *testing.T) {
|
||||
for _, serials := range [][2]string{{"", "USB-SERIAL"}, {"USB-SERIAL", ""}, {"", ""}, {"UNKNOWN", "USB-SERIAL"}, {"USB-SERIAL", "UNKNOWN"}} {
|
||||
t.Run(fmt.Sprintf("%s/%s", serials[0], serials[1]), func(t *testing.T) {
|
||||
snapshot := usbAliasSnapshot(serials[0], serials[1])
|
||||
adapter := NewMonitorAdapter(NewRegistry(nil))
|
||||
var id string
|
||||
for cycle := 0; cycle < 4; cycle++ {
|
||||
adapter.PopulateFromSnapshot(snapshot)
|
||||
views := adapter.PhysicalDisks()
|
||||
if len(views) != 1 {
|
||||
t.Fatalf("cycle %d: disks = %d, want one linked USB device", cycle, len(views))
|
||||
}
|
||||
if cycle == 0 {
|
||||
id = views[0].ID()
|
||||
} else if views[0].ID() != id {
|
||||
t.Fatal("canonical ID changed")
|
||||
}
|
||||
payload, err := json.Marshal(adapter.GetAll())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var resources []Resource
|
||||
if err := json.Unmarshal(payload, &resources); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
count := 0
|
||||
for _, r := range resources {
|
||||
if r.Type != ResourceTypePhysicalDisk {
|
||||
continue
|
||||
}
|
||||
count++
|
||||
if !hasDataSource(r.Sources, SourceAgent) || !hasDataSource(r.Sources, SourceProxmox) || r.ParentID == nil {
|
||||
t.Fatalf("missing source/parent: %+v", r)
|
||||
}
|
||||
if r.PhysicalDisk == nil || r.PhysicalDisk.SizeBytes != 239_000_000_000 || r.PhysicalDisk.Temperature != 31 {
|
||||
t.Fatalf("lost disk facts: %+v", r.PhysicalDisk)
|
||||
}
|
||||
if serials[0] == "USB-SERIAL" || serials[1] == "USB-SERIAL" {
|
||||
if r.PhysicalDisk.Serial != "USB-SERIAL" {
|
||||
t.Fatalf("lost serial: %+v", r.PhysicalDisk)
|
||||
}
|
||||
}
|
||||
}
|
||||
if count != 1 {
|
||||
t.Fatalf("JSON disks = %d", count)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssue2076USBDoesNotMergeUnsafeAliases(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
change func(*models.StateSnapshot)
|
||||
}{
|
||||
{"conflicting serials", func(s *models.StateSnapshot) { s.Hosts[0].Sensors.SMART[0].Serial = "OTHER-SERIAL" }},
|
||||
{"different device same size", func(s *models.StateSnapshot) { s.Hosts[0].Sensors.SMART[0].Device = "sdy" }},
|
||||
{"controller member", func(s *models.StateSnapshot) { s.Hosts[0].Sensors.SMART[0].Target = "megaraid,0" }},
|
||||
{"conflicting controllers", func(s *models.StateSnapshot) {
|
||||
s.Hosts[0].Sensors.SMART[0].Controller = "controller-a"
|
||||
s.PhysicalDisks[0].Controller = "controller-b"
|
||||
}},
|
||||
{"different hosts", func(s *models.StateSnapshot) {
|
||||
s.Hosts[0].LinkedNodeID = ""
|
||||
s.Hosts[0].Hostname = "other"
|
||||
s.Nodes[0].LinkedAgentID = ""
|
||||
}},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
s := usbAliasSnapshot("", "USB-SERIAL")
|
||||
tc.change(&s)
|
||||
rr := NewRegistry(nil)
|
||||
rr.IngestSnapshot(s)
|
||||
if got := len(rr.ListByType(ResourceTypePhysicalDisk)); got != 2 {
|
||||
t.Fatalf("disks=%d, want separate observations", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIssue2076USBLinkedDiskAmbiguity(t *testing.T) {
|
||||
for _, source := range []DataSource{SourceAgent, SourceProxmox} {
|
||||
t.Run(string(source), func(t *testing.T) {
|
||||
other := SourceAgent
|
||||
if source == SourceAgent {
|
||||
other = SourceProxmox
|
||||
}
|
||||
rr := NewRegistry(nil)
|
||||
parent := "host:linked"
|
||||
incoming := Resource{Type: ResourceTypePhysicalDisk, ParentID: &parent, PhysicalDisk: &PhysicalDiskMeta{DevPath: "sdx", DiskType: "usb"}}
|
||||
for _, id := range []string{"a", "b"} {
|
||||
rr.resources[id] = &Resource{ID: id, Type: ResourceTypePhysicalDisk, ParentID: &parent, Sources: []DataSource{other}, PhysicalDisk: &PhysicalDiskMeta{DevPath: "/dev/sdx", Serial: "SERIAL-" + id, DiskType: "usb"}}
|
||||
}
|
||||
if got := rr.resolveLinkedPhysicalDisk(source, incoming); got != "" {
|
||||
t.Fatalf("ambiguous path matched %q", got)
|
||||
}
|
||||
delete(rr.resources, "b")
|
||||
if got := rr.resolveLinkedPhysicalDisk(source, incoming); got != "a" {
|
||||
t.Fatalf("unique missing-identity alias matched %q", got)
|
||||
}
|
||||
incoming.PhysicalDisk.WWN = "DIFFERENT-WWN"
|
||||
if got := rr.resolveLinkedPhysicalDisk(source, incoming); got != "" {
|
||||
t.Fatalf("conflicting hardware identity matched %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user