Skip to content

Commit adda5de

Browse files
committed
Use GetMappingsRequest instead of GetIndexRequest in QueryPlanningTool
QueryPlanningTool only needs index mappings but was using GetIndexRequest which requires the broader indices:admin/get permission. This change switches to GetMappingsRequest which only requires indices:admin/mappings/get, following the principle of least privilege and aligning with how other tools (knn, query_assistant) request mappings. Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
1 parent 3b174ad commit adda5de

File tree

2 files changed

+28
-30
lines changed

2 files changed

+28
-30
lines changed

ml-algorithms/src/main/java/org/opensearch/ml/engine/tools/QueryPlanningTool.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828

2929
import org.apache.commons.text.StringSubstitutor;
3030
import org.opensearch.action.admin.cluster.storedscripts.GetStoredScriptRequest;
31-
import org.opensearch.action.admin.indices.get.GetIndexRequest;
32-
import org.opensearch.action.admin.indices.get.GetIndexResponse;
31+
import org.opensearch.action.admin.indices.mapping.get.GetMappingsRequest;
32+
import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse;
3333
import org.opensearch.action.search.SearchRequest;
3434
import org.opensearch.action.search.SearchResponse;
3535
import org.opensearch.action.support.IndicesOptions;
@@ -363,20 +363,19 @@ public void onFailure(Exception e) {
363363

364364
private void getIndexMappingAsync(String indexName, ActionListener<String> listener) {
365365
try {
366-
GetIndexRequest getIndexRequest = new GetIndexRequest()
367-
.indices(indexName)
366+
GetMappingsRequest getMappingsRequest = new GetMappingsRequest().indices(indexName)
368367
.indicesOptions(IndicesOptions.strictExpand())
369368
.local(false)
370369
.clusterManagerNodeTimeout(DEFAULT_CLUSTER_MANAGER_NODE_TIMEOUT);
371370

372-
client.admin().indices().getIndex(getIndexRequest, new ActionListener<GetIndexResponse>() {
371+
client.admin().indices().getMappings(getMappingsRequest, new ActionListener<GetMappingsResponse>() {
373372
@Override
374-
public void onResponse(GetIndexResponse getIndexResponse) {
373+
public void onResponse(GetMappingsResponse getMappingsResponse) {
375374
try {
376375
// When indexName is a wildcard pattern or alias, the response keys are
377376
// concrete index names, not the original pattern/alias. Pick the first
378377
// mapping since indices matching a pattern/alias generally share the same mapping.
379-
Map<String, MappingMetadata> mappings = getIndexResponse.mappings();
378+
Map<String, MappingMetadata> mappings = getMappingsResponse.mappings();
380379
if (mappings == null || mappings.isEmpty()) {
381380
listener
382381
.onFailure(

ml-algorithms/src/test/java/org/opensearch/ml/engine/tools/QueryPlanningToolTests.java

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
import org.mockito.Mock;
5858
import org.mockito.MockitoAnnotations;
5959
import org.opensearch.action.admin.cluster.storedscripts.GetStoredScriptResponse;
60-
import org.opensearch.action.admin.indices.get.GetIndexResponse;
60+
import org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse;
6161
import org.opensearch.action.search.SearchResponse;
6262
import org.opensearch.cluster.metadata.MappingMetadata;
6363
import org.opensearch.common.xcontent.XContentHelper;
@@ -105,8 +105,8 @@ public class QueryPlanningToolTests {
105105
private QueryPlanningTool.Factory factory;
106106

107107
// Common test objects
108-
private ArgumentCaptor<ActionListener<GetIndexResponse>> actionListenerCaptor;
109-
private GetIndexResponse getIndexResponse;
108+
private ArgumentCaptor<ActionListener<GetMappingsResponse>> actionListenerCaptor;
109+
private GetMappingsResponse getIndexResponse;
110110
private MappingMetadata mapping;
111111
private String mockedSearchResponseString;
112112

@@ -157,13 +157,12 @@ public void mockSampleDoc() throws IOException {
157157

158158
public void mockGetIndexMapping() {
159159
// Mock the getIndex operation for getIndexMappingAsync (following IndexMappingToolTests pattern)
160-
ArgumentCaptor<ActionListener<GetIndexResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
160+
ArgumentCaptor<ActionListener<GetMappingsResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
161161
this.actionListenerCaptor = captor;
162-
doNothing().when(indicesAdminClient).getIndex(any(), actionListenerCaptor.capture());
162+
doNothing().when(indicesAdminClient).getMappings(any(), actionListenerCaptor.capture());
163163

164-
// Create a real GetIndexResponse with real MappingMetadata
165-
this.getIndexResponse = mock(GetIndexResponse.class);
166-
when(getIndexResponse.indices()).thenReturn(new String[] { "testIndex" });
164+
// Create a real GetMappingsResponse with real MappingMetadata
165+
this.getIndexResponse = mock(GetMappingsResponse.class);
167166

168167
// Create real MappingMetadata with actual source
169168
String mappingSource = "{\"properties\":{\"title\":{\"type\":\"text\"}}}";
@@ -887,11 +886,11 @@ public void testCreateWithExtraFieldsInSearchTemplates() {
887886
public void testRunWithNonExistentIndex() throws InterruptedException {
888887

889888
doAnswer(invocation -> {
890-
org.opensearch.core.action.ActionListener<org.opensearch.action.admin.indices.get.GetIndexResponse> listener = invocation
889+
org.opensearch.core.action.ActionListener<org.opensearch.action.admin.indices.mapping.get.GetMappingsResponse> listener = invocation
891890
.getArgument(1);
892891
listener.onFailure(new org.opensearch.index.IndexNotFoundException("non_existent_index"));
893892
return null;
894-
}).when(indicesAdminClient).getIndex(any(), any());
893+
}).when(indicesAdminClient).getMappings(any(), any());
895894

896895
QueryPlanningTool tool = new QueryPlanningTool("llmGenerated", queryGenerationTool, client, null, null);
897896
final CompletableFuture<String> future = new CompletableFuture<>();
@@ -1083,19 +1082,19 @@ public void testGetIndexMapping_ErrorPaths() throws ExecutionException, Interrup
10831082
}).when(queryGenerationTool).run(any(), any());
10841083

10851084
// Case 1: onResponse mapping null -> NPE inside try caught
1086-
ArgumentCaptor<ActionListener<GetIndexResponse>> captor1 = ArgumentCaptor.forClass(ActionListener.class);
1085+
ArgumentCaptor<ActionListener<GetMappingsResponse>> captor1 = ArgumentCaptor.forClass(ActionListener.class);
10871086
doAnswer(invocation -> {
10881087
// capture and call onResponse with a response missing mapping
1089-
ActionListener<GetIndexResponse> al = invocation.getArgument(1);
1088+
ActionListener<GetMappingsResponse> al = invocation.getArgument(1);
10901089
if (actionListenerCaptor == null) {
10911090
actionListenerCaptor = captor1;
10921091
}
10931092
al.onResponse(getIndexResponse); // getIndexResponse will be mocked below
10941093
return null;
1095-
}).when(indicesAdminClient).getIndex(any(), any());
1094+
}).when(indicesAdminClient).getMappings(any(), any());
10961095

10971096
// Mock getIndexResponse to return no mapping for index
1098-
getIndexResponse = mock(GetIndexResponse.class);
1097+
getIndexResponse = mock(GetMappingsResponse.class);
10991098
when(getIndexResponse.mappings()).thenReturn(Collections.emptyMap());
11001099

11011100
CompletableFuture<String> f1 = new CompletableFuture<>();
@@ -1110,10 +1109,10 @@ public void testGetIndexMapping_ErrorPaths() throws ExecutionException, Interrup
11101109

11111110
// Case 2: onFailure path with generic exception (non IndexNotFoundException)
11121111
doAnswer(invocation -> {
1113-
ActionListener<GetIndexResponse> al = invocation.getArgument(1);
1112+
ActionListener<GetMappingsResponse> al = invocation.getArgument(1);
11141113
al.onFailure(new RuntimeException("boom"));
11151114
return null;
1116-
}).when(indicesAdminClient).getIndex(any(), any());
1115+
}).when(indicesAdminClient).getMappings(any(), any());
11171116

11181117
CompletableFuture<String> f2 = new CompletableFuture<>();
11191118
ActionListener<String> l2 = ActionListener.wrap(f2::complete, f2::completeExceptionally);
@@ -1126,7 +1125,7 @@ public void testGetIndexMapping_ErrorPaths() throws ExecutionException, Interrup
11261125
assertTrue(ix2.getCause().getMessage().contains("Failed to extract index mapping"));
11271126

11281127
// Case 3: outer try-catch (getIndex throws synchronously)
1129-
doAnswer(invocation -> { throw new RuntimeException("sync"); }).when(indicesAdminClient).getIndex(any(), any());
1128+
doAnswer(invocation -> { throw new RuntimeException("sync"); }).when(indicesAdminClient).getMappings(any(), any());
11301129

11311130
CompletableFuture<String> f3 = new CompletableFuture<>();
11321131
ActionListener<String> l3 = ActionListener.wrap(f3::complete, f3::completeExceptionally);
@@ -1511,11 +1510,11 @@ public void testRunWithAliasResolvingToMultipleIndices() throws ExecutionExcepti
15111510
mockSampleDoc();
15121511

15131512
// Mock getIndex to return mappings keyed by multiple concrete indices (simulating alias resolution)
1514-
ArgumentCaptor<ActionListener<GetIndexResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
1515-
doNothing().when(indicesAdminClient).getIndex(any(), captor.capture());
1513+
ArgumentCaptor<ActionListener<GetMappingsResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
1514+
doNothing().when(indicesAdminClient).getMappings(any(), captor.capture());
15161515
this.actionListenerCaptor = captor;
15171516

1518-
GetIndexResponse multiIndexResponse = mock(GetIndexResponse.class);
1517+
GetMappingsResponse multiIndexResponse = mock(GetMappingsResponse.class);
15191518
String mappingSource = "{\"properties\":{\"title\":{\"type\":\"text\"}}}";
15201519
MappingMetadata mapping1 = new MappingMetadata(
15211520
"logs_march",
@@ -1557,10 +1556,10 @@ public void testRunWithAliasResolvingToMultipleIndices() throws ExecutionExcepti
15571556

15581557
@Test
15591558
public void testGetIndexMapping_NullMappingsFails() throws ExecutionException, InterruptedException {
1560-
ArgumentCaptor<ActionListener<GetIndexResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
1561-
doNothing().when(indicesAdminClient).getIndex(any(), captor.capture());
1559+
ArgumentCaptor<ActionListener<GetMappingsResponse>> captor = ArgumentCaptor.forClass(ActionListener.class);
1560+
doNothing().when(indicesAdminClient).getMappings(any(), captor.capture());
15621561

1563-
GetIndexResponse response = mock(GetIndexResponse.class);
1562+
GetMappingsResponse response = mock(GetMappingsResponse.class);
15641563
when(response.mappings()).thenReturn(null);
15651564

15661565
QueryPlanningTool tool = new QueryPlanningTool(LLM_GENERATED_TYPE_FIELD, queryGenerationTool, client, null, null);

0 commit comments

Comments
 (0)