Repository: incubator-slider Updated Branches: refs/heads/develop 3fbe9ff64 -> 72932530f
[SLIDER-931] Security permissions on set up ZK path are too lax Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/72932530 Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/72932530 Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/72932530 Branch: refs/heads/develop Commit: 72932530f764087b549b6d052ac45d51e24d1893 Parents: 3fbe9ff Author: Steve Loughran <[email protected]> Authored: Fri Sep 4 15:55:27 2015 +0100 Committer: Steve Loughran <[email protected]> Committed: Fri Sep 4 15:55:27 2015 +0100 ---------------------------------------------------------------------- .../org/apache/slider/client/SliderClient.java | 59 ++++++++++++++------ .../test_command_log/appConfig.json | 3 +- .../common/tools/TestZKIntegration.groovy | 4 +- 3 files changed, 47 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/72932530/slider-core/src/main/java/org/apache/slider/client/SliderClient.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java index 323bc73..b993cc5 100644 --- a/slider-core/src/main/java/org/apache/slider/client/SliderClient.java +++ b/slider-core/src/main/java/org/apache/slider/client/SliderClient.java @@ -176,6 +176,7 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.InterruptedIOException; import java.io.PrintStream; import java.io.StringWriter; import java.io.Writer; @@ -516,12 +517,11 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe client.deleteRecursive(zkPath); return true; } - } catch (InterruptedException | BadConfigException | KeeperException ignored) { - e = ignored; + } catch (InterruptedException | BadConfigException | KeeperException ex) { + e = ex; } if (e != null) { - log.debug("Unable to recursively delete zk node {}", zkPath); - log.debug("Reason: ", e); + log.warn("Unable to recursively delete zk node {}", zkPath, e); } return false; @@ -532,6 +532,31 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe */ @VisibleForTesting public String createZookeeperNode(String clusterName, Boolean nameOnly) throws YarnException, IOException { + try { + return createZookeeperNodeInner(clusterName, nameOnly); + } catch (KeeperException.NodeExistsException e) { + return null; + } catch (KeeperException e) { + return null; + } catch (InterruptedException e) { + throw new InterruptedIOException(e.toString()); + } + } + + /** + * Create the zookeeper node associated with the calling user and the cluster + * -throwing exceptions on any failure + * @param clusterName cluster name + * @param nameOnly create the path, not the node + * @return the path, with the node created + * @throws YarnException + * @throws IOException + * @throws KeeperException + * @throws InterruptedException + */ + @VisibleForTesting + public String createZookeeperNodeInner(String clusterName, Boolean nameOnly) + throws YarnException, IOException, KeeperException, InterruptedException { String user = getUsername(); String zkPath = ZKIntegration.mkClusterPath(user, clusterName); if (nameOnly) { @@ -539,20 +564,22 @@ public class SliderClient extends AbstractSliderLaunchedService implements RunSe } ZKIntegration client = getZkClient(clusterName, user); if (client != null) { - try { - List<ACL> zkperms = new ArrayList<>(); - zkperms.addAll(ZooDefs.Ids.CREATOR_ALL_ACL); - zkperms.addAll(ZooDefs.Ids.READ_ACL_UNSAFE); - client.createPath(zkPath, "", - zkperms, - CreateMode.PERSISTENT); - return zkPath; - } catch (InterruptedException | KeeperException e) { - log.warn("Unable to create default zk node {}", zkPath, e); + // set up the permissions. This must be done differently on a secure cluster from an insecure + // one + List<ACL> zkperms = new ArrayList<ACL>(); + if (UserGroupInformation.isSecurityEnabled()) { + zkperms.add(new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.AUTH_IDS)); + zkperms.add(new ACL(ZooDefs.Perms.READ, ZooDefs.Ids.ANYONE_ID_UNSAFE)); + } else { + zkperms.add(new ACL(ZooDefs.Perms.ALL, ZooDefs.Ids.ANYONE_ID_UNSAFE)); } + client.createPath(zkPath, "", + zkperms, + CreateMode.PERSISTENT); + return zkPath; + } else { + return null; } - - return null; } /** http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/72932530/slider-core/src/test/app_packages/test_command_log/appConfig.json ---------------------------------------------------------------------- diff --git a/slider-core/src/test/app_packages/test_command_log/appConfig.json b/slider-core/src/test/app_packages/test_command_log/appConfig.json index b9383ec..f53225e 100644 --- a/slider-core/src/test/app_packages/test_command_log/appConfig.json +++ b/slider-core/src/test/app_packages/test_command_log/appConfig.json @@ -9,7 +9,8 @@ "site.global.app_root": "${AGENT_WORK_ROOT}/app/install/command-logger", "site.cl-site.logfile.location": "${AGENT_LOG_ROOT}/operations.log", "site.cl-site.datetime.format": "%A, %d. %B %Y %I:%M%p", - "site.cl-site.pattern.for.test.to.verify": "verify this pattern" + "site.cl-site.pattern.for.test.to.verify": "verify this pattern", + "create.default.zookeeper.node": true }, "components": { "COMMAND_LOGGER": { http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/72932530/slider-core/src/test/groovy/org/apache/slider/common/tools/TestZKIntegration.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/common/tools/TestZKIntegration.groovy b/slider-core/src/test/groovy/org/apache/slider/common/tools/TestZKIntegration.groovy index 6fed4cb..1ea9001 100644 --- a/slider-core/src/test/groovy/org/apache/slider/common/tools/TestZKIntegration.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/common/tools/TestZKIntegration.groovy @@ -108,7 +108,7 @@ class TestZKIntegration extends YarnZKMiniClusterTestBase implements KeysForTest public void testCreateAndDeleteDefaultZKPath() throws Throwable { MockSliderClient client = new MockSliderClient() - String path = client.createZookeeperNode("cl1", true) + String path = client.createZookeeperNodeInner("cl1", true) zki = client.lastZKIntegration String zkPath = ZKIntegration.mkClusterPath(USER, "cl1") @@ -118,7 +118,7 @@ class TestZKIntegration extends YarnZKMiniClusterTestBase implements KeysForTest zki = createZKIntegrationInstance(getZKBinding(), "cl1", true, false, CONNECT_TIMEOUT); assert !zki.exists(zkPath) - path = client.createZookeeperNode("cl1", false) + path = client.createZookeeperNodeInner("cl1", false) zki = client.lastZKIntegration assert zki assert zkPath == "/services/slider/users/" + USER + "/cl1", "zkPath must be as expected"
