From 233d446be1bc1bc77c7c1c45386086a732897afd Mon Sep 17 00:00:00 2001 From: Jian He Date: Thu, 16 Oct 2014 16:37:21 -0700 Subject: [PATCH] YARN-2621. Simplify the output when the user doesn't have the access for getDomain(s). Contributed by Zhijie Shen --- hadoop-yarn-project/CHANGES.txt | 3 ++ .../server/timeline/TimelineDataManager.java | 38 ++++++------------ .../webapp/TestTimelineWebServices.java | 39 +++++++------------ 3 files changed, 28 insertions(+), 52 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index f85735e1b98..432f35af6d1 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -357,6 +357,9 @@ Release 2.6.0 - UNRELEASED YARN-2312. Deprecated old ContainerId#getId API and updated MapReduce to use ContainerId#getContainerId instead. (Tsuyoshi OZAWA via jianhe) + YARN-2621. Simplify the output when the user doesn't have the access for + getDomain(s). (Zhijie Shen via jianhe) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/TimelineDataManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/TimelineDataManager.java index 79b924a9c8c..3b6aafa2801 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/TimelineDataManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/main/java/org/apache/hadoop/yarn/server/timeline/TimelineDataManager.java @@ -361,8 +361,7 @@ public class TimelineDataManager extends AbstractService { /** * Get a single domain of the particular ID. If callerUGI is not the owner - * or the admin of the domain, we need to hide the details from him, and - * only allow him to see the ID. + * or the admin of the domain, null will be returned. */ public TimelineDomain getDomain(String domainId, UserGroupInformation callerUGI) throws YarnException, IOException { @@ -370,9 +369,6 @@ public class TimelineDataManager extends AbstractService { if (domain != null) { if (timelineACLsManager.checkAccess(callerUGI, domain)) { return domain; - } else { - hideDomainDetails(domain); - return domain; } } return null; @@ -380,34 +376,22 @@ public class TimelineDataManager extends AbstractService { /** * Get all the domains that belong to the given owner. If callerUGI is not - * the owner or the admin of the domain, we need to hide the details from - * him, and only allow him to see the ID. + * the owner or the admin of the domain, empty list is going to be returned. */ public TimelineDomains getDomains(String owner, UserGroupInformation callerUGI) throws YarnException, IOException { TimelineDomains domains = store.getDomains(owner); boolean hasAccess = true; - boolean isChecked = false; - for (TimelineDomain domain : domains.getDomains()) { - // The owner for each domain is the same, just need to check on - if (!isChecked) { - hasAccess = timelineACLsManager.checkAccess(callerUGI, domain); - isChecked = true; - } - if (!hasAccess) { - hideDomainDetails(domain); - } + if (domains.getDomains().size() > 0) { + // The owner for each domain is the same, just need to check one + hasAccess = timelineACLsManager.checkAccess( + callerUGI, domains.getDomains().get(0)); + } + if (hasAccess) { + return domains; + } else { + return new TimelineDomains(); } - return domains; - } - - private static void hideDomainDetails(TimelineDomain domain) { - domain.setDescription(null); - domain.setOwner(null); - domain.setReaders(null); - domain.setWriters(null); - domain.setCreatedTime(null); - domain.setModifiedTime(null); } private static boolean extendFields(EnumSet fieldEnums) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestTimelineWebServices.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestTimelineWebServices.java index f429a97be81..e117a2fef44 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestTimelineWebServices.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-applicationhistoryservice/src/test/java/org/apache/hadoop/yarn/server/timeline/webapp/TestTimelineWebServices.java @@ -807,7 +807,7 @@ public class TestTimelineWebServices extends JerseyTest { .get(ClientResponse.class); Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getType()); TimelineDomain domain = response.getEntity(TimelineDomain.class); - verifyDomain(domain, "domain_id_1", true); + verifyDomain(domain, "domain_id_1"); } @Test @@ -823,7 +823,7 @@ public class TestTimelineWebServices extends JerseyTest { .get(ClientResponse.class); Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getType()); TimelineDomain domain = response.getEntity(TimelineDomain.class); - verifyDomain(domain, "domain_id_1", true); + verifyDomain(domain, "domain_id_1"); response = r.path("ws").path("v1").path("timeline") .path("domain").path("domain_id_1") @@ -831,8 +831,8 @@ public class TestTimelineWebServices extends JerseyTest { .accept(MediaType.APPLICATION_JSON) .get(ClientResponse.class); Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getType()); - domain = response.getEntity(TimelineDomain.class); - verifyDomain(domain, "domain_id_1", false); + Assert.assertEquals(ClientResponse.Status.NOT_FOUND, + response.getClientResponseStatus()); } finally { timelineACLsManager.setAdminACLsManager(oldAdminACLsManager); } @@ -851,7 +851,7 @@ public class TestTimelineWebServices extends JerseyTest { Assert.assertEquals(2, domains.getDomains().size()); for (int i = 0; i < domains.getDomains().size(); ++i) { verifyDomain(domains.getDomains().get(i), - i == 0 ? "domain_id_4" : "domain_id_1", true); + i == 0 ? "domain_id_4" : "domain_id_1"); } } @@ -871,7 +871,7 @@ public class TestTimelineWebServices extends JerseyTest { Assert.assertEquals(2, domains.getDomains().size()); for (int i = 0; i < domains.getDomains().size(); ++i) { verifyDomain(domains.getDomains().get(i), - i == 0 ? "domain_id_4" : "domain_id_1", true); + i == 0 ? "domain_id_4" : "domain_id_1"); } response = r.path("ws").path("v1").path("timeline") @@ -882,11 +882,7 @@ public class TestTimelineWebServices extends JerseyTest { .get(ClientResponse.class); Assert.assertEquals(MediaType.APPLICATION_JSON_TYPE, response.getType()); domains = response.getEntity(TimelineDomains.class); - Assert.assertEquals(2, domains.getDomains().size()); - for (int i = 0; i < domains.getDomains().size(); ++i) { - verifyDomain(domains.getDomains().get(i), - i == 0 ? "domain_id_4" : "domain_id_1", false); - } + Assert.assertEquals(0, domains.getDomains().size()); } finally { timelineACLsManager.setAdminACLsManager(oldAdminACLsManager); } @@ -978,22 +974,15 @@ public class TestTimelineWebServices extends JerseyTest { } } - private static void verifyDomain(TimelineDomain domain, - String domainId, boolean hasAccess) { + private static void verifyDomain(TimelineDomain domain, String domainId) { Assert.assertNotNull(domain); Assert.assertEquals(domainId, domain.getId()); // The specific values have been verified in TestMemoryTimelineStore - Assert.assertTrue(hasAccess && domain.getDescription() != null || - !hasAccess && domain.getDescription() == null); - Assert.assertTrue(hasAccess && domain.getOwner() != null || - !hasAccess && domain.getOwner() == null); - Assert.assertTrue(hasAccess && domain.getReaders() != null || - !hasAccess && domain.getReaders() == null); - Assert.assertTrue(hasAccess && domain.getWriters() != null || - !hasAccess && domain.getWriters() == null); - Assert.assertTrue(hasAccess && domain.getCreatedTime() != null || - !hasAccess && domain.getCreatedTime() == null); - Assert.assertTrue(hasAccess && domain.getModifiedTime() != null || - !hasAccess && domain.getModifiedTime() == null); + Assert.assertNotNull(domain.getDescription()); + Assert.assertNotNull(domain.getOwner()); + Assert.assertNotNull(domain.getReaders()); + Assert.assertNotNull(domain.getWriters()); + Assert.assertNotNull(domain.getCreatedTime()); + Assert.assertNotNull(domain.getModifiedTime()); } }