Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 6E7B31766C for ; Thu, 2 Oct 2014 21:57:45 +0000 (UTC) Received: (qmail 24485 invoked by uid 500); 2 Oct 2014 21:57:45 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 24413 invoked by uid 500); 2 Oct 2014 21:57:45 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 24403 invoked by uid 99); 2 Oct 2014 21:57:45 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 02 Oct 2014 21:57:45 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id DDDD4A17FB1; Thu, 2 Oct 2014 21:57:44 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: zjshen@apache.org To: common-commits@hadoop.apache.org Message-Id: <7516ad62e63b42e6a25460bfd490a7ed@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: YARN-2527. Fixed the potential NPE in ApplicationACLsManager and added test cases for it. Contributed by Benoy Antony. Date: Thu, 2 Oct 2014 21:57:44 +0000 (UTC) Repository: hadoop Updated Branches: refs/heads/branch-2 ebeb9da80 -> ecaae67ab YARN-2527. Fixed the potential NPE in ApplicationACLsManager and added test cases for it. Contributed by Benoy Antony. (cherry picked from commit 1c93025a1b370db46e345161dbc15e03f829823f) Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/ecaae67a Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/ecaae67a Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/ecaae67a Branch: refs/heads/branch-2 Commit: ecaae67ab32f842627c0bee06bba7b4773972a67 Parents: ebeb9da Author: Zhijie Shen Authored: Thu Oct 2 14:55:37 2014 -0700 Committer: Zhijie Shen Committed: Thu Oct 2 14:57:27 2014 -0700 ---------------------------------------------------------------------- hadoop-yarn-project/CHANGES.txt | 3 + .../server/security/ApplicationACLsManager.java | 22 ++- .../security/TestApplicationACLsManager.java | 180 +++++++++++++++++++ 3 files changed, 199 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/ecaae67a/hadoop-yarn-project/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index b4b0b75..6aeb961 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -494,6 +494,9 @@ Release 2.6.0 - UNRELEASED YARN-2624. Resource Localization fails on a cluster due to existing cache directories (Anubhav Dhoot via jlowe) + YARN-2527. Fixed the potential NPE in ApplicationACLsManager and added test + cases for it. (Benoy Antony via zjshen) + Release 2.5.1 - 2014-09-05 INCOMPATIBLE CHANGES http://git-wip-us.apache.org/repos/asf/hadoop/blob/ecaae67a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/security/ApplicationACLsManager.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/security/ApplicationACLsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/security/ApplicationACLsManager.java index 75c8478..e8e3cb5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/security/ApplicationACLsManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/server/security/ApplicationACLsManager.java @@ -41,6 +41,8 @@ public class ApplicationACLsManager { private static final Log LOG = LogFactory .getLog(ApplicationACLsManager.class); + private static AccessControlList DEFAULT_YARN_APP_ACL + = new AccessControlList(YarnConfiguration.DEFAULT_YARN_APP_ACL); private final Configuration conf; private final AdminACLsManager adminAclsManager; private final ConcurrentMap> applicationACLS @@ -100,18 +102,26 @@ public class ApplicationACLsManager { if (!areACLsEnabled()) { return true; } - - AccessControlList applicationACL = this.applicationACLS - .get(applicationId).get(applicationAccessType); - if (applicationACL == null) { + AccessControlList applicationACL = DEFAULT_YARN_APP_ACL; + Map acls = this.applicationACLS + .get(applicationId); + if (acls == null) { if (LOG.isDebugEnabled()) { + LOG.debug("ACL not found for application " + + applicationId + " owned by " + + applicationOwner + ". Using default [" + + YarnConfiguration.DEFAULT_YARN_APP_ACL + "]"); + } + } else { + AccessControlList applicationACLInMap = acls.get(applicationAccessType); + if (applicationACLInMap != null) { + applicationACL = applicationACLInMap; + } else if (LOG.isDebugEnabled()) { LOG.debug("ACL not found for access-type " + applicationAccessType + " for application " + applicationId + " owned by " + applicationOwner + ". Using default [" + YarnConfiguration.DEFAULT_YARN_APP_ACL + "]"); } - applicationACL = - new AccessControlList(YarnConfiguration.DEFAULT_YARN_APP_ACL); } // Allow application-owner for any type of access on the application http://git-wip-us.apache.org/repos/asf/hadoop/blob/ecaae67a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/server/security/TestApplicationACLsManager.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/server/security/TestApplicationACLsManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/server/security/TestApplicationACLsManager.java new file mode 100644 index 0000000..2db1da9 --- /dev/null +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/server/security/TestApplicationACLsManager.java @@ -0,0 +1,180 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.yarn.server.security; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.yarn.api.records.ApplicationAccessType; +import org.apache.hadoop.yarn.api.records.ApplicationId; +import org.apache.hadoop.yarn.conf.YarnConfiguration; +import org.junit.Test; + +public class TestApplicationACLsManager { + + private static final String ADMIN_USER = "adminuser"; + private static final String APP_OWNER = "appuser"; + private static final String TESTUSER1 = "testuser1"; + private static final String TESTUSER2 = "testuser2"; + private static final String TESTUSER3 = "testuser3"; + + @Test + public void testCheckAccess() { + Configuration conf = new Configuration(); + conf.setBoolean(YarnConfiguration.YARN_ACL_ENABLE, + true); + conf.set(YarnConfiguration.YARN_ADMIN_ACL, + ADMIN_USER); + ApplicationACLsManager aclManager = new ApplicationACLsManager(conf); + Map aclMap = + new HashMap(); + aclMap.put(ApplicationAccessType.VIEW_APP, TESTUSER1 + "," + TESTUSER3); + aclMap.put(ApplicationAccessType.MODIFY_APP, TESTUSER1); + ApplicationId appId = ApplicationId.newInstance(1, 1); + aclManager.addApplication(appId, aclMap); + + //User in ACL, should be allowed access + UserGroupInformation testUser1 = UserGroupInformation + .createRemoteUser(TESTUSER1); + assertTrue(aclManager.checkAccess(testUser1, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(testUser1, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + //User NOT in ACL, should not be allowed access + UserGroupInformation testUser2 = UserGroupInformation + .createRemoteUser(TESTUSER2); + assertFalse(aclManager.checkAccess(testUser2, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertFalse(aclManager.checkAccess(testUser2, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + //User has View access, but not modify access + UserGroupInformation testUser3 = UserGroupInformation + .createRemoteUser(TESTUSER3); + assertTrue(aclManager.checkAccess(testUser3, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertFalse(aclManager.checkAccess(testUser3, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + //Application Owner should have all access + UserGroupInformation appOwner = UserGroupInformation + .createRemoteUser(APP_OWNER); + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + //Admin should have all access + UserGroupInformation adminUser = UserGroupInformation + .createRemoteUser(ADMIN_USER); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + } + + @Test + public void testCheckAccessWithNullACLS() { + Configuration conf = new Configuration(); + conf.setBoolean(YarnConfiguration.YARN_ACL_ENABLE, + true); + conf.set(YarnConfiguration.YARN_ADMIN_ACL, + ADMIN_USER); + ApplicationACLsManager aclManager = new ApplicationACLsManager(conf); + UserGroupInformation appOwner = UserGroupInformation + .createRemoteUser(APP_OWNER); + ApplicationId appId = ApplicationId.newInstance(1, 1); + //Application ACL is not added + + //Application Owner should have all access even if Application ACL is not added + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + + //Admin should have all access + UserGroupInformation adminUser = UserGroupInformation + .createRemoteUser(ADMIN_USER); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + // A regular user should Not have access + UserGroupInformation testUser1 = UserGroupInformation + .createRemoteUser(TESTUSER1); + assertFalse(aclManager.checkAccess(testUser1, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertFalse(aclManager.checkAccess(testUser1, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + } + + @Test + public void testCheckAccessWithPartialACLS() { + Configuration conf = new Configuration(); + conf.setBoolean(YarnConfiguration.YARN_ACL_ENABLE, + true); + conf.set(YarnConfiguration.YARN_ADMIN_ACL, + ADMIN_USER); + ApplicationACLsManager aclManager = new ApplicationACLsManager(conf); + UserGroupInformation appOwner = UserGroupInformation + .createRemoteUser(APP_OWNER); + // Add only the VIEW ACLS + Map aclMap = + new HashMap(); + aclMap.put(ApplicationAccessType.VIEW_APP, TESTUSER1 ); + ApplicationId appId = ApplicationId.newInstance(1, 1); + aclManager.addApplication(appId, aclMap); + + //Application Owner should have all access even if Application ACL is not added + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(appOwner, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + + //Admin should have all access + UserGroupInformation adminUser = UserGroupInformation + .createRemoteUser(ADMIN_USER); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertTrue(aclManager.checkAccess(adminUser, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + // testuser1 should have view access only + UserGroupInformation testUser1 = UserGroupInformation + .createRemoteUser(TESTUSER1); + assertTrue(aclManager.checkAccess(testUser1, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertFalse(aclManager.checkAccess(testUser1, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + + // A testuser2 should Not have access + UserGroupInformation testUser2 = UserGroupInformation + .createRemoteUser(TESTUSER2); + assertFalse(aclManager.checkAccess(testUser2, ApplicationAccessType.VIEW_APP, + APP_OWNER, appId)); + assertFalse(aclManager.checkAccess(testUser2, ApplicationAccessType.MODIFY_APP, + APP_OWNER, appId)); + } +}