fix(search): scope alias lookups to cluster prefix on shared OpenSearch clusters#27466
Merged
fix(search): scope alias lookups to cluster prefix on shared OpenSearch clusters#27466
Conversation
On shared OpenSearch/Elasticsearch clusters where tenant roles only
grant indices:admin/aliases/get on their own prefix, the orphaned
index cleanup and metrics refresh were failing with 403 Forbidden
because listIndicesByPrefix("") and getAllIndexStats() issued
unscoped GET /*/_alias and stats("*") requests.
Route both through a shared buildScopedPattern() that substitutes
{clusterAlias}_* when the caller passes an empty prefix and a
cluster alias is configured, so each deployment only reads its own
indices. Explicit non-empty prefixes are already cluster-qualified
by their callers and are left untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
bot
added
backend
safe to test
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes permission failures on shared OpenSearch/Elasticsearch clusters by scoping previously-unbounded alias and stats lookups to the deployment’s configured clusterAlias prefix, preventing GET /*/_alias and stats("*") from hitting indices outside the tenant’s allowed namespace.
Changes:
- Updated
listIndicesByPrefixto use a cluster-scoped pattern ({clusterAlias}_*) when called with an empty/null prefix and aclusterAliasis configured. - Updated
getAllIndexStatsto use the same scoped pattern instead of hard-coded"*". - Added unit tests (ES + OS) asserting the emitted alias lookup pattern for empty/null prefix with/without
clusterAlias.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchIndexManager.java | Introduces scoped pattern builder and applies it to alias listing + index stats retrieval. |
| openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchIndexManager.java | Mirrors scoped pattern behavior for Elasticsearch alias listing + index stats retrieval. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/opensearch/OpenSearchIndexManagerTest.java | Adds coverage to validate scoping behavior for empty/null prefixes. |
| openmetadata-service/src/test/java/org/openmetadata/service/search/elasticsearch/ElasticSearchIndexManagerTest.java | Adds coverage to validate scoping behavior for empty/null prefixes. |
Verifies that on a shared search cluster where the app is configured
with clusterAlias="openmetadata", the orphaned-index cleanup and
index-listing paths only read / touch indices matching
{clusterAlias}_*.
The test provisions a "foreign tenant" directly against the real
OpenSearch/Elasticsearch container by creating indices under a
different prefix (foreigntenant_it_orphans_*), then asserts:
1. listIndicesByPrefix("") never returns foreign-prefixed indices
2. getAllIndexStats() never returns foreign-prefixed indices
3. OrphanedIndexCleaner.cleanupOrphanedIndices() only deletes
orphans under the configured cluster prefix, leaving foreign
tenant indices (both orphaned and live) intact
Security plugin is disabled in the IT bootstrap, so the exact 403
cannot be reproduced — but the behavioral guarantee that prevents
it (never issuing unscoped GET /*/_alias) is verified here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tern Centralize the cluster-prefix separator so the scoped wildcard is built from the same constant used by getIndexName() / getAlias(). Addresses review feedback on #27466. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Use IndexMapping.INDEX_NAME_SEPARATOR in unit test assertions for parity with production code. - Rewrite the IT's cleanup test as a read-only discovery test via findOrphanedRebuildIndices(). cleanupOrphanedIndices() is a globally-scoped destructive op that could race with parallel ITs creating _rebuild_ indices under the same shared openmetadata_* namespace. Discovery-scope is the invariant that produces the 403 prevention; per-index deletion is already covered by unit tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@BeforeAll was calling createIndex() directly, which returns 400 if the index already exists from a prior failed run (or a re-run against a reused container). Delete first, then create, so setUp is safe to rerun. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code Review ✅ ApprovedScopes alias lookups to the cluster prefix on shared OpenSearch clusters to improve multi-tenancy isolation. No issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
pmbrull
approved these changes
Apr 17, 2026
Contributor
🔴 Playwright Results — 1 failure(s), 17 flaky✅ 3667 passed · ❌ 1 failed · 🟡 17 flaky · ⏭️ 89 skipped
Genuine Failures (failed on all attempts)❌
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
403 Forbiddenon shared OpenSearch/Elasticsearch clusters where tenant roles only grantindices:admin/aliases/geton their own prefix. The orphaned index cleanup and metrics refresh were issuing unscopedGET /*/_aliasandstats("*")calls that the tenant role is not allowed to perform.listIndicesByPrefixandgetAllIndexStatsthrough a sharedbuildScopedPattern()in bothOpenSearchIndexManagerandElasticSearchIndexManager. When the caller passes an empty/null prefix and aclusterAliasis configured, the request is scoped to{clusterAlias}_*so each deployment only reads its own indices. Explicit non-empty prefixes (already cluster-qualified by their callers) are left untouched; deployments with no cluster alias keep the existing*behavior.Context
Reported in open-metadata/openmetadata-collate#3557:
The unscoped calls come from:
OrphanedIndexCleaner.findAllRebuildIndices()→listIndicesByPrefix("")SearchIndexMetrics.countTotalIndices()→listIndicesByPrefix("")getAllIndexStats()→ hard-codedstats("*")Both managers already held a
clusterAliasfield (fromElasticSearchConfiguration.getClusterAlias()); this PR simply uses it for the two pattern builders that were ignoring it. Callers that already pass cluster-qualified prefixes (DefaultRecreateHandler,OpenMetadataOperations, internaladdAliasesInternal) are unaffected.Test plan
mvn test -Dtest='OpenSearchIndexManagerTest,ElasticSearchIndexManagerTest,OrphanedIndexCleanerTest,SearchIndexMetricsTest'→ 129/129 passtestListIndicesByPrefix_EmptyPrefixScopesToClusterAliasandtestListIndicesByPrefix_EmptyPrefixWithoutClusterAliasUsesWildcardfor both ES and OS managers, asserting the emittedGetAliasRequestindex pattern viaArgumentCaptormvn spotless:apply— no formatting drift{clusterAlias}_*indices🤖 Generated with Claude Code