YARN-6314. Potential infinite redirection on YARN log redirection web service. Contributed by Xuan Gong.

(cherry picked from commit 5a9dda796f0e73060ada794ad5752cc6a237ab2e)

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);

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)

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";
 }

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);

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));
   }