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 2014/10/16 23:52:49 UTC
git commit: Fix redirect loop when accessing rewritten paths.
Repository: incubator-aurora
Updated Branches:
refs/heads/master 6fea534df -> b931a5c7b
Fix redirect loop when accessing rewritten paths.
Testing Done:
./gradlew build -Pq
Also started up multiple schedulers in vagrant image and confirmed
that I'm properly redirected when accessing the non-leading scheduler.
Bugs closed: AURORA-841
Reviewed at https://reviews.apache.org/r/26790/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/b931a5c7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/b931a5c7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/b931a5c7
Branch: refs/heads/master
Commit: b931a5c7b44018d24f7dc4c203615bbe22820f4d
Parents: 6fea534
Author: Joshua Cohen <jc...@twopensource.com>
Authored: Thu Oct 16 14:48:59 2014 -0700
Committer: Kevin Sweeney <ke...@apache.org>
Committed: Thu Oct 16 14:48:59 2014 -0700
----------------------------------------------------------------------
.../scheduler/http/JettyServerModule.java | 6 +-
.../aurora/scheduler/http/LeaderRedirect.java | 7 +-
.../scheduler/http/LeaderRedirectTest.java | 83 ++++++++++++++++++--
3 files changed, 87 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/b931a5c7/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java b/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
index fe1e111..958f60c 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
@@ -86,9 +86,12 @@ import static com.twitter.common.application.modules.LocalServiceRegistry.LocalS
* requests (for static assets) similar to what we currently do with path specs.
*/
public class JettyServerModule extends AbstractModule {
-
private static final Logger LOG = Logger.getLogger(JettyServerModule.class.getName());
+ // The name of the request attribute where the path for the current request before it was
+ // rewritten is stored.
+ static final String ORIGINAL_PATH_ATTRIBUTE_NAME = "originalPath";
+
@Nonnegative
@CmdLine(name = "http_port",
help = "The port to start an HTTP server on. Default value will choose a random port.")
@@ -163,6 +166,7 @@ public class JettyServerModule extends AbstractModule {
private RewriteHandler getRewriteHandler(HandlerCollection rootHandler) {
RewriteHandler rewrites = new RewriteHandler();
+ rewrites.setOriginalPathAttribute(ORIGINAL_PATH_ATTRIBUTE_NAME);
rewrites.setRewriteRequestURI(true);
rewrites.setRewritePathInfo(true);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/b931a5c7/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
index e50de1e..cf17385 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java
@@ -130,7 +130,12 @@ public class LeaderRedirect {
.append(target.getHostText())
.append(':')
.append(target.getPort())
- .append(req.getRequestURI());
+ .append(
+ // If Jetty rewrote the path, we want to be sure to redirect to the original path
+ // rather than the rewritten path to be sure it's a route the UI code recognizes.
+ Optional.fromNullable(
+ req.getAttribute(JettyServerModule.ORIGINAL_PATH_ATTRIBUTE_NAME))
+ .or(req.getRequestURI()));
String queryString = req.getQueryString();
if (queryString != null) {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/b931a5c7/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java b/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java
index 53c7088..c3bf5ea 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java
@@ -16,6 +16,8 @@ package org.apache.aurora.scheduler.http;
import java.util.Arrays;
import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+
import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.collect.ImmutableMap;
@@ -82,20 +84,26 @@ public class LeaderRedirectTest extends EasyMockTest {
monitorCapture = new Capture<>();
expect(schedulers.watch(capture(monitorCapture))).andReturn(null);
+ }
+
+ private void replayAndMonitor() throws Exception {
control.replay();
leaderRedirector.monitor();
}
- @Ignore("http://jira.local.twitter.com/browse/MESOS-323")
+ @Ignore("https://issues.apache.org/jira/browse/AURORA-842")
@Test
- public void testLeader() {
+ public void testLeader() throws Exception {
+ replayAndMonitor();
publishSchedulers(localPort(HTTP_PORT));
assertEquals(Optional.<HostAndPort>absent(), leaderRedirector.getRedirect());
}
@Test
- public void testNotLeader() {
+ public void testNotLeader() throws Exception {
+ replayAndMonitor();
+
HostAndPort remote = HostAndPort.fromParts("foobar", HTTP_PORT);
publishSchedulers(remote);
@@ -103,7 +111,9 @@ public class LeaderRedirectTest extends EasyMockTest {
}
@Test
- public void testLeaderOnSameHost() {
+ public void testLeaderOnSameHost() throws Exception {
+ replayAndMonitor();
+
HostAndPort local = localPort(555);
publishSchedulers(local);
@@ -111,19 +121,25 @@ public class LeaderRedirectTest extends EasyMockTest {
}
@Test
- public void testNoLeaders() {
+ public void testNoLeaders() throws Exception {
+ replayAndMonitor();
+
assertEquals(Optional.<HostAndPort>absent(), leaderRedirector.getRedirect());
}
@Test
- public void testMultipleLeaders() {
+ public void testMultipleLeaders() throws Exception {
+ replayAndMonitor();
+
publishSchedulers(HostAndPort.fromParts("foobar", 500), HostAndPort.fromParts("baz", 800));
assertEquals(Optional.<HostAndPort>absent(), leaderRedirector.getRedirect());
}
@Test
- public void testBadServiceInstance() {
+ public void testBadServiceInstance() throws Exception {
+ replayAndMonitor();
+
ServiceInstance badLocal = new ServiceInstance()
.setAdditionalEndpoints(ImmutableMap.of("foo", new Endpoint("localhost", 500)));
@@ -132,6 +148,59 @@ public class LeaderRedirectTest extends EasyMockTest {
assertEquals(Optional.<HostAndPort>absent(), leaderRedirector.getRedirect());
}
+ private HttpServletRequest mockRequest(String attributeValue, String queryString) {
+ HttpServletRequest mockRequest = createMock(HttpServletRequest.class);
+ expect(mockRequest.getScheme()).andReturn("http");
+ expect(mockRequest.getAttribute(JettyServerModule.ORIGINAL_PATH_ATTRIBUTE_NAME))
+ .andReturn(attributeValue);
+ expect(mockRequest.getRequestURI()).andReturn("/some/path");
+ expect(mockRequest.getQueryString()).andReturn(queryString);
+
+ return mockRequest;
+ }
+
+ @Test
+ public void testRedirectTargetNoAttribute() throws Exception {
+ HttpServletRequest mockRequest = mockRequest(null, null);
+
+ replayAndMonitor();
+
+ HostAndPort remote = HostAndPort.fromParts("foobar", HTTP_PORT);
+ publishSchedulers(remote);
+
+ assertEquals(
+ Optional.of("http://foobar:500/some/path"),
+ leaderRedirector.getRedirectTarget(mockRequest));
+ }
+
+ @Test
+ public void testRedirectTargetWithAttribute() throws Exception {
+ HttpServletRequest mockRequest = mockRequest("/the/original/path", null);
+
+ replayAndMonitor();
+
+ HostAndPort remote = HostAndPort.fromParts("foobar", HTTP_PORT);
+ publishSchedulers(remote);
+
+ assertEquals(
+ Optional.of("http://foobar:500/the/original/path"),
+ leaderRedirector.getRedirectTarget(mockRequest));
+ }
+
+ @Test
+ public void testRedirectTargetQueryString() throws Exception {
+ HttpServletRequest mockRequest = mockRequest(null, "bar=baz");
+
+ replayAndMonitor();
+
+ HostAndPort remote = HostAndPort.fromParts("foobar", HTTP_PORT);
+ publishSchedulers(remote);
+
+ assertEquals(
+ Optional.of("http://foobar:500/some/path?bar=baz"),
+ leaderRedirector.getRedirectTarget(mockRequest));
+ }
+
private void publishSchedulers(HostAndPort... schedulerHttpEndpoints) {
publishSchedulers(ImmutableSet.copyOf(Iterables.transform(Arrays.asList(schedulerHttpEndpoints),
CREATE_INSTANCE)));