You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by as...@apache.org on 2018/12/01 13:52:39 UTC
[incubator-druid] branch master updated: remove
AbstractResourceFilter.isApplicable because it is not (#6691)
This is an automated email from the ASF dual-hosted git repository.
asdf2014 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 43adb39 remove AbstractResourceFilter.isApplicable because it is not (#6691)
43adb39 is described below
commit 43adb391c2d0aeaca40316cba37ea0440fb46d55
Author: Clint Wylie <cj...@gmail.com>
AuthorDate: Sat Dec 1 05:52:31 2018 -0800
remove AbstractResourceFilter.isApplicable because it is not (#6691)
* remove AbstractResourceFilter.isApplicable because it is not, add tests for OverlordResource.doShutdown and OverlordResource.shutdownTasksForDatasource
* cleanup
---
.../basic/BasicSecurityResourceFilter.java | 18 ----
.../http/security/SupervisorResourceFilter.java | 15 ---
.../overlord/http/security/TaskResourceFilter.java | 14 ---
.../overlord/http/OverlordResourceTest.java | 102 +++++++++++++++++----
.../OverlordSecurityResourceFilterTest.java | 5 -
.../http/security/AbstractResourceFilter.java | 2 -
.../server/http/security/ConfigResourceFilter.java | 9 --
.../http/security/DatasourceResourceFilter.java | 19 ----
.../server/http/security/RulesResourceFilter.java | 14 ---
.../server/http/security/StateResourceFilter.java | 15 ---
.../http/security/SecurityResourceFilterTest.java | 11 ---
11 files changed, 85 insertions(+), 139 deletions(-)
diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java
index abb904c..8294ec1 100644
--- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java
+++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java
@@ -19,7 +19,6 @@
package org.apache.druid.security.basic;
-import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ContainerRequest;
import org.apache.druid.java.util.common.StringUtils;
@@ -33,15 +32,9 @@ import org.apache.druid.server.security.ResourceType;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.Response;
-import java.util.List;
public class BasicSecurityResourceFilter extends AbstractResourceFilter
{
- private static final List<String> APPLICABLE_PATHS = ImmutableList.of(
- "/druid-ext/basic-security/authentication",
- "/druid-ext/basic-security/authorization"
- );
-
private static final String SECURITY_RESOURCE_NAME = "security";
@Inject
@@ -76,15 +69,4 @@ public class BasicSecurityResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- for (String path : APPLICABLE_PATHS) {
- if (requestPath.startsWith(path) && !requestPath.equals(path)) {
- return true;
- }
- }
- return false;
- }
}
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java
index 5179a89..e834c2e 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java
@@ -23,7 +23,6 @@ import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ContainerRequest;
@@ -41,7 +40,6 @@ import org.apache.druid.server.security.ResourceAction;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.PathSegment;
import javax.ws.rs.core.Response;
-import java.util.List;
public class SupervisorResourceFilter extends AbstractResourceFilter
{
@@ -109,17 +107,4 @@ public class SupervisorResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- List<String> applicablePaths = ImmutableList.of("druid/indexer/v1/supervisor/");
- for (String path : applicablePaths) {
- if (requestPath.startsWith(path) && !requestPath.equals(path)) {
- return true;
- }
- }
- return false;
- }
-
}
diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
index 26c927f..8109961 100644
--- a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
+++ b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
@@ -22,7 +22,6 @@ package org.apache.druid.indexing.overlord.http.security;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ContainerRequest;
@@ -41,7 +40,6 @@ import org.apache.druid.server.security.ResourceType;
import javax.ws.rs.WebApplicationException;
import javax.ws.rs.core.PathSegment;
import javax.ws.rs.core.Response;
-import java.util.List;
/**
* Use this ResourceFilter when the datasource information is present after "task" segment in the request Path
@@ -110,16 +108,4 @@ public class TaskResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- List<String> applicablePaths = ImmutableList.of("druid/indexer/v1/task/");
- for (String path : applicablePaths) {
- if (requestPath.startsWith(path) && !requestPath.equals(path)) {
- return true;
- }
- }
- return false;
- }
}
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 52955f0..4558de2 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
@@ -36,6 +36,7 @@ import org.apache.druid.indexing.common.task.NoopTask;
import org.apache.druid.indexing.common.task.Task;
import org.apache.druid.indexing.overlord.IndexerMetadataStorageAdapter;
import org.apache.druid.indexing.overlord.TaskMaster;
+import org.apache.druid.indexing.overlord.TaskQueue;
import org.apache.druid.indexing.overlord.TaskRunner;
import org.apache.druid.indexing.overlord.TaskRunnerWorkItem;
import org.apache.druid.indexing.overlord.TaskStorageQueryAdapter;
@@ -125,20 +126,10 @@ public class OverlordResourceTest
);
}
- private void expectAuthorizationTokenCheck()
+ @After
+ public void tearDown()
{
- 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).atLeastOnce();
- EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT))
- .andReturn(authenticationResult)
- .anyTimes();
-
- req.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, false);
- EasyMock.expectLastCall().anyTimes();
-
- req.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
- EasyMock.expectLastCall().anyTimes();
+ EasyMock.verify(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req);
}
@Test
@@ -916,10 +907,88 @@ public class OverlordResourceTest
Assert.assertEquals(new TaskStatusResponse("othertask", null), taskStatusResponse2);
}
- @After
- public void tearDown()
+ @Test
+ public void testShutdownTask()
{
- EasyMock.verify(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req);
+ // This is disabled since OverlordResource.doShutdown is annotated with TaskResourceFilter
+ // This should be fixed in https://github.com/apache/incubator-druid/issues/6685.
+ // expectAuthorizationTokenCheck();
+ TaskQueue mockQueue = EasyMock.createMock(TaskQueue.class);
+ EasyMock.expect(taskMaster.isLeader()).andReturn(true).anyTimes();
+ EasyMock.expect(taskMaster.getTaskRunner()).andReturn(
+ Optional.of(taskRunner)
+ ).anyTimes();
+ EasyMock.expect(taskMaster.getTaskQueue()).andReturn(
+ Optional.of(mockQueue)
+ ).anyTimes();
+ mockQueue.shutdown("id_1", "Shutdown request from user");
+ EasyMock.expectLastCall();
+
+ EasyMock.replay(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req, mockQueue);
+
+ final Map<String, Integer> response = (Map<String, Integer>) overlordResource
+ .doShutdown("id_1")
+ .getEntity();
+ Assert.assertEquals("id_1", response.get("task"));
+ }
+
+ @Test
+ public void testShutdownAllTasks()
+ {
+ // This is disabled since OverlordResource.shutdownTasksForDataSource is annotated with DatasourceResourceFilter
+ // This should be fixed in https://github.com/apache/incubator-druid/issues/6685.
+ // expectAuthorizationTokenCheck();
+ TaskQueue mockQueue = EasyMock.createMock(TaskQueue.class);
+ EasyMock.expect(taskMaster.isLeader()).andReturn(true).anyTimes();
+ EasyMock.expect(taskMaster.getTaskRunner()).andReturn(
+ Optional.of(taskRunner)
+ ).anyTimes();
+ EasyMock.expect(taskMaster.getTaskQueue()).andReturn(
+ Optional.of(mockQueue)
+ ).anyTimes();
+ EasyMock.expect(taskStorageQueryAdapter.getActiveTaskInfo("datasource")).andStubReturn(ImmutableList.of(
+ new TaskInfo(
+ "id_1",
+ DateTime.now(ISOChronology.getInstanceUTC()),
+ TaskStatus.success("id_1"),
+ "datasource",
+ getTaskWithIdAndDatasource("id_1", "datasource")
+ ),
+ new TaskInfo(
+ "id_2",
+ DateTime.now(ISOChronology.getInstanceUTC()),
+ TaskStatus.success("id_2"),
+ "datasource",
+ getTaskWithIdAndDatasource("id_2", "datasource")
+ )
+ ));
+ mockQueue.shutdown("id_1", "Shutdown request from user");
+ EasyMock.expectLastCall();
+ mockQueue.shutdown("id_2", "Shutdown request from user");
+ EasyMock.expectLastCall();
+
+ EasyMock.replay(taskRunner, taskMaster, taskStorageQueryAdapter, indexerMetadataStorageAdapter, req, mockQueue);
+
+ final Map<String, Integer> response = (Map<String, Integer>) overlordResource
+ .shutdownTasksForDataSource("datasource")
+ .getEntity();
+ Assert.assertEquals("datasource", response.get("dataSource"));
+ }
+
+ 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).atLeastOnce();
+ EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT))
+ .andReturn(authenticationResult)
+ .atLeastOnce();
+
+ req.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, false);
+ EasyMock.expectLastCall().anyTimes();
+
+ req.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+ EasyMock.expectLastCall().anyTimes();
}
private Task getTaskWithIdAndDatasource(String id, String datasource)
@@ -975,5 +1044,4 @@ public class OverlordResourceTest
}
}
-
}
diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
index 27dd6a3..16c8afe 100644
--- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
+++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
@@ -33,13 +33,11 @@ import org.apache.druid.indexing.overlord.supervisor.SupervisorManager;
import org.apache.druid.indexing.overlord.supervisor.SupervisorResource;
import org.apache.druid.indexing.overlord.supervisor.SupervisorSpec;
import org.apache.druid.indexing.worker.http.WorkerResource;
-import org.apache.druid.server.http.security.AbstractResourceFilter;
import org.apache.druid.server.http.security.ResourceFilterTestHelper;
import org.apache.druid.server.security.AuthorizerMapper;
import org.apache.druid.server.security.ForbiddenException;
import org.easymock.EasyMock;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -171,7 +169,6 @@ public class OverlordSecurityResourceFilterTest extends ResourceFilterTestHelper
EasyMock.expect(request.getMethod()).andReturn(requestMethod).anyTimes();
EasyMock.replay(req, request, authorizerMapper);
resourceFilter.getRequestFilter().filter(request);
- Assert.assertTrue(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(requestPath));
}
@Test(expected = ForbiddenException.class)
@@ -180,7 +177,6 @@ public class OverlordSecurityResourceFilterTest extends ResourceFilterTestHelper
setUpMockExpectations(requestPath, false, requestMethod);
EasyMock.expect(request.getEntity(Task.class)).andReturn(noopTask).anyTimes();
EasyMock.replay(req, request, authorizerMapper);
- Assert.assertTrue(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(requestPath));
try {
resourceFilter.getRequestFilter().filter(request);
}
@@ -195,7 +191,6 @@ public class OverlordSecurityResourceFilterTest extends ResourceFilterTestHelper
final String badRequestPath = WORD.matcher(requestPath).replaceAll("droid");
EasyMock.expect(request.getPath()).andReturn(badRequestPath).anyTimes();
EasyMock.replay(req, request, authorizerMapper);
- Assert.assertFalse(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(badRequestPath));
}
@After
diff --git a/server/src/main/java/org/apache/druid/server/http/security/AbstractResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/AbstractResourceFilter.java
index 1f2a245..4119e0b 100644
--- a/server/src/main/java/org/apache/druid/server/http/security/AbstractResourceFilter.java
+++ b/server/src/main/java/org/apache/druid/server/http/security/AbstractResourceFilter.java
@@ -92,6 +92,4 @@ public abstract class AbstractResourceFilter implements ResourceFilter, Containe
}
return action;
}
-
- public abstract boolean isApplicable(String requestPath);
}
diff --git a/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java
index 9ebe98d..7a45ca1 100644
--- a/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java
+++ b/server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java
@@ -68,13 +68,4 @@ public class ConfigResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- return requestPath.startsWith("druid/worker/v1") ||
- requestPath.startsWith("druid/indexer/v1") ||
- requestPath.startsWith("status/properties") ||
- requestPath.startsWith("druid/coordinator/v1/config");
- }
}
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 c46277b..e32446c 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
@@ -21,7 +21,6 @@ package org.apache.druid.server.http.security;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ContainerRequest;
@@ -34,7 +33,6 @@ import org.apache.druid.server.security.ResourceAction;
import org.apache.druid.server.security.ResourceType;
import javax.ws.rs.core.PathSegment;
-import java.util.List;
/**
* Use this ResourceFilter when the datasource information is present after "datasources" segment in the request Path
@@ -93,21 +91,4 @@ public class DatasourceResourceFilter extends AbstractResourceFilter
Preconditions.checkNotNull(dataSourceName);
return dataSourceName;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- List<String> applicablePaths = ImmutableList.of(
- "druid/coordinator/v1/datasources/",
- "druid/coordinator/v1/metadata/datasources/",
- "druid/v2/datasources/",
- "druid/indexer/v1/datasources"
- );
- for (String path : applicablePaths) {
- if (requestPath.startsWith(path) && !requestPath.equals(path)) {
- return true;
- }
- }
- return false;
- }
}
diff --git a/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java
index d59aa20..f314c77 100644
--- a/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java
+++ b/server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java
@@ -21,7 +21,6 @@ package org.apache.druid.server.http.security;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.inject.Inject;
import com.sun.jersey.spi.container.ContainerRequest;
@@ -34,7 +33,6 @@ import org.apache.druid.server.security.ResourceAction;
import org.apache.druid.server.security.ResourceType;
import javax.ws.rs.core.PathSegment;
-import java.util.List;
/**
@@ -89,16 +87,4 @@ public class RulesResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- List<String> applicablePaths = ImmutableList.of("druid/coordinator/v1/rules/");
- for (String path : applicablePaths) {
- if (requestPath.startsWith(path) && !requestPath.equals(path)) {
- return true;
- }
- }
- return false;
- }
}
diff --git a/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java b/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java
index cd97b52..b3231dc 100644
--- a/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java
+++ b/server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java
@@ -74,19 +74,4 @@ public class StateResourceFilter extends AbstractResourceFilter
return request;
}
-
- @Override
- public boolean isApplicable(String requestPath)
- {
- return requestPath.startsWith("druid/broker/v1") ||
- requestPath.startsWith("druid/coordinator/v1") ||
- requestPath.startsWith("druid/historical/v1") ||
- requestPath.startsWith("druid/indexer/v1") ||
- requestPath.startsWith("druid/coordinator/v1/rules") ||
- requestPath.startsWith("druid/coordinator/v1/tiers") ||
- requestPath.startsWith("druid/worker/v1") ||
- requestPath.startsWith("druid/coordinator/v1/servers") ||
- requestPath.startsWith("druid/v2") ||
- requestPath.startsWith("status");
- }
}
diff --git a/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java b/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
index a4c4156..0d3be7f 100644
--- a/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
+++ b/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
@@ -105,7 +105,6 @@ public class SecurityResourceFilterTest extends ResourceFilterTestHelper
{
setUpMockExpectations(requestPath, true, requestMethod);
EasyMock.replay(req, request, authorizerMapper);
- Assert.assertTrue(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(requestPath));
resourceFilter.getRequestFilter().filter(request);
EasyMock.verify(req, request, authorizerMapper);
}
@@ -115,7 +114,6 @@ public class SecurityResourceFilterTest extends ResourceFilterTestHelper
{
setUpMockExpectations(requestPath, false, requestMethod);
EasyMock.replay(req, request, authorizerMapper);
- Assert.assertTrue(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(requestPath));
try {
resourceFilter.getRequestFilter().filter(request);
Assert.fail();
@@ -125,13 +123,4 @@ public class SecurityResourceFilterTest extends ResourceFilterTestHelper
}
EasyMock.verify(req, request, authorizerMapper);
}
-
- @Test
- public void testResourcesFilteringBadPath()
- {
- EasyMock.replay(req, request, authorizerMapper);
- final String badRequestPath = WORD.matcher(requestPath).replaceAll("droid");
- Assert.assertFalse(((AbstractResourceFilter) resourceFilter.getRequestFilter()).isApplicable(badRequestPath));
- EasyMock.verify(req, request, authorizerMapper);
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org