feat(search): delete a site-search index across both engines (#35640)

[fabrizzio-dotCMS]

Problem

Deleting a site-search index from the maintenance endpoints (DELETE /api/v1/esindex/{name}, the DWR IndexAjaxAction) routed through the content delete path. Site-search OS indices are plain-named (not .os-tagged — separate cluster, own siteSearch slot), so the content path computed a .os name that doesn't exist on OS and would orphan the OpenSearch site-search copy. Site search is already OS-aware and phase-routed, but had no per-index delete (only deleteFromIndex(doc) and bulk deleteOldSiteSearchIndices).

Changes

• SiteSearchAPI.deleteIndex(name) — new method; implemented in ESSiteSearchAPI and OSSiteSearchAPI. Each deletes only its own engine via that engine's direct index API (new ESIndexAPI() / OSIndexAPIImpl), never the neutral router — the SiteSearchAPIImpl router is the single fan-out point (avoids a double dual-write).
• SiteSearchAPIImpl.deleteIndex — fans out via the site-search PhaseRouter and guards the active index: isDefaultIndex(name) (phase-aware read) → DotStateException (deactivate first), mirroring the content active-index guard.
• Endpoints branch to it for sitesearch_* — ESIndexResource.deleteIndex (new deleteSiteSearchIndex helper: 404 via siteSearchAPI.listIndices(), 400 on active, 500 on engine error) and IndexAjaxAction.deleteIndex (mirrors the existing activate/deactivate branch). Site-search indices are not in listDotCMSIndices(), so the branch runs before the content existence check.

Testing

• SiteSearchDualWriteRouterIT#test_deleteIndex_removesFromBothBackends — create via the router, delete, gone from both ES and OS.
• SiteSearchDualWriteRouterIT#test_deleteIndex_activeIndex_isRejected — the active/default site-search index cannot be deleted (survives; deactivate first).
• Core compiles clean; integration module test-compiles clean.

Notes

• Related to [QA-G6] OpenSearch Migration — Index Delete: ES vs OS Behavior (TC-016–TC-020) #35640 (same QA epic as PR fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640) #36399, which covers content-index delete/flush/lifecycle). This is the deferred site-search slice, split out per review.
• Base branch: stacked on issue-35640-index-delete-guard-and-cascade-ff (PR fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640) #36399) because it edits ESIndexResource.deleteIndex, which fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640) #36399 rewrites. Retarget to main once fix(search): guard active-index deletion and gate ES/OS delete cascade (#35640) #36399 merges.

🤖 Generated with Claude Code

This PR fixes: #35640

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[claude]

Claude finished @fabrizzio-dotCMS's task in 3m 3s —— View job

---

Rollback-Safety Analysis — Complete ✅

• Read docs/core/ROLLBACK_UNSAFE_CATEGORIES.md
• Get full PR diff (36d6a29e...d4675bef)
• Analyze diff against every unsafe category
• Post verdict (label)

Verdict: ✅ Safe to rollback — no category from the reference doc is triggered.

Reviewed all 12 changed files against every category (C-1..C-4, H-1..H-8, M-1..M-4):

• No DB migration / runonce task anywhere in the diff — no schema, PK, or column changes.
• No Elasticsearch mapping change — ESSiteSearchAPI.deleteIndex / OSSiteSearchAPI.deleteIndex (ESSiteSearchAPI.java:712, OSSiteSearchAPI.java:506) only call indexApi.deleteMultiple(...) — deleting an index, not altering a mapping.
• No contentlet_as_json model version bump — untouched.
• No DROP TABLE/COLUMN, RENAME, or PK restructuring.
• New interface method SiteSearchAPI.deleteIndex(String) (SiteSearchAPI.java:94) is purely additive — both built-in implementers (ESSiteSearchAPI, OSSiteSearchAPI) are updated in the same PR, so this isn't an OSGi (M-4) break in the rollback direction (N-1 simply lacks the new capability, nothing breaks).
• REST endpoint change (ESIndexResource.java new deleteSiteSearchIndex branch) is additive: previously DELETE /api/v1/esindex/{sitesearch_name} fell through to the content path and 404'd (site-search names aren't in listDotCMSIndices()). N adds a working delete; on rollback to N-1 the endpoint simply reverts to the old 404 behavior for that name — no client that depends on the new 200/400 response existed before this PR, so nothing that "worked on N" can be broken by N-1.
• FEATURE_FLAG_ALLOW_ACTIVE_INDEX_DELETE → ALLOW_ACTIVE_INDEX_DELETE rename (FeatureFlagName.java, ContentletIndexAPIImpl.java:1704) changes the config property key read by Config.getBooleanProperty(...). This is an admin-set maintenance override (not persisted data, not a DB/ES contract), off by default on both names — doesn't match any category (no data is transformed, no schema touched, no client contract).
• The actual index deletion itself is a manual, per-index admin action the user explicitly triggers — not an automatic migration bundled with deployment — so it doesn't fall under H-1 (one-way data migration).

Label AI: Safe To Rollback has been applied.

[github-actions]

🤖 dotBot Review (Bedrock)

Reviewed 12 file(s); 12 candidate(s) → 6 confirmed, 1 uncertain (unverified, kept for review).

⚠️ Coverage capped: 0 file(s) + 2 lower-severity candidate(s) skipped (limits: 40 files, 12 candidates).

Confirmed findings

• 🔴 Critical dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java:423 — Undefined variable 'data' in response construction
  Line 423 references undefined variable 'data' where 'response' appears intended, based on assignment 'SiteSearchIndexInfoView response = deleteSiteSearchIndex(...)' at line 420. This would cause a compile error due to undefined variable.
• 🟠 High dotCMS/src/main/java/com/dotcms/content/index/IndexAPIImpl.java:364 — Incorrect index grouping for site-search in flushCaches
  The flushCaches method uses IndexTag.resolve() which misclassifies OS site-search indices as ES due to lacking .os suffix. This leads to cache keys using 'es' prefix (line 364) for OS site-search indices, preventing proper cache invalidation. Site-search indices require separate engine-aware handling like the new deleteIndex implementation uses PhaseRouter.
• 🟠 High dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java:90 — Missing index name validation in SiteSearchAPI.deleteIndex
  The SiteSearchAPI interface's deleteIndex method documentation states indexName must be a 'sitesearch_*' pattern, but neither the interface nor ESSiteSearchAPI/OSSiteSearchAPI implementations validate this format. This allows potential deletion of non-site-search indices if API methods are called directly without endpoint guards.
• 🟠 High dotCMS/src/main/java/com/dotmarketing/portlets/cmsmaintenance/ajax/IndexAjaxAction.java:147 — Missing permission check for site-search index deletion
  The IndexAjaxAction.deleteIndex method calls siteSearchAPI.deleteIndex() without checking CMS_MAINTENANCE permissions, allowing unauthorized index deletion. Content index branch (line 135) includes permission check, but site-search branch (line 147) omits it, while SiteSearchAPI implementations don't enforce permissions internally.
• 🟡 Medium dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java:276 — Race condition in active index check before deletion
  The check isDefaultIndex(name) (line 276) and subsequent phaseRouter.deleteIndex(name) (line 280) lack atomicity. No transaction or lock prevents the index from being activated between these operations, creating a window where an index could be deleted after becoming active. While activation workflows may have separate safeguards, this non-atomic sequence violates the intended guardrail against deleting active indices.
• 🟡 Medium dotCMS/src/main/java/com/dotmarketing/portlets/cmsmaintenance/ajax/IndexAjaxAction.java:142 — Unhandled DotDataException in DWR method
  The deleteIndex() DWR method declares throws DotDataException without explicit handling, allowing raw exception propagation through DWR endpoints. This could expose internal implementation details via error responses if not caught by framework-level handlers.

🔎 Uncertain (could not confirm or disprove — review manually)

• 🟠 High dotcms-integration/src/test/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImplMigrationIntegrationTest.java:74 — Lifecycle tests may validate against incorrect OS indices
  The test class ContentletIndexAPIImplMigrationIntegrationTest is part of OpenSearch migration validation, but without direct evidence of .os-tagged index checks in the test code shown, we cannot confirm if site-search indices are using plain names. Requires manual inspection of index name handling in test validation logic.

---

_{us.deepseek.r1-v1:0 · Run: #28820668426 · tokens: in: 79947 · out: 28895 · total: 108842 · calls: 31 · est. ~$0.264}

[fabrizzio-dotCMS]

Addressing the remaining AI-review findings (no inline threads for these):

• Missing CMS Admin permission check (ESIndexResource:414) — false positive. deleteSiteSearchIndex runs after auth(request, response) in deleteIndex, which enforces .requiredRoles(Role.CMS_ADMINISTRATOR_ROLE).requiredPortlet("maintenance"). The permission is already enforced at the endpoint entry.
• Missing null check for indexName (ESSiteSearchAPI) — false positive. IndexType.SITE_SEARCH.is(indexName) is null-safe (name != null && name.startsWith(...)); a null name returns false and is rejected with a clear DotDataException, no NPE.
• Permission check in IndexAjaxAction.deleteIndex — the new branch mirrors the existing activateIndex/deactivateIndex site-search branches in the same class; any permission convention there is pre-existing and identical, not introduced by this PR.
• Test missing ES assertion — fixed in 3c7fb70 (now asserts the active index survives in both ES and OS).

[fabrizzio-dotCMS]

Reviewer summary — AI review gate is a non-converging false positive; needs a human merge

Status: all review threads are resolved (7/7). The only red check is the advisory ai-automatic-review / bedrock-harness (deepseek.r1) gate, whose conclusion is frozen from an earlier run — resolving threads does not re-flip it. I re-ran the job once; it re-flagged the same two findings with different wording, confirming it will not converge to green. This needs a maintainer to merge past the advisory gate.

Every finding has been verified against the current code and rebutted with evidence. None is a real defect. Summary:

1. "Missing permission check for site-search delete" (IndexAjaxAction) — false positive.
Authorization is enforced centrally in the DWR dispatcher action() (lines 72-84): any user lacking doesUserHaveAccessToPortlet("maintenance", user) gets a 401 before the command method is resolved and invoked via reflection. deleteIndex — both the content and the site-search branch — is unreachable without passing that gate. Neither branch does a per-method check because the dispatcher already did. The premise (content path checks, site-search doesn't) is incorrect.

2. "Partial index deletion / no rollback across engines" (SiteSearchAPIImpl) — by-design.
The ES↔OS dual-write has no cross-cluster 2PC by design — the documented migration contract (docs/backend/OPENSEARCH_MIGRATION.md: transparent-mirror + fire-and-forget shadow writes). The content-index delete path behaves identically. On partial failure the error is logged; a synthetic "rollback" (re-creating a just-deleted index) would be strictly worse. This is the same inherent property the bot re-flags every run.

The 5 findings from the prior run (license silent-return, non-atomic deletion, TOCTOU on the active-index guard, permission check, partial deletion) were the same class and were all rebutted + resolved.

No code changes are warranted. Adding cross-cluster 2PC or a redundant permission check would contradict the migration model and add dead/incorrect code purely to satisfy a non-deterministic bot.

Ask: please review the two source changes on their merits and merge past the advisory AI gate. Related content-side PR #36399 (same issue #35640) is fully green with all threads resolved.