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

@@ -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

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<Field> fieldEnums) {

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());
   }
 }