From f8471f1c6403c486424fea49584d6d80d0c09beb Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 29 Nov 2018 16:18:51 -0800 Subject: [PATCH 1/3] Fix overlord APIs and console --- .../overlord/http/OverlordResource.java | 18 ++++++--- .../indexer_static/js/console-0.0.1.js | 2 +- .../overlord/http/OverlordResourceTest.java | 37 ++++++++++++++----- 3 files changed, 40 insertions(+), 17 deletions(-) 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 d0d7a434b499..8978f6093d4a 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.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 Response apply(TaskQueue taskQueue) } @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( @@ -1001,9 +1003,11 @@ public Response doGetReports( } @GET - @Path("/dataSources/{dataSource}") + @Path("/datasources/{dataSource}") @Produces(MediaType.APPLICATION_JSON) - public Response getRunningTasksByDataSource(@PathParam("dataSource") String dataSource, + @ResourceFilters(DatasourceResourceFilter.class) + public Response getRunningTasksByDataSource( + @PathParam("dataSource") String dataSource, @Context HttpServletRequest request) { Optional ts = taskMaster.getTaskRunner(); @@ -1013,10 +1017,12 @@ public Response getRunningTasksByDataSource(@PathParam("dataSource") String data 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(); + .entity("No running tasks found for the datasource : " + dataSource).build(); } - List taskRunnerWorkItemList = runningTasks.stream() - .filter(task -> dataSource.equals(task.getDataSource())).collect(Collectors.toList()); + List taskRunnerWorkItemList = runningTasks + .stream() + .filter(task -> dataSource.equals(task.getDataSource())) + .collect(Collectors.toList()); return Response.ok(taskRunnerWorkItemList).build(); } 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 b4994f9c917a..bda6094e63f1 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 e3885f755967..5ac89bb587f4 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 Access authorize(AuthenticationResult authenticationResult, Resource reso ); } - 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 void testKillPendingSegments() @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 void testGetTaskPayload() throws Exception @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"); @@ -913,12 +919,18 @@ public void testGetTaskStatus() throws Exception @Test public void testGetRunningTasksByDataSource() { + // 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(); 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))); + 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( @@ -926,24 +938,29 @@ public void testGetRunningTasksByDataSource() EasyMock.replay(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req); List responseObjects = (List) overlordResource.getRunningTasksByDataSource("ds_test", req) - .getEntity(); + .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())); + Assert.assertEquals("DataSource Check", "ds_test", responseObjects.get(0).getDataSource()); } @Test public void testGetRunningTasksByDataSourceNeg() { - 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(); 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))); + 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( @@ -953,7 +970,7 @@ public void testGetRunningTasksByDataSourceNeg() Assert.assertTrue(taskStorageQueryAdapter.getTask("id_1").isPresent()); Assert.assertTrue(taskStorageQueryAdapter.getTask("id_2").isPresent()); List responseObjects = (List) overlordResource.getRunningTasksByDataSource("ds_NA", req) - .getEntity(); + .getEntity(); Assert.assertEquals(0, responseObjects.size()); } From d2606aa969a8798589542b7036da9142c1f49bff Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 29 Nov 2018 17:21:39 -0800 Subject: [PATCH 2/3] remove getRunningTasksByDataSource --- docs/content/operations/api-reference.md | 2 +- .../overlord/http/OverlordResource.java | 24 -------- .../overlord/http/OverlordResourceTest.java | 59 ------------------- 3 files changed, 1 insertion(+), 84 deletions(-) diff --git a/docs/content/operations/api-reference.md b/docs/content/operations/api-reference.md index 6b605f38cb96..5d3fda344584 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 8978f6093d4a..84f4ac91c5ad 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 @@ -1002,30 +1002,6 @@ public Response doGetReports( } } - @GET - @Path("/datasources/{dataSource}") - @Produces(MediaType.APPLICATION_JSON) - @ResourceFilters(DatasourceResourceFilter.class) - 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/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 5ac89bb587f4..52955f09c259 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 @@ -916,65 +916,6 @@ public void testGetTaskStatus() throws Exception Assert.assertEquals(new TaskStatusResponse("othertask", null), taskStatusResponse2); } - @Test - public void testGetRunningTasksByDataSource() - { - // 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(); - - 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.assertEquals("DataSource Check", "ds_test", responseObjects.get(0).getDataSource()); - } - - @Test - public void testGetRunningTasksByDataSourceNeg() - { - // 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(); - - 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() { From 7b84c37b8d9aa96718b47e93d0d20318b8c87e7f Mon Sep 17 00:00:00 2001 From: Jihoon Son Date: Thu, 29 Nov 2018 18:05:47 -0800 Subject: [PATCH 3/3] add missing path to isApplicable --- .../druid/server/http/security/DatasourceResourceFilter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 6f62f94d3a27..c46277b18f79 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 boolean isApplicable(String requestPath) 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)) {