aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kevi...@apache.org
Subject aurora git commit: Use Shiro to generate audit messages when available.
Date Wed, 30 Sep 2015 23:07:17 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 488f29f70 -> 33d7e2170


Use Shiro to generate audit messages when available.

This ensures that Shiro is always used to generate audit messages when available.

Testing Done:
./gradlew build

Reviewed at https://reviews.apache.org/r/38906/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/33d7e217
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/33d7e217
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/33d7e217

Branch: refs/heads/master
Commit: 33d7e2170a86f54722a02a2dc9cb1e09fb52df25
Parents: 488f29f
Author: Kevin Sweeney <kevints@apache.org>
Authored: Wed Sep 30 16:06:53 2015 -0700
Committer: Kevin Sweeney <kevints@apache.org>
Committed: Wed Sep 30 16:06:53 2015 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/thrift/AuditMessages.java  | 57 ++++++++++++++++++++
 .../thrift/SchedulerThriftInterface.java        | 26 +++------
 .../scheduler/thrift/AuditMessagesTest.java     | 51 ++++++++++++++++++
 .../thrift/SchedulerThriftInterfaceTest.java    | 18 ++++---
 .../aurora/scheduler/thrift/ThriftIT.java       |  8 +++
 5 files changed, 134 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java b/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java
new file mode 100644
index 0000000..3a29a62
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/AuditMessages.java
@@ -0,0 +1,57 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.thrift;
+
+import java.util.Objects;
+import java.util.Optional;
+
+import javax.inject.Inject;
+import javax.inject.Provider;
+
+import com.google.common.annotations.VisibleForTesting;
+
+import org.apache.shiro.subject.Subject;
+
+/**
+ * Generates audit messages with usernames from Shiro if available and falls back to
+ * caller-specified ones if not.
+ */
+@VisibleForTesting
+class AuditMessages {
+  private final Provider<Optional<Subject>> subjectProvider;
+
+  @Inject
+  AuditMessages(Provider<Optional<Subject>> subjectProvider) {
+    this.subjectProvider = Objects.requireNonNull(subjectProvider);
+  }
+
+  private String getShiroUserNameOr(String defaultUser) {
+    return subjectProvider.get()
+        .map(Subject::getPrincipal)
+        .map(Object::toString)
+        .orElse(defaultUser);
+  }
+
+  com.google.common.base.Optional<String> transitionedBy(String user) {
+    return com.google.common.base.Optional.of("Transition forced by " + getShiroUserNameOr(user));
+  }
+
+  com.google.common.base.Optional<String> killedBy(String user) {
+    return com.google.common.base.Optional.of("Killed by " + getShiroUserNameOr(user));
+  }
+
+  com.google.common.base.Optional<String> restartedBy(String user) {
+    return com.google.common.base.Optional.of("Restarted by " + getShiroUserNameOr(user));
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/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 d69bc39..14b5a00 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -202,6 +202,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
   private final UUIDGenerator uuidGenerator;
   private final JobUpdateController jobUpdateController;
   private final ReadOnlyScheduler.Iface readOnlyScheduler;
+  private final AuditMessages auditMessages;
 
   @Inject
   SchedulerThriftInterface(
@@ -217,7 +218,8 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
       TaskIdGenerator taskIdGenerator,
       UUIDGenerator uuidGenerator,
       JobUpdateController jobUpdateController,
-      ReadOnlyScheduler.Iface readOnlyScheduler) {
+      ReadOnlyScheduler.Iface readOnlyScheduler,
+      AuditMessages auditMessages) {
 
     this.storage = requireNonNull(storage);
     this.lockManager = requireNonNull(lockManager);
@@ -232,6 +234,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
     this.uuidGenerator = requireNonNull(uuidGenerator);
     this.jobUpdateController = requireNonNull(jobUpdateController);
     this.readOnlyScheduler = requireNonNull(readOnlyScheduler);
+    this.auditMessages = requireNonNull(auditMessages);
   }
 
   @Override
@@ -583,7 +586,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
               taskId,
               Optional.absent(),
               ScheduleStatus.KILLING,
-              killedByMessage(context.getIdentity()));
+              auditMessages.killedBy(context.getIdentity()));
         }
 
         return tasksKilled
@@ -639,7 +642,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
                   taskId,
                   Optional.absent(),
                   ScheduleStatus.RESTARTING,
-                  restartedByMessage(context.getIdentity()));
+                  auditMessages.restartedBy(context.getIdentity()));
             }
           }
         });
@@ -736,7 +739,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
             taskId,
             Optional.absent(),
             status,
-            transitionMessage(context.getIdentity()));
+            auditMessages.transitionedBy(context.getIdentity()));
       }
     });
 
@@ -1343,21 +1346,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
   }
 
   @VisibleForTesting
-  static Optional<String> transitionMessage(String user) {
-    return Optional.of("Transition forced by " + user);
-  }
-
-  @VisibleForTesting
-  static Optional<String> killedByMessage(String user) {
-    return Optional.of("Killed by " + user);
-  }
-
-  @VisibleForTesting
-  static Optional<String> restartedByMessage(String user) {
-    return Optional.of("Restarted by " + user);
-  }
-
-  @VisibleForTesting
   static String noCronScheduleMessage(IJobKey jobKey) {
     return String.format("Job %s has no cron schedule", JobKeys.canonicalString(jobKey));
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java
new file mode 100644
index 0000000..0f8748d
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/AuditMessagesTest.java
@@ -0,0 +1,51 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.thrift;
+
+import java.util.Optional;
+
+import org.apache.aurora.common.testing.easymock.EasyMockTest;
+import org.apache.shiro.subject.Subject;
+import org.junit.Test;
+
+import static org.easymock.EasyMock.expect;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertThat;
+
+public class AuditMessagesTest extends EasyMockTest {
+  @Test
+  public void testEmptySubject() {
+    AuditMessages emptyMessages = new AuditMessages(Optional::empty);
+
+    control.replay();
+
+    assertThat(emptyMessages.killedBy("legacy").get(), containsString("legacy"));
+    assertThat(emptyMessages.restartedBy("legacy").get(), containsString("legacy"));
+    assertThat(emptyMessages.transitionedBy("legacy").get(), containsString("legacy"));
+  }
+
+  @Test
+  public void testPresentSubject() {
+    Subject subject = createMock(Subject.class);
+    AuditMessages presentMessages = new AuditMessages(() -> Optional.of(subject));
+
+    expect(subject.getPrincipal()).andReturn("shiro").times(3);
+
+    control.replay();
+
+    assertThat(presentMessages.killedBy("legacy").get(), containsString("shiro"));
+    assertThat(presentMessages.restartedBy("legacy").get(), containsString("shiro"));
+    assertThat(presentMessages.transitionedBy("legacy").get(), containsString("shiro"));
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/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 ac89618..83f65a7 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -159,11 +159,8 @@ import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAX_TA
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NOOP_JOB_UPDATE_MESSAGE;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NO_CRON;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.jobAlreadyExistsMessage;
-import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.killedByMessage;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.noCronScheduleMessage;
 import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.notScheduledCronMessage;
-import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.restartedByMessage;
-import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.transitionMessage;
 import static org.easymock.EasyMock.anyInt;
 import static org.easymock.EasyMock.anyObject;
 import static org.easymock.EasyMock.eq;
@@ -195,6 +192,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   private UUIDGenerator uuidGenerator;
   private JobUpdateController jobUpdateController;
   private ReadOnlyScheduler.Iface readOnlyScheduler;
+  private AuditMessages auditMessages;
 
   @Before
   public void setUp() throws Exception {
@@ -214,6 +212,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     uuidGenerator = createMock(UUIDGenerator.class);
     jobUpdateController = createMock(JobUpdateController.class);
     readOnlyScheduler = createMock(ReadOnlyScheduler.Iface.class);
+    auditMessages = createMock(AuditMessages.class);
 
     thrift = getResponseProxy(
         new SchedulerThriftInterface(
@@ -229,7 +228,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
             taskIdGenerator,
             uuidGenerator,
             jobUpdateController,
-            readOnlyScheduler));
+            readOnlyScheduler,
+            auditMessages));
   }
 
   private static AuroraAdmin.Iface getResponseProxy(final AuroraAdmin.Iface realThrift) {
@@ -644,12 +644,13 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   private void expectTransitionsToKilling() {
+    expect(auditMessages.killedBy(USER)).andReturn(Optional.of("test"));
     expect(stateManager.changeState(
         storageUtil.mutableStoreProvider,
         TASK_ID,
         Optional.absent(),
         ScheduleStatus.KILLING,
-        killedByMessage(USER))).andReturn(StateChangeResult.SUCCESS);
+        Optional.of("test"))).andReturn(StateChangeResult.SUCCESS);
   }
 
   @Test
@@ -874,12 +875,13 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   public void testForceTaskState() throws Exception {
     ScheduleStatus status = ScheduleStatus.FAILED;
 
+    expect(auditMessages.transitionedBy(USER)).andReturn(Optional.of("test"));
     expect(stateManager.changeState(
         storageUtil.mutableStoreProvider,
         TASK_ID,
         Optional.absent(),
         ScheduleStatus.FAILED,
-        Optional.of(transitionMessage(USER).get()))).andReturn(StateChangeResult.SUCCESS);
+        Optional.of("test"))).andReturn(StateChangeResult.SUCCESS);
 
     expectAuth(ROOT, true);
     expectAuth(ROOT, false);
@@ -972,12 +974,14 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
         Query.instanceScoped(JOB_KEY, shards).active(),
         buildScheduledTask());
 
+    expect(auditMessages.restartedBy(USER))
+        .andReturn(Optional.of("test"));
     expect(stateManager.changeState(
         storageUtil.mutableStoreProvider,
         TASK_ID,
         Optional.absent(),
         ScheduleStatus.RESTARTING,
-        restartedByMessage(USER))).andReturn(StateChangeResult.SUCCESS);
+        Optional.of("test"))).andReturn(StateChangeResult.SUCCESS);
 
     control.replay();
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/33d7e217/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
index 4bd8b12..f06481b 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
@@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.thrift;
 
 import java.util.Arrays;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import com.google.common.collect.ImmutableMap;
@@ -22,6 +23,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.google.inject.Provides;
 
 import org.apache.aurora.auth.CapabilityValidator;
 import org.apache.aurora.auth.CapabilityValidator.Capability;
@@ -50,6 +52,7 @@ import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
 import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.apache.aurora.scheduler.thrift.auth.ThriftAuthModule;
 import org.apache.aurora.scheduler.updater.JobUpdateController;
+import org.apache.shiro.subject.Subject;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -173,6 +176,11 @@ public class ThriftIT extends EasyMockTest {
             bind(IServerInfo.class).toInstance(IServerInfo.build(new ServerInfo()));
             bindMock(CronPredictor.class);
           }
+
+          @Provides
+          Optional<Subject> provideSubject() {
+            return Optional.of(createMock(Subject.class));
+          }
         }
     );
     thrift = injector.getInstance(AnnotatedAuroraAdmin.class);


Mime
View raw message