From 34424e98a618a9fefce800746168be2b72e17de9 Mon Sep 17 00:00:00 2001 From: Junping Du Date: Tue, 14 Mar 2017 02:56:18 -0700 Subject: [PATCH] YARN-6314. Potential infinite redirection on YARN log redirection web service. Contributed by Xuan Gong. (cherry picked from commit 5a9dda796f0e73060ada794ad5752cc6a237ab2e) --- .../webapp/AHSWebServices.java | 32 +++++++++++++++---- .../webapp/TestAHSWebServices.java | 17 ++++++++++ .../server/webapp/YarnWebServiceParams.java | 1 + .../nodemanager/webapp/NMWebServices.java | 6 +++- .../nodemanager/webapp/TestNMWebServices.java | 6 ++++ 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java index c296aaa6ad..619519934f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/AHSWebServices.java @@ -28,6 +28,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -235,6 +236,8 @@ public ContainerInfo getContainer(@Context HttpServletRequest req, * The container ID * @param nmId * The Node Manager NodeId + * @param redirected_from_node + * Whether this is a redirected request from NM * @return * The log file's name and current file size */ @@ -245,7 +248,9 @@ public Response getContainerLogsInfo( @Context HttpServletRequest req, @Context HttpServletResponse res, @PathParam(YarnWebServiceParams.CONTAINER_ID) String containerIdStr, - @QueryParam(YarnWebServiceParams.NM_ID) String nmId) { + @QueryParam(YarnWebServiceParams.NM_ID) String nmId, + @QueryParam(YarnWebServiceParams.REDIRECTED_FROM_NODE) + @DefaultValue("false") boolean redirected_from_node) { ContainerId containerId = null; init(res); try { @@ -253,6 +258,7 @@ public Response getContainerLogsInfo( } catch (IllegalArgumentException e) { throw new BadRequestException("invalid container id, " + containerIdStr); } + ApplicationId appId = containerId.getApplicationAttemptId() .getApplicationId(); AppInfo appInfo; @@ -297,9 +303,12 @@ public Response getContainerLogsInfo( // make sure nodeHttpAddress is not null and not empty. Otherwise, // we would only get log meta for aggregated logs instead of // re-directing the request - if (nodeHttpAddress == null || nodeHttpAddress.isEmpty()) { + if (nodeHttpAddress == null || nodeHttpAddress.isEmpty() + || redirected_from_node) { // return log meta for the aggregated logs if exists. // It will also return empty log meta for the local logs. + // If this is the redirect request from NM, we should not + // re-direct the request back. Simply output the aggregated log meta. return getContainerLogMeta(appId, appOwner, null, containerIdStr, true); } @@ -338,6 +347,8 @@ public Response getContainerLogsInfo( * the size of the log file * @param nmId * The Node Manager NodeId + * @param redirected_from_node + * Whether this is the redirect request from NM * @return * The contents of the container's log file */ @@ -352,9 +363,11 @@ public Response getContainerLogFile(@Context HttpServletRequest req, @PathParam(YarnWebServiceParams.CONTAINER_LOG_FILE_NAME) String filename, @QueryParam(YarnWebServiceParams.RESPONSE_CONTENT_FORMAT) String format, @QueryParam(YarnWebServiceParams.RESPONSE_CONTENT_SIZE) String size, - @QueryParam(YarnWebServiceParams.NM_ID) String nmId) { + @QueryParam(YarnWebServiceParams.NM_ID) String nmId, + @QueryParam(YarnWebServiceParams.REDIRECTED_FROM_NODE) + boolean redirected_from_node) { return getLogs(req, res, containerIdStr, filename, format, - size, nmId); + size, nmId, redirected_from_node); } //TODO: YARN-4993: Refactory ContainersLogsBlock, AggregatedLogsBlock and @@ -371,7 +384,9 @@ public Response getLogs(@Context HttpServletRequest req, @PathParam(YarnWebServiceParams.CONTAINER_LOG_FILE_NAME) String filename, @QueryParam(YarnWebServiceParams.RESPONSE_CONTENT_FORMAT) String format, @QueryParam(YarnWebServiceParams.RESPONSE_CONTENT_SIZE) String size, - @QueryParam(YarnWebServiceParams.NM_ID) String nmId) { + @QueryParam(YarnWebServiceParams.NM_ID) String nmId, + @QueryParam(YarnWebServiceParams.REDIRECTED_FROM_NODE) + @DefaultValue("false") boolean redirected_from_node) { init(res); ContainerId containerId; try { @@ -426,8 +441,11 @@ public Response getLogs(@Context HttpServletRequest req, nodeHttpAddress = containerInfo.getNodeHttpAddress(); // make sure nodeHttpAddress is not null and not empty. Otherwise, // we would only get aggregated logs instead of re-directing the - // request - if (nodeHttpAddress == null || nodeHttpAddress.isEmpty()) { + // request. + // If this is the redirect request from NM, we should not re-direct the + // request back. Simply output the aggregated logs. + if (nodeHttpAddress == null || nodeHttpAddress.isEmpty() + || redirected_from_node) { // output the aggregated logs return sendStreamOutputResponse(appId, appOwner, null, containerIdStr, filename, format, length, true); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TestAHSWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TestAHSWebServices.java index c2cfb3b438..dc692a5963 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TestAHSWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/applicationhistoryservice/webapp/TestAHSWebServices.java @@ -771,6 +771,23 @@ public void testContainerLogsForRunningApps() throws Exception { assertTrue(responseText.contains("LogAggregationType: " + ContainerLogAggregationType.LOCAL)); assertTrue(responseText.contains(AHSWebServices.getNoRedirectWarning())); + + // If this is the redirect request, we would not re-direct the request + // back and get the aggregated logs. + String content1 = "Hello." + containerId1; + NodeId nodeId1 = NodeId.fromString(NM_ID); + TestContainerLogsUtils.createContainerLogFileInRemoteFS(conf, fs, + rootLogDir, containerId1, nodeId1, fileName, user, content1, true); + response = r.path("ws").path("v1") + .path("applicationhistory").path("containers") + .path(containerId1.toString()).path("logs").path(fileName) + .queryParam("user.name", user) + .queryParam(YarnWebServiceParams.REDIRECTED_FROM_NODE, "true") + .accept(MediaType.TEXT_PLAIN).get(ClientResponse.class); + responseText = response.getEntity(String.class); + assertTrue(responseText.contains(content1)); + assertTrue(responseText.contains("LogAggregationType: " + + ContainerLogAggregationType.AGGREGATED)); } @Test(timeout = 10000) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/YarnWebServiceParams.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/YarnWebServiceParams.java index 87e540fc30..479cc75813 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/YarnWebServiceParams.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/YarnWebServiceParams.java @@ -34,4 +34,5 @@ public interface YarnWebServiceParams { String RESPONSE_CONTENT_FORMAT = "format"; String RESPONSE_CONTENT_SIZE = "size"; String NM_ID = "nm.id"; + String REDIRECTED_FROM_NODE = "redirected_from_node"; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java index 4e72746f79..04a889fdc5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/NMWebServices.java @@ -499,7 +499,11 @@ private Response createRedirectResponse(HttpServletRequest httpRequest, String requestParams = WebAppUtils.removeQueryParams(httpRequest, YarnWebServiceParams.NM_ID); if (requestParams != null && !requestParams.isEmpty()) { - redirectPath.append("?" + requestParams); + redirectPath.append("?" + requestParams + "&" + + YarnWebServiceParams.REDIRECTED_FROM_NODE + "=true"); + } else { + redirectPath.append("?" + YarnWebServiceParams.REDIRECTED_FROM_NODE + + "=true"); } ResponseBuilder res = Response.status( HttpServletResponse.SC_TEMPORARY_REDIRECT); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java index d9fd2899ee..16411715c5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServices.java @@ -383,6 +383,8 @@ public void testNMRedirect() { assertTrue(redirectURL.contains(noExistContainerId.toString())); assertTrue(redirectURL.contains("/logs/" + fileName)); assertTrue(redirectURL.contains("user.name=" + "user")); + assertTrue(redirectURL.contains( + YarnWebServiceParams.REDIRECTED_FROM_NODE + "=true")); assertFalse(redirectURL.contains(YarnWebServiceParams.NM_ID)); // check the new api @@ -397,6 +399,8 @@ public void testNMRedirect() { assertTrue(redirectURL.contains(noExistContainerId.toString())); assertTrue(redirectURL.contains("/logs/" + fileName)); assertTrue(redirectURL.contains("user.name=" + "user")); + assertTrue(redirectURL.contains( + YarnWebServiceParams.REDIRECTED_FROM_NODE + "=true")); assertFalse(redirectURL.contains(YarnWebServiceParams.NM_ID)); requestURI = r.path("ws").path("v1").path("node") @@ -409,6 +413,8 @@ public void testNMRedirect() { assertTrue(redirectURL.contains(LOGSERVICEWSADDR)); assertTrue(redirectURL.contains(noExistContainerId.toString())); assertTrue(redirectURL.contains("user.name=" + "user")); + assertTrue(redirectURL.contains( + YarnWebServiceParams.REDIRECTED_FROM_NODE + "=true")); assertFalse(redirectURL.contains(YarnWebServiceParams.NM_ID)); }