Exclude system indices from remote cluster state

[MdTanwer]

Summary

Exclude system indices from remote cluster state publish, restore, and manifest verification using the existing SystemIndices registry.

Fixes #21043.

Problem

Remote cluster state persists index metadata only — not index documents. System indices such as .opendistro_security store their configuration as documents on local disk.

When remote cluster state was enabled, system index metadata was uploaded alongside user indices. After a node restart with empty local storage (e.g. Kubernetes emptyDir), OpenSearch restored that metadata from remote while the on-disk documents were gone. Plugins like security then saw the index as already existing (ResourceAlreadyExistsException) and never finished initialization (OpenSearch Security not initialized).

Solution

Introduce RemoteClusterStateIndexFilter, backed by SystemIndices, and apply it consistently across remote cluster state paths:

• Full write (writeFullMetadata): skip system index metadata and routing on upload
• Incremental write (writeIncrementalMetadata): skip system indices from upload/diff computation; strip them from the previous manifest map used for incremental updates
• Restore (getClusterStateForManifest, getClusterStateUsingDiff): filter system indices out of downloaded manifests before rebuilding cluster state
• Manifest verification (verifyManifestMatchesClusterState): compare manifest entries against the filtered set of cluster indices so publication succeeds when system indices are present locally

GatewayMetaState.RemotePersistedState delegates manifest checks to RemoteClusterStateService, and Node wires SystemIndices into the service constructor.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit ea66c85)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Possible Issue In writeIncrementalMetadata, the system index entries are removed from allUploadedIndexMetadata (the map used to derive the previous uploaded indices). However, the corresponding remote blob files for previously uploaded system indices are not added to a cleanup/delete path either - they are silently dropped from the manifest map without being deleted from the remote store. If a cluster previously published system indices (before this fix), upgrading will leave orphaned system index metadata files in the remote repository forever. Consider deleting these orphaned blobs or documenting the migration cleanup path. indexFilter.removeSystemIndicesFromUploadedMetadataMap(allUploadedIndexMetadata); List<IndexMetadata> toUpload = new ArrayList<>(); // We prepare a map that contains the previous index metadata for the indexes for which version has changed. Map<String, IndexMetadata> prevIndexMetadataByName = new HashMap<>(); for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { indicesToBeDeletedFromRemote.remove(indexName); continue; } Possible Issue filterUploadedIndexMetadata is used to filter routing tables via manifest.getIndicesRouting(), but filterIndexRoutingTables (which operates on IndexRoutingTable) is also defined. The restore path in getClusterStateForManifest filters manifest.getIndicesRouting() (List) using filterUploadedIndexMetadata, which only filters by name. If a system index was previously uploaded to remote routing (pre-fix), it will be excluded from restore - but the routing diff in getClusterStateUsingDiff may still reference deletions that assume those entries exist. Verify that restore paths handle pre-existing remote routing entries for system indices consistently. includeEphemeral ? indexFilter.filterUploadedIndexMetadata(manifest.getIndicesRouting()) : emptyList(), Assertion Behavior Change The verifyManifestAndClusterState method previously performed inline asserts that would fire when assertions are enabled. It now delegates to remoteClusterStateService.verifyManifestMatchesClusterState, which still uses asserts internally. The wrapping method, however, no longer returns true explicitly when assertions are disabled - it relies on the delegated method's return. Since verifyManifestMatchesClusterState is only reachable from within assert statements, behavior is preserved, but confirm all callers invoke it inside assert and not as a boolean check, otherwise the method will return true unconditionally when asserts are off, masking mismatches. private boolean verifyManifestAndClusterState(ClusterMetadataManifest manifest, ClusterState clusterState) { return remoteClusterStateService.verifyManifestMatchesClusterState(manifest, clusterState); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to ea66c85

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Ensure stale system index entries are cleaned When a previously non-system index becomes a system index (or naming changes cause filtering to flip), removing it from indicesToBeDeletedFromRemote here prevents its prior remote metadata from being cleaned up. Consider explicitly deleting any prior remote upload for newly-filtered indices instead of just skipping, otherwise stale metadata may remain on the remote store indefinitely. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [460-465] for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { - indicesToBeDeletedFromRemote.remove(indexName); + // Leave it in indicesToBeDeletedFromRemote so any stale remote entry gets cleaned up continue; } Suggestion importance[1-10]: 6 __ Why: This raises a valid concern about stale remote metadata for indices that transition to system status. The suggestion has merit, though the actual impact depends on whether such transitions occur in practice.	Low
	Filtered routing may break checksum validation Filtering manifest.getIndicesRouting() post-hoc may cause a mismatch between the requested routing entries and what readClusterStateInParallel expects given the rest of the manifest. If older nodes uploaded routing for indices that are now considered system indices, those will be silently dropped, producing a cluster state inconsistent with the manifest checksum used for validation later. Consider validating consistency or filtering only when reading state generated by this same node version. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [1628] +includeEphemeral ? indexFilter.filterUploadedIndexMetadata(manifest.getIndicesRouting()) : emptyList(), - Suggestion importance[1-10]: 5 __ Why: The concern about checksum validation mismatch is valid, but the improved_code is identical to the existing_code, providing no concrete fix. It only raises a question for the author to verify.	Low
Possible issue	Replace asserts with runtime checks The method returns true unconditionally and only uses assert statements, which are disabled at runtime by default. This means the verification will be silently skipped in production and the method effectively does nothing. Replace asserts with explicit checks that return false (or throw) on mismatch so callers receive accurate results. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [395-408] public boolean verifyManifestMatchesClusterState(ClusterMetadataManifest manifest, ClusterState clusterState) { - assert manifest != null : "ClusterMetadataManifest is null"; - assert clusterState != null : "ClusterState is null"; + if (manifest == null || clusterState == null) { + return false; + } List<IndexMetadata> indicesInManifest = indexFilter.filterIndexMetadata(clusterState.metadata().indices().values()); - assert indicesInManifest.size() == manifest.getIndices().size() - : "Number of indices in last accepted state and manifest are different"; - manifest.getIndices().forEach(md -> { - assert clusterState.metadata().indices().containsKey(md.getIndexName()) : "Last accepted state does not contain the index : " - + md.getIndexName(); - assert clusterState.metadata().indices().get(md.getIndexName()).getIndexUUID().equals(md.getIndexUUID()) - : "Last accepted state and manifest do not have same UUID for index : " + md.getIndexName(); - }); + if (indicesInManifest.size() != manifest.getIndices().size()) { + return false; + } + for (UploadedIndexMetadata md : manifest.getIndices()) { + IndexMetadata indexMetadata = clusterState.metadata().indices().get(md.getIndexName()); + if (indexMetadata == null || indexMetadata.getIndexUUID().equals(md.getIndexUUID()) == false) { + return false; + } + } return true; } Suggestion importance[1-10]: 3 __ Why: The original code in GatewayMetaState also used asserts and returned true, so this maintains existing behavior. The suggestion is a stylistic/defensive improvement but not strictly a bug fix since asserts are intentional here.	Low

---

Previous suggestions

Suggestions up to commit 910631b

Category	Suggestion	Impact
Possible issue	Allow cleanup of stale remote system indices When transitioning from a previous version where a system index was uploaded to remote state, indicesToBeDeletedFromRemote would contain it; the new code removes it from the delete list and skips upload, leaving stale system-index entries in remote blob storage forever. Consider letting system indices that exist in the previous remote state proceed to deletion (i.e., do not remove from indicesToBeDeletedFromRemote) so legacy remote-only system index uploads are cleaned up. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [460-465] for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { - indicesToBeDeletedFromRemote.remove(indexName); + // Allow previously-uploaded system indices to be deleted from remote; do not remove from indicesToBeDeletedFromRemote continue; } Suggestion importance[1-10]: 7 __ Why: This identifies a potentially valid concern: legacy remote-stored system indices may never be cleaned up due to the early remove from the deletion list, potentially leaving orphaned blob entries.	Medium
General	Warn when filtering system indices on read Filtering on the read path means that if a manifest was uploaded before this change (and still contains system indices), those system indices will be silently dropped during recovery. This may cause real data loss for clusters that had legitimately persisted system index metadata remotely. Consider either (a) only filtering on write and tolerating extra entries on read, or (b) logging a warning when a system index is filtered from a downloaded manifest so operators are aware. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [1615-1628] +List<UploadedIndexMetadata> filteredIndices = indexFilter.filterUploadedIndexMetadata(manifest.getIndices()); +if (filteredIndices.size() != manifest.getIndices().size()) { + logger.warn("Filtered out system indices from downloaded manifest for cluster UUID [{}]", manifest.getClusterUUID()); +} clusterState = readClusterStateInParallel( ClusterState.builder(new ClusterName(clusterName)).build(), manifest, manifest.getClusterUUID(), localNodeId, - indexFilter.filterUploadedIndexMetadata(manifest.getIndices()), + filteredIndices, manifest.getCustomMetadataMap(), - ... - includeEphemeral ? indexFilter.filterUploadedIndexMetadata(manifest.getIndicesRouting()) : emptyList(), Suggestion importance[1-10]: 5 __ Why: Adding a warning log for filtered system indices on read could help operators detect potential silent data loss from legacy manifests, but it's an enhancement rather than a critical fix.	Low
	Assertions disabled at runtime skip validation The method relies entirely on assert statements which are disabled at runtime in production JVMs (no -ea flag), so verifyManifestMatchesClusterState will silently always return true without performing any validation. Since this replaces inline asserts that were also runtime-disabled, the behavior matches the original, but for the verification to be meaningful, conditions should throw explicit exceptions (e.g. IllegalStateException) instead of relying on assertions. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [395-408] public boolean verifyManifestMatchesClusterState(ClusterMetadataManifest manifest, ClusterState clusterState) { - assert manifest != null : "ClusterMetadataManifest is null"; - assert clusterState != null : "ClusterState is null"; + if (manifest == null) throw new IllegalStateException("ClusterMetadataManifest is null"); + if (clusterState == null) throw new IllegalStateException("ClusterState is null"); List<IndexMetadata> indicesInManifest = indexFilter.filterIndexMetadata(clusterState.metadata().indices().values()); - assert indicesInManifest.size() == manifest.getIndices().size() - : "Number of indices in last accepted state and manifest are different"; + if (indicesInManifest.size() != manifest.getIndices().size()) { + throw new IllegalStateException("Number of indices in last accepted state and manifest are different"); + } manifest.getIndices().forEach(md -> { - assert clusterState.metadata().indices().containsKey(md.getIndexName()) : "Last accepted state does not contain the index : " - + md.getIndexName(); - assert clusterState.metadata().indices().get(md.getIndexName()).getIndexUUID().equals(md.getIndexUUID()) - : "Last accepted state and manifest do not have same UUID for index : " + md.getIndexName(); + if (!clusterState.metadata().indices().containsKey(md.getIndexName())) { + throw new IllegalStateException("Last accepted state does not contain the index : " + md.getIndexName()); + } + if (!clusterState.metadata().indices().get(md.getIndexName()).getIndexUUID().equals(md.getIndexUUID())) { + throw new IllegalStateException("Last accepted state and manifest do not have same UUID for index : " + md.getIndexName()); + } }); return true; } Suggestion importance[1-10]: 3 __ Why: The original inline assertions also relied on assert, so the refactor preserves existing behavior; converting to exceptions is a behavioral change that may not be desired and is outside the PR's scope.	Low
	Filter system indices before lookup call filterUploadedIndexMetadata early-returns the same list instance when empty and otherwise returns a stream-collected List, but the contract here may be a mutable list elsewhere. More importantly, applying the system-index filter after getUpdatedIndexRoutingTableMetadata is redundant if manifest.getIndicesRouting() was already filtered upstream during write — but on the read path, manifests from older versions may still contain system indices, so this is correct. However, consider also filtering diff.getIndicesRoutingUpdated() (the index name list) before passing it in, to avoid unnecessary work and possible NPEs inside getUpdatedIndexRoutingTableMetadata for unknown system indices. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [1713-1720] updatedIndexRouting.addAll( - indexFilter.filterUploadedIndexMetadata( - remoteRoutingTableService.getUpdatedIndexRoutingTableMetadata( - diff.getIndicesRoutingUpdated(), - manifest.getIndicesRouting() - ) + remoteRoutingTableService.getUpdatedIndexRoutingTableMetadata( + diff.getIndicesRoutingUpdated().stream().filter(indexFilter::includeIndex).collect(Collectors.toList()), + manifest.getIndicesRouting() ) ); Suggestion importance[1-10]: 3 __ Why: Minor optimization; the current code is correct since post-filtering achieves the same result. The benefit is marginal.	Low

Suggestions up to commit b1bc906

Category	Suggestion	Impact
Possible bug	Avoid filtering deletions for system indices The filter on deletedIndicesRouting will incorrectly drop system index names from the deletion list. If a system index was previously uploaded (e.g., before this filter existed or before becoming a system index) and now appears in the diff's deletes, removing it from deletedIndicesRouting will leave orphaned remote routing entries. Only the upload list should be filtered; deletions should still be processed. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [498-499] indicesRoutingToUpload.removeIf(indexRoutingTable -> indexFilter.includeIndex(indexRoutingTable.getIndex().getName()) == false); -deletedIndicesRouting.removeIf(indexName -> indexFilter.includeIndex(indexName) == false); Suggestion importance[1-10]: 7 __ Why: Valid concern: filtering deletedIndicesRouting could leave orphaned remote routing entries for indices that became system indices or were uploaded prior to this filter, though the practical likelihood depends on usage patterns.	Medium
	Allow cleanup of previously uploaded system indices Removing system indices from indicesToBeDeletedFromRemote here means that if an index existed in the previous cluster state as a non-system index (and was uploaded), but is now considered a system index, it will never be deleted from the remote store, creating orphaned data. Consider letting such indices remain queued for deletion. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [460-465] for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { - indicesToBeDeletedFromRemote.remove(indexName); continue; } Suggestion importance[1-10]: 7 __ Why: Reasonable edge case: an index transitioning from user to system could leave orphaned remote data. The suggestion addresses a real but uncommon scenario impacting cleanup correctness.	Medium
General	Avoid verification overhead in production The previous implementation was guarded by assert and was free in production builds. The new method always runs the filter and constructs a list even when assertions are disabled, since it is called from a regular if (not within an assert). Wrap the call in assert or guard the body to avoid runtime overhead on the hot publication path. server/src/main/java/org/opensearch/gateway/GatewayMetaState.java [805-807] private boolean verifyManifestAndClusterState(ClusterMetadataManifest manifest, ClusterState clusterState) { - return remoteClusterStateService.verifyManifestMatchesClusterState(manifest, clusterState); + assert remoteClusterStateService.verifyManifestMatchesClusterState(manifest, clusterState); + return true; } Suggestion importance[1-10]: 6 __ Why: Valid performance observation: the original code used assert statements which are eliminated in production, while the new code unconditionally runs filtering on the publication path, adding overhead.	Low

Suggestions up to commit e551f2f

Category	Suggestion	Impact
General	Allow cleanup of stale remote system indices When a previously non-system index becomes a system index (or a cluster is upgraded with this change while system indices already exist in the remote state), the corresponding entry in indicesToBeDeletedFromRemote is removed here, so the stale system-index metadata file in remote storage will never be cleaned up. Consider letting the deletion proceed (i.e., do not remove from indicesToBeDeletedFromRemote) so that the previously-uploaded system index metadata is removed from remote. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [462-465] for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { - indicesToBeDeletedFromRemote.remove(indexName); + // Leave any prior remote entry in indicesToBeDeletedFromRemote so it gets cleaned up. continue; } Suggestion importance[1-10]: 7 __ Why: Identifies a real upgrade-path concern: stale system-index metadata previously uploaded to remote storage would never be cleaned up because the entry is removed from indicesToBeDeletedFromRemote. This is a meaningful correctness issue worth addressing.	Medium
	Preserve deletion of legacy system index entries The indexFilter.filterUploadedIndexMetadata for updatedIndexRouting should be applied consistently with how the filter is used for getClusterStateForManifest. However, more importantly, diff.getIndicesRoutingDeleted() (if applicable) and diff.getIndicesDeleted() are not filtered against system indices. If a system index was previously uploaded (before this change) and then deleted, the deletion entry should still be processed; but the new code in writeIncrementalMetadata does deletedIndicesRouting.removeIf(indexName -> indexFilter.includeIndex(indexName) == false), which will skip cleaning up legacy system-index routing entries on upgraded clusters. Consider preserving deletion processing for previously-uploaded system indices to allow cleanup during the upgrade path. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [498-499] -List<UploadedIndexMetadata> updatedIndexRouting = new ArrayList<>(); -if (manifest.getCodecVersion() == CODEC_V2 || manifest.getCodecVersion() == CODEC_V3) { - updatedIndexRouting.addAll( - indexFilter.filterUploadedIndexMetadata( - remoteRoutingTableService.getUpdatedIndexRoutingTableMetadata( - diff.getIndicesRoutingUpdated(), - manifest.getIndicesRouting() - ) - ) - ); -} +// Do not filter out deletions of previously uploaded system indices, to allow cleanup on upgrade. +// indicesRoutingToUpload.removeIf already prevents new system-index uploads. +indicesRoutingToUpload.removeIf(indexRoutingTable -> indexFilter.includeIndex(indexRoutingTable.getIndex().getName()) == false); Suggestion importance[1-10]: 5 __ Why: Valid concern about cleanup of legacy system index routing entries on upgrade paths, but the improved_code doesn't actually modify the problematic line (deletedIndicesRouting.removeIf); it just repeats one of the existing lines, making the fix unclear.	Low
Possible issue	Handle null indices routing list manifest.getIndicesRouting() may return null for older codec versions, and filterUploadedIndexMetadata only short-circuits on isEmpty() but does not handle null, which would cause a NullPointerException. Add a null check in filterUploadedIndexMetadata (similar to filterIndexRoutingTables) or guard the call site. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [1628] -includeEphemeral ? indexFilter.filterUploadedIndexMetadata(manifest.getIndicesRouting()) : emptyList(), +includeEphemeral ? indexFilter.filterUploadedIndexMetadata(manifest.getIndicesRouting() == null ? emptyList() : manifest.getIndicesRouting()) : emptyList(), Suggestion importance[1-10]: 6 __ Why: Reasonable defensive null check, since filterUploadedIndexMetadata only checks isEmpty(). The impact depends on whether manifest.getIndicesRouting() can actually be null in this path, but it is a worthwhile robustness improvement.	Low

Suggestions up to commit a90841e

Category	Suggestion	Impact
Possible issue	Prevent assertion/NPE for filtered indices The diff's getIndicesUpdated() may contain index names that were filtered out of manifest.getIndices() if a previous manifest was written before this filter existed (or for system indices stored historically). The assert uploadedIndexMetadataOptional.isPresent() will fail / NPE in production. Filter out system indices before the lookup and tolerate missing entries. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [1668-1677] -List<UploadedIndexMetadata> updatedIndices = indexFilter.filterUploadedIndexMetadata( - diff.getIndicesUpdated().stream().map(idx -> { - Optional<UploadedIndexMetadata> uploadedIndexMetadataOptional = manifest.getIndices() - .stream() - .filter(idx2 -> idx2.getIndexName().equals(idx)) - .findFirst(); - assert uploadedIndexMetadataOptional.isPresent() == true; - return uploadedIndexMetadataOptional.get(); - }).collect(Collectors.toList()) -); +List<UploadedIndexMetadata> updatedIndices = diff.getIndicesUpdated().stream() + .filter(indexFilter::includeIndex) + .map(idx -> manifest.getIndices().stream().filter(idx2 -> idx2.getIndexName().equals(idx)).findFirst()) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toList()); Suggestion importance[1-10]: 6 __ Why: Valid concern about legacy manifests potentially containing system indices in getIndicesUpdated(). The current code applies the filter after the lookup, so the assertion would still fire before filtering. However, in practice getIndicesUpdated() comes from the diff and should align with manifest.getIndices(), making this an edge case.	Low
General	Allow cleanup of legacy system index entries deletedIndicesRouting may contain system indices that were previously uploaded to remote state before this filter was introduced; removing them here prevents cleanup of legacy system-index routing entries. Consider keeping deletions for previously uploaded system indices to allow cleanup, or only filter indicesRoutingToUpload. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [479-480] indicesRoutingToUpload.removeIf(indexRoutingTable -> indexFilter.includeIndex(indexRoutingTable.getIndex().getName()) == false); -deletedIndicesRouting.removeIf(indexName -> indexFilter.includeIndex(indexName) == false); Suggestion importance[1-10]: 5 __ Why: Reasonable point that filtering deletedIndicesRouting could prevent cleanup of pre-existing system index routing entries from older versions. The impact is moderate but the reasoning has merit for backward compatibility.	Low
	Do not skip cleanup of legacy entries Removing system indices from indicesToBeDeletedFromRemote will skip deletion of any pre-existing system index entries that were uploaded to remote state in older versions (before this filter). Leave such entries in the deletion set so legacy system-index metadata in remote state gets cleaned up on the next incremental write. server/src/main/java/org/opensearch/gateway/remote/RemoteClusterStateService.java [443-446] for (final IndexMetadata indexMetadata : clusterState.metadata().indices().values()) { String indexName = indexMetadata.getIndex().getName(); if (indexFilter.includeIndex(indexName) == false) { - indicesToBeDeletedFromRemote.remove(indexName); continue; } Suggestion importance[1-10]: 5 __ Why: Similar valid concern about cleanup of legacy system index metadata. Leaving them in indicesToBeDeletedFromRemote would allow cleanup, though the current behavior may be intentional to avoid disruption.	Low

[github-actions]

❌ Gradle check result for a90841e: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit e551f2f

[github-actions]

❌ Gradle check result for e551f2f: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit b1bc906

[github-actions]

Persistent review updated to latest commit 910631b

[github-actions]

Persistent review updated to latest commit ea66c85

[github-actions]

❌ Gradle check result for ea66c85: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?