Use GetMappingsRequest instead of GetIndexRequest in QueryPlanningTool

[owaiskazi19]

Description

QueryPlanningTool uses GetIndexRequest to retrieve index mappings for query planning, but it only accesses .mappings() from the response. GetIndexRequest requires the indices:admin/get permission, which also exposes index settings, aliases, and defaults — none of which QPT uses.

This PR switches to GetMappingsRequest / GetMappingsResponse, which only requires indices:admin/mappings/get. This:

• Follows the principle of least privilege
• Aligns with how knn_full_access and query_assistant_access roles already handle mapping retrieval
• Enables the ml_full_access role to support Agentic Search (QueryPlanningTool) with the narrower indices:admin/mappings/get permission instead of the broader indices:admin/get

Related Issue

Resolves #4775

Changes

QueryPlanningTool.java:

• GetIndexRequest → GetMappingsRequest
• GetIndexResponse → GetMappingsResponse
• client.admin().indices().getIndex() → client.admin().indices().getMappings()
• Response handling unchanged — .mappings() returns the same Map<String, MappingMetadata>

QueryPlanningToolTests.java:

• All mocks updated to use GetMappingsResponse
• All indicesAdminClient.getIndex() → indicesAdminClient.getMappings()

Check List

• New functionality includes testing
• New functionality has been documented
  • N/A (internal API change, no user-facing doc change)
• Commits are signed off as per the DCO using --signoff

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 00b97d2)

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 Stale Variable Name The field getIndexResponse in the test class was renamed from GetIndexResponse type to GetMappingsResponse type, but the variable name getIndexResponse was kept unchanged. This is misleading and could cause confusion during maintenance. It should be renamed to getMappingsResponse or similar to reflect the new type. private GetMappingsResponse getIndexResponse; Stale Comment The comment on line 1512 still says "Mock getIndex to return mappings keyed by multiple concrete indices" — it references the old getIndex method. It should be updated to reference getMappings for accuracy. // Mock getIndex to return mappings keyed by multiple concrete indices (simulating alias resolution)

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 00b97d2

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Add missing mappings stub on mocked response After removing the when(getIndexResponse.indices()).thenReturn(...) stub, the mock for getIndexResponse.mappings() is never set up in mockGetIndexMapping(). Tests that call mockGetIndexMapping() and then invoke actionListenerCaptor.getValue().onResponse(getIndexResponse) will receive null from getMappingsResponse.mappings(), causing unexpected IllegalStateException failures instead of exercising the happy path. Add a when(getIndexResponse.mappings()).thenReturn(...) stub with the expected mapping data. ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/QueryPlanningToolTests.java [165-169] this.getIndexResponse = mock(GetMappingsResponse.class); // Create real MappingMetadata with actual source String mappingSource = "{\"properties\":{\"title\":{\"type\":\"text\"}}}"; this.mapping = new MappingMetadata("testIndex", XContentHelper.convertToMap(JsonXContent.jsonXContent, mappingSource, true)); + Map<String, MappingMetadata> mappingsMap = new HashMap<>(); + mappingsMap.put("testIndex", this.mapping); + when(getIndexResponse.mappings()).thenReturn(mappingsMap); Suggestion importance[1-10]: 8 __ Why: The old code had when(getIndexResponse.indices()).thenReturn(new String[] { "testIndex" }) which was removed, but no when(getIndexResponse.mappings()).thenReturn(...) stub was added in mockGetIndexMapping(). Without this stub, tests using mockGetIndexMapping() that call onResponse(getIndexResponse) will get null from getMappingsResponse.mappings(), causing unexpected IllegalStateException failures in happy-path tests.	Medium

---

Previous suggestions

Suggestions up to commit 702c3c0

Category	Suggestion	Impact
Possible issue	Remove unsupported method call on new request type GetMappingsRequest does not have a local() method — that method belongs to GetIndexRequest. Calling a non-existent method will cause a compilation error. Remove the .local(false) call since GetMappingsRequest does not support it. ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java [366-370] GetMappingsRequest getMappingsRequest = new GetMappingsRequest() .indices(indexName) .indicesOptions(IndicesOptions.strictExpand()) - .local(false) .clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); Suggestion importance[1-10]: 8 __ Why: GetMappingsRequest does not have a .local(false) method (that belongs to GetIndexRequest), so this would cause a compilation error. Removing it is necessary for the code to compile correctly.	Medium

Suggestions up to commit 16120a8

Category	Suggestion	Impact
Possible issue	Remove unsupported method call on new request type GetMappingsRequest does not have a local(boolean) method — this method exists on GetIndexRequest but not on GetMappingsRequest. This will cause a compilation error. Remove the .local(false) call since it is not supported by GetMappingsRequest. ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java [366-370] GetMappingsRequest getMappingsRequest = new GetMappingsRequest() .indices(indexName) .indicesOptions(IndicesOptions.strictExpand()) - .local(false) .clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); Suggestion importance[1-10]: 8 __ Why: GetMappingsRequest does not have a .local(boolean) method like GetIndexRequest does, so this would cause a compilation error. Removing the unsupported .local(false) call is necessary for the code to compile correctly.	Medium

Suggestions up to commit adda5de

Category	Suggestion	Impact
Possible issue	Remove unsupported method calls on new request type GetMappingsRequest does not have a local() method or a clusterManagerNodeTimeout() method in the same way GetIndexRequest does. These methods may not exist on GetMappingsRequest, which would cause a compilation error. Verify that these methods are available on GetMappingsRequest and remove or replace any unsupported method calls. ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java [366-369] GetMappingsRequest getMappingsRequest = new GetMappingsRequest().indices(indexName) - .indicesOptions(IndicesOptions.strictExpand()) - .local(false) - .clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT); + .indicesOptions(IndicesOptions.strictExpand()); Suggestion importance[1-10]: 7 __ Why: GetMappingsRequest may not support .local() and .clusterManagerNodeTimeout() methods unlike GetIndexRequest, which could cause a compilation error. This is a potentially valid concern about API compatibility that warrants verification.	Medium

[github-actions]

Persistent review updated to latest commit 16120a8

[github-actions]

Persistent review updated to latest commit 702c3c0

[rithinpullela]

Thanks for the PR Owais!

[github-actions]

Persistent review updated to latest commit 00b97d2

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.41%. Comparing base (3b174ad) to head (00b97d2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #4785   +/-   ##
=========================================
  Coverage     77.41%   77.41%           
- Complexity    11900    11901    +1     
=========================================
  Files           963      963           
  Lines         53310    53309    -1     
  Branches       6500     6500           
=========================================
+ Hits          41268    41269    +1     
- Misses         9287     9289    +2     
+ Partials       2755     2751    -4     

Flag	Coverage Δ
ml-commons	77.41% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.