Skip to content

Commit c9f72a0

Browse files
committed
Fix shard follow task cleaner under security (elastic#52347)
The shard follow task cleaner executes on behalf of the user to clean up a shard follow task after the follower index has been deleted. Otherwise, these persistent tasks are left laying around, and they fail to execute because the follower index has been deleted. In the face of security, attempts to complete these persistent tasks would fail. This is because these cleanups are executed under the system context (this makes sense, they are happening on behalf of the user after the user has executed an action) but the system role was never granted the permission for persistent task completion. This commit addresses this by adding this cluster privilege to the system role.
1 parent 012420a commit c9f72a0

File tree

5 files changed

+44
-3
lines changed

5 files changed

+44
-3
lines changed

x-pack/plugin/ccr/qa/security/follower-roles.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,10 @@ ccruser:
88
- read
99
- write
1010
- manage_follow_index
11+
- names: [ 'clean-follower' ]
12+
privileges:
13+
- monitor
14+
- read
15+
- write
16+
- delete_index
17+
- manage_follow_index

x-pack/plugin/ccr/qa/security/leader-roles.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ ccruser:
22
cluster:
33
- read_ccr
44
indices:
5-
- names: [ 'allowed-index', 'forget-leader', 'logs-eu-*' ]
5+
- names: [ 'allowed-index', 'clean-leader', 'forget-leader', 'logs-eu-*' ]
66
privileges:
77
- monitor
88
- read

x-pack/plugin/ccr/qa/security/src/test/java/org/elasticsearch/xpack/ccr/FollowIndexSecurityIT.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,32 @@ public void testForgetFollower() throws IOException {
225225
}
226226
}
227227

228+
public void testCleanShardFollowTaskAfterDeleteFollower() throws Exception {
229+
final String cleanLeader = "clean-leader";
230+
final String cleanFollower = "clean-follower";
231+
if ("leader".equals(targetCluster)) {
232+
logger.info("running against leader cluster");
233+
final Settings indexSettings = Settings.builder()
234+
.put("index.number_of_replicas", 0)
235+
.put("index.number_of_shards", 1)
236+
.put("index.soft_deletes.enabled", true)
237+
.build();
238+
createIndex(cleanLeader, indexSettings);
239+
} else {
240+
logger.info("running against follower cluster");
241+
followIndex(client(), "leader_cluster", cleanLeader, cleanFollower);
242+
243+
final Request request = new Request("DELETE", "/" + cleanFollower);
244+
final Response response = client().performRequest(request);
245+
assertOK(response);
246+
// the shard follow task should have been cleaned up on behalf of the user, see ShardFollowTaskCleaner
247+
assertBusy(() -> {
248+
Map<String, Object> clusterState = toMap(adminClient().performRequest(new Request("GET", "/_cluster/state")));
249+
List<?> tasks = (List<?>) XContentMapValues.extractValue("metadata.persistent_tasks.tasks", clusterState);
250+
assertThat(tasks.size(), equalTo(0));
251+
assertThat(countCcrNodeTasks(), equalTo(0));
252+
});
253+
}
254+
}
255+
228256
}

x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/ShardFollowTaskCleaner.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,12 @@ public void clusterChanged(final ClusterChangedEvent event) {
6868
CompletionPersistentTaskAction.Request request =
6969
new CompletionPersistentTaskAction.Request(persistentTask.getId(), persistentTask.getAllocationId(), infe);
7070
threadPool.generic().submit(() -> {
71+
/*
72+
* We are executing under the system context, on behalf of the user to clean up the shard follow task after the follower
73+
* index was deleted. This is why the system role includes the privilege for persistent task completion.
74+
*/
75+
assert threadPool.getThreadContext().isSystemContext();
7176
client.execute(CompletionPersistentTaskAction.INSTANCE, request, new ActionListener<PersistentTaskResponse>() {
72-
7377
@Override
7478
public void onResponse(PersistentTaskResponse persistentTaskResponse) {
7579
logger.debug("task [{}] cleaned up", persistentTask.getId());

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import org.elasticsearch.index.seqno.RetentionLeaseActions;
99
import org.elasticsearch.index.seqno.RetentionLeaseBackgroundSyncAction;
1010
import org.elasticsearch.index.seqno.RetentionLeaseSyncAction;
11+
import org.elasticsearch.persistent.CompletionPersistentTaskAction;
1112
import org.elasticsearch.transport.TransportActionProxy;
1213
import org.elasticsearch.xpack.core.security.support.Automatons;
1314

@@ -33,7 +34,8 @@ public final class SystemPrivilege extends Privilege {
3334
RetentionLeaseActions.Add.ACTION_NAME + "*", // needed for CCR to add retention leases
3435
RetentionLeaseActions.Remove.ACTION_NAME + "*", // needed for CCR to remove retention leases
3536
RetentionLeaseActions.Renew.ACTION_NAME + "*", // needed for CCR to renew retention leases
36-
"indices:admin/settings/update" // needed for DiskThresholdMonitor.markIndicesReadOnly
37+
"indices:admin/settings/update", // needed for DiskThresholdMonitor.markIndicesReadOnly
38+
CompletionPersistentTaskAction.NAME // needed for ShardFollowTaskCleaner
3739
);
3840

3941
private static final Predicate<String> PREDICATE = (action) -> {

0 commit comments

Comments
 (0)