[jira] [Comment Edited] (HADOOP-15836) Review of AccessControlList

["BELUGA BEHR (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-15836?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16705357#comment-16705357 ] 

BELUGA BEHR edited comment on HADOOP-15836 at 11/30/18 10:25 PM:
-----------------------------------------------------------------

OK, so I am providing a new patch with lessons learned from the failed unit tests and from HADOOP-12640. The two unit tests for MAPREDUCE-7155 and YARN-8928 pass locally.

I have made one functional change from how things are now:
{code:java}
    // The space in the group list is acceptable.
    // However, there is no test (and therefore ambiguity)
    // about what to do if the user list has a space in it.
    // There is only a test for a space in the group list
    Iterator<String> iter;    
    acl = new AccessControlList("drwho,joe tardis, users");
    users = acl.getUsers();
    assertEquals(users.size(), 2);
    iter = users.iterator();
    assertEquals(iter.next(), "drwho");
    assertEquals(iter.next(), "joe");
    groups = acl.getGroups();
    assertEquals(groups.size(), 2);
    iter = groups.iterator();
    assertEquals(iter.next(), "tardis");
    assertEquals(iter.next(), "users");
{code}
 

I have made both situations fail as an invalid format. To avoid any ambiguity, the format should be strict: {{u1,u2<white-space>g1,g2}}
{code:java|title=Patch}
  @Test(expected = IllegalArgumentException.class)
  public void testSpaceInGroupString() {
    // Proper format is: u1,u2 g1,g2
    new AccessControlList("drwho,joe tardis, group2");
  }

  @Test(expected = IllegalArgumentException.class)
  public void testSpaceInUSerString() {
    // Proper format is: u1,u2 g1,g2
    new AccessControlList("drwho, joe tardis,group2");
  }
{code}


was (Author: belugabehr):
OK, so I am providing a new patch with lessons learned from the failed unit tests and from HADOOP-12640. The two unit tests for MAPREDUCE-7155 and YARN-8928 pass locally.

I have made one functional change from how things are now:
{code:java}
    // The space in the group list is acceptable.
    // However, there is no test (and therefore ambiguity)
    // about what to do if the user list has a space in it.
    // There is only a test for a space in the group list
    Iterator<String> iter;    
    acl = new AccessControlList("drwho,joe tardis, users");
    users = acl.getUsers();
    assertEquals(users.size(), 2);
    iter = users.iterator();
    assertEquals(iter.next(), "drwho");
    assertEquals(iter.next(), "joe");
    groups = acl.getGroups();
    assertEquals(groups.size(), 2);
    iter = groups.iterator();
    assertEquals(iter.next(), "tardis");
    assertEquals(iter.next(), "users");
{code}
 

I have made both situations fail as an invalid format. To avoid any ambiguity, the format should be strict: {{u1,u2 g1,g2}}
{code:java|title=Patch}
  @Test(expected = IllegalArgumentException.class)
  public void testSpaceInGroupString() {
    // Proper format is: u1,u2 g1,g2
    new AccessControlList("drwho,joe tardis, group2");
  }

  @Test(expected = IllegalArgumentException.class)
  public void testSpaceInUSerString() {
    // Proper format is: u1,u2 g1,g2
    new AccessControlList("drwho, joe tardis,group2");
  }
{code}

> Review of AccessControlList
> ---------------------------
>
>                 Key: HADOOP-15836
>                 URL: https://issues.apache.org/jira/browse/HADOOP-15836
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: common, security
>    Affects Versions: 3.2.0
>            Reporter: BELUGA BEHR
>            Assignee: BELUGA BEHR
>            Priority: Minor
>             Fix For: 3.3.0
>
>         Attachments: HADOOP-15836.1.patch, HADOOP-15836.2.patch, assertEqualACLStrings.patch
>
>
> * Improve unit tests (expected / actual were backwards)
> * Unit test expected elements to be in order but the class's return Collections were unordered
> * Formatting cleanup
> * Removed superfluous white space
> * Remove use of LinkedList
> * Removed superfluous code
> * Use {{unmodifiable}} Collections where JavaDoc states that caller must not manipulate the data structure



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org