From 333c4d6b40358b77f17c6551f66f42532f2b19d1 Mon Sep 17 00:00:00 2001 From: Xore Date: Sun, 27 Sep 2026 05:23:53 +0200 Subject: [PATCH 1/4] fix(arkime): un-shadow db.pl and make the translation actually run (#3343) into composable ones, and left the refusal it could not get past open: Elasticsearch refuses to create a legacy template whose patterns match an existing composable one, and single-node-replica-default matches "*", so db.pl could never install or refresh any of Arkime's templates again. On an Arkime version bump that needed a template upgrade, arkime-init's db.pl would fail under `set -e`, the done-marker would not be written, and capture and viewer would wait on it forever. It was worse than that. arkime-init's command is a YAML `>` folded block scalar, and a folded scalar joins every run of adjacent non-empty lines with a single space. The node invocation sat directly under its comment with no blank line between, so the whole thing rendered as one `fi` line with a trailing comment: fi # #3343: translate ... See arkime/composable-templates.js. /opt/arkime/bin/node /opt/arkime/composable-templates.js composable-templates.js had therefore never once run, and #3346's fix was inert while still reading as correct in the YAML. bash -n accepts the result either way and nothing else exercised the rendering. composable-templates.js gains a `shadow` subcommand, run before db.pl and restored after: it deletes every composable template that covers an Arkime index family, stashing each body first, so db.pl has nothing to collide with. Coverage is decided by Elasticsearch's own simpleMatch glob semantics rather than by name, so a renamed or second catch-all is shadowed too -- the previous approach of testing one invented probe name missed even arkime_sessions3-2*, which overlaps arkime_sessions3-* just as hard. The restore runs in a `finally`, because the shadowed set includes the replicas catch-all for every template-less index family in the stack and leaving it deleted would turn one failure into a yellow cluster. compose.yml wraps db.pl in a bounded three-attempt retry, which covers the one race a shadow cannot: a template created *while* db.pl runs, by elasticsearch-setup's catch-all landing mid-run. Both db.pl verbs are safe to re-run, and the loop exits non-zero without writing the done marker rather than wedging the deploy permanently. elasticsearch-setup.sh's comment claimed the catch-all "explicitly excluded" Arkime, which was never true -- composable templates have no exclusion syntax, so the "-arkime_..." entries were literal index names matching nothing. The inert entries are gone and the comment now names the mechanism that actually works. Its 60s wait for Arkime's legacy template is kept: it is the common case and needs no shadow/restore cycle, but it is no longer load-bearing. Refs #3343 --- .../analysis/elasticsearch-setup.sh | 113 ++- .../arkime/composable-templates.js | 215 +++++- arcane/home/honeypot-init/compose.yml | 84 ++- tests/docs/test_3343_fix.py | 676 ++++++++++++++++++ 4 files changed, 997 insertions(+), 91 deletions(-) create mode 100644 tests/docs/test_3343_fix.py diff --git a/arcane/home/honeypot-init/analysis/elasticsearch-setup.sh b/arcane/home/honeypot-init/analysis/elasticsearch-setup.sh index 799934ba..71a0f71f 100644 --- a/arcane/home/honeypot-init/analysis/elasticsearch-setup.sh +++ b/arcane/home/honeypot-init/analysis/elasticsearch-setup.sh @@ -1556,48 +1556,46 @@ curl -fsS -X PUT "$es_url/_all/_settings?expand_wildcards=all" \ # number_of_replicas explicitly and outranks this one on priority, so this # only ever applies to an index nothing more specific already covers. # -# EXCEPT arkime_sessions3-*/arkime_history_v1-*, explicitly excluded below. -# Elasticsearch's own documented precedence rule: when ANY composable index -# template (the modern _index_template API, what this whole script and -# every "priority": N template above uses) matches an index, EVERY legacy -# template (the old _template API) is ignored outright for that index, not -# merged -- even a priority-1 catch-all like this one wins outright over a -# legacy template with no priority concept at all. Arkime's own db.pl -# still creates its real field-typing templates (arkime_sessions3_template/ -# _ecs_template, arkime_history_v1_template) via that legacy API, and this -# catch-all's original "*" pattern silently shadowed them completely -- -# confirmed live: every arkime_sessions3-* index's source.ip/destination.ip -# fell through to Elasticsearch's own dynamic string default (text + -# .keyword) instead of a real `ip`-typed field, breaking session-detail -# lookups outright ("TypeError: Cannot create property 'keyword' on -# string" in Arkime's own viewer, since it assumes an object-typed IP -# field it can attach a .keyword accessor to). Both excluded index -# families already set their own number_of_replicas: 0 in their real -# legacy templates, so excluding them here doesn't reintroduce the -# yellow-cluster problem this template exists to prevent -- it only lets -# their own already-correct settings apply uncontested again. Every OTHER -# arkime_* index (dstats, files, stats, users, etc) has no legacy template -# of its own and still needs this catch-all, so the exclusion is scoped to -# exactly these two, not arkime_* broadly. -# Wait for Arkime's own legacy template before adding ANY composable template that overlaps it. -# Elasticsearch allows a composable template to overlap an existing legacy one -# (it only warns), but REFUSES the reverse: +# #3343: the "EXCEPT arkime_sessions3-*/arkime_history_v1-*, explicitly +# excluded below" claim that used to stand here was simply false. Composable +# index templates have no exclusion syntax, so the "-arkime_..." entries this +# template carried matched nothing but literal index names -- the "*" really +# did match every arkime_sessions3-* index. The inert entries are gone, and so +# is any pretence that this catch-all leaves Arkime alone. It does not. +# +# That is not only a mapping wart, it is why Arkime could not be installed at +# all. Elasticsearch applies NO legacy template to an index once ANY +# composable template matches it (even a priority-1 catch-all like this one +# wins outright over a legacy template, which has no priority concept at all), +# and it refuses the reverse direction outright: # # illegal_argument_exception: legacy template [arkime_sessions3_template] has # index patterns [arkime_sessions3-*] matching patterns from existing -# composable templates [arkime-sessions3-ip-fix, ...] -- use composable +# composable templates [single-node-replica-default, ...] -- use composable # templates (/_index_template) instead # -# arkime-init and elasticsearch-setup are both honeypot-init one-shots with no -# depends_on between them, so they race. On the 2026-09-04 rebuild this script -# won, and `db.pl init` then failed with the above and exited 255 -- Arkime got -# no session indices at all, while hp-arkime-capture and -viewer both looked -# healthy. Ordering the composable templates behind it makes Arkime win deterministically, -# without coupling the whole of this script to arkime-init succeeding (a hard -# depends_on would let one Arkime failure block every template here). +# `db.pl init` DELETEs and re-creates all three of Arkime's legacy templates, +# so against this catch-all it fails and exits 255, and Arkime ends up with no +# session indices at all while hp-arkime-capture and -viewer both look healthy. +# +# arkime-init now owns Arkime's mappings properly: it deletes this catch-all +# for the duration of db.pl, puts it straight back afterwards, and translates +# Arkime's own legacy templates into full composable equivalents at priority 11 +# -- above this priority-1 one -- so the real field typing wins. See +# arkime/composable-templates.js. That is what replaced the +# arkime-sessions3-ip-fix fragment which used to stand below: it decided every +# sessions index alone (firstPacket long, no wordSplit analyzer, 1 replica) and +# left source.ip/destination.ip as text + .keyword, breaking session-detail +# lookups outright ("TypeError: Cannot create property 'keyword' on string" in +# Arkime's own viewer, since it assumes an object-typed IP field it can attach +# a .keyword accessor to). # -# Bounded, and non-fatal on timeout: a deployment with arkime-init disabled -# entirely should still get this mapping fix rather than hang. +# Waiting for Arkime's own legacy template before adding this catch-all is +# still worth doing -- it is the common case and needs no shadow/restore cycle +# at all -- but it is no longer load-bearing, since arkime-init now handles +# either order itself. It stays bounded and non-fatal: this script must not +# depend on arkime-init succeeding, or one Arkime failure would block every +# template here. arkime_legacy_wait=60 while (( arkime_legacy_wait > 0 )); do if curl -fsS -o /dev/null "$es_url/_template/arkime_sessions3_template" 2>/dev/null; then @@ -1609,38 +1607,31 @@ while (( arkime_legacy_wait > 0 )); do done if (( arkime_legacy_wait <= 0 )); then echo "elasticsearch-setup: Arkime legacy template did not appear within 60s --" \ - "adding the composable templates anyway (arkime-init may be disabled; if it" \ - "runs later it will fail its own template creation, see #2961)" + "adding the composable catch-all anyway (arkime-init may be disabled; if it" \ + "runs later it shadows this template and regenerates Arkime's own, so the" \ + "worst case is a needless shadow/restore cycle, not a failure -- #3343)" fi curl -fsS -X PUT "$es_url/_index_template/single-node-replica-default" \ -H 'Content-Type: application/json' \ - --data-binary '{"index_patterns":["*","-arkime_sessions3-*","-arkime_history_v1-*"],"priority":1,"template":{"settings":{"index.number_of_replicas":0}}}' >/dev/null + --data-binary '{"index_patterns":["*"],"priority":1,"template":{"settings":{"index.number_of_replicas":0}}}' >/dev/null -# #3343: no Arkime template is created here any more. What the comment that -# used to stand here called "under-documented legacy-template merge behavior" -# was plain shadowing: Elasticsearch applies NO legacy template to an index -# once any composable template matches it, and single-node-replica-default -# above matches "*" (composable templates have no exclusion syntax -- the -# "-arkime_..." entries are literal names). So neither of Arkime's legacy -# templates ever applied, and the arkime-sessions3-ip-fix fragment that stood -# here decided every sessions index alone (firstPacket long, no analyzer, 1 -# replica). arkime-init now translates Arkime's legacy templates into full -# composable ones right after db.pl (arkime/composable-templates.js) and -# deletes arkime-sessions3-ip-fix. -# # #3283: arkime_sessions3-*'s retention is NOT here, with the other twelve # policies, and that placement is the fix rather than an omission. This -# script's own catch-all cannot reach that family (it is excluded above, and -# the generated composable template is what creates the indices), and an -# index template naming an ILM policy that does not exist yet fails index -# creation outright. This job and arkime-init are independent one-shots that -# race -- the wait above exists because of exactly that -- and arkime-capture -# waits only on arkime-init.done, so a policy created here could not be -# relied on to exist before the first sessions index is created. -# composable-templates.js therefore installs arkime-sessions-30d immediately -# before the template that names it, and adopts the indices already on disk -# in the same run. One definition, one owner, an ordering that cannot lose. +# script's own catch-all cannot reach that family: since #3343 its pattern is +# a bare "*", so it really does match arkime_sessions3-* -- what keeps it off +# that family is that the composable template arkime-init generates for it +# outranks this one on priority, and composable templates replace rather than +# merge, so nothing set here would survive to govern an index. The generated +# template is also what creates those indices. An index template naming an +# ILM policy that does not exist yet fails index creation outright. This job +# and arkime-init are independent one-shots that race -- the wait above exists +# because of exactly that -- and arkime-capture waits only on arkime-init.done, +# so a policy created here could not be relied on to exist before the first +# sessions index is created. composable-templates.js therefore installs +# arkime-sessions-30d immediately before the template that names it, and +# adopts the indices already on disk in the same run. One definition, one +# owner, an ordering that cannot lose. echo echo "elasticsearch-setup: GeoIP, retention policies, and event templates installed" diff --git a/arcane/home/honeypot-init/arkime/composable-templates.js b/arcane/home/honeypot-init/arkime/composable-templates.js index a4424595..b3a81c38 100644 --- a/arcane/home/honeypot-init/arkime/composable-templates.js +++ b/arcane/home/honeypot-init/arkime/composable-templates.js @@ -1,23 +1,61 @@ -// composable-templates.js -- run by arkime-init right after db.pl (#3343). +// composable-templates.js -- run by arkime-init either side of db.pl (#3343). // // Arkime's db.pl installs LEGACY index templates (arkime_sessions3_template, -// arkime_sessions3_ecs_template, arkime_history_v1_template). Elasticsearch -// applies no legacy template to an index once any composable template matches -// it, and this cluster always has one: elasticsearch-setup.sh's -// single-node-replica-default matches "*" (composable templates have no -// exclusion syntax, so its "-arkime_..." entries are literal names). Every -// arkime_sessions3-* index was therefore created from dynamic defaults -- +// arkime_sessions3_ecs_template, arkime_history_v1_template) and re-installs +// them on every init/upgrade run. Elasticsearch applies no legacy template to +// an index once ANY composable template matches it, and this cluster always +// has one: elasticsearch-setup.sh's single-node-replica-default matches "*". +// (It used to carry "-arkime_sessions3-*" alongside, hoping to exclude Arkime; +// composable templates have no exclusion syntax, so those were literal index +// names matching nothing and the claim was false. #3343 removed them rather +// than leaving the comment asserting a mechanism that does not exist.) +// Every arkime_sessions3-* index was therefore created from dynamic defaults -- // firstPacket as long instead of date, node/tags as text, no wordSplit // analyzer, no dynamic templates. // -// This translates Arkime's live legacy templates into one composable template -// per index family, merged the way Elasticsearch merges legacy templates -// (ascending order, later wins; dynamic_templates by name), plus the three -// things this cluster needs on top: number_of_replicas 0 (single node, #3283), -// ip-typed source.ip/destination.ip (the viewer's fixSessionFields crashes on -// text IPs, #1191), and delete-only retention on the sessions family (#3283). -// Regenerated on every arkime-init run, so it follows whatever templates the -// running Arkime version's db.pl installed. +// Two subcommands, run either side of db.pl by honeypot-init/compose.yml: +// +// shadow before db.pl: delete every composable template that would cover +// an Arkime index family -- the two generated below, the #1191 +// arkime-sessions3-ip-fix fragment on clusters that predate them, +// and single-node-replica-default -- stashing each body first. +// Decided by glob, not by name, so a renamed or added catch-all is +// covered too. +// generate after db.pl (no argument): read the live legacy templates, merge +// them the way Elasticsearch merges legacy templates, install one +// composable template per family, then put everything the shadow +// stashed back. +// +// Why the shadow cannot be skipped: a composable template covering those +// families is exactly what makes db.pl's own template installation fail. +// +// illegal_argument_exception: legacy template [arkime_sessions3_template] has +// index patterns [arkime_sessions3-*] matching patterns from existing +// composable templates [arkime-sessions3, single-node-replica-default] -- +// use composable templates (/_index_template) instead +// +// Only a CREATE is refused. Re-PUTting a legacy template whose index patterns +// are unchanged is only a warning -- ES 9.5's +// MetadataIndexTemplateService.innerPutTemplate takes the "may be ignored in +// favor of a composable template" branch whenever isUpdateAndPatternsAreUnchanged +// holds -- so a steady-state redeploy survives even unshadowed. `db.pl init` is +// the case that is not: it DELETEs all three legacy templates and creates them +// again, and it is exactly what a fresh cluster and a half-finished earlier +// deploy both need. So a single composable template left behind by a failed +// run refuses init, the next deploy fails the same way, and the stack can +// never finish initialising again without a hand-deleted template: the thing +// that would have to be cleaned up is created by the one-shot that already +// succeeded. Shadowing makes that self-healing; the retry in compose.yml +// covers the one race a shadow cannot -- a template created *while* db.pl runs. +// +// The generated templates translate Arkime's live legacy templates into one +// composable template per index family, merged the way Elasticsearch merges +// legacy templates (ascending order, later wins; dynamic_templates by name), +// plus the three things this cluster needs on top: number_of_replicas 0 +// (single node, #3283), ip-typed source.ip/destination.ip (the viewer's +// fixSessionFields crashes on text IPs, #1191), and delete-only retention on +// the sessions family (#3283). Regenerated on every arkime-init run, so it +// follows whatever templates the running Arkime version's db.pl installed. // // #3283 retention, and why it lives here rather than in // elasticsearch-setup.sh next to the other twelve policies: @@ -61,9 +99,21 @@ // half-applied state. 'use strict'; +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + const ES = (process.env.ARKIME__elasticsearch || 'http://elasticsearch:9200').replace(/\/+$/, ''); const PRIORITY = 11; // above the #1191 ip-fix template (10) it replaces, so there is no gap const LEGACY_IP_FIX = 'arkime-sessions3-ip-fix'; +// /tmp on purpose: the stash only has to outlive the two subcommands in one +// arkime-init run. If db.pl dies, the shadowed templates stay deleted until the +// next deploy, which re-PUTs the catch-all (elasticsearch-setup.sh always does) +// and re-shadows + regenerates here -- so the loss is one deploy of +// number_of_replicas on newly created indices, not a permanent one. Kept out of +// the shared init-markers volume, which is a *.done readiness contract. +const SHADOW_FILE = process.env.SHADOW_FILE || '/tmp/arkime-composable-shadow.json'; +const DRY_RUN = Boolean(process.env.DRY_RUN); // #3283: same knob, same 30-day default as elasticsearch-setup.sh's // `retention_days="${HONEYPOT_RETENTION_DAYS:-30}"`. Junk falls back rather @@ -103,6 +153,11 @@ const FAMILIES = [ }, ]; +// The literal stem of every real index in a family -- anything with a longer +// suffix is still an index of that family, so this is what "does this template +// cover Arkime" has to be asked about. +const STEMS = FAMILIES.map((f) => f.pattern.replace(/\*+$/, '')); + const isObj = (v) => v !== null && typeof v === 'object' && !Array.isArray(v); // Recursive object merge; `over` wins on scalar and array conflicts. @@ -134,8 +189,37 @@ function mergeDynamicTemplates(listsAscending) { return out; } -async function es(method, path, body) { - const res = await fetch(ES + path, { +// Elasticsearch answers "does this template cover that index" with +// Regex.simpleMatch: '*' matches any run of characters, '?' exactly one, +// everything else literal, with no escape character. Mirror that exactly +// instead of hard-coding the names of the templates that happen to overlap +// today, so a catch-all that is renamed, or a second one added later, is +// shadowed too -- that is the whole point of the exercise. +function simpleMatch(pattern, value) { + const source = '^' + pattern.replace(/[.+^${}()|[\]\\]/g, '\\$&').replace(/\*/g, '.*').replace(/\?/g, '.') + '$'; + return new RegExp(source).test(value); +} + +// A template covers a family when its patterns can match *some* index of that +// family, not only the shortest one: `arkime_sessions3-2*` is a per-month +// refinement of a real family, and db.pl's `arkime_sessions3-*` overlaps it just +// as hard as the bare `*` does. Two conditions together cover the shapes that +// actually occur -- the pattern matches the stem outright (a bare `*`, an +// `arkime_*`, an `arkime_sessions3?*`), or the pattern's literal (wildcard- +// free) prefix starts with the stem (`arkime_sessions3-2*`, +// `arkime_history_v1-?`). Testing one invented probe name instead would miss +// both while looking perfectly correct. +function coversStem(patterns, stem) { + return patterns.some( + (pattern) => simpleMatch(pattern, stem) || pattern.split(/[*?]/)[0].startsWith(stem) + ); +} + +const coversArkime = (template) => + (template.index_patterns || []).some((pattern) => STEMS.some((stem) => coversStem([pattern], stem))); + +async function es(method, path_, body) { + const res = await fetch(ES + path_, { method, headers: body ? { 'content-type': 'application/json' } : {}, body: body ? JSON.stringify(body) : undefined, @@ -144,6 +228,75 @@ async function es(method, path, body) { return { status: res.status, json: text ? JSON.parse(text) : {} }; } +// ------------------------------------------------------------- the shadow ---- + +function readShadow() { + try { + return JSON.parse(fs.readFileSync(SHADOW_FILE, 'utf8')); + } catch (err) { + if (err.code === 'ENOENT') return {}; + throw new Error(`reading ${SHADOW_FILE}: ${err.message}`); + } +} + +function writeShadow(shadow) { + fs.mkdirSync(path.dirname(SHADOW_FILE), { recursive: true }); + fs.writeFileSync(SHADOW_FILE, JSON.stringify(shadow, null, 2) + '\n'); +} + +async function shadow() { + const listed = await es('GET', '/_index_template'); + if (listed.status !== 200) throw new Error(`GET _index_template: HTTP ${listed.status}`); + const shadowed = readShadow(); + // Anything already in the stash is already deleted, so re-running shadow is + // a no-op for it and a crash mid-run loses nothing. + const doomed = (listed.json.index_templates || []).filter( + (entry) => !(entry.name in shadowed) && coversArkime(entry.index_template || {}) + ); + if (!doomed.length) { + console.log('shadow: no composable template covers an Arkime index family -- nothing to delete'); + return; + } + for (const entry of doomed) { + if (DRY_RUN) { + console.log(`shadow (dry run): would stash and delete ${entry.name}`); + continue; + } + // Stash before deleting, never the other way round: a crash in between + // then leaves a stashed entry for a template that still exists, and + // re-PUTting that is a no-op, whereas deleting first can lose the body. + shadowed[entry.name] = entry.index_template; + writeShadow(shadowed); + const del = await es('DELETE', `/_index_template/${entry.name}`); + if (del.status !== 200) throw new Error(`DELETE _index_template/${entry.name}: HTTP ${del.status}`); + console.log(`shadow: ${entry.name} stashed in ${SHADOW_FILE} and deleted so db.pl can install Arkime's legacy templates`); + } +} + +async function restore(keepOut) { + const shadowed = readShadow(); + // Never resurrect these from the stash: the generated templates were just + // rebuilt from the legacy templates db.pl installed a moment ago (or were + // deliberately skipped because no legacy template exists), and the ip-fix + // fragment is the #1191 workaround this file replaced. + for (const name of keepOut) delete shadowed[name]; + for (const name of Object.keys(shadowed)) { + if (DRY_RUN) { + console.log(`restore (dry run): would put ${name} back`); + continue; + } + const put = await es('PUT', `/_index_template/${name}`, shadowed[name]); + if (put.status !== 200) { + throw new Error(`restoring _index_template/${name}: HTTP ${put.status} ${JSON.stringify(put.json)}`); + } + delete shadowed[name]; + console.log(`restore: ${name} put back`); + } + if (!DRY_RUN) writeShadow(shadowed); +} + +// ------------------------------------------------------------ the generate ---- + function build(family, legacyTemplates) { const sorted = [...legacyTemplates].sort((a, b) => (a.order || 0) - (b.order || 0)); let settings = {}; @@ -255,7 +408,11 @@ async function adoptExisting(pattern, policy) { console.log(`${pattern}: ${adopted} index(es) adopted, ${kept} already on ${policy}`); } -async function main() { +// Returns the names it installed, so restore() can leave those alone. +async function generate() { + const installed = new Set(); + // #3283: the families this run actually translated, so adoption can be told + // apart from a family that was skipped because no legacy template exists. const managed = []; for (const family of FAMILIES) { const found = []; @@ -273,7 +430,7 @@ async function main() { // #3283: before the template, never after. See the header comment. if (family.retention) await ensurePolicy(family.retention, `${RETENTION_DAYS}d`); const body = build(family, found); - if (process.env.DRY_RUN) { + if (DRY_RUN) { console.log(JSON.stringify({ [family.name]: body })); continue; } @@ -297,6 +454,7 @@ async function main() { throw new Error(`${family.name}: simulate shows index.lifecycle.name=${lifecycle}, expected ${family.retention}`); } } + installed.add(family.name); console.log(`${family.name}: composable template installed from ${found.length} legacy template(s), priority ${PRIORITY}`); } // #3283: retention reaches the indices that already exist. Before the @@ -305,10 +463,27 @@ async function main() { for (const family of managed) { if (family.retention) await adoptExisting(family.pattern, family.retention); } - if (process.env.DRY_RUN) return; + if (DRY_RUN) return installed; const old = await es('DELETE', `/_index_template/${LEGACY_IP_FIX}`); if (old.status === 200) console.log(`${LEGACY_IP_FIX}: removed (superseded by arkime-sessions3)`); else if (old.status !== 404) throw new Error(`DELETE _index_template/${LEGACY_IP_FIX}: HTTP ${old.status}`); + return installed; +} + +async function main() { + const mode = process.argv[2] || 'generate'; + if (mode === 'shadow') return shadow(); + if (mode !== 'generate') throw new Error(`unknown subcommand "${mode}" (expected "shadow" or "generate")`); + // The restore runs even when generate() throws: the shadowed templates + // include the replicas catch-all for every index family in the stack, and + // leaving it deleted because a session mapping went wrong would turn one + // failure into a yellow cluster until the next deploy. + const installed = new Set(); + try { + for (const name of await generate()) installed.add(name); + } finally { + await restore(new Set([...installed, LEGACY_IP_FIX])); + } } main().catch((err) => { diff --git a/arcane/home/honeypot-init/compose.yml b/arcane/home/honeypot-init/compose.yml index 01b079ff..80d10019 100644 --- a/arcane/home/honeypot-init/compose.yml +++ b/arcane/home/honeypot-init/compose.yml @@ -319,16 +319,80 @@ services: # so it can never wipe existing session data. - if /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 info 2>/dev/null \ - | grep 'DB Version' | grep -vq -- '-1'; then - echo RED | /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 upgradenoprompt - else - echo INIT | /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 init - fi - # #3343: translate the legacy templates db.pl just installed into - # composable ones -- Elasticsearch ignores legacy templates wherever a - # composable template matches, and elasticsearch-setup's "*" catch-all - # always does. See arkime/composable-templates.js. + # #3343: run it with nothing composable covering Arkime's own index + + # families. Elasticsearch refuses to CREATE a legacy template whose + + # patterns match an existing composable template, and db.pl init + + # deletes and re-creates all three of them, so any composable template + + # left over from an earlier run fails db.pl under `set -e`. The one + + # that matters is elasticsearch-setup's "*" catch-all: it is created by + + # a different one-shot that has already succeeded by then, so one + + # unlucky deploy (db.pl init racing that catch-all) leaves a template + + # standing that every later deploy trips over -- the stack could never + + # finish initialising again without a hand-deleted template. `shadow` + + # stashes and deletes whatever covers an Arkime family; the subcommand + + # after db.pl puts it all back and regenerates the Arkime templates + + # from the legacy ones. See arkime/composable-templates.js. + # + + # The retry covers the one race a shadow cannot: a template created + + # *while* db.pl is running, by elasticsearch-setup's catch-all landing + + # mid-run. Both db.pl verbs are safe to re-run, so re-shadow and try + + # again rather than wedge the deploy permanently. + + arkime_db_run() { + if /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 info 2>/dev/null \ + | grep 'DB Version' | grep -vq -- '-1'; then + echo RED | /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 upgradenoprompt + else + echo INIT | /opt/arkime/db/db.pl --insecure http://elasticsearch:9200 init + fi + } + + arkime_db_runs=0 + + until /opt/arkime/bin/node /opt/arkime/composable-templates.js shadow && arkime_db_run; do + # A doubled dollar sign is compose's escape for a literal one; a bare + # reference would be interpolated by compose instead, warning that the + # variable is not set (this is a shell counter, not an .env one) -- + # including here in the comment, which is interpolated too. + arkime_db_runs=$(( arkime_db_runs + 1 )) + if [ "$$arkime_db_runs" -ge 3 ]; then + echo "arkime-init: db.pl failed $$arkime_db_runs times -- not writing the done marker" + exit 1 + fi + echo "arkime-init: db.pl failed, re-shadowing the composable templates and retrying" + done + + # #3343: translate the legacy templates db.pl just installed (or + + # refreshed) into composable ones -- Elasticsearch ignores legacy + + # templates wherever a composable template matches, and this cluster + + # always has one. Also restores everything the shadow stashed. + # + + # #3343: this call was folded into the comment above it by the `>` + + # block scalar and so has never actually run; the blank line between + + # the comment and the command is what keeps it out of the comment. + /opt/arkime/bin/node /opt/arkime/composable-templates.js # Create the admin user once, with a real local password (#122, #134, diff --git a/tests/docs/test_3343_fix.py b/tests/docs/test_3343_fix.py new file mode 100644 index 00000000..80d2be98 --- /dev/null +++ b/tests/docs/test_3343_fix.py @@ -0,0 +1,676 @@ +#!/usr/bin/env python3 +"""Regression tests for #3343: Arkime must be able to install its own index +templates, and the translation of them into composable templates must actually +run. + +The issue was filed because Elasticsearch applies no *legacy* template to an +index as soon as any composable template matches it, so Arkime's `db.pl` +templates silently never applied -- `firstPacket` mapped `long` instead of +`date`, no `wordSplit` analyzer, no `dynamic_templates`, IPs typed as text. +#3346 added `arkime/composable-templates.js` to translate them, and left the +residual refusal ("Elasticsearch refuses to create *or update* any legacy +template while the `*` catch-all exists, so `db.pl` can never refresh its +templates again") open. + +What could quietly undo this, and is therefore asserted here rather than +assumed: + +* **the translation call being folded into a comment.** `arkime-init`'s + command is a YAML `>` folded block scalar, and a folded scalar joins every + run of adjacent non-empty lines with a single space. On main the node + invocation sat directly under its comment with no blank line between, so the + whole thing rendered as one `fi` line with a trailing comment and + `composable-templates.js` had *never once run* -- #3346's fix was inert + while still reading as correct in the YAML. The other 90 lines of that + command folded cleanly, and `bash -n` accepts the result either way, so + nothing caught it. These tests render the scalar the way Compose does and + assert the commands survive as commands. +* `db.pl` being left to race the catch-all instead of having it removed for + the duration -- the `shadow` subcommand, and the bounded retry for the one + race a shadow cannot cover (a template created *while* `db.pl` runs). +* the shadow deleting more than it should: every other family's template + (`dionaea-*`, `ml-anomalies`, ...) has to survive, and the stashed bodies + have to come back, or one fix becomes a cluster-wide replica regression. +* a hand-rolled list of the templates that happen to overlap today. The + matcher is Elasticsearch's own `simpleMatch` glob semantics, so a renamed or + second catch-all is covered too; that is asserted behaviourally. +* the catch-all's comment claiming an Arkime exclusion that composable + templates cannot express. Those `-arkime_...` entries were literal index + names matching nothing, and the prose told the next reader they were + excluded. + +The `tests/docs/` CI row installs pytest and nothing else (see quality.yml), +so the render is done by a small dependency-free folded-scalar reader, with a +PyYAML cross-check where PyYAML happens to be available. The Node tests are +skipped where Node is not, the same way test_3321_fix.py guards its optional +pieces. +""" +from __future__ import annotations + +import json +import os +import pathlib +import shutil +import subprocess +import threading +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + +import pytest + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] +COMPOSE = REPO_ROOT / "arcane/home/honeypot-init/compose.yml" +ES_SETUP = REPO_ROOT / "arcane/home/honeypot-init/analysis/elasticsearch-setup.sh" +SCRIPT = REPO_ROOT / "arcane/home/honeypot-init/arkime/composable-templates.js" + +NODE = shutil.which("node") +node_only = pytest.mark.skipif(NODE is None, reason="node not installed") + +# The two Arkime index families #3343 is about, as db.pl names them. +SESSIONS_LEGACY = { + "order": 99, + "index_patterns": ["arkime_sessions3-*"], + "settings": {"index": {"refresh_interval": "60s"}}, + "mappings": { + "properties": {"firstPacket": {"type": "date"}, "node": {"type": "keyword"}}, + "dynamic_templates": [ + {"*Ip": {"match_mapping_type": "ip", "mapping": {"type": "ip"}}}, + {"*Tokens": {"match_mapping_type": "string", "mapping": {"type": "text", "analyzer": "wordSplit"}}}, + ], + }, +} +SESSIONS_ECS_LEGACY = { + "order": 1, + "index_patterns": ["arkime_sessions3-*"], + "settings": {"index": {"total_fields": {"limit": 10000}}}, + "mappings": { + "properties": {"tags": {"type": "keyword"}}, + # The ECS catch-all. If it ended up ahead of *Ip/*Tokens every IP and + # token field would be a string, which is the exact regression #3283 + # found on the live cluster. + "dynamic_templates": [ + {"strings_as_keyword": {"match_mapping_type": "string", "mapping": {"type": "keyword"}}} + ], + }, +} +HISTORY_LEGACY = { + "order": 99, + "index_patterns": ["arkime_history_v1-*"], + "settings": {"index": {"refresh_interval": "60s"}}, + "mappings": {"properties": {"sourceIP": {"type": "ip"}}}, +} + +CATCH_ALL = { + "index_patterns": ["*"], + "priority": 1, + "template": {"settings": {"index.number_of_replicas": 0}}, +} +IP_FIX = { + "index_patterns": ["arkime_sessions3-*"], + "priority": 10, + "template": {"mappings": {"properties": {"source": {"properties": {"ip": {"type": "ip"}}}}}}, +} +# Neither of these has anything to do with Arkime. Losing them is the failure +# mode a too-eager shadow would cause. +DIONAEA = {"index_patterns": ["dionaea-*"], "priority": 5, "template": {"settings": {"index.number_of_shards": 1}}} +ML = {"index_patterns": ["ml-anomalies", "ml-worker-*"], "priority": 5, "template": {}} + + +# ------------------------------------------------------------ the render ---- + + +def unfold_service_command(text: str, service: str) -> str: + """Render a service's `command:` the way Compose does, with no dependencies. + + Only what this repository actually uses is implemented: a `command:` list + whose script is one `>` folded block scalar. In a folded scalar every run + of adjacent non-empty lines is joined with a single space and each empty + line becomes a newline -- which is exactly the rule that swallowed the + `node` call on main, so getting it right is the point of the helper. + """ + lines = text.splitlines() + + # Exactly two spaces of indent: a service key directly under `services:`, + # not a nested mapping that happens to share the name. + start = next( + (i for i, ln in enumerate(lines) + if ln.strip() == f"{service}:" and ln.startswith(" ") and not ln.startswith(" ")), + None, + ) + assert start is not None, f"service {service} not found" + try: + cmd = next(i for i in range(start, len(lines)) if lines[i].strip() == "command:") + except StopIteration: + raise AssertionError(f"{service} has no command:") from None + + def indent(ln: str) -> int: + return len(ln) - len(ln.lstrip()) + + key_indent = indent(lines[cmd]) + # The list items (`- /bin/sh`, `- -c`, `- >`) sit at one level in from the + # key; the script itself is the block scalar under the `- >` item. + item_indent, body_start = None, None + for i in range(cmd + 1, len(lines)): + ln = lines[i] + if not ln.strip(): + continue + if indent(ln) <= key_indent: + break + if item_indent is None: + item_indent = indent(ln) + if indent(ln) == item_indent and ln.lstrip()[1:].strip() == ">": + body_start = i + 1 + break + assert body_start is not None, f"{service}'s command is not a single folded block scalar" + + body: list[str] = [] + for ln in lines[body_start:]: + if not ln.strip(): + body.append("") + continue + if indent(ln) <= item_indent: + break # dedented past the list: that is the next key + body.append(ln) + + # A folded scalar's content indentation is that of its first non-empty line. + # Only lines at exactly that indentation fold together: a *more* indented + # line is literal content, which is why the shell bodies below survive while + # the comment block above them collapses onto one line. + pad = indent(next(ln for ln in body if ln.strip())) + out: list[str] = [] + foldable = False # the previous output line may still take a folded continuation + blank = False + for ln in body: + if not ln.strip(): + blank = True + continue + text = ln[pad:].rstrip() if len(ln) >= pad else ln.strip() + if indent(ln) > pad: + out.append(text) + foldable = False + elif out and foldable and not blank: + out[-1] += " " + text.strip() # the fold that hid the node call + foldable = True + else: + out.append(text) + foldable = True + blank = False + return "\n".join(out) + + +@pytest.fixture(scope="module") +def arkime_init_command() -> str: + return unfold_service_command(COMPOSE.read_text(encoding="utf-8"), "arkime-init") + + +# ------------------------------------------------------- the fold defect ---- + + +def test_composable_templates_call_is_not_folded_into_a_comment(arkime_init_command): + """The #3346 fix is only a fix if the call survives YAML folding. + + On main this rendered as a single `fi` line whose trailing comment + happened to contain the whole invocation, so the script never ran. + """ + commands = [ln for ln in arkime_init_command.splitlines() if ln.startswith("/opt/arkime/bin/node")] + assert commands == ["/opt/arkime/bin/node /opt/arkime/composable-templates.js"], ( + "composable-templates.js is not a standalone command in the rendered " + f"arkime-init script; it is folded into a comment. Rendered lines " + f"mentioning it: {[ln for ln in arkime_init_command.splitlines() if 'composable-templates.js' in ln]}" + ) + + +def test_no_command_line_in_arkime_init_is_swallowed_by_a_comment(arkime_init_command): + """Generalisation of the above, so the next command added does not repeat it. + + Any rendered line that starts a shell command but has a `#` before its + first non-blank character other than its own shebang is code that YAML + folded into prose. Cheap to assert and it fails loudly instead of turning + into a silent no-op. + """ + offenders = [] + for n, line in enumerate(arkime_init_command.splitlines(), 1): + stripped = line.lstrip() + if not stripped or stripped.startswith("#"): + continue + # A command whose first token is quoted or bracketed is not one we can + # recognise; only flag the shapes this command actually uses. + first = stripped.split()[0] + if first.startswith(("'", '"')): + continue + if "#" in line[: len(line) - len(stripped)]: + offenders.append((n, line)) + assert not offenders, f"commands folded into a comment: {offenders}" + + +def test_render_matches_pyyaml_when_available(arkime_init_command): + """Keep the dependency-free reader honest against a real YAML parser. + + Compared with blank lines dropped: they decide *whether* two lines fold + together, which is what this helper has to get right, but how many of them + PyYAML emits around a more-indented block is cosmetic and never changes + what the shell does. + """ + yaml = pytest.importorskip("yaml", reason="PyYAML not installed") + parsed = yaml.safe_load(COMPOSE.read_text(encoding="utf-8")) + real = parsed["services"]["arkime-init"]["command"][2] + assert [ln for ln in real.splitlines() if ln.strip()] == \ + [ln for ln in arkime_init_command.splitlines() if ln.strip()] + + +def test_rendered_arkime_init_is_valid_shell(arkime_init_command): + """`bash -n` passes on main too, but it must pass on the fixed script as well.""" + out = subprocess.run(["bash", "-n"], input=arkime_init_command, capture_output=True, text=True) + assert out.returncode == 0, out.stderr + + +# ------------------------------------------------- shadow before db.pl ---- + + +def test_shadow_runs_before_db_pl_and_restore_after(arkime_init_command): + lines = [ln.strip() for ln in arkime_init_command.splitlines() if ln.strip()] + + shadow = next(i for i, ln in enumerate(lines) if ln.startswith("until /opt/arkime/bin/node") and ln.endswith("; do")) + db_run = next(i for i, ln in enumerate(lines) if ln.startswith("arkime_db_run()")) + generate = next(i for i, ln in enumerate(lines) if ln == "/opt/arkime/bin/node /opt/arkime/composable-templates.js") + + assert " shadow && arkime_db_run" in lines[shadow], lines[shadow] + assert db_run < shadow, "the db.pl helper must be defined before the loop that calls it" + assert generate > shadow, "the restore/regenerate pass must come after the shadow" + # Exactly two invocations: one shadow, one generate. A third would mean a + # subcommand crept in un-reviewed. + invocations = [ln for ln in lines if "/opt/arkime/bin/node /opt/arkime/composable-templates.js" in ln] + assert len(invocations) == 2, invocations + assert invocations[0].endswith(" shadow && arkime_db_run; do") + assert invocations[1] == "/opt/arkime/bin/node /opt/arkime/composable-templates.js" + + +def test_db_pl_retry_is_bounded_and_fails_loudly(arkime_init_command): + """Three attempts, then give up -- and never write the readiness marker. + + `arkime-capture` and `arkime-viewer` both block on + `/markers/arkime-init.done`, so an arkime-init that fails quietly instead + of loudly would be an outage that looks healthy. + """ + lines = [ln.strip() for ln in arkime_init_command.splitlines() if ln.strip()] + loop = lines.index(next(ln for ln in lines if ln.startswith("until /opt/arkime/bin/node"))) + end = lines.index("done", loop) + body = lines[loop + 1:end] + + assert any('-ge 3' in ln for ln in body), body + assert "exit 1" in body, "a persistent db.pl failure must abort arkime-init, not loop forever" + marker = lines.index("mkdir -p /markers && touch /markers/arkime-init.done") + assert end < marker + # `set -e` plus the explicit exit: the marker line must be unreachable on + # the failure path, which it is because the loop exits the process first. + assert lines[0] == "set -e" + + +def test_shell_counters_are_compose_escaped(arkime_init_command): + """`$$`, not `$` -- and not in a comment either, which Compose interpolates too. + + A bare `$arkime_db_runs` makes `docker compose config` warn that the + variable is not set. It is a shell counter, not an .env one; the only + reference in the file was the doubled-dollar form this asserts, and it is + the sole such escape in `arcane/` outside honeypot-conpot. + """ + shell_lines = [ln for ln in arkime_init_command.splitlines() if "arkime_db_runs" in ln] + assert shell_lines, "the retry counter is gone" + for ln in shell_lines: + assert "$arkime_db_runs" not in ln.replace("$$arkime_db_runs", ""), ( + f"a bare $arkime_db_runs makes docker compose config warn: {ln.strip()!r}" + ) + + +# ------------------------------------------- the catch-all's false claim ---- + + +def test_catch_all_no_longer_claims_an_arkime_exclusion_it_cannot_express(): + """Composable templates have no exclusion syntax; `-arkime_...` was a name. + + Those entries matched nothing but an index literally called + `-arkime_sessions3-*`, and the prose asserted they were "explicitly + excluded below" -- so the next reader believed Arkime was untouched. + """ + text = ES_SETUP.read_text(encoding="utf-8") + put = next(ln for ln in text.splitlines() if "_index_template/single-node-replica-default" in ln) + body = text[text.index(put):][:400] + + assert '"-arkime_sessions3-*"' not in text, "the inert exclusion entry is back in the catch-all" + assert '"-arkime_history_v1-*"' not in text + assert "explicitly excluded below" not in text, "the false exclusion claim is back in the comment" + + # What it does now, and the reason, must be stated where a reader lands. + assert '"index_patterns":["*"]' in body, "the catch-all should be a plain [*] wildcard now" + assert "no exclusion syntax" in text + assert "arkime/composable-templates.js" in text, "the comment should point at the real mechanism" + + +def test_elasticsearch_setup_is_still_valid_shell(): + out = subprocess.run(["bash", "-n"], input=ES_SETUP.read_text(encoding="utf-8"), capture_output=True, text=True) + assert out.returncode == 0, out.stderr + + +def test_elasticsearch_setup_keeps_a_bounded_non_fatal_arkime_wait(): + """The wait is an ordering nicety, not the fix -- it must not become a hang.""" + text = ES_SETUP.read_text(encoding="utf-8") + assert "arkime_legacy_wait=60" in text + assert "arkime_legacy_wait=$(( arkime_legacy_wait - 2 ))" in text + # The old message promised a permanent failure; the shadow makes the worst + # case a needless shadow/restore cycle instead. + assert "see #2961" not in text, "the timeout message still claims db.pl cannot recover" + + +# ------------------------------------------- composable-templates.js shape ---- + + +def test_script_declares_both_subcommands(): + text = SCRIPT.read_text(encoding="utf-8") + assert "if (mode === 'shadow') return shadow();" in text + assert "'generate'" in text + assert "unknown subcommand" in text, "an unknown subcommand must fail loudly" + + +def test_script_uses_elasticsearch_glob_semantics_not_a_hardcoded_list(): + """A renamed or second catch-all has to be shadowed too. + + The whole point is to stop hard-coding the names of the templates that + happen to overlap today, so the matcher must implement `simpleMatch` rather + than compare against a list. + """ + text = SCRIPT.read_text(encoding="utf-8") + assert "function simpleMatch(pattern, value)" in text + assert "STEMS" in text, "coverage has to be decided per family, not by one invented name" + assert "coversStem" in text + assert "single-node-replica-default" not in text.split("async function shadow()")[1].split("async function restore")[0], ( + "shadow() must decide what to delete by pattern, not by name" + ) + + +def test_script_does_not_resurrect_what_generate_just_rebuilt(): + text = SCRIPT.read_text(encoding="utf-8") + assert "const LEGACY_IP_FIX = 'arkime-sessions3-ip-fix';" in text + assert "restore(new Set([...installed, LEGACY_IP_FIX]))" in text + + +# ------------------------------------------------- functional: node + stub ES ---- + + +class _StubElasticsearch: + """Just enough of ES to drive composable-templates.js end to end.""" + + def __init__(self, index_templates: dict, legacy_templates: dict): + self.index_templates = dict(index_templates) + self.legacy_templates = dict(legacy_templates) + self.deleted: list[str] = [] + self.put_index_templates: dict = {} + self.calls: list[str] = [] + + def handle(self, method: str, path: str, body) -> tuple[int, dict]: + self.calls.append(f"{method} {path}") + if method == "GET" and path == "/_index_template": + return 200, {"index_templates": [ + {"name": name, "index_template": tmpl} for name, tmpl in self.index_templates.items() + ]} + if method == "DELETE" and path.startswith("/_index_template/"): + name = path.rsplit("/", 1)[1] + self.deleted.append(name) + if name in self.index_templates: + del self.index_templates[name] + return 200, {"acknowledged": True} + return 404, {"error": "resource_not_found_exception"} + if method == "PUT" and path.startswith("/_index_template/"): + name = path.rsplit("/", 1)[1] + self.put_index_templates[name] = body + self.index_templates[name] = body + return 200, {"acknowledged": True} + if method == "POST" and path.startswith("/_index_template/_simulate_index/"): + return 200, {"template": {"settings": {"index": {"number_of_replicas": "0"}}}} + if method == "GET" and path.startswith("/_template/"): + name = path.rsplit("/", 1)[1] + if name in self.legacy_templates: + return 200, {name: self.legacy_templates[name]} + return 404, {"error": "resource_not_found_exception"} + raise AssertionError(f"stub Elasticsearch got an unexpected {method} {path}") + + +class _Handler(BaseHTTPRequestHandler): + stub: _StubElasticsearch + + def _respond(self): + length = int(self.headers.get("content-length") or 0) + raw = self.rfile.read(length) if length else b"" + body = json.loads(raw) if raw else None + status, payload = self.stub.handle(self.command, self.path, body) + out = json.dumps(payload).encode() + self.send_response(status) + self.send_header("content-type", "application/json") + self.send_header("content-length", str(len(out))) + self.end_headers() + self.wfile.write(out) + + do_GET = do_PUT = do_POST = do_DELETE = _respond + + def log_message(self, *_args): # keep pytest output readable + pass + + +@contextmanager +def _serving(stub: _StubElasticsearch): + handler = type("H", (_Handler,), {"stub": stub}) + server = ThreadingHTTPServer(("127.0.0.1", 0), handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + try: + yield f"http://127.0.0.1:{server.server_address[1]}" + finally: + server.shutdown() + server.server_close() + + +def _run(url: str, shadow_file: pathlib.Path, *args: str) -> subprocess.CompletedProcess: + env = {**os.environ, "ARKIME__elasticsearch": url, "SHADOW_FILE": str(shadow_file)} + return subprocess.run([NODE, str(SCRIPT), *args], capture_output=True, text=True, env=env, timeout=60) + + +@contextmanager +def _stubbed(): + stub = _StubElasticsearch( + index_templates={ + "single-node-replica-default": CATCH_ALL, + "arkime-sessions3-ip-fix": IP_FIX, + "dionaea-30d": DIONAEA, + "ml-anomalies": ML, + }, + legacy_templates={ + "arkime_sessions3_template": SESSIONS_LEGACY, + "arkime_sessions3_ecs_template": SESSIONS_ECS_LEGACY, + "arkime_history_v1_template": HISTORY_LEGACY, + }, + ) + with _serving(stub) as url: + yield stub, url + + +@node_only +def test_shadow_deletes_only_arkime_covering_templates_and_stashes_their_bodies(tmp_path): + with _stubbed() as (stub, url): + shadow_file = tmp_path / "shadow.json" + out = _run(url, shadow_file, "shadow") + assert out.returncode == 0, out.stderr + + assert set(stub.deleted) == {"single-node-replica-default", "arkime-sessions3-ip-fix"}, stub.deleted + assert set(stub.index_templates) == {"dionaea-30d", "ml-anomalies"}, "a non-Arkime template was deleted" + + stashed = json.loads(shadow_file.read_text()) + assert set(stashed) == {"single-node-replica-default", "arkime-sessions3-ip-fix"} + # A stash that kept only the name would restore an empty template. + assert stashed["single-node-replica-default"] == CATCH_ALL + assert stashed["arkime-sessions3-ip-fix"] == IP_FIX + + +@node_only +def test_generate_rebuilds_arkimes_templates_and_restores_everything_else(tmp_path): + with _stubbed() as (stub, url): + shadow_file = tmp_path / "shadow.json" + assert _run(url, shadow_file, "shadow").returncode == 0 + + out = _run(url, shadow_file) + assert out.returncode == 0, out.stderr + + installed = stub.put_index_templates + assert "arkime-sessions3" in installed, installed.keys() + assert "arkime-history-v1" in installed + + # Everything that is not Arkime's own generated template came back... + assert "single-node-replica-default" in installed + assert installed["single-node-replica-default"] == CATCH_ALL + assert set(stub.index_templates) == {"dionaea-30d", "ml-anomalies", "arkime-sessions3", "arkime-history-v1", + "single-node-replica-default"} + # ...and the stash is empty, so a crash later cannot replay a stale body. + assert json.loads(shadow_file.read_text()) == {} + + +@node_only +def test_generated_sessions_template_carries_arkimes_real_mappings(tmp_path): + with _stubbed() as (stub, url): + shadow_file = tmp_path / "shadow.json" + assert _run(url, shadow_file, "shadow").returncode == 0 + assert _run(url, shadow_file).returncode == 0 + + tmpl = stub.put_index_templates["arkime-sessions3"] + assert tmpl["index_patterns"] == ["arkime_sessions3-*"] + # Above the ip-fix fragment (10) it replaces and the catch-all (1). + assert tmpl["priority"] == 11 + assert tmpl["template"]["settings"]["index"]["number_of_replicas"] == "0" + # The live-cluster symptom from the issue: long instead of date. + assert tmpl["template"]["mappings"]["properties"]["firstPacket"]["type"] == "date" + assert tmpl["template"]["mappings"]["properties"]["node"]["type"] == "keyword" + # The ip typing Arkime's ECS template provides and the #1191 fragment used to fake. + assert tmpl["template"]["mappings"]["properties"]["source"]["properties"]["ip"]["type"] == "ip" + assert tmpl["template"]["mappings"]["properties"]["destination"]["properties"]["ip"]["type"] == "ip" + + names = [next(iter(e)) for e in tmpl["template"]["mappings"]["dynamic_templates"]] + # Higher order (99) first, so its *Ip/*Tokens rules are not shadowed by + # the ECS catch-all at order 1. Reversed, every IP is a keyword again. + assert names == ["*Ip", "*Tokens", "strings_as_keyword"], names + + history = stub.put_index_templates["arkime-history-v1"] + assert history["template"]["settings"]["index"]["number_of_replicas"] == "0" + # No ipFix for the history family, matching the issue's per-family scope. + assert "source" not in history["template"]["mappings"]["properties"] + + +@node_only +def test_shadow_uses_glob_semantics_not_a_name_list(tmp_path): + """A catch-all nobody anticipated must still be shadowed. + + Asking "does this match one invented probe name" instead of "can this match + an index of the family" would have kept `arkime_sessions3-2*` and + `arkime_history_v1-?` alive -- both of which overlap db.pl's + `arkime_sessions3-*` / `arkime_history_v1-*` just as hard as a bare `*`. + """ + stub = _StubElasticsearch( + index_templates={ + "renamed-defaults": CATCH_ALL, + "sessions-day-2": {"index_patterns": ["arkime_sessions3-2*"], "priority": 3, "template": {}}, + "history-one": {"index_patterns": ["arkime_history_v1-?"], "priority": 3, "template": {}}, + "sessions-any-year": {"index_patterns": ["arkime_sessions3?*"], "priority": 3, "template": {}}, + "arkime-stem-glob": {"index_patterns": ["arkime_*"], "priority": 3, "template": {}}, + "dionaea-30d": DIONAEA, + "ml-anomalies": ML, + }, + legacy_templates={}, + ) + with _serving(stub) as url: + out = _run(url, tmp_path / "shadow.json", "shadow") + assert out.returncode == 0, out.stderr + assert set(stub.deleted) == { + "renamed-defaults", "sessions-day-2", "history-one", "sessions-any-year", "arkime-stem-glob", + }, stub.deleted + assert set(stub.index_templates) == {"dionaea-30d", "ml-anomalies"}, "a non-Arkime template was deleted" + + +@node_only +def test_shadow_is_a_noop_on_a_cluster_with_nothing_to_shadow(tmp_path): + """The first deploy on a clean cluster must not fail for lack of templates.""" + stub = _StubElasticsearch(index_templates={"dionaea-30d": DIONAEA, "ml-anomalies": ML}, legacy_templates={}) + with _serving(stub) as url: + out = _run(url, tmp_path / "shadow.json", "shadow") + assert out.returncode == 0, out.stderr + assert "nothing to delete" in out.stdout + assert stub.deleted == [] + + +@node_only +def test_shadow_is_idempotent_across_the_retry_loop(tmp_path): + """compose.yml re-shadows before every db.pl attempt; the second must be free. + + Re-deleting a template that is already in the stash would re-PUT a body the + first attempt never got round to replacing, and re-deleting something the + restore has already put back is a no-op by construction. + """ + stub = _StubElasticsearch( + index_templates={"single-node-replica-default": CATCH_ALL, "dionaea-30d": DIONAEA}, + legacy_templates={}, + ) + with _serving(stub) as url: + shadow_file = tmp_path / "shadow.json" + assert _run(url, shadow_file, "shadow").returncode == 0 + assert stub.deleted == ["single-node-replica-default"] + out = _run(url, shadow_file, "shadow") + assert out.returncode == 0, out.stderr + assert stub.deleted == ["single-node-replica-default"], "a second shadow re-deleted something" + assert "nothing to delete" in out.stdout + assert json.loads(shadow_file.read_text()) == {"single-node-replica-default": CATCH_ALL} + + +@node_only +def test_generate_survives_a_crash_in_the_middle(tmp_path): + """`set -e` + a half-restored cluster is the outage this whole issue is about.""" + stub = _StubElasticsearch( + index_templates={"single-node-replica-default": CATCH_ALL}, + # _simulate_index reports the wrong replica count, so generate() throws + # after its PUT. + legacy_templates={"arkime_sessions3_template": SESSIONS_LEGACY}, + ) + with _serving(stub) as url: + shadow_file = tmp_path / "shadow.json" + assert _run(url, shadow_file, "shadow").returncode == 0 + + # Make the simulation disagree with what generate() just installed. + original = stub.handle + + def handle(method, path, body): + if method == "POST": + return 200, {"template": {"settings": {"index": {"number_of_replicas": "1"}}}} + return original(method, path, body) + + stub.handle = handle # type: ignore[method-assign] + out = _run(url, shadow_file) + assert out.returncode != 0, "a wrong replica count must fail the run" + assert "single-node-replica-default" in stub.index_templates, ( + "the catch-all must be restored even when generate() fails, or every " + "template-less index in the stack loses number_of_replicas until the next deploy" + ) + + +@node_only +def test_dry_run_changes_nothing(tmp_path): + stub = _StubElasticsearch(index_templates={"single-node-replica-default": CATCH_ALL}, legacy_templates={}) + with _serving(stub) as url: + env = {**os.environ, "ARKIME__elasticsearch": url, "SHADOW_FILE": str(tmp_path / "s.json"), "DRY_RUN": "1"} + out = subprocess.run([NODE, str(SCRIPT), "shadow"], capture_output=True, text=True, env=env, timeout=60) + assert out.returncode == 0, out.stderr + assert "dry run" in out.stdout + assert stub.deleted == [] + assert set(stub.index_templates) == {"single-node-replica-default"} + assert not (tmp_path / "s.json").exists() + + +@node_only +def test_unknown_subcommand_fails_loudly(tmp_path): + stub = _StubElasticsearch(index_templates={}, legacy_templates={}) + with _serving(stub) as url: + out = _run(url, tmp_path / "s.json", "translate") + assert out.returncode != 0 + assert "unknown subcommand" in out.stderr From 717e7fce2f06413f6b9938c247a27cb5fd1d8570 Mon Sep 17 00:00:00 2001 From: Xore Date: Sun, 27 Sep 2026 11:26:44 +0200 Subject: [PATCH 2/4] fix(arkime): keep the template stash out of world-readable /tmp (#3343) CodeQL flagged js/insecure-temp-file on the shadow file: it was written to a fixed path directly in /tmp, so anything on the host could have pre-created it as a symlink or read the stash. The stash holds the full body of every Arkime template this script deletes from the cluster, so that is worth closing. The path stays fixed because the shadow and generate passes are separate processes that must share it -- a fresh mkdtemp per process would hand each one its own empty stash and the generate pass would have nothing to restore. The fix is the permissions instead: the parent directory is created 0700 and the file 0600, and neither is widened on rewrite. os.tmpdir() honours TMPDIR and SHADOW_FILE still overrides the location outright. Asserted in tests/docs/test_3343_fix.py so a later change cannot quietly drop the modes back to the mkdir/write defaults. --- .../arkime/composable-templates.js | 17 ++++++++++--- tests/docs/test_3343_fix.py | 24 +++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/arcane/home/honeypot-init/arkime/composable-templates.js b/arcane/home/honeypot-init/arkime/composable-templates.js index b3a81c38..13cb7eda 100644 --- a/arcane/home/honeypot-init/arkime/composable-templates.js +++ b/arcane/home/honeypot-init/arkime/composable-templates.js @@ -112,7 +112,16 @@ const LEGACY_IP_FIX = 'arkime-sessions3-ip-fix'; // and re-shadows + regenerates here -- so the loss is one deploy of // number_of_replicas on newly created indices, not a permanent one. Kept out of // the shared init-markers volume, which is a *.done readiness contract. -const SHADOW_FILE = process.env.SHADOW_FILE || '/tmp/arkime-composable-shadow.json'; +// The stash is one deploy's worth of deleted templates and the only record +// that db.pl's originals existed. The `shadow` and `generate` passes are two +// separate processes, so the path is a fixed name under os.tmpdir() rather +// than a fresh mkdtemp (which would not be shared). writeShadow() creates the +// parent 0700 and the file 0600, so it is not exposed through a +// world-writable /tmp -- CodeQL js/insecure-temp-file, and a real +// pre-creation/symlink window on a shared host. SHADOW_FILE still moves it. +const SHADOW_FILE = + process.env.SHADOW_FILE || + path.join(os.tmpdir(), 'arkime-composable-shadow', 'shadow.json'); const DRY_RUN = Boolean(process.env.DRY_RUN); // #3283: same knob, same 30-day default as elasticsearch-setup.sh's @@ -240,8 +249,10 @@ function readShadow() { } function writeShadow(shadow) { - fs.mkdirSync(path.dirname(SHADOW_FILE), { recursive: true }); - fs.writeFileSync(SHADOW_FILE, JSON.stringify(shadow, null, 2) + '\n'); + // 0700 dir + 0600 file, and never widened on rewrite: the stash holds the + // full body of templates that were deleted from the cluster. + fs.mkdirSync(path.dirname(SHADOW_FILE), { recursive: true, mode: 0o700 }); + fs.writeFileSync(SHADOW_FILE, JSON.stringify(shadow, null, 2) + '\n', { mode: 0o600 }); } async function shadow() { diff --git a/tests/docs/test_3343_fix.py b/tests/docs/test_3343_fix.py index 80d2be98..c5e5b20a 100644 --- a/tests/docs/test_3343_fix.py +++ b/tests/docs/test_3343_fix.py @@ -49,6 +49,7 @@ import json import os +import stat import pathlib import shutil import subprocess @@ -674,3 +675,26 @@ def test_unknown_subcommand_fails_loudly(tmp_path): out = _run(url, tmp_path / "s.json", "translate") assert out.returncode != 0 assert "unknown subcommand" in out.stderr + + +@node_only +def test_shadow_stash_is_not_world_readable(tmp_path): + """The stash holds the full body of templates deleted from the cluster, so + it must not land world-readable. CodeQL flags a direct /tmp write + (js/insecure-temp-file); the fix keeps a fixed path -- the shadow and + generate passes are separate processes sharing it -- but creates the + parent 0700 and the file 0600.""" + shadow = tmp_path / "nested" / "s.json" + stub = _StubElasticsearch( + index_templates={ + "single-node-replica-default": CATCH_ALL, + "arkime-sessions3-ip-fix": IP_FIX, + }, + legacy_templates={}, + ) + with _serving(stub) as url: + out = _run(url, shadow, "shadow") + assert out.returncode == 0, out.stderr + assert shadow.exists() + assert stat.S_IMODE(shadow.stat().st_mode) == 0o600, oct(shadow.stat().st_mode) + assert stat.S_IMODE(shadow.parent.stat().st_mode) == 0o700, oct(shadow.parent.stat().st_mode) From 7a80c9cec0d41a3467336ba6b37bf74cefde7e4a Mon Sep 17 00:00:00 2001 From: Xore Date: Sun, 27 Sep 2026 12:15:58 +0200 Subject: [PATCH 3/4] test(arkime): teach the #3343 stub the routes #3283 added to generate() Conflict resolution follow-up for the rebase onto main. #3391 (the #3283 retention work) landed in composable-templates.js between this branch being cut and the rebase, and it widened the contract generate() has with Elasticsearch: it now installs _ilm/policy/arkime-sessions-30d immediately before the template that names it, verifies index.lifecycle.name through _simulate_index, and adopts the arkime_sessions3-* indices already on disk. The #3343 suite drives the real script against a stub Elasticsearch, so two of its tests failed on the rebase with an AssertionError naming an "unexpected" PUT /_ilm/policy/arkime-sessions-30d. No assertion changed -- the stub is what was stale, and it is what had to learn the new routes. Its _simulate_index answer changes from a canned {"number_of_replicas": "0"} to a composition of the templates the script really PUT, which is what Elasticsearch does and what the script's own post-install verification depends on. That is strictly stronger than what it replaces: the canned reply reported 0 replicas whatever the script installed, so a generated template that dropped number_of_replicas would still have passed. Composed, it fails. Dropping the retention line from build() now fails 18 tests across both suites rather than passing silently. The adoption routes answer with an empty index list, so the #3343 assertions stay about templates and the shadow; the adoption contract itself remains asserted in tests/docs/test_3283_fix.py, which owns it. Refs #3343, #3283 --- tests/docs/test_3343_fix.py | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/tests/docs/test_3343_fix.py b/tests/docs/test_3343_fix.py index c5e5b20a..a6aea097 100644 --- a/tests/docs/test_3343_fix.py +++ b/tests/docs/test_3343_fix.py @@ -400,13 +400,32 @@ def test_script_does_not_resurrect_what_generate_just_rebuilt(): class _StubElasticsearch: """Just enough of ES to drive composable-templates.js end to end.""" - def __init__(self, index_templates: dict, legacy_templates: dict): + def __init__(self, index_templates: dict, legacy_templates: dict, indices=()): self.index_templates = dict(index_templates) self.legacy_templates = dict(legacy_templates) + # #3283: the arkime_sessions3-* names the cluster already holds, which + # `generate` adopts onto its policy. Empty by default so the #3343 + # assertions stay about templates; the adoption contract itself is + # asserted in tests/docs/test_3283_fix.py, which owns that behaviour. + self.indices = list(indices) self.deleted: list[str] = [] self.put_index_templates: dict = {} self.calls: list[str] = [] + def compose(self, probe: str) -> dict: + """Answer `_simulate_index` by composing what the script really sent. + + Elasticsearch composes every matching composable template, so a canned + reply would let the script's own post-install verification pass no + matter what it installed. Composing from `put_index_templates` means a + template that dropped a setting fails here instead of in production. + """ + pattern = probe.replace("composable-check", "*") + for tmpl in self.put_index_templates.values(): + if pattern in (tmpl.get("index_patterns") or []): + return tmpl.get("template", {}) + return {} + def handle(self, method: str, path: str, body) -> tuple[int, dict]: self.calls.append(f"{method} {path}") if method == "GET" and path == "/_index_template": @@ -426,12 +445,24 @@ def handle(self, method: str, path: str, body) -> tuple[int, dict]: self.index_templates[name] = body return 200, {"acknowledged": True} if method == "POST" and path.startswith("/_index_template/_simulate_index/"): - return 200, {"template": {"settings": {"index": {"number_of_replicas": "0"}}}} + return 200, {"template": self.compose(path.rsplit("/", 1)[-1])} if method == "GET" and path.startswith("/_template/"): name = path.rsplit("/", 1)[1] if name in self.legacy_templates: return 200, {name: self.legacy_templates[name]} return 404, {"error": "resource_not_found_exception"} + # #3283: the policy `generate` installs immediately before the template + # that names it, so the ordering that makes that safe can never depend + # on this stub refusing the call. + if method == "PUT" and path.startswith("/_ilm/policy/"): + return 200, {"acknowledged": True} + if method == "GET" and path.startswith("/_cat/indices/"): + return 200, [{"index": name} for name in self.indices] + if method == "GET" and path.endswith("/_settings?flat_settings=true"): + name = path.lstrip("/").split("/", 1)[0] + return 200, {name: {"settings": {}}} + if method == "PUT" and path.endswith("/_settings"): + return 200, {"acknowledged": True} raise AssertionError(f"stub Elasticsearch got an unexpected {method} {path}") From 34c5d3ecca17591c1625001b76af7a9d6828b4f9 Mon Sep 17 00:00:00 2001 From: CI-fix lane Date: Sun, 27 Sep 2026 12:30:07 +0200 Subject: [PATCH 4/4] test(arkime): drive #3283's adoption from the #3343 stub, and pin the ordering Follow-up to 7a80c9ce, which taught this stub the routes #3283 added. That commit answered them, but answered _cat/indices with an empty list and _settings with a constant, so the adoption branch was never actually entered from this suite -- the routes were registered and dead. tests/docs/ test_3283_fix.py owns the adoption contract and pins it properly; this change is about the *combined* run, which is what this suite owns. The stub now models the one field the script reads: a name maps to whatever index.lifecycle.name it carries, or None for unmanaged. _stubbed() seeds one unmanaged sessions index, so the functional tests drive the real path, and put_ilm_policies records the body so the ordering can be asserted rather than assumed. New test: the run that restores the catch-all is the same run that installs arkime-sessions-30d and adopts what predates it, and the policy is installed before the template that names it. That ordering is the part that breaks silently -- an index template naming an ILM policy that does not exist yet fails index creation outright, and Elasticsearch validates it at index creation rather than here, so nothing upstream would report it. Non-vacuous, checked by mutating the script and reverting: * moving ensurePolicy after the template PUT fails the new test ("the policy was installed after the template naming it") * deleting the adoption loop fails it ("arkime_sessions3-2026.09.01 was not adopted onto the policy: None") Local: pytest tests/docs/ 521 passed, 1 xfailed (main: 497 passed, same xfail); test_3343 + test_3283 together 46 passed. scripts/tests unchanged from main: the same 2 pre-existing test_compose_drift_watch_sweep failures that fail on a pristine main checkout on this host. --- tests/docs/test_3343_fix.py | 63 +++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/tests/docs/test_3343_fix.py b/tests/docs/test_3343_fix.py index a6aea097..2cc3a582 100644 --- a/tests/docs/test_3343_fix.py +++ b/tests/docs/test_3343_fix.py @@ -116,6 +116,11 @@ DIONAEA = {"index_patterns": ["dionaea-*"], "priority": 5, "template": {"settings": {"index.number_of_shards": 1}}} ML = {"index_patterns": ["ml-anomalies", "ml-worker-*"], "priority": 5, "template": {}} +# A sessions index that predates the generated template and carries no +# lifecycle name -- what #3283's adoption pass exists to pick up. +PRE_EXISTING_SESSIONS = "arkime_sessions3-2026.09.01" +SESSIONS_POLICY = "arkime-sessions-30d" + # ------------------------------------------------------------ the render ---- @@ -407,9 +412,14 @@ def __init__(self, index_templates: dict, legacy_templates: dict, indices=()): # `generate` adopts onto its policy. Empty by default so the #3343 # assertions stay about templates; the adoption contract itself is # asserted in tests/docs/test_3283_fix.py, which owns that behaviour. - self.indices = list(indices) + # + # A name maps to whatever index.lifecycle.name it currently carries, or + # to None for "unmanaged, adopt me". A list is also accepted and reads + # as all-unmanaged, so the fixtures above stay terse. + self.indices = {name: None for name in indices} if not isinstance(indices, dict) else dict(indices) self.deleted: list[str] = [] self.put_index_templates: dict = {} + self.put_ilm_policies: dict = {} self.calls: list[str] = [] def compose(self, probe: str) -> dict: @@ -453,15 +463,26 @@ def handle(self, method: str, path: str, body) -> tuple[int, dict]: return 404, {"error": "resource_not_found_exception"} # #3283: the policy `generate` installs immediately before the template # that names it, so the ordering that makes that safe can never depend - # on this stub refusing the call. + # on this stub refusing the call. The body is recorded so the ordering + # can be asserted rather than assumed. if method == "PUT" and path.startswith("/_ilm/policy/"): + self.put_ilm_policies[path.rsplit("/", 1)[1]] = body return 200, {"acknowledged": True} if method == "GET" and path.startswith("/_cat/indices/"): return 200, [{"index": name} for name in self.indices] if method == "GET" and path.endswith("/_settings?flat_settings=true"): name = path.lstrip("/").split("/", 1)[0] - return 200, {name: {"settings": {}}} + if name not in self.indices: + return 404, {"error": "resource_not_found_exception"} + # Only the lifecycle name is modelled; a real reply carries every + # flat setting, and the script reads this one. + settings = {"index.lifecycle.name": self.indices[name]} if self.indices[name] else {} + return 200, {name: {"settings": settings}} if method == "PUT" and path.endswith("/_settings"): + name = path.lstrip("/").split("/", 1)[0] + if name not in self.indices: + return 404, {"error": "resource_not_found_exception"} + self.indices[name] = (body or {}).get("index.lifecycle.name") return 200, {"acknowledged": True} raise AssertionError(f"stub Elasticsearch got an unexpected {method} {path}") @@ -518,6 +539,11 @@ def _stubbed(): "arkime_sessions3_ecs_template": SESSIONS_ECS_LEGACY, "arkime_history_v1_template": HISTORY_LEGACY, }, + # #3283's adoption pass runs inside the same generate() that restores + # the catch-all, so seeding one unmanaged index means the functional + # tests below drive that path too rather than leaving it to a stub + # that never receives the call. + indices=[PRE_EXISTING_SESSIONS], ) with _serving(stub) as url: yield stub, url @@ -562,6 +588,37 @@ def test_generate_rebuilds_arkimes_templates_and_restores_everything_else(tmp_pa assert json.loads(shadow_file.read_text()) == {} +@node_only +def test_the_restoring_run_is_also_the_one_that_installs_retention(tmp_path): + """#3283's obligations, checked from this suite's own fixture. + + The shadow/restore cycle and the retention pass are one run of one script + now, so the run that puts the catch-all back is the same run that installs + the policy and adopts the indices already on disk. The ordering is the + part that breaks silently: an index template naming an ILM policy that + does not exist yet fails index creation outright, and Elasticsearch + validates it at index creation, not here. + """ + with _stubbed() as (stub, url): + shadow_file = tmp_path / "shadow.json" + assert _run(url, shadow_file, "shadow").returncode == 0 + out = _run(url, shadow_file) + assert out.returncode == 0, out.stderr + + assert SESSIONS_POLICY in stub.put_ilm_policies, sorted(stub.put_ilm_policies) + assert stub.indices[PRE_EXISTING_SESSIONS] == SESSIONS_POLICY, ( + f"{PRE_EXISTING_SESSIONS} was not adopted onto the policy: " + f"{stub.indices[PRE_EXISTING_SESSIONS]!r}" + ) + + order = [c for c in stub.calls + if c.startswith(("PUT /_ilm/policy/", "PUT /_index_template/arkime-sessions3"))] + assert order.index(f"PUT /_ilm/policy/{SESSIONS_POLICY}") < \ + order.index("PUT /_index_template/arkime-sessions3"), ( + f"the policy was installed after the template naming it: {order}" + ) + + @node_only def test_generated_sessions_template_carries_arkimes_real_mappings(tmp_path): with _stubbed() as (stub, url):