-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Tasks: Only require task permissions #35667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ef69ce7
02b3746
701cb55
18c8bd5
0771769
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.client; | ||
|
||
import org.elasticsearch.action.Action; | ||
import org.elasticsearch.action.ActionListener; | ||
import org.elasticsearch.action.ActionRequest; | ||
import org.elasticsearch.action.ActionResponse; | ||
import org.elasticsearch.action.support.ContextPreservingActionListener; | ||
import org.elasticsearch.common.util.concurrent.ThreadContext; | ||
|
||
import java.util.function.Supplier; | ||
|
||
/** | ||
* A {@linkplain Client} that sends requests with the | ||
* {@link ThreadContext#stashWithOrigin origin} set to a particular | ||
* value and calls its {@linkplain ActionListener} in its original | ||
* {@link ThreadContext}. | ||
*/ | ||
public final class OriginSettingClient extends FilterClient { | ||
|
||
private final String origin; | ||
|
||
public OriginSettingClient(Client in, String origin) { | ||
super(in); | ||
this.origin = origin; | ||
} | ||
|
||
@Override | ||
protected <Request extends ActionRequest, Response extends ActionResponse> | ||
void doExecute(Action<Response> action, Request request, ActionListener<Response> listener) { | ||
final Supplier<ThreadContext.StoredContext> supplier = in().threadPool().getThreadContext().newRestorableContext(false); | ||
try (ThreadContext.StoredContext ignore = in().threadPool().getThreadContext().stashWithOrigin(origin)) { | ||
super.doExecute(action, request, new ContextPreservingActionListener<>(supplier, listener)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,21 +25,19 @@ | |
import org.elasticsearch.action.ActionRequest; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.cancel.CancelTasksRequest; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.cancel.CancelTasksResponse; | ||
import org.elasticsearch.action.support.ContextPreservingActionListener; | ||
import org.elasticsearch.client.Client; | ||
import org.elasticsearch.client.OriginSettingClient; | ||
import org.elasticsearch.cluster.ClusterState; | ||
import org.elasticsearch.cluster.ClusterStateObserver; | ||
import org.elasticsearch.cluster.service.ClusterService; | ||
import org.elasticsearch.common.Nullable; | ||
import org.elasticsearch.common.unit.TimeValue; | ||
import org.elasticsearch.common.util.concurrent.ThreadContext; | ||
import org.elasticsearch.node.NodeClosedException; | ||
import org.elasticsearch.persistent.PersistentTasksCustomMetaData.PersistentTask; | ||
import org.elasticsearch.tasks.TaskId; | ||
import org.elasticsearch.threadpool.ThreadPool; | ||
|
||
import java.util.function.Predicate; | ||
import java.util.function.Supplier; | ||
|
||
/** | ||
* This service is used by persistent tasks and allocated persistent tasks to communicate changes | ||
|
@@ -50,15 +48,14 @@ public class PersistentTasksService { | |
|
||
private static final Logger logger = LogManager.getLogger(PersistentTasksService.class); | ||
|
||
private static final String ACTION_ORIGIN_TRANSIENT_NAME = "action.origin"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that the server has |
||
private static final String PERSISTENT_TASK_ORIGIN = "persistent_tasks"; | ||
|
||
private final Client client; | ||
private final ClusterService clusterService; | ||
private final ThreadPool threadPool; | ||
|
||
public PersistentTasksService(ClusterService clusterService, ThreadPool threadPool, Client client) { | ||
this.client = client; | ||
this.client = new OriginSettingClient(client, PERSISTENT_TASK_ORIGIN); | ||
this.clusterService = clusterService; | ||
this.threadPool = threadPool; | ||
} | ||
|
@@ -98,12 +95,7 @@ void sendCancelRequest(final long taskId, final String reason, final ActionListe | |
request.setTaskId(new TaskId(clusterService.localNode().getId(), taskId)); | ||
request.setReason(reason); | ||
try { | ||
final ThreadContext threadContext = client.threadPool().getThreadContext(); | ||
final Supplier<ThreadContext.StoredContext> supplier = threadContext.newRestorableContext(false); | ||
|
||
try (ThreadContext.StoredContext ignore = stashWithOrigin(threadContext, PERSISTENT_TASK_ORIGIN)) { | ||
client.admin().cluster().cancelTasks(request, new ContextPreservingActionListener<>(supplier, listener)); | ||
} | ||
client.admin().cluster().cancelTasks(request, listener); | ||
} catch (Exception e) { | ||
listener.onFailure(e); | ||
} | ||
|
@@ -140,14 +132,8 @@ public void sendRemoveRequest(final String taskId, final ActionListener<Persiste | |
private <Req extends ActionRequest, Resp extends PersistentTaskResponse> | ||
void execute(final Req request, final Action<Resp> action, final ActionListener<PersistentTask<?>> listener) { | ||
try { | ||
final ThreadContext threadContext = client.threadPool().getThreadContext(); | ||
final Supplier<ThreadContext.StoredContext> supplier = threadContext.newRestorableContext(false); | ||
|
||
try (ThreadContext.StoredContext ignore = stashWithOrigin(threadContext, PERSISTENT_TASK_ORIGIN)) { | ||
client.execute(action, request, | ||
new ContextPreservingActionListener<>(supplier, | ||
ActionListener.wrap(r -> listener.onResponse(r.getTask()), listener::onFailure))); | ||
} | ||
client.execute(action, request, | ||
ActionListener.wrap(r -> listener.onResponse(r.getTask()), listener::onFailure)); | ||
} catch (Exception e) { | ||
listener.onFailure(e); | ||
} | ||
|
@@ -233,10 +219,4 @@ default void onTimeout(TimeValue timeout) { | |
onFailure(new IllegalStateException("Timed out when waiting for persistent task after " + timeout)); | ||
} | ||
} | ||
|
||
public static ThreadContext.StoredContext stashWithOrigin(ThreadContext threadContext, String origin) { | ||
final ThreadContext.StoredContext storedContext = threadContext.stashContext(); | ||
threadContext.putTransient(ACTION_ORIGIN_TRANSIENT_NAME, origin); | ||
return storedContext; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,14 +27,14 @@ | |
import org.elasticsearch.action.TaskOperationFailure; | ||
import org.elasticsearch.action.admin.cluster.health.ClusterHealthAction; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.cancel.CancelTasksResponse; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskRequest; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.get.GetTaskResponse; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksAction; | ||
import org.elasticsearch.action.admin.cluster.node.tasks.list.ListTasksResponse; | ||
import org.elasticsearch.action.admin.indices.refresh.RefreshAction; | ||
import org.elasticsearch.action.admin.indices.upgrade.post.UpgradeAction; | ||
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryAction; | ||
import org.elasticsearch.action.bulk.BulkAction; | ||
import org.elasticsearch.action.get.GetResponse; | ||
import org.elasticsearch.action.index.IndexAction; | ||
import org.elasticsearch.action.index.IndexResponse; | ||
import org.elasticsearch.action.search.SearchAction; | ||
|
@@ -85,7 +85,6 @@ | |
import static org.elasticsearch.common.unit.TimeValue.timeValueMillis; | ||
import static org.elasticsearch.common.unit.TimeValue.timeValueSeconds; | ||
import static org.elasticsearch.http.HttpTransportSettings.SETTING_HTTP_MAX_HEADER_SIZE; | ||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; | ||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; | ||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; | ||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows; | ||
|
@@ -725,12 +724,6 @@ public void testTasksWaitForAllTask() throws Exception { | |
} | ||
|
||
public void testTaskStoringSuccesfulResult() throws Exception { | ||
// Randomly create an empty index to make sure the type is created automatically | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed this because it made writing the assertions about setting the origin very difficult and it isn't really a supported configuration. |
||
if (randomBoolean()) { | ||
logger.info("creating an empty results index with custom settings"); | ||
assertAcked(client().admin().indices().prepareCreate(TaskResultsService.TASK_INDEX)); | ||
} | ||
|
||
registerTaskManageListeners(TestTaskPlugin.TestTaskAction.NAME); // we need this to get task id of the process | ||
|
||
// Start non-blocking test task | ||
|
@@ -743,23 +736,20 @@ public void testTaskStoringSuccesfulResult() throws Exception { | |
TaskInfo taskInfo = events.get(0); | ||
TaskId taskId = taskInfo.getTaskId(); | ||
|
||
GetResponse resultDoc = client() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I replaced these with the tasks API calls because it gives me a good opportunity to assert that they find the documents while setting the origin. |
||
.prepareGet(TaskResultsService.TASK_INDEX, TaskResultsService.TASK_TYPE, taskId.toString()).get(); | ||
assertTrue(resultDoc.isExists()); | ||
|
||
Map<String, Object> source = resultDoc.getSource(); | ||
@SuppressWarnings("unchecked") | ||
Map<String, Object> task = (Map<String, Object>) source.get("task"); | ||
assertEquals(taskInfo.getTaskId().getNodeId(), task.get("node")); | ||
assertEquals(taskInfo.getAction(), task.get("action")); | ||
assertEquals(Long.toString(taskInfo.getId()), task.get("id").toString()); | ||
|
||
@SuppressWarnings("unchecked") | ||
Map<String, Object> result = (Map<String, Object>) source.get("response"); | ||
TaskResult taskResult = client().admin().cluster() | ||
.getTask(new GetTaskRequest().setTaskId(taskId)).get().getTask(); | ||
assertTrue(taskResult.isCompleted()); | ||
assertNull(taskResult.getError()); | ||
|
||
assertEquals(taskInfo.getTaskId(), taskResult.getTask().getTaskId()); | ||
assertEquals(taskInfo.getType(), taskResult.getTask().getType()); | ||
assertEquals(taskInfo.getAction(), taskResult.getTask().getAction()); | ||
assertEquals(taskInfo.getDescription(), taskResult.getTask().getDescription()); | ||
assertEquals(taskInfo.getStartTime(), taskResult.getTask().getStartTime()); | ||
assertEquals(taskInfo.getHeaders(), taskResult.getTask().getHeaders()); | ||
Map<?, ?> result = taskResult.getResponseAsMap(); | ||
assertEquals("0", result.get("failure_count").toString()); | ||
|
||
assertNull(source.get("failure")); | ||
|
||
assertNoFailures(client().admin().indices().prepareRefresh(TaskResultsService.TASK_INDEX).get()); | ||
|
||
SearchResponse searchResponse = client().prepareSearch(TaskResultsService.TASK_INDEX) | ||
|
@@ -797,25 +787,21 @@ public void testTaskStoringFailureResult() throws Exception { | |
TaskInfo failedTaskInfo = events.get(0); | ||
TaskId failedTaskId = failedTaskInfo.getTaskId(); | ||
|
||
GetResponse failedResultDoc = client() | ||
.prepareGet(TaskResultsService.TASK_INDEX, TaskResultsService.TASK_TYPE, failedTaskId.toString()) | ||
.get(); | ||
assertTrue(failedResultDoc.isExists()); | ||
|
||
Map<String, Object> source = failedResultDoc.getSource(); | ||
@SuppressWarnings("unchecked") | ||
Map<String, Object> task = (Map<String, Object>) source.get("task"); | ||
assertEquals(failedTaskInfo.getTaskId().getNodeId(), task.get("node")); | ||
assertEquals(failedTaskInfo.getAction(), task.get("action")); | ||
assertEquals(Long.toString(failedTaskInfo.getId()), task.get("id").toString()); | ||
|
||
@SuppressWarnings("unchecked") | ||
Map<String, Object> error = (Map<String, Object>) source.get("error"); | ||
TaskResult taskResult = client().admin().cluster() | ||
.getTask(new GetTaskRequest().setTaskId(failedTaskId)).get().getTask(); | ||
assertTrue(taskResult.isCompleted()); | ||
assertNull(taskResult.getResponse()); | ||
|
||
assertEquals(failedTaskInfo.getTaskId(), taskResult.getTask().getTaskId()); | ||
assertEquals(failedTaskInfo.getType(), taskResult.getTask().getType()); | ||
assertEquals(failedTaskInfo.getAction(), taskResult.getTask().getAction()); | ||
assertEquals(failedTaskInfo.getDescription(), taskResult.getTask().getDescription()); | ||
assertEquals(failedTaskInfo.getStartTime(), taskResult.getTask().getStartTime()); | ||
assertEquals(failedTaskInfo.getHeaders(), taskResult.getTask().getHeaders()); | ||
Map<?, ?> error = (Map<?, ?>) taskResult.getErrorAsMap(); | ||
assertEquals("Simulating operation failure", error.get("reason")); | ||
assertEquals("illegal_state_exception", error.get("type")); | ||
|
||
assertNull(source.get("result")); | ||
|
||
GetTaskResponse getResponse = expectFinishedTask(failedTaskId); | ||
assertNull(getResponse.getTask().getResponse()); | ||
assertEquals(error, getResponse.getTask().getErrorAsMap()); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stuck this here because it felt like a fairly right place at the time. Now that I look at it again maybe it should be on something a little more centrally located. But it is about actions so I stuck it on an action. I'm not sure of a better spot.