Repository: aurora Updated Branches: refs/heads/master ddbb9657e -> ba174ba38
Remove TaskQuery from killTasks RPC. Bugs closed: AURORA-1591, AURORA-1592 Reviewed at https://reviews.apache.org/r/45701/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/ba174ba3 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/ba174ba3 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/ba174ba3 Branch: refs/heads/master Commit: ba174ba38cd4390723e475e2817f885426c83ad6 Parents: ddbb965 Author: Bill Farner <[email protected]> Authored: Wed Apr 6 11:55:41 2016 -0700 Committer: Bill Farner <[email protected]> Committed: Wed Apr 6 11:55:41 2016 -0700 ---------------------------------------------------------------------- .../thrift/org/apache/aurora/gen/api.thrift | 4 +- .../http/api/security/HttpSecurityModule.java | 5 +- .../ShiroAuthorizingParamInterceptor.java | 35 --------- .../thrift/SchedulerThriftInterface.java | 29 ++----- .../thrift/aop/AnnotatedAuroraAdmin.java | 2 - .../python/apache/aurora/client/api/__init__.py | 2 +- .../http/api/security/HttpSecurityIT.java | 49 +++--------- .../ShiroAuthorizingParamInterceptorTest.java | 79 +------------------- .../thrift/SchedulerThriftInterfaceTest.java | 47 +----------- src/test/python/apache/aurora/api_util.py | 2 +- .../aurora/client/api/test_scheduler_client.py | 14 ++-- 11 files changed, 36 insertions(+), 232 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/api/src/main/thrift/org/apache/aurora/gen/api.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/api.thrift b/api/src/main/thrift/org/apache/aurora/gen/api.thrift index 2412490..47ab500 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/api.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/api.thrift @@ -991,8 +991,8 @@ service AuroraSchedulerManager extends ReadOnlyScheduler { /** Restarts a batch of shards. */ Response restartShards(5: JobKey job, 3: set<i32> shardIds) - /** Initiates a kill on tasks. TODO(maxim): remove TaskQuery in AURORA-1591. */ - Response killTasks(1: TaskQuery query, 4: JobKey job, 5: set<i32> instances) + /** Initiates a kill on tasks. */ + Response killTasks(4: JobKey job, 5: set<i32> instances) /** * Adds new instances with the TaskConfig of the existing instance pointed by the key. http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java index e328620..5bba496 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java +++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java @@ -16,6 +16,7 @@ package org.apache.aurora.scheduler.http.api.security; import java.lang.reflect.Method; import java.util.Optional; import java.util.Set; + import javax.servlet.Filter; import com.google.common.annotations.VisibleForTesting; @@ -53,7 +54,6 @@ import static org.apache.aurora.scheduler.http.H2ConsoleModule.H2_PATH; import static org.apache.aurora.scheduler.http.H2ConsoleModule.H2_PERM; import static org.apache.aurora.scheduler.http.api.ApiModule.API_PATH; import static org.apache.aurora.scheduler.spi.Permissions.Domain.THRIFT_AURORA_ADMIN; -import static org.apache.aurora.scheduler.spi.Permissions.Domain.THRIFT_AURORA_SCHEDULER_MANAGER; import static org.apache.shiro.guice.web.ShiroWebModule.guiceFilterModule; import static org.apache.shiro.web.filter.authc.AuthenticatingFilter.PERMISSIVE; @@ -242,8 +242,7 @@ public class HttpSecurityModule extends ServletModule { AURORA_SCHEDULER_MANAGER_SERVICE.or(AURORA_ADMIN_SERVICE), authenticatingInterceptor); - MethodInterceptor apiInterceptor = new ShiroAuthorizingParamInterceptor( - THRIFT_AURORA_SCHEDULER_MANAGER); + MethodInterceptor apiInterceptor = new ShiroAuthorizingParamInterceptor(); requestInjection(apiInterceptor); bindInterceptor( Matchers.subclassesOf(AuroraSchedulerManager.Iface.class), http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java index d9039c9..b078ef0 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java +++ b/src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java @@ -49,13 +49,9 @@ import org.apache.aurora.gen.LockKey; import org.apache.aurora.gen.Response; import org.apache.aurora.gen.ResponseCode; import org.apache.aurora.gen.TaskConfig; -import org.apache.aurora.gen.TaskQuery; import org.apache.aurora.scheduler.base.JobKeys; -import org.apache.aurora.scheduler.base.Query; -import org.apache.aurora.scheduler.http.api.security.FieldGetter.AbstractFieldGetter; import org.apache.aurora.scheduler.http.api.security.FieldGetter.IdentityFieldGetter; import org.apache.aurora.scheduler.spi.Permissions; -import org.apache.aurora.scheduler.spi.Permissions.Domain; import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.thrift.Responses; import org.apache.shiro.authz.Permission; @@ -88,20 +84,6 @@ import static com.google.common.base.Preconditions.checkState; * is invoked, otherwise this interceptor will not allow the invocation to proceed. */ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { - @VisibleForTesting - static final FieldGetter<TaskQuery, JobKey> QUERY_TO_JOB_KEY = - new AbstractFieldGetter<TaskQuery, JobKey>(TaskQuery.class, JobKey.class) { - @Override - public Optional<JobKey> apply(TaskQuery input) { - Optional<Set<IJobKey>> targetJobs = JobKeys.from(Query.arbitrary(input)); - if (targetJobs.isPresent() && targetJobs.get().size() == 1) { - return Optional.of(Iterables.getOnlyElement(targetJobs.get())) - .transform(IJobKey::newBuilder); - } else { - return Optional.absent(); - } - } - }; private static class JobKeyGetter { private final int index; @@ -153,7 +135,6 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { LOCK_KEY_GETTER, JOB_UPDATE_KEY_GETTER, ADD_INSTANCES_CONFIG_GETTER, - QUERY_TO_JOB_KEY, INSTANCE_KEY_GETTER, new IdentityFieldGetter<>(JobKey.class)); @@ -285,18 +266,12 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { private final LoadingCache<Method, Function<Object[], Optional<JobKey>>> authorizingParamGetters = CacheBuilder.newBuilder().build(LOADER); - private final Domain domain; - private volatile boolean initialized; private Provider<Subject> subjectProvider; private AtomicLong authorizationFailures; private AtomicLong badRequests; - ShiroAuthorizingParamInterceptor(Domain domain) { - this.domain = requireNonNull(domain); - } - @Inject void initialize(Provider<Subject> newSubjectProvider, StatsProvider statsProvider) { checkState(!initialized); @@ -309,11 +284,6 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { } @VisibleForTesting - Permission makeWildcardPermission(String methodName) { - return Permissions.createUnscopedPermission(domain, methodName); - } - - @VisibleForTesting Permission makeTargetPermission(String methodName, IJobKey jobKey) { return Permissions.createJobScopedPermission(methodName, jobKey); } @@ -323,12 +293,7 @@ class ShiroAuthorizingParamInterceptor implements MethodInterceptor { checkState(initialized); Method method = invocation.getMethod(); - // Short-circuit request processing restrictions if the caller would be allowed to - // operate on every possible job key. This allows a broadly-scoped TaskQuery. Subject subject = subjectProvider.get(); - if (subject.isPermitted(makeWildcardPermission(method.getName()))) { - return invocation.proceed(); - } Optional<IJobKey> jobKey = authorizingParamGetters .getUnchecked(invocation.getMethod()) http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java index 9e6ea3c..86088d9 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -122,8 +122,6 @@ import org.slf4j.LoggerFactory; import static java.util.Objects.requireNonNull; -import static com.google.common.base.CharMatcher.WHITESPACE; - import static org.apache.aurora.common.base.MorePreconditions.checkNotBlank; import static org.apache.aurora.gen.ResponseCode.INVALID_REQUEST; import static org.apache.aurora.gen.ResponseCode.LOCK_ERROR; @@ -416,29 +414,14 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { } @Override - public Response killTasks( - @Nullable TaskQuery mutableQuery, - @Nullable JobKey mutableJob, - @Nullable Set<Integer> instances) { - - final Query.Builder query; + public Response killTasks(@Nullable JobKey mutableJob, @Nullable Set<Integer> instances) { Response response = empty(); - if (mutableQuery == null) { - IJobKey jobKey = JobKeys.assertValid(IJobKey.build(mutableJob)); - if (instances == null || Iterables.isEmpty(instances)) { - query = implicitKillQuery(Query.jobScoped(jobKey)); - } else { - query = implicitKillQuery(Query.instanceScoped(jobKey, instances)); - } + IJobKey jobKey = JobKeys.assertValid(IJobKey.build(mutableJob)); + Query.Builder query; + if (instances == null || Iterables.isEmpty(instances)) { + query = implicitKillQuery(Query.jobScoped(jobKey)); } else { - requireNonNull(mutableQuery); - addMessage(response, "The TaskQuery field is deprecated."); - - if (mutableQuery.getJobName() != null && WHITESPACE.matchesAllOf(mutableQuery.getJobName())) { - return invalidRequest(String.format("Invalid job name: '%s'", mutableQuery.getJobName())); - } - - query = implicitKillQuery(Query.arbitrary(mutableQuery)); + query = implicitKillQuery(Query.instanceScoped(jobKey, instances)); } return storage.write(storeProvider -> { http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java index f295992..73a7128 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java @@ -25,7 +25,6 @@ import org.apache.aurora.gen.JobKey; import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.JobUpdateRequest; import org.apache.aurora.gen.Response; -import org.apache.aurora.gen.TaskQuery; import org.apache.aurora.scheduler.http.api.security.AuthorizingParam; import org.apache.thrift.TException; @@ -62,7 +61,6 @@ public interface AnnotatedAuroraAdmin extends AuroraAdmin.Iface { @Override Response killTasks( - @AuthorizingParam @Nullable TaskQuery query, @AuthorizingParam @Nullable JobKey job, @Nullable Set<Integer> instances) throws TException; http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/main/python/apache/aurora/client/api/__init__.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/client/api/__init__.py b/src/main/python/apache/aurora/client/api/__init__.py index 18a10e2..294c590 100644 --- a/src/main/python/apache/aurora/client/api/__init__.py +++ b/src/main/python/apache/aurora/client/api/__init__.py @@ -111,7 +111,7 @@ class AuroraClientAPI(object): log.info("Instances to be killed: %s" % instances) instances = frozenset([int(s) for s in instances]) - return self._scheduler_proxy.killTasks(None, job_key.to_thrift(), instances) + return self._scheduler_proxy.killTasks(job_key.to_thrift(), instances) def check_status(self, job_key): self._assert_valid_job_key(job_key) http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java index 9031217..b20900d 100644 --- a/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java +++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java @@ -38,12 +38,10 @@ import org.apache.aurora.gen.JobKey; import org.apache.aurora.gen.Response; import org.apache.aurora.gen.ResponseCode; import org.apache.aurora.scheduler.base.JobKeys; -import org.apache.aurora.scheduler.base.Query; import org.apache.aurora.scheduler.http.AbstractJettyTest; import org.apache.aurora.scheduler.http.H2ConsoleModule; import org.apache.aurora.scheduler.http.api.ApiModule; import org.apache.aurora.scheduler.storage.entities.IJobKey; -import org.apache.aurora.scheduler.storage.entities.ITaskQuery; import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin; import org.apache.aurora.scheduler.thrift.aop.MockDecoratedThrift; import org.apache.http.HttpResponse; @@ -227,7 +225,7 @@ public class HttpSecurityIT extends AbstractJettyTest { private void assertKillTasksFails(AuroraAdmin.Client client) throws TException { try { - client.killTasks(null, null, null); + client.killTasks(null, null); fail("killTasks should fail."); } catch (TTransportException e) { // Expected. @@ -236,70 +234,45 @@ public class HttpSecurityIT extends AbstractJettyTest { @Test public void testAuroraSchedulerManager() throws TException, ServletException, IOException { - expect(auroraAdmin.killTasks(null, null, null)).andReturn(OK); - expect(auroraAdmin.killTasks(null, null, null)).andReturn(OK); - JobKey job = JobKeys.from("role", "env", "name").newBuilder(); - ITaskQuery jobScopedQuery = Query.jobScoped(IJobKey.build(job)).get(); - ITaskQuery adsScopedQuery = Query.jobScoped(ADS_STAGING_JOB).get(); - expect(auroraAdmin.killTasks(adsScopedQuery.newBuilder(), null, null)).andReturn(OK); - expect(auroraAdmin.killTasks(null, ADS_STAGING_JOB.newBuilder(), null)).andReturn(OK); - expectShiroAfterAuthFilter().times(24); + expect(auroraAdmin.killTasks(job, null)).andReturn(OK).times(2); + expect(auroraAdmin.killTasks(ADS_STAGING_JOB.newBuilder(), null)).andReturn(OK); + expectShiroAfterAuthFilter().atLeastOnce(); replayAndStart(); assertEquals( OK, - getAuthenticatedClient(WFARNER).killTasks(null, null, null)); + getAuthenticatedClient(WFARNER).killTasks(job, null)); assertEquals( OK, - getAuthenticatedClient(ROOT).killTasks(null, null, null)); + getAuthenticatedClient(ROOT).killTasks(job, null)); assertEquals( ResponseCode.INVALID_REQUEST, - getAuthenticatedClient(UNPRIVILEGED).killTasks(null, null, null).getResponseCode()); - assertEquals( - ResponseCode.AUTH_FAILED, - getAuthenticatedClient(UNPRIVILEGED) - .killTasks(jobScopedQuery.newBuilder(), null, null) - .getResponseCode()); + getAuthenticatedClient(UNPRIVILEGED).killTasks(null, null).getResponseCode()); assertEquals( ResponseCode.AUTH_FAILED, getAuthenticatedClient(UNPRIVILEGED) - .killTasks(null, job, null) + .killTasks(job, null) .getResponseCode()); assertEquals( ResponseCode.INVALID_REQUEST, - getAuthenticatedClient(BACKUP_SERVICE).killTasks(null, null, null).getResponseCode()); - assertEquals( - ResponseCode.AUTH_FAILED, - getAuthenticatedClient(BACKUP_SERVICE) - .killTasks(jobScopedQuery.newBuilder(), null, null) - .getResponseCode()); + getAuthenticatedClient(BACKUP_SERVICE).killTasks(null, null).getResponseCode()); assertEquals( ResponseCode.AUTH_FAILED, getAuthenticatedClient(BACKUP_SERVICE) - .killTasks(null, job, null) - .getResponseCode()); - assertEquals( - ResponseCode.AUTH_FAILED, - getAuthenticatedClient(DEPLOY_SERVICE) - .killTasks(jobScopedQuery.newBuilder(), null, null) + .killTasks(job, null) .getResponseCode()); assertEquals( ResponseCode.AUTH_FAILED, getAuthenticatedClient(DEPLOY_SERVICE) - .killTasks(null, job, null) + .killTasks(job, null) .getResponseCode()); assertEquals( OK, - getAuthenticatedClient(DEPLOY_SERVICE) - .killTasks(adsScopedQuery.newBuilder(), null, null)); - assertEquals( - OK, getAuthenticatedClient(DEPLOY_SERVICE).killTasks( - null, ADS_STAGING_JOB.newBuilder(), null)); http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java index 503f0c3..05f4a18 100644 --- a/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java +++ b/src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java @@ -18,7 +18,6 @@ import java.util.concurrent.atomic.AtomicLong; import com.google.common.base.Function; import com.google.common.base.Optional; -import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.UncheckedExecutionException; import com.google.inject.AbstractModule; import com.google.inject.Guice; @@ -33,7 +32,6 @@ import org.apache.aurora.gen.Response; import org.apache.aurora.gen.ResponseCode; import org.apache.aurora.gen.TaskQuery; import org.apache.aurora.scheduler.base.JobKeys; -import org.apache.aurora.scheduler.spi.Permissions.Domain; import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.thrift.Responses; import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin; @@ -43,17 +41,13 @@ import org.apache.thrift.TException; import org.junit.Before; import org.junit.Test; -import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.QUERY_TO_JOB_KEY; import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.SHIRO_AUTHORIZATION_FAILURES; import static org.apache.aurora.scheduler.http.api.security.ShiroAuthorizingParamInterceptor.SHIRO_BAD_REQUESTS; import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { - private static final Domain DOMAIN = Domain.THRIFT_AURORA_SCHEDULER_MANAGER; - private ShiroAuthorizingParamInterceptor interceptor; private Subject subject; @@ -66,7 +60,7 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { @Before public void setUp() { - interceptor = new ShiroAuthorizingParamInterceptor(DOMAIN); + interceptor = new ShiroAuthorizingParamInterceptor(); subject = createMock(Subject.class); statsProvider = createMock(StatsProvider.class); thrift = createMock(AnnotatedAuroraAdmin.class); @@ -110,8 +104,6 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { JobConfiguration jobConfiguration = new JobConfiguration().setKey(JOB_KEY.newBuilder()); Response response = Responses.ok(); - expect(subject.isPermitted(interceptor.makeWildcardPermission("createJob"))) - .andReturn(false); expect(subject .isPermitted(interceptor.makeTargetPermission("createJob", JOB_KEY))) .andReturn(true); @@ -123,25 +115,7 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { } @Test - public void testKillTasksWithWildcardPermission() throws TException { - Response response = Responses.ok(); - - // TODO(maxim): Remove wildcard (unscoped) permissions when TaskQuery is gone from killTasks - // AURORA-1592. - expect(subject.isPermitted(interceptor.makeWildcardPermission("killTasks"))) - .andReturn(true); - expect(thrift.killTasks(new TaskQuery(), null, null)) - .andReturn(response); - - replayAndInitialize(); - - assertSame(response, decoratedThrift.killTasks(new TaskQuery(), null, null)); - } - - @Test public void testKillTasksWithTargetedPermission() throws TException { - expect(subject.isPermitted(interceptor.makeWildcardPermission("killTasks"))) - .andReturn(false); expect(subject.isPermitted(interceptor.makeTargetPermission("killTasks", JOB_KEY))) .andReturn(false); expect(subject.getPrincipal()).andReturn("zmanji"); @@ -150,72 +124,27 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { assertEquals( ResponseCode.AUTH_FAILED, - decoratedThrift.killTasks(null, JOB_KEY.newBuilder(), null).getResponseCode()); + decoratedThrift.killTasks(JOB_KEY.newBuilder(), null).getResponseCode()); } @Test public void testKillTasksInvalidJobKey() throws TException { - expect(subject.isPermitted(interceptor.makeWildcardPermission("killTasks"))) - .andReturn(false); - replayAndInitialize(); assertEquals( ResponseCode.INVALID_REQUEST, decoratedThrift.killTasks( - null, JOB_KEY.newBuilder().setName(null), null).getResponseCode()); } @Test - public void testExtractTaskQuerySingleJobKey() { - replayAndInitialize(); - - assertEquals( - JOB_KEY.newBuilder(), - QUERY_TO_JOB_KEY - .apply(new TaskQuery() - .setRole(JOB_KEY.getRole()) - .setEnvironment(JOB_KEY.getEnvironment()) - .setJobName(JOB_KEY.getName())) - .orNull()); - - assertEquals( - JOB_KEY.newBuilder(), - QUERY_TO_JOB_KEY.apply(new TaskQuery().setJobKeys(ImmutableSet.of(JOB_KEY.newBuilder()))) - .orNull()); - } - - @Test - public void testExtractTaskQueryBroadlyScoped() { - control.replay(); - - assertNull(QUERY_TO_JOB_KEY.apply(new TaskQuery().setRole("role")).orNull()); - } - - @Test - public void testExtractTaskQueryMultiScoped() { - // TODO(ksweeney): Reconsider behavior here, this is possibly too restrictive as it - // will mean that only admins are authorized to operate on multiple jobs at once regardless - // of whether they share a common role. - control.replay(); - - assertNull(QUERY_TO_JOB_KEY - .apply( - new TaskQuery().setJobKeys( - ImmutableSet.of(JOB_KEY.newBuilder(), JOB_KEY.newBuilder().setName("other")))) - .orNull()); - } - - @Test public void testHandlesMultipleAnnotations() { control.replay(); Function<Object[], Optional<JobKey>> func = interceptor.getAuthorizingParamGetters().getUnchecked(Params.class.getMethods()[0]); - func.apply(new Object[]{new TaskQuery(), null, null}); func.apply(new Object[]{null, new JobKey(), null}); func.apply(new Object[]{null, null, new JobUpdateRequest()}); } @@ -227,7 +156,7 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { Function<Object[], Optional<JobKey>> func = interceptor.getAuthorizingParamGetters().getUnchecked(Params.class.getMethods()[0]); - func.apply(new Object[]{new TaskQuery(), new JobKey(), null}); + func.apply(new Object[]{new JobConfiguration(), new JobKey(), null}); } @Test(expected = UncheckedExecutionException.class) @@ -254,7 +183,7 @@ public class ShiroAuthorizingParamInterceptorTest extends EasyMockTest { private interface Params { Response test( - @AuthorizingParam TaskQuery query, + @AuthorizingParam JobConfiguration jobConfig, @AuthorizingParam JobKey job, @AuthorizingParam JobUpdateRequest request); } http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java index 2b557e2..3facb13 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -70,7 +70,6 @@ import org.apache.aurora.gen.ScheduledTask; import org.apache.aurora.gen.StartJobUpdateResult; import org.apache.aurora.gen.TaskConfig; import org.apache.aurora.gen.TaskConstraint; -import org.apache.aurora.gen.TaskQuery; import org.apache.aurora.gen.ValueConstraint; import org.apache.aurora.scheduler.TaskIdGenerator; import org.apache.aurora.scheduler.base.JobKeys; @@ -590,20 +589,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { } @Test - public void testKillByJobName() throws Exception { - Query.Builder query = Query.arbitrary(new TaskQuery().setJobName("job")).active(); - storageUtil.expectTaskFetch(query, buildScheduledTask()); - lockManager.assertNotLocked(LOCK_KEY); - expectTransitionsToKilling(); - - control.replay(); - - Response response = thrift.killTasks(query.get().newBuilder(), null, null); - assertOkResponse(response); - assertMessageMatches(response, "The TaskQuery field is deprecated."); - } - - @Test public void testJobScopedKillsActive() throws Exception { Query.Builder query = Query.unscoped().byJob(JOB_KEY).active(); storageUtil.expectTaskFetch(query, buildScheduledTask()); @@ -612,7 +597,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertOkResponse(thrift.killTasks(null, JOB_KEY.newBuilder(), null)); + assertOkResponse(thrift.killTasks(JOB_KEY.newBuilder(), null)); } @Test @@ -624,7 +609,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - assertOkResponse(thrift.killTasks(null, JOB_KEY.newBuilder(), ImmutableSet.of(1))); + assertOkResponse(thrift.killTasks(JOB_KEY.newBuilder(), ImmutableSet.of(1))); } @Test @@ -642,31 +627,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { assertResponse( LOCK_ERROR, - thrift.killTasks(null, JOB_KEY.newBuilder(), null)); - } - - @Test - public void testKillByTaskId() throws Exception { - // A non-admin user may kill their own tasks when specified by task IDs. - Query.Builder query = Query.taskScoped("taskid").active(); - // This query happens twice - once for authentication (without consistency) and once again - // to perform the state change (within a write transaction). - storageUtil.expectTaskFetch(query, buildScheduledTask()); - lockManager.assertNotLocked(LOCK_KEY); - expectTransitionsToKilling(); - - control.replay(); - - assertOkResponse(thrift.killTasks(query.get().newBuilder(), null, null)); - } - - @Test - public void testKillTasksInvalidJobName() throws Exception { - TaskQuery query = new TaskQuery().setJobName(""); - - control.replay(); - - assertResponse(INVALID_REQUEST, thrift.killTasks(query, null, null)); + thrift.killTasks(JOB_KEY.newBuilder(), null)); } @Test @@ -676,7 +637,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { control.replay(); - Response response = thrift.killTasks(null, JOB_KEY.newBuilder(), null); + Response response = thrift.killTasks(JOB_KEY.newBuilder(), null); assertOkResponse(response); assertMessageMatches(response, SchedulerThriftInterface.NO_TASKS_TO_KILL_MESSAGE); } http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/test/python/apache/aurora/api_util.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/api_util.py b/src/test/python/apache/aurora/api_util.py index 1497332..4fde4ba 100644 --- a/src/test/python/apache/aurora/api_util.py +++ b/src/test/python/apache/aurora/api_util.py @@ -85,7 +85,7 @@ class SchedulerThriftApiSpec(ReadOnlyScheduler.Iface): def restartShards(self, job, shardIds): pass - def killTasks(self, query, jobKey, instances): + def killTasks(self, jobKey, instances): pass def addInstances(self, config, key, count): http://git-wip-us.apache.org/repos/asf/aurora/blob/ba174ba3/src/test/python/apache/aurora/client/api/test_scheduler_client.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/client/api/test_scheduler_client.py b/src/test/python/apache/aurora/client/api/test_scheduler_client.py index 0b14b09..bd7c834 100644 --- a/src/test/python/apache/aurora/client/api/test_scheduler_client.py +++ b/src/test/python/apache/aurora/client/api/test_scheduler_client.py @@ -137,12 +137,9 @@ class TestSchedulerProxyInjection(unittest.TestCase): self.make_scheduler_proxy().getJobs(ROLE) def test_killTasks(self): - self.mock_thrift_client.killTasks( - IgnoreArg(), - IsA(JobKey), - IgnoreArg()).AndReturn(DEFAULT_RESPONSE) + self.mock_thrift_client.killTasks(IsA(JobKey), IgnoreArg()).AndReturn(DEFAULT_RESPONSE) self.mox.ReplayAll() - self.make_scheduler_proxy().killTasks(None, JobKey(), {0}) + self.make_scheduler_proxy().killTasks(JobKey(), {0}) def test_getQuota(self): self.mock_thrift_client.getQuota(IgnoreArg()).AndReturn(DEFAULT_RESPONSE) @@ -195,12 +192,11 @@ class TestSchedulerProxyInjection(unittest.TestCase): self.make_scheduler_proxy().pulseJobUpdate('update_id') def test_raise_auth_error(self): - self.mock_thrift_client.killTasks(TaskQuery(), None, None).AndRaise( - TRequestsTransport.AuthError()) + self.mock_thrift_client.killTasks(None, None).AndRaise(TRequestsTransport.AuthError()) self.mock_scheduler_client.get_failed_auth_message().AndReturn('failed auth') self.mox.ReplayAll() with pytest.raises(scheduler_client.SchedulerProxy.AuthError): - self.make_scheduler_proxy().killTasks(TaskQuery(), None, None) + self.make_scheduler_proxy().killTasks(None, None) class TestSchedulerProxyAdminInjection(TestSchedulerProxyInjection): @@ -460,7 +456,7 @@ class TestSchedulerClient(unittest.TestCase): client.get.return_value = mock_scheduler_client proxy = scheduler_client.SchedulerProxy(Cluster(name='local')) - proxy.killTasks(None, JobKey(), None) + proxy.killTasks(JobKey(), None) assert mock_thrift_client.killTasks.call_count == 3
