fix: Prevent findings history deletion by alert retention period

[nagendramohan]

The getHistoryIndexToDelete method incorrectly returned the index name for deletion when the index had no matching alias in the history type being checked. A findings index with no alert-history alias would be deleted after exceeding alert_history_retention_period instead of finding_history_retention_period.

Fix: return null (do not delete) when an index does not belong to the history type being evaluated (alias == null). Only delete when the index positively matches the history type's alias AND exceeds that type's retention period.

Also makes getHistoryIndexToDelete package-private static for testability and adds unit tests covering:

• Findings index not deleted by alert retention check
• Findings index deleted when exceeding finding retention
• Findings index not deleted when younger than retention

Resolves #1759

Description

Fixes the getHistoryIndexToDelete method in DetectorIndexManagementService which incorrectly deleted findings indices based on alert_history_retention_period instead of finding_history_retention_period.

Root cause: The method is called once per history type (alert, finding, correlation, IOC) for each index. When an index had no matching alias for the history type being checked (e.g., a findings index checked against alert history aliases), the method fell through and returned the index for deletion. It should have returned null (skip — this index doesn't belong to me).

Fix: Return null when no alias match is found, meaning the index doesn't belong to this history type. Only return the index name for deletion when it positively matches the type's alias AND exceeds that type's retention period.

Also makes getHistoryIndexToDelete package-private static (uses no instance state) and adds 3 unit tests.

Related Issues

Resolves #1759

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ No major issues detected

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Distinguish write alias from read alias When historyEnabled is true, returning null immediately upon matching the write alias short-circuits deletion for the current write index, which is correct. However, when historyEnabled is true and the index has the alias but is NOT the write index (rolled-over index still carrying the alias), the code returns the index name for deletion only via the historyEnabled=false branch. Verify that the alias match logic correctly distinguishes write alias vs. read alias to avoid deleting the active write index prematurely. src/main/java/org/opensearch/securityanalytics/indexmanagment/DetectorIndexManagementService.java [407-416] if (alias != null) { - if (historyEnabled) { + if (historyEnabled && Boolean.TRUE.equals(alias.writeIndex())) { // If the index has the write alias and history is enabled, don't delete the index return null; } - // Index belongs to this history type but history is disabled — delete it + // Index belongs to this history type and is expired — delete it return indexMetadata.getIndex().getName(); } // Index does not belong to this history type — do not delete return null; Suggestion importance[1-10]: 4 __ Why: The suggestion raises a valid concern about distinguishing write alias from read alias, but it's primarily a "verify" request and the proposed change alters existing behavior that wasn't part of the PR's original scope. The impact is moderate but speculative without more context on the alias iteration logic.	Low

[nagendramohan]

Gentle bump 🙂 — this fixes a retention bug where findings-history indices could be deleted on the alert retention period (and vice-versa) because getHistoryIndexToDelete didn't check the index belongs to the history type being processed, causing data to be removed earlier than configured. Small, tested change. Could a maintainer take a look when there's bandwidth? Happy to address feedback. Thanks!