Skip to content

Commit c11118a

Browse files
authored
Propagate original indices in NodeTermsEnumRequest (#78740)
This fix ensures that we provide the original list of indices as part of the node-level terms enum request. Closes #77508
1 parent 3d6f4ec commit c11118a

File tree

5 files changed

+138
-31
lines changed

5 files changed

+138
-31
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/NodeTermsEnumRequest.java

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
*/
77
package org.elasticsearch.xpack.core.termsenum.action;
88

9+
import org.elasticsearch.Version;
910
import org.elasticsearch.action.IndicesRequest;
11+
import org.elasticsearch.action.OriginalIndices;
1012
import org.elasticsearch.action.support.IndicesOptions;
1113
import org.elasticsearch.common.io.stream.StreamInput;
1214
import org.elasticsearch.common.io.stream.StreamOutput;
@@ -26,22 +28,26 @@
2628
*/
2729
public class NodeTermsEnumRequest extends TransportRequest implements IndicesRequest {
2830

29-
private String field;
30-
private String string;
31-
private String searchAfter;
32-
private long taskStartedTimeMillis;
33-
private long nodeStartedTimeMillis;
34-
private boolean caseInsensitive;
35-
private int size;
36-
private long timeout;
31+
private final String field;
32+
private final String string;
33+
private final String searchAfter;
34+
private final long taskStartedTimeMillis;
35+
private final boolean caseInsensitive;
36+
private final int size;
37+
private final long timeout;
3738
private final QueryBuilder indexFilter;
38-
private Set<ShardId> shardIds;
39-
private String nodeId;
39+
private final Set<ShardId> shardIds;
40+
private final String nodeId;
41+
private final OriginalIndices originalIndices;
42+
43+
private long nodeStartedTimeMillis;
4044

41-
public NodeTermsEnumRequest(final String nodeId,
45+
public NodeTermsEnumRequest(OriginalIndices originalIndices,
46+
final String nodeId,
4247
final Set<ShardId> shardIds,
4348
TermsEnumRequest request,
4449
long taskStartTimeMillis) {
50+
this.originalIndices = originalIndices;
4551
this.field = request.field();
4652
this.string = request.string();
4753
this.searchAfter = request.searchAfter();
@@ -70,6 +76,15 @@ public NodeTermsEnumRequest(StreamInput in) throws IOException {
7076
for (int i = 0; i < numShards; i++) {
7177
shardIds.add(new ShardId(in));
7278
}
79+
if (in.getVersion().onOrAfter(Version.V_7_15_1)) {
80+
originalIndices = OriginalIndices.readOriginalIndices(in);
81+
} else {
82+
String[] indicesNames = shardIds.stream()
83+
.map(ShardId::getIndexName)
84+
.distinct()
85+
.toArray(String[]::new);
86+
this.originalIndices = new OriginalIndices(indicesNames, null);
87+
}
7388
}
7489

7590
@Override
@@ -92,6 +107,9 @@ public void writeTo(StreamOutput out) throws IOException {
92107
for (ShardId shardId : shardIds) {
93108
shardId.writeTo(out);
94109
}
110+
if (out.getVersion().onOrAfter(Version.V_7_15_1)) {
111+
OriginalIndices.writeOriginalIndices(originalIndices, out);
112+
}
95113
}
96114

97115
public String field() {
@@ -152,16 +170,12 @@ public QueryBuilder indexFilter() {
152170

153171
@Override
154172
public String[] indices() {
155-
HashSet<String> indicesNames = new HashSet<>();
156-
for (ShardId shardId : shardIds) {
157-
indicesNames.add(shardId.getIndexName());
158-
}
159-
return indicesNames.toArray(new String[0]);
173+
return originalIndices.indices();
160174
}
161175

162176
@Override
163177
public IndicesOptions indicesOptions() {
164-
return null;
178+
return originalIndices.indicesOptions();
165179
}
166180

167181
public boolean remove(ShardId shardId) {

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/termsenum/action/TransportTermsEnumAction.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,8 @@ protected void doExecute(Task task, TermsEnumRequest request, ActionListener<Ter
136136
new AsyncBroadcastAction(task, request, listener).start();
137137
}
138138

139-
protected NodeTermsEnumRequest newNodeRequest(final String nodeId,
139+
protected NodeTermsEnumRequest newNodeRequest(final OriginalIndices originalIndices,
140+
final String nodeId,
140141
final Set<ShardId> shardIds,
141142
TermsEnumRequest request,
142143
long taskStartMillis) {
@@ -145,14 +146,14 @@ protected NodeTermsEnumRequest newNodeRequest(final String nodeId,
145146
// final ClusterState clusterState = clusterService.state();
146147
// final Set<String> indicesAndAliases = indexNameExpressionResolver.resolveExpressions(clusterState, request.indices());
147148
// final AliasFilter aliasFilter = searchService.buildAliasFilter(clusterState, shard.getIndexName(), indicesAndAliases);
148-
return new NodeTermsEnumRequest(nodeId, shardIds, request, taskStartMillis);
149+
return new NodeTermsEnumRequest(originalIndices, nodeId, shardIds, request, taskStartMillis);
149150
}
150151

151152
protected NodeTermsEnumResponse readShardResponse(StreamInput in) throws IOException {
152153
return new NodeTermsEnumResponse(in);
153154
}
154155

155-
protected Map<String, Set<ShardId>> getNodeBundles(ClusterState clusterState, TermsEnumRequest request, String[] concreteIndices) {
156+
protected Map<String, Set<ShardId>> getNodeBundles(ClusterState clusterState, String[] concreteIndices) {
156157
// Group targeted shards by nodeId
157158
Map<String, Set<ShardId>> fastNodeBundles = new HashMap<>();
158159
for (String indexName : concreteIndices) {
@@ -162,9 +163,7 @@ protected Map<String, Set<ShardId>> getNodeBundles(ClusterState clusterState, Te
162163
GroupShardsIterator<ShardIterator> shards = clusterService.operationRouting()
163164
.searchShards(clusterState, singleIndex, null, null);
164165

165-
Iterator<ShardIterator> shardsForIndex = shards.iterator();
166-
while (shardsForIndex.hasNext()) {
167-
ShardIterator copiesOfShard = shardsForIndex.next();
166+
for (ShardIterator copiesOfShard : shards) {
168167
ShardRouting selectedCopyOfShard = null;
169168
for (ShardRouting copy : copiesOfShard) {
170169
// Pick the first active node with a copy of the shard
@@ -177,7 +176,7 @@ protected Map<String, Set<ShardId>> getNodeBundles(ClusterState clusterState, Te
177176
break;
178177
}
179178
String nodeId = selectedCopyOfShard.currentNodeId();
180-
Set<ShardId> bundle = null;
179+
final Set<ShardId> bundle;
181180
if (fastNodeBundles.containsKey(nodeId)) {
182181
bundle = fastNodeBundles.get(nodeId);
183182
} else {
@@ -388,7 +387,7 @@ protected NodeTermsEnumResponse dataNodeOperation(NodeTermsEnumRequest request,
388387
if (termsList.size() >= shard_size) {
389388
break;
390389
}
391-
};
390+
}
392391

393392
} catch (Exception e) {
394393
error = ExceptionsHelper.stackTrace(e);
@@ -415,7 +414,7 @@ private boolean canAccess(
415414

416415
if (indexAccessControl != null) {
417416
final boolean dls = indexAccessControl.getDocumentPermissions().hasDocumentLevelPermissions();
418-
if ( dls && licenseChecker.get()) {
417+
if (dls && licenseChecker.get()) {
419418
// Check to see if any of the roles defined for the current user rewrite to match_all
420419

421420
SecurityContext securityContext = new SecurityContext(clusterService.getSettings(), threadContext);
@@ -467,20 +466,20 @@ protected class AsyncBroadcastAction {
467466
private final Task task;
468467
private final TermsEnumRequest request;
469468
private ActionListener<TermsEnumResponse> listener;
470-
private final ClusterState clusterState;
471469
private final DiscoveryNodes nodes;
472470
private final int expectedOps;
473471
private final AtomicInteger counterOps = new AtomicInteger();
474472
private final AtomicReferenceArray<Object> atomicResponses;
475473
private final Map<String, Set<ShardId>> nodeBundles;
474+
private final OriginalIndices localIndices;
476475
private final Map<String, OriginalIndices> remoteClusterIndices;
477476

478477
protected AsyncBroadcastAction(Task task, TermsEnumRequest request, ActionListener<TermsEnumResponse> listener) {
479478
this.task = task;
480479
this.request = request;
481480
this.listener = listener;
482481

483-
clusterState = clusterService.state();
482+
ClusterState clusterState = clusterService.state();
484483

485484
ClusterBlockException blockException = checkGlobalBlock(clusterState, request);
486485
if (blockException != null) {
@@ -489,7 +488,7 @@ protected AsyncBroadcastAction(Task task, TermsEnumRequest request, ActionListen
489488

490489
this.remoteClusterIndices = remoteClusterService.groupIndices(request.indicesOptions(), request.indices(),
491490
idx -> indexNameExpressionResolver.hasIndexAbstraction(idx, clusterState));
492-
OriginalIndices localIndices = remoteClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
491+
this.localIndices = remoteClusterIndices.remove(RemoteClusterAware.LOCAL_CLUSTER_GROUP_KEY);
493492

494493
// update to concrete indices
495494
String[] concreteIndices = localIndices == null ? new String[0] :
@@ -501,7 +500,7 @@ protected AsyncBroadcastAction(Task task, TermsEnumRequest request, ActionListen
501500

502501
nodes = clusterState.nodes();
503502
logger.trace("resolving shards based on cluster state version [{}]", clusterState.version());
504-
nodeBundles = getNodeBundles(clusterState, request, concreteIndices);
503+
nodeBundles = getNodeBundles(clusterState, concreteIndices);
505504
expectedOps = nodeBundles.size() + remoteClusterIndices.size();
506505

507506
atomicResponses = new AtomicReferenceArray<>(expectedOps);
@@ -556,7 +555,7 @@ protected void performOperation(final String nodeId, final Set<ShardId> shardIds
556555
onNodeFailure(nodeId, opsIndex, null);
557556
} else {
558557
try {
559-
final NodeTermsEnumRequest nodeRequest = newNodeRequest(nodeId, shardIds, request, task.getStartTime());
558+
final NodeTermsEnumRequest nodeRequest = newNodeRequest(localIndices, nodeId, shardIds, request, task.getStartTime());
560559
nodeRequest.setParentTask(clusterService.localNode().getId(), task.getId());
561560
DiscoveryNode node = nodes.get(nodeId);
562561
if (node == null) {

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/terms_enum/10_basic.yml

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ setup:
3838
]
3939
}
4040
41+
- do:
42+
security.put_role:
43+
name: "dls_alias_role"
44+
body: >
45+
{
46+
"indices": [
47+
{ "names": ["alias_security"], "privileges": ["read"], "query": "{\"term\": {\"ck\": \"const\"}}" }
48+
]
49+
}
50+
4151
- do:
4252
security.put_role:
4353
name: "dls_none_role"
@@ -57,6 +67,16 @@ setup:
5767
"full_name" : "user with access to all docs in test_security index (using DLS)"
5868
}
5969
70+
- do:
71+
security.put_user:
72+
username: "dls_alias_user"
73+
body: >
74+
{
75+
"password" : "x-pack-test-password",
76+
"roles" : [ "dls_alias_role" ],
77+
"full_name" : "user with access to all docs in test_security index (using DLS)"
78+
}
79+
6080
- do:
6181
security.put_role:
6282
name: "dls_some_role"
@@ -143,6 +163,8 @@ setup:
143163
indices.create:
144164
index: test_security
145165
body:
166+
aliases:
167+
alias_security: {}
146168
settings:
147169
index:
148170
number_of_shards: 1
@@ -198,6 +220,16 @@ teardown:
198220
security.delete_role:
199221
name: "dls_all_role"
200222
ignore: 404
223+
224+
- do:
225+
security.delete_user:
226+
username: "dls_alias_user"
227+
ignore: 404
228+
229+
- do:
230+
security.delete_role:
231+
name: "dls_alias_role"
232+
ignore: 404
201233
- do:
202234
security.delete_role:
203235
name: "dls_none_role"
@@ -285,6 +317,7 @@ teardown:
285317
index: test_k
286318
body: {"field": "foo"}
287319
- length: {terms: 1}
320+
288321
---
289322
"Test search after keyword field":
290323
- do:
@@ -385,6 +418,7 @@ teardown:
385418
terms_enum:
386419
index: test_*
387420
body: {"field": "foo", "string":"b", "timeout": "2m"}
421+
388422
---
389423
"Test security":
390424

@@ -402,6 +436,13 @@ teardown:
402436
body: {"field": "foo", "string":"b"}
403437
- length: {terms: 1}
404438

439+
- do:
440+
headers: { Authorization: "Basic ZGxzX2FsaWFzX3VzZXI6eC1wYWNrLXRlc3QtcGFzc3dvcmQ=" } # dls_alias_user sees all docs through the alias
441+
terms_enum:
442+
index: alias_security
443+
body: { "field": "foo", "string": "b" }
444+
- length: { terms: 1 }
445+
405446
- do:
406447
headers: { Authorization: "Basic ZGxzX3NvbWVfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" } # dls_some_user sees selected docs
407448
terms_enum:

x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/multi_cluster/120_terms_enum.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,20 @@ setup:
2121
]
2222
}
2323
24+
- do:
25+
security.put_role:
26+
name: "terms_enum_alias_role"
27+
body: >
28+
{
29+
"cluster": ["all"],
30+
"indices": [
31+
{
32+
"names": ["my_remote_cluster:terms_enum_alias"],
33+
"privileges": ["read"]
34+
}
35+
]
36+
}
37+
2438
- do:
2539
security.put_user:
2640
username: "joe_all"
@@ -30,6 +44,15 @@ setup:
3044
"roles" : [ "terms_enum_all_role" ]
3145
}
3246
47+
- do:
48+
security.put_user:
49+
username: "joe_alias"
50+
body: >
51+
{
52+
"password": "s3krit-password",
53+
"roles" : [ "terms_enum_alias_role" ]
54+
}
55+
3356
- do:
3457
security.put_role:
3558
name: "terms_enum_none_role"
@@ -82,6 +105,10 @@ teardown:
82105
security.delete_user:
83106
username: "joe_all"
84107
ignore: 404
108+
- do:
109+
security.delete_user:
110+
username: "joe_alias"
111+
ignore: 404
85112
- do:
86113
security.delete_user:
87114
username: "joe_none"
@@ -94,6 +121,10 @@ teardown:
94121
security.delete_role:
95122
name: "terms_enum_all_role"
96123
ignore: 404
124+
- do:
125+
security.delete_role:
126+
name: "terms_enum_alias_role"
127+
ignore: 404
97128
- do:
98129
security.delete_role:
99130
name: "terms_enum_none_role"
@@ -123,6 +154,15 @@ teardown:
123154
- match: { terms.0: "zar" }
124155
- match: { complete: true }
125156

157+
- do:
158+
headers: { Authorization: "Basic am9lX2FsaWFzOnMza3JpdC1wYXNzd29yZA==" } # joe_alias can see all docs through alias
159+
terms_enum:
160+
index: my_remote_cluster:terms_enum_alias
161+
body: { "field": "foo", "search_after": "foobar" }
162+
- length: { terms: 1 }
163+
- match: { terms.0: "zar" }
164+
- match: { complete: true }
165+
126166
- do:
127167
headers: { Authorization: "Basic am9lX25vbmU6czNrcml0LXBhc3N3b3Jk" } # joe_none can't see docs
128168
terms_enum:

x-pack/qa/multi-cluster-search-security/src/test/resources/rest-api-spec/test/remote_cluster/10_basic.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,17 @@ setup:
7979
]
8080
}
8181
82+
- do:
83+
security.put_role:
84+
name: "terms_enum_alias_role"
85+
body: >
86+
{
87+
"cluster": ["monitor"],
88+
"indices": [
89+
{ "names": ["terms_enum_alias"], "privileges": ["read"], "query": "{\"term\": {\"ck\": \"const\"}}" }
90+
]
91+
}
92+
8293
- do:
8394
security.put_role:
8495
name: "terms_enum_none_role"
@@ -373,6 +384,8 @@ setup:
373384
indices.create:
374385
index: terms_enum_index
375386
body:
387+
aliases:
388+
terms_enum_alias: {}
376389
settings:
377390
index:
378391
number_of_shards: 1

0 commit comments

Comments
 (0)