aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: Remove TaskQuery from killTasks RPC.
Date Wed, 06 Apr 2016 18:55:36 GMT
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 <wfarner@apache.org>
Authored: Wed Apr 6 11:55:41 2016 -0700
Committer: Bill Farner <wfarner@apache.org>
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
 


Mime
View raw message