fix(migration): guard activateIndex against a missing OpenSearch counterpart (#36360) - #36880
fix(migration): guard activateIndex against a missing OpenSearch counterpart (#36360)#36880fabrizzio-dotCMS wants to merge 5 commits into
Conversation
…terpart (#36360) Activating an old/backup index is the rollback mechanism, but if that index predates the migration it has no OpenSearch copy. activateIndex only repoints both stores by name (no create/reconcile), so pointing at a missing .os index silently diverges ES from OS and detonates at Phase 3 (no ES fallback → empty/error). Add a narrow guard: during the migration (phases 1/2/3), refuse to activate an index whose OpenSearch counterpart does not exist (DotStateException → 400). This does NOT block rollback to a migration-era index that has its .os copy — only the genuinely broken case. Escape hatch: ALLOW_ACTIVATE_INDEX_WITHOUT_OS_MIRROR=true forces it. (The non-blocking alternative — allow the rollback and rebuild the OS copy async via the reindex machinery, with the readiness gate refusing Phase 3 until it's ready — is the deferred follow-up.) Tests: ContentletIndexAPIImplActivateGuardTest (6) — blocks in phases 1/2/3 when the OS counterpart is missing; allows when present, in Phase 0, and under the override. Fixed the shared FakeContentletIndexOperations.indexAPI() to report existence so the existing phase-2/3 activate tests in ContentletIndexAPIImplPhaseTest keep passing (14). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @fabrizzio-dotCMS's task in 2m 21s —— View job Code Review — activateIndex OpenSearch-counterpart guard
New Issues
Notes (non-blocking)
Both findings are 🟡 Medium — non-blocking. The core guard is correct and the "simple first" tradeoff is clearly documented in the PR. • |
…removed (#36360) Confirms the guard only applies while migration is started (phases 1/2/3): with no phase configured, MigrationPhase.current() defaults to Phase 0 and activation (rollback) proceeds. So setting the phase to 0 or removing the flag re-enables unrestricted rollback. 7/7. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#36360) When activate/clear/delete is rejected by a guard (DotStateException), modIndex returned a 400 but the portlet showed nothing. Push the reason as a system message (toast) via sendAdminMessage so the operator sees it in the UI, not only in the server log. Also log the resolved indexAction instead of the raw query param, which fixes the misleading "Rejected 'null'" when the UI omits ?action=activate (relies on the default). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ge (#36360) The guard's rejection reached the UI toast with internal jargon (OpenSearch, the .os physical name, "Phase 3", the override config key), which also exposed the migration to a regular admin. Split it: the full diagnostic (counterpart name + override) goes to the operator log; the exception message that surfaces in the UI is now friendly and neutral — "'<index>' can't be set as the active index right now because its search data isn't ready yet. Run a full reindex and try again." Toast severity softened to WARNING. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#36360) Per feedback the UI message should be honest, not migration-neutral: state the real reason (no OpenSearch counterpart) and the real fix (suspend the migration, or run a full reindex). The physical counterpart name and override key stay in the operator log only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Closing — a hard existence guard on
At activation time, two cases are indistinguishable by a point-in-time existence check:
Both are "no OS copy right now", so the guard blocks the legitimate catch-up too — breaking normal migration operation. CI confirmed it: 6 ITs in the OpenSearch Upgrade Suite failed for exactly this reason. The real risk (reactivating an un-mirrored backup → empty search at Phase 3) is a known, accepted characteristic of the optimistic-mirror model; the intended mitigation is the migration-readiness endpoint (#36849) — it detects a Finding documented in |
…tchup) (#36360) Record the finding from the reverted activate guard (PR #36880): activateIndex/ deactivateIndex are name-driven pointer-store updates that deliberately don't validate cluster existence — a load-bearing part of the catchup model (ES/OS names diverge, OS may not be built yet). A hard "block activate when the OS counterpart is missing" guard cannot tell "not built yet" (legitimate catchup) from "never built" (old backup) at activation time, so it breaks normal migration operation (6 OpenSearch Upgrade Suite ITs). The intended mitigation for the un-mirrored-backup risk is the migration-readiness endpoint (detect MISSING_COUNTERPART + gate Phase-3), not a per-op guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
Activating an old/inactive index is dotCMS's rollback mechanism (revert to a previous reindex). But if that index predates the migration, it never went through the OpenSearch create fan-out, so it has no OpenSearch counterpart.
ContentletIndexAPIImpl.activateIndexis phase-aware but only repoints both stores by name (toPhysicalName→cluster_X.<name>.os) — noindexExists, no create, no reconcile. Reactivating such an index during the migration is unguarded:.osindex that doesn't exist; shadow writes swallowed).deletealready has a phase-aware guard (assertIndexNotActive);activatehad none.Fix — a narrow guard
During the migration (phases 1/2/3),
activateIndexnow refuses to activate an index whose OpenSearch counterpart does not exist, throwingDotStateException(→ 400) with a clear message..oscopy still works. Only the genuinely-broken case (no OS copy) is blocked.ALLOW_ACTIVATE_INDEX_WITHOUT_OS_MIRROR=trueforces the activation (accepting that OpenSearch is left pointing at a missing index until rebuilt) — mirroring theALLOW_ACTIVE_INDEX_DELETEoverride on the delete guard.Known trade-off (deliberate, "simple first")
A hard block conflicts with rollback: you can't roll back to a pre-migration index during the migration without first rebuilding it (or setting the override). The non-blocking alternative — allow the rollback, repoint ES instantly (stays transparent), and rebuild the OpenSearch copy asynchronously via the existing reindex machinery, with the readiness gate refusing Phase 3 until it's ready — is the intended follow-up. This PR is the cheap, safe first step that stops the silent Phase-3 detonation.
Tests
ContentletIndexAPIImplActivateGuardTest(6, surefire): blocks in phases 1/2/3 when the OS counterpart is missing; allows when it's present, in Phase 0, and under the override flag. Set-backed engine fakes control OS existence — no cluster needed.FakeContentletIndexOperations.indexAPI()(previously threw) to report existence, so the existing phase-2/3activateIndextests inContentletIndexAPIImplPhaseTest(14) keep passing under the new precondition.CI note: migration integration tests that activate an index in a dual/OS phase now require the OS counterpart to exist (real bootstrap normally provides it). Watch the OpenSearch upgrade / phase suites.
Notes
main. Tied to [QA-G17] OpenSearch Migration — Site Search Functional Validation (Phases 0–3, $sitesearch viewtool) #36360 (no new issue).🤖 Generated with Claude Code