From 5092c94195a63bd2c3e36d5a74b4c061cea1b847 Mon Sep 17 00:00:00 2001 From: naganarasimha Date: Mon, 4 Apr 2016 16:25:03 +0530 Subject: [PATCH] YARN-4746. yarn web services should convert parse failures of appId, appAttemptId and containerId to 400. Contributed by Bibin A Chundatt --- .../hadoop/yarn/util/ConverterUtils.java | 16 ++++++++-- .../hadoop/yarn/webapp/util/WebAppUtils.java | 22 +++++++++++++ .../yarn/server/webapp/WebServices.java | 22 ++++++++++--- .../nodemanager/webapp/NMWebServices.java | 6 ++-- .../webapp/TestNMWebServicesApps.java | 9 ++++-- .../resourcemanager/webapp/RMWebServices.java | 32 ++----------------- .../webapp/TestRMWebServicesApps.java | 24 ++++++++------ .../TestRMWebServicesAppsModification.java | 10 ++++-- 8 files changed, 87 insertions(+), 54 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java index e9674cfc147..acd29fb02d6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/ConverterUtils.java @@ -122,8 +122,20 @@ public static String toString(ApplicationId appId) { public static ApplicationId toApplicationId(RecordFactory recordFactory, String appIdStr) { Iterator it = _split(appIdStr).iterator(); - it.next(); // prefix. TODO: Validate application prefix - return toApplicationId(recordFactory, it); + if (!it.next().equals(APPLICATION_PREFIX)) { + throw new IllegalArgumentException("Invalid ApplicationId prefix: " + + appIdStr + ". The valid ApplicationId should start with prefix " + + APPLICATION_PREFIX); + } + try { + return toApplicationId(recordFactory, it); + } catch (NumberFormatException n) { + throw new IllegalArgumentException("Invalid ApplicationId: " + appIdStr, + n); + } catch (NoSuchElementException e) { + throw new IllegalArgumentException("Invalid ApplicationId: " + appIdStr, + e); + } } private static ApplicationId toApplicationId(RecordFactory recordFactory, diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java index f8e67ee1fb3..faf4a774447 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/webapp/util/WebAppUtils.java @@ -33,9 +33,14 @@ import org.apache.hadoop.http.HttpServer2; import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.yarn.conf.YarnConfiguration; +import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.conf.HAUtil; import org.apache.hadoop.yarn.exceptions.YarnRuntimeException; +import org.apache.hadoop.yarn.factories.RecordFactory; +import org.apache.hadoop.yarn.util.ConverterUtils; import org.apache.hadoop.yarn.util.RMHAUtils; +import org.apache.hadoop.yarn.webapp.BadRequestException; +import org.apache.hadoop.yarn.webapp.NotFoundException; @Private @Evolving @@ -378,4 +383,21 @@ static String getPassword(Configuration conf, String alias) { } return password; } + + public static ApplicationId parseApplicationId(RecordFactory recordFactory, + String appId) { + if (appId == null || appId.isEmpty()) { + throw new NotFoundException("appId, " + appId + ", is empty or null"); + } + ApplicationId aid = null; + try { + aid = ConverterUtils.toApplicationId(recordFactory, appId); + } catch (Exception e) { + throw new BadRequestException(e); + } + if (aid == null) { + throw new NotFoundException("app with id " + appId + " not found"); + } + return aid; + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/WebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/WebServices.java index 40e40c98421..19ea30136e8 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/WebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/webapp/WebServices.java @@ -429,7 +429,12 @@ protected static ApplicationId parseApplicationId(String appId) { if (appId == null || appId.isEmpty()) { throw new NotFoundException("appId, " + appId + ", is empty or null"); } - ApplicationId aid = ConverterUtils.toApplicationId(appId); + ApplicationId aid = null; + try { + aid = ConverterUtils.toApplicationId(appId); + } catch (Exception e) { + throw new BadRequestException(e); + } if (aid == null) { throw new NotFoundException("appId is null"); } @@ -442,8 +447,12 @@ protected static ApplicationAttemptId parseApplicationAttemptId( throw new NotFoundException("appAttemptId, " + appAttemptId + ", is empty or null"); } - ApplicationAttemptId aaid = - ConverterUtils.toApplicationAttemptId(appAttemptId); + ApplicationAttemptId aaid = null; + try { + aaid = ConverterUtils.toApplicationAttemptId(appAttemptId); + } catch (Exception e) { + throw new BadRequestException(e); + } if (aaid == null) { throw new NotFoundException("appAttemptId is null"); } @@ -455,7 +464,12 @@ protected static ContainerId parseContainerId(String containerId) { throw new NotFoundException("containerId, " + containerId + ", is empty or null"); } - ContainerId cid = ConverterUtils.toContainerId(containerId); + ContainerId cid = null; + try { + cid = ConverterUtils.toContainerId(containerId); + } catch (Exception e) { + throw new BadRequestException(e); + } if (cid == null) { throw new NotFoundException("containerId is null"); } 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 de6d219deaa..fddeb042e5f 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 @@ -58,6 +58,7 @@ import org.apache.hadoop.yarn.webapp.BadRequestException; import org.apache.hadoop.yarn.webapp.NotFoundException; import org.apache.hadoop.yarn.webapp.WebApp; +import org.apache.hadoop.yarn.webapp.util.WebAppUtils; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -143,10 +144,7 @@ public AppsInfo getNodeApps(@QueryParam("state") String stateQuery, @Produces({ MediaType.APPLICATION_JSON, MediaType.APPLICATION_XML }) public AppInfo getNodeApp(@PathParam("appid") String appId) { init(); - ApplicationId id = ConverterUtils.toApplicationId(recordFactory, appId); - if (id == null) { - throw new NotFoundException("app with id " + appId + " not found"); - } + ApplicationId id = WebAppUtils.parseApplicationId(recordFactory, appId); Application app = this.nmContext.getApplications().get(id); if (app == null) { throw new NotFoundException("app with id " + appId + " not found"); 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/TestNMWebServicesApps.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServicesApps.java index e274abb2aac..3cd8faf0389 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServicesApps.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/webapp/TestNMWebServicesApps.java @@ -565,11 +565,14 @@ public void testNodeSingleAppsInvalid() throws JSONException, Exception { String type = exception.getString("exception"); String classname = exception.getString("javaClassName"); WebServicesTestUtils.checkStringMatch("exception message", - "For input string: \"foo\"", message); + "java.lang.IllegalArgumentException: Invalid ApplicationId prefix: " + + "app_foo_0000. The valid ApplicationId should start with prefix" + + " application", + message); WebServicesTestUtils.checkStringMatch("exception type", - "NumberFormatException", type); + "BadRequestException", type); WebServicesTestUtils.checkStringMatch("exception classname", - "java.lang.NumberFormatException", classname); + "org.apache.hadoop.yarn.webapp.BadRequestException", classname); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java index 0ed8a4ed00f..8036af42f05 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/RMWebServices.java @@ -662,14 +662,7 @@ private static void countApp( public AppInfo getApp(@Context HttpServletRequest hsr, @PathParam("appid") String appId) { init(); - if (appId == null || appId.isEmpty()) { - throw new NotFoundException("appId, " + appId + ", is empty or null"); - } - ApplicationId id; - id = ConverterUtils.toApplicationId(recordFactory, appId); - if (id == null) { - throw new NotFoundException("appId is null"); - } + ApplicationId id = WebAppUtils.parseApplicationId(recordFactory, appId); RMApp app = rm.getRMContext().getRMApps().get(id); if (app == null) { throw new NotFoundException("app with id: " + appId + " not found"); @@ -684,14 +677,7 @@ public AppAttemptsInfo getAppAttempts(@Context HttpServletRequest hsr, @PathParam("appid") String appId) { init(); - if (appId == null || appId.isEmpty()) { - throw new NotFoundException("appId, " + appId + ", is empty or null"); - } - ApplicationId id; - id = ConverterUtils.toApplicationId(recordFactory, appId); - if (id == null) { - throw new NotFoundException("appId is null"); - } + ApplicationId id = WebAppUtils.parseApplicationId(recordFactory, appId); RMApp app = rm.getRMContext().getRMApps().get(id); if (app == null) { throw new NotFoundException("app with id: " + appId + " not found"); @@ -1303,19 +1289,7 @@ public Void run() throws IOException, } private RMApp getRMAppForAppId(String appId) { - - if (appId == null || appId.isEmpty()) { - throw new NotFoundException("appId, " + appId + ", is empty or null"); - } - ApplicationId id; - try { - id = ConverterUtils.toApplicationId(recordFactory, appId); - } catch (NumberFormatException e) { - throw new NotFoundException("appId is invalid"); - } - if (id == null) { - throw new NotFoundException("appId is invalid"); - } + ApplicationId id = WebAppUtils.parseApplicationId(recordFactory, appId); RMApp app = rm.getRMContext().getRMApps().get(id); if (app == null) { throw new NotFoundException("app with id: " + appId + " not found"); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesApps.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesApps.java index 0328a7a291b..72cccf66420 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesApps.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesApps.java @@ -1194,11 +1194,13 @@ public void testInvalidApp() throws JSONException, Exception { String type = exception.getString("exception"); String classname = exception.getString("javaClassName"); WebServicesTestUtils.checkStringMatch("exception message", - "For input string: \"invalid\"", message); + "java.lang.IllegalArgumentException: Invalid ApplicationId:" + + " application_invalid_12", + message); WebServicesTestUtils.checkStringMatch("exception type", - "NumberFormatException", type); + "BadRequestException", type); WebServicesTestUtils.checkStringMatch("exception classname", - "java.lang.NumberFormatException", classname); + "org.apache.hadoop.yarn.webapp.BadRequestException", classname); } finally { rm.stop(); @@ -1500,15 +1502,17 @@ public void testAppAttemtpsDefault() throws JSONException, Exception { public void testInvalidAppAttempts() throws JSONException, Exception { rm.start(); MockNM amNodeManager = rm.registerNode("127.0.0.1:1234", 2048); - rm.submitApp(CONTAINER_MB); + RMApp app = rm.submitApp(CONTAINER_MB); amNodeManager.nodeHeartbeat(true); WebResource r = resource(); try { r.path("ws").path("v1").path("cluster").path("apps") - .path("application_invalid_12").accept(MediaType.APPLICATION_JSON) + .path(app.getApplicationId().toString()).path("appattempts") + .path("appattempt_invalid_12_000001") + .accept(MediaType.APPLICATION_JSON) .get(JSONObject.class); - fail("should have thrown exception on invalid appid"); + fail("should have thrown exception on invalid appAttempt"); } catch (UniformInterfaceException ue) { ClientResponse response = ue.getResponse(); @@ -1521,11 +1525,13 @@ public void testInvalidAppAttempts() throws JSONException, Exception { String type = exception.getString("exception"); String classname = exception.getString("javaClassName"); WebServicesTestUtils.checkStringMatch("exception message", - "For input string: \"invalid\"", message); + "java.lang.IllegalArgumentException: Invalid AppAttemptId:" + + " appattempt_invalid_12_000001", + message); WebServicesTestUtils.checkStringMatch("exception type", - "NumberFormatException", type); + "BadRequestException", type); WebServicesTestUtils.checkStringMatch("exception classname", - "java.lang.NumberFormatException", classname); + "org.apache.hadoop.yarn.webapp.BadRequestException", classname); } finally { rm.stop(); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesAppsModification.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesAppsModification.java index 9abcf9c8d5b..38b32e96e17 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesAppsModification.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/webapp/TestRMWebServicesAppsModification.java @@ -568,17 +568,21 @@ public void testSingleAppKillInvalidId() throws Exception { MockNM amNodeManager = rm.registerNode("127.0.0.1:1234", 2048); amNodeManager.nodeHeartbeat(true); String[] testAppIds = { "application_1391705042196_0001", "random_string" }; - for (String testAppId : testAppIds) { + for (int i = 0; i < testAppIds.length; i++) { AppState info = new AppState("KILLED"); ClientResponse response = - this.constructWebResource("apps", testAppId, "state") + this.constructWebResource("apps", testAppIds[i], "state") .accept(MediaType.APPLICATION_XML) .entity(info, MediaType.APPLICATION_XML).put(ClientResponse.class); if (!isAuthenticationEnabled()) { assertEquals(Status.UNAUTHORIZED, response.getClientResponseStatus()); continue; } - assertEquals(Status.NOT_FOUND, response.getClientResponseStatus()); + if (i == 0) { + assertEquals(Status.NOT_FOUND, response.getClientResponseStatus()); + } else { + assertEquals(Status.BAD_REQUEST, response.getClientResponseStatus()); + } } rm.stop(); }