asdf2014 closed pull request #6691: remove AbstractResourceFilter.isApplicable
because it is not
URL: https://github.com/apache/incubator-druid/pull/6691
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 abb904c2564..8294ec1c6bd 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 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 ContainerRequest filter(ContainerRequest request)
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 5179a89cd1c..e834c2e875e 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.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 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 boolean apply(PathSegment input)
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 26c927f02fc..8109961558c 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 @@
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 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 boolean apply(PathSegment input)
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 52955f09c25..4558de2863e 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.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 Access authorize(AuthenticationResult
authenticationResult, Resource reso
);
}
- 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 void testGetTaskStatus() throws Exception
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 String getDataSource()
}
}
-
}
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 27dd6a38fe7..16c8afea185 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.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 void testResourcesFilteringAccess()
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 void testDatasourcesResourcesFilteringNoAccess()
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 void testDatasourcesResourcesFilteringBadPath()
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 1f2a245dd19..4119e0b9b74 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 @@ protected Action getAction(ContainerRequest request)
}
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 9ebe98d4c7a..7a45ca1d5bb 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 ContainerRequest filter(ContainerRequest request)
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 c46277b18f7..e32446cfe5c 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 @@
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.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 boolean apply(PathSegment input)
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 d59aa20de11..f314c77d743 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 @@
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.ResourceType;
import javax.ws.rs.core.PathSegment;
-import java.util.List;
/**
@@ -89,16 +87,4 @@ public boolean apply(PathSegment input)
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 cd97b5289ae..b3231dc2b37 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 ContainerRequest filter(ContainerRequest request)
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 a4c4156560d..0d3be7fce0b 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 void testResourcesFilteringAccess()
{
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 void testResourcesFilteringNoAccess()
{
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 void testResourcesFilteringNoAccess()
}
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);
- }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
