You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by cw...@apache.org on 2018/11/30 07:45:36 UTC

[incubator-druid] branch master updated: Fix overlord api and console (#6686)

This is an automated email from the ASF dual-hosted git repository.

cwylie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git


The following commit(s) were added to refs/heads/master by this push:
     new d6539ab  Fix overlord api and console (#6686)
d6539ab is described below

commit d6539abd0a2d56d2160a3da43fe70738f825855b
Author: Jihoon Son <ji...@apache.org>
AuthorDate: Thu Nov 29 23:45:28 2018 -0800

    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 +-
 .../indexing/overlord/http/OverlordResource.java   | 24 ++-------
 .../resources/indexer_static/js/console-0.0.1.js   |  2 +-
 .../overlord/http/OverlordResourceTest.java        | 62 ++++------------------
 .../http/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 6b605f3..5d3fda3 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 d0d7a43..84f4ac9 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<TaskRunner> ts = taskMaster.getTaskRunner();
-    if (!ts.isPresent()) {
-      return Response.status(Response.Status.NOT_FOUND).entity("No tasks are running").build();
-    }
-    Collection<? extends TaskRunnerWorkItem> 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<TaskRunnerWorkItem> taskRunnerWorkItemList = runningTasks.stream()
-        .filter(task -> dataSource.equals(task.getDataSource())).collect(Collectors.toList());
-    return Response.ok(taskRunnerWorkItemList).build();
-  }
-
   private <T> Response asLeaderWith(Optional<T> x, Function<T, Response> 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 b4994f9..bda6094 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 e3885f7..52955f0 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<String> tasksIds = ImmutableList.of("id_1", "id_2");
-    EasyMock.<Collection<? extends TaskRunnerWorkItem>>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<TaskRunnerWorkItem> 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<String> tasksIds = ImmutableList.of("id_1", "id_2");
-    EasyMock.<Collection<? extends TaskRunnerWorkItem>>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<TaskRunnerWorkItem> 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 6f62f94..c46277b 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<String> 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)) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org