From d6539abd0a2d56d2160a3da43fe70738f825855b Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 29 Nov 2018 23:45:28 -0800 Subject: [PATCH] Fix overlord api and console (#6686) * Fix overlord APIs and console * remove getRunningTasksByDataSource * add missing path to isApplicable --- docs/content/operations/api-reference.md | 2 +- .../overlord/http/OverlordResource.java | 24 +------ .../indexer_static/js/console-0.0.1.js | 2 +- .../overlord/http/OverlordResourceTest.java | 62 +++---------------- .../security/DatasourceResourceFilter.java | 3 +- 5 files changed, 17 insertions(+), 76 deletions(-) diff --git a/docs/content/operations/api-reference.md b/docs/content/operations/api-reference.md index 6b605f38cb9..5d3fda34458 100644 --- a/docs/content/operations/api-reference.md +++ b/docs/content/operations/api-reference.md @@ -405,7 +405,7 @@ Endpoint for submitting tasks and supervisor specs to the overlord. Returns the Shuts down a task. -* `druid/indexer/v1/task/{dataSource}/shutdownAllTasks` +* `druid/indexer/v1/datasources/{dataSource}/shutdownAllTasks` Shuts down all tasks for a dataSource. diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java index d0d7a434b49..84f4ac91c5a 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/OverlordResource.java @@ -59,6 +59,7 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.metadata.EntryExistsException; import org.apache.druid.server.http.security.ConfigResourceFilter; +import org.apache.druid.server.http.security.DatasourceResourceFilter; import org.apache.druid.server.http.security.StateResourceFilter; import org.apache.druid.server.security.Access; import org.apache.druid.server.security.Action; @@ -338,8 +339,9 @@ public class OverlordResource } @POST - @Path("/task/{dataSource}/shutdownAllTasks") + @Path("/datasources/{dataSource}/shutdownAllTasks") @Produces(MediaType.APPLICATION_JSON) + @ResourceFilters(DatasourceResourceFilter.class) public Response shutdownTasksForDataSource(@PathParam("dataSource") final String dataSource) { return asLeaderWith( @@ -1000,26 +1002,6 @@ public class OverlordResource } } - @GET - @Path("/dataSources/{dataSource}") - @Produces(MediaType.APPLICATION_JSON) - public Response getRunningTasksByDataSource(@PathParam("dataSource") String dataSource, - @Context HttpServletRequest request) - { - Optional ts = taskMaster.getTaskRunner(); - if (!ts.isPresent()) { - return Response.status(Response.Status.NOT_FOUND).entity("No tasks are running").build(); - } - Collection runningTasks = ts.get().getRunningTasks(); - if (runningTasks == null || runningTasks.isEmpty()) { - return Response.status(Response.Status.NOT_FOUND) - .entity("No running tasks found for the datasource : " + dataSource).build(); - } - List taskRunnerWorkItemList = runningTasks.stream() - .filter(task -> dataSource.equals(task.getDataSource())).collect(Collectors.toList()); - return Response.ok(taskRunnerWorkItemList).build(); - } - private Response asLeaderWith(Optional x, Function f) { if (x.isPresent()) { diff --git a/indexing-service/src/main/resources/indexer_static/js/console-0.0.1.js b/indexing-service/src/main/resources/indexer_static/js/console-0.0.1.js index b4994f9c917..bda6094e63f 100644 --- a/indexing-service/src/main/resources/indexer_static/js/console-0.0.1.js +++ b/indexing-service/src/main/resources/indexer_static/js/console-0.0.1.js @@ -24,7 +24,7 @@ var killTask = function(taskId) { if(confirm('Do you really want to kill: '+taskId)) { $.ajax({ type:'POST', - url: '/druid/indexer/v1/task/'+ taskId +'/terminate', + url: '/druid/indexer/v1/task/'+ taskId +'/shutdown', data: '' }).done(function(data) { setTimeout(function() { location.reload(true) }, 750); diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java index e3885f75596..52955f09c25 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java @@ -125,11 +125,11 @@ public class OverlordResourceTest ); } - public void expectAuthorizationTokenCheck() + private void expectAuthorizationTokenCheck() { AuthenticationResult authenticationResult = new AuthenticationResult("druid", "druid", null, null); EasyMock.expect(req.getAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH)).andReturn(null).anyTimes(); - EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).anyTimes(); + EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).atLeastOnce(); EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)) .andReturn(authenticationResult) .anyTimes(); @@ -833,7 +833,10 @@ public class OverlordResourceTest @Test public void testGetTaskPayload() throws Exception { - expectAuthorizationTokenCheck(); + // This is disabled since OverlordResource.getTaskStatus() is annotated with TaskResourceFilter which is supposed to + // set authorization token properly, but isn't called in this test. + // This should be fixed in https://github.com/apache/incubator-druid/issues/6685. + // expectAuthorizationTokenCheck(); final NoopTask task = NoopTask.create("mydatasource"); EasyMock.expect(taskStorageQueryAdapter.getTask("mytask")) .andReturn(Optional.of(task)); @@ -861,7 +864,10 @@ public class OverlordResourceTest @Test public void testGetTaskStatus() throws Exception { - expectAuthorizationTokenCheck(); + // This is disabled since OverlordResource.getTaskStatus() is annotated with TaskResourceFilter which is supposed to + // set authorization token properly, but isn't called in this test. + // This should be fixed in https://github.com/apache/incubator-druid/issues/6685. + // expectAuthorizationTokenCheck(); final Task task = NoopTask.create("mytask", 0); final TaskStatus status = TaskStatus.running("mytask"); @@ -910,54 +916,6 @@ public class OverlordResourceTest Assert.assertEquals(new TaskStatusResponse("othertask", null), taskStatusResponse2); } - @Test - public void testGetRunningTasksByDataSource() - { - - List tasksIds = ImmutableList.of("id_1", "id_2"); - EasyMock.>expect(taskRunner.getRunningTasks()).andReturn( - ImmutableList.of( - new MockTaskRunnerWorkItem(tasksIds.get(0), null), - new MockTaskRunnerWorkItem(tasksIds.get(1), null))); - EasyMock.expect(taskStorageQueryAdapter.getTask(tasksIds.get(0))).andReturn( - Optional.of(getTaskWithIdAndDatasource(tasksIds.get(0), "deny"))).once(); - EasyMock.expect(taskStorageQueryAdapter.getTask(tasksIds.get(1))).andReturn( - Optional.of(getTaskWithIdAndDatasource(tasksIds.get(1), "allow"))).once(); - - EasyMock.replay(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req); - List responseObjects = (List) overlordResource.getRunningTasksByDataSource("ds_test", req) - .getEntity(); - - Assert.assertEquals(2, responseObjects.size()); - Assert.assertEquals(taskStorageQueryAdapter.getTask("id_1").get().getId(), responseObjects.get(0).getTaskId()); - Assert.assertEquals(taskStorageQueryAdapter.getTask("id_2").get().getId(), responseObjects.get(1).getTaskId()); - Assert.assertTrue("DataSource Check", "ds_test".equals(responseObjects.get(0).getDataSource())); - } - - @Test - public void testGetRunningTasksByDataSourceNeg() - { - expectAuthorizationTokenCheck(); - - List tasksIds = ImmutableList.of("id_1", "id_2"); - EasyMock.>expect(taskRunner.getRunningTasks()).andReturn( - ImmutableList.of( - new MockTaskRunnerWorkItem(tasksIds.get(0), null), - new MockTaskRunnerWorkItem(tasksIds.get(1), null))); - EasyMock.expect(taskStorageQueryAdapter.getTask(tasksIds.get(0))).andReturn( - Optional.of(getTaskWithIdAndDatasource(tasksIds.get(0), "deny"))).once(); - EasyMock.expect(taskStorageQueryAdapter.getTask(tasksIds.get(1))).andReturn( - Optional.of(getTaskWithIdAndDatasource(tasksIds.get(1), "allow"))).once(); - - EasyMock.replay(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req); - Assert.assertTrue(taskStorageQueryAdapter.getTask("id_1").isPresent()); - Assert.assertTrue(taskStorageQueryAdapter.getTask("id_2").isPresent()); - List responseObjects = (List) overlordResource.getRunningTasksByDataSource("ds_NA", req) - .getEntity(); - - Assert.assertEquals(0, responseObjects.size()); - } - @After public void tearDown() { diff --git a/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java index 6f62f94d3a2..c46277b18f7 100644 --- a/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java +++ b/server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java @@ -100,7 +100,8 @@ public class DatasourceResourceFilter extends AbstractResourceFilter List applicablePaths = ImmutableList.of( "druid/coordinator/v1/datasources/", "druid/coordinator/v1/metadata/datasources/", - "druid/v2/datasources/" + "druid/v2/datasources/", + "druid/indexer/v1/datasources" ); for (String path : applicablePaths) { if (requestPath.startsWith(path) && !requestPath.equals(path)) {