Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

remove AbstractResourceFilter.isApplicable because it is not (#6691) 

* remove AbstractResourceFilter.isApplicable because it is not, add tests for OverlordResource.doShutdown and OverlordResource.shutdownTasksForDatasource

* cleanup
This commit is contained in:
Clint Wylie 2018-12-01 05:52:31 -08:00 committed by Benedict Jin
parent e2bedab665
commit 43adb391c2
11 changed files with 85 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/BasicSecurityResourceFilter.java
View File

@ -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;
}
}

15
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/SupervisorResourceFilter.java
View File

@ -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;
}
}

14
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/TaskResourceFilter.java
View File

@ -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;
}
}

102
indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/OverlordResourceTest.java
View File

@ -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
}
}
}

5
indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
View File

@ -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

2
server/src/main/java/org/apache/druid/server/http/security/AbstractResourceFilter.java
View File

@ -92,6 +92,4 @@ public abstract class AbstractResourceFilter implements ResourceFilter, Containe
}
return action;
}
public abstract boolean isApplicable(String requestPath);
}

9
server/src/main/java/org/apache/druid/server/http/security/ConfigResourceFilter.java
View File

@ -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");
}
}

19
server/src/main/java/org/apache/druid/server/http/security/DatasourceResourceFilter.java
View File

@ -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;
}
}

14
server/src/main/java/org/apache/druid/server/http/security/RulesResourceFilter.java
View File

@ -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;
}
}

15
server/src/main/java/org/apache/druid/server/http/security/StateResourceFilter.java
View File

@ -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");
}
}

11
server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
View File

@ -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);
}
}