You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ke...@apache.org on 2015/10/01 01:07:17 UTC
aurora git commit: Use Shiro to generate audit messages when
available.
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 <ke...@apache.org>
Authored: Wed Sep 30 16:06:53 2015 -0700
Committer: Kevin Sweeney <ke...@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);