Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 8A5EF200CCB for ; Thu, 20 Jul 2017 18:06:14 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 88ACC16B9A6; Thu, 20 Jul 2017 16:06:14 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 7EE9216B9A4 for ; Thu, 20 Jul 2017 18:06:13 +0200 (CEST) Received: (qmail 53991 invoked by uid 500); 20 Jul 2017 16:06:12 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 53978 invoked by uid 99); 20 Jul 2017 16:06:12 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 20 Jul 2017 16:06:12 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 5FABCE04AA; Thu, 20 Jul 2017 16:06:12 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jlowe@apache.org To: common-commits@hadoop.apache.org Message-Id: <3e7550d464014fee8a47a506b9c20e4f@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: hadoop git commit: YARN-6837. Null LocalResource visibility or resource type can crash the nodemanager. Contributed by Jinjiang Ling Date: Thu, 20 Jul 2017 16:06:12 +0000 (UTC) archived-at: Thu, 20 Jul 2017 16:06:14 -0000 Repository: hadoop Updated Branches: refs/heads/trunk 0ba8cda13 -> c8df3668e YARN-6837. Null LocalResource visibility or resource type can crash the nodemanager. Contributed by Jinjiang Ling Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/c8df3668 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/c8df3668 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/c8df3668 Branch: refs/heads/trunk Commit: c8df3668ecc37c2d58cad35520a762eaec3c8539 Parents: 0ba8cda Author: Jason Lowe Authored: Thu Jul 20 11:03:04 2017 -0500 Committer: Jason Lowe Committed: Thu Jul 20 11:03:04 2017 -0500 ---------------------------------------------------------------------- .../impl/pb/ContainerLaunchContextPBImpl.java | 8 ++ .../TestApplicationClientProtocolRecords.java | 52 +++++++++++ .../containermanager/ContainerManagerImpl.java | 6 ++ .../containermanager/TestContainerManager.java | 90 ++++++++++++++++++++ 4 files changed, 156 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/c8df3668/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerLaunchContextPBImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerLaunchContextPBImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerLaunchContextPBImpl.java index f07a9d6..d722cc5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerLaunchContextPBImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerLaunchContextPBImpl.java @@ -222,6 +222,14 @@ extends ContainerLaunchContext { throw new NullPointerException( "Null resource URL for local resource " + rsrcEntry.getKey() + " : " + rsrcEntry.getValue()); + } else if (rsrcEntry.getValue().getType() == null) { + throw new NullPointerException( + "Null resource type for local resource " + rsrcEntry.getKey() + " : " + + rsrcEntry.getValue()); + } else if (rsrcEntry.getValue().getVisibility() == null) { + throw new NullPointerException( + "Null resource visibility for local resource " + rsrcEntry.getKey() + " : " + + rsrcEntry.getValue()); } } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/c8df3668/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/impl/pb/TestApplicationClientProtocolRecords.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/impl/pb/TestApplicationClientProtocolRecords.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/impl/pb/TestApplicationClientProtocolRecords.java index 8773d11..6c51516 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/impl/pb/TestApplicationClientProtocolRecords.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/api/records/impl/pb/TestApplicationClientProtocolRecords.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.io.DataOutputBuffer; import org.apache.hadoop.security.Credentials; import org.apache.hadoop.yarn.api.records.ApplicationAccessType; @@ -32,6 +33,7 @@ import org.apache.hadoop.yarn.api.records.ContainerLaunchContext; import org.apache.hadoop.yarn.api.records.LocalResource; import org.apache.hadoop.yarn.api.records.LocalResourceType; import org.apache.hadoop.yarn.api.records.LocalResourceVisibility; +import org.apache.hadoop.yarn.api.records.URL; import org.apache.hadoop.yarn.factories.RecordFactory; import org.apache.hadoop.yarn.factory.providers.RecordFactoryProvider; import org.junit.Assert; @@ -95,4 +97,54 @@ public class TestApplicationClientProtocolRecords { Assert.assertTrue(e.getMessage().contains("Null resource URL for local resource")); } } + + /* + * This test validates the scenario in which the client sets a null value for + * local resource type. + */ + @Test + public void testCLCPBImplNullResourceType() throws IOException { + RecordFactory recordFactory = RecordFactoryProvider.getRecordFactory(null); + try { + LocalResource resource = recordFactory.newRecordInstance(LocalResource.class); + resource.setResource(URL.fromPath(new Path("."))); + resource.setSize(-1); + resource.setVisibility(LocalResourceVisibility.APPLICATION); + resource.setType(null); + resource.setTimestamp(System.currentTimeMillis()); + Map localResources = + new HashMap(); + localResources.put("null_type_resource", resource); + ContainerLaunchContext containerLaunchContext = recordFactory.newRecordInstance(ContainerLaunchContext.class); + containerLaunchContext.setLocalResources(localResources); + Assert.fail("Setting an invalid local resource should be an error!"); + } catch (NullPointerException e) { + Assert.assertTrue(e.getMessage().contains("Null resource type for local resource")); + } + } + + /* + * This test validates the scenario in which the client sets a null value for + * local resource type. + */ + @Test + public void testCLCPBImplNullResourceVisibility() throws IOException { + RecordFactory recordFactory = RecordFactoryProvider.getRecordFactory(null); + try { + LocalResource resource = recordFactory.newRecordInstance(LocalResource.class); + resource.setResource(URL.fromPath(new Path("."))); + resource.setSize(-1); + resource.setVisibility(null); + resource.setType(LocalResourceType.FILE); + resource.setTimestamp(System.currentTimeMillis()); + Map localResources = + new HashMap(); + localResources.put("null_visibility_resource", resource); + ContainerLaunchContext containerLaunchContext = recordFactory.newRecordInstance(ContainerLaunchContext.class); + containerLaunchContext.setLocalResources(localResources); + Assert.fail("Setting an invalid local resource should be an error!"); + } catch (NullPointerException e) { + Assert.assertTrue(e.getMessage().contains("Null resource visibility for local resource")); + } + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/c8df3668/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java index b1d634a..167d15d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/ContainerManagerImpl.java @@ -1018,6 +1018,12 @@ public class ContainerManagerImpl extends CompositeService implements if (rsrc.getValue() == null || rsrc.getValue().getResource() == null) { throw new YarnException( "Null resource URL for local resource " + rsrc.getKey() + " : " + rsrc.getValue()); + } else if (rsrc.getValue().getType() == null) { + throw new YarnException( + "Null resource type for local resource " + rsrc.getKey() + " : " + rsrc.getValue()); + } else if (rsrc.getValue().getVisibility() == null) { + throw new YarnException( + "Null resource visibility for local resource " + rsrc.getKey() + " : " + rsrc.getValue()); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/c8df3668/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java index 60df7cb8..ba0ecce 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/TestContainerManager.java @@ -1899,4 +1899,94 @@ public class TestContainerManager extends BaseContainerManagerTest { Assert.assertTrue(response.getFailedRequests().get(cId).getMessage() .contains("Null resource URL for local resource")); } + + @Test + public void testStartContainerFailureWithNullTypeLocalResource() + throws Exception { + containerManager.start(); + LocalResource rsrc_alpha = + recordFactory.newRecordInstance(LocalResource.class); + rsrc_alpha.setResource(URL.fromPath(new Path("./"))); + rsrc_alpha.setSize(-1); + rsrc_alpha.setVisibility(LocalResourceVisibility.APPLICATION); + rsrc_alpha.setType(null); + rsrc_alpha.setTimestamp(System.currentTimeMillis()); + Map localResources = + new HashMap(); + localResources.put("null_type_resource", rsrc_alpha); + ContainerLaunchContext containerLaunchContext = + recordFactory.newRecordInstance(ContainerLaunchContext.class); + ContainerLaunchContext spyContainerLaunchContext = + Mockito.spy(containerLaunchContext); + Mockito.when(spyContainerLaunchContext.getLocalResources()) + .thenReturn(localResources); + + ContainerId cId = createContainerId(0); + String user = "start_container_fail"; + Token containerToken = + createContainerToken(cId, DUMMY_RM_IDENTIFIER, context.getNodeId(), + user, context.getContainerTokenSecretManager()); + StartContainerRequest request = StartContainerRequest + .newInstance(spyContainerLaunchContext, containerToken); + + // start containers + List startRequest = + new ArrayList(); + startRequest.add(request); + StartContainersRequest requestList = + StartContainersRequest.newInstance(startRequest); + + StartContainersResponse response = + containerManager.startContainers(requestList); + Assert.assertTrue(response.getFailedRequests().size() == 1); + Assert.assertTrue(response.getSuccessfullyStartedContainers().size() == 0); + Assert.assertTrue(response.getFailedRequests().containsKey(cId)); + Assert.assertTrue(response.getFailedRequests().get(cId).getMessage() + .contains("Null resource type for local resource")); + } + + @Test + public void testStartContainerFailureWithNullVisibilityLocalResource() + throws Exception { + containerManager.start(); + LocalResource rsrc_alpha = + recordFactory.newRecordInstance(LocalResource.class); + rsrc_alpha.setResource(URL.fromPath(new Path("./"))); + rsrc_alpha.setSize(-1); + rsrc_alpha.setVisibility(null); + rsrc_alpha.setType(LocalResourceType.FILE); + rsrc_alpha.setTimestamp(System.currentTimeMillis()); + Map localResources = + new HashMap(); + localResources.put("null_visibility_resource", rsrc_alpha); + ContainerLaunchContext containerLaunchContext = + recordFactory.newRecordInstance(ContainerLaunchContext.class); + ContainerLaunchContext spyContainerLaunchContext = + Mockito.spy(containerLaunchContext); + Mockito.when(spyContainerLaunchContext.getLocalResources()) + .thenReturn(localResources); + + ContainerId cId = createContainerId(0); + String user = "start_container_fail"; + Token containerToken = + createContainerToken(cId, DUMMY_RM_IDENTIFIER, context.getNodeId(), + user, context.getContainerTokenSecretManager()); + StartContainerRequest request = StartContainerRequest + .newInstance(spyContainerLaunchContext, containerToken); + + // start containers + List startRequest = + new ArrayList(); + startRequest.add(request); + StartContainersRequest requestList = + StartContainersRequest.newInstance(startRequest); + + StartContainersResponse response = + containerManager.startContainers(requestList); + Assert.assertTrue(response.getFailedRequests().size() == 1); + Assert.assertTrue(response.getSuccessfullyStartedContainers().size() == 0); + Assert.assertTrue(response.getFailedRequests().containsKey(cId)); + Assert.assertTrue(response.getFailedRequests().get(cId).getMessage() + .contains("Null resource visibility for local resource")); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org