Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-9295

Add a thorough test of the full KMS code path

    Details

    • Type: Test
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.7.1
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: security, test
    • Labels:
      None
    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      TestKMS does a good job of testing the ACLs directly, but they are tested out of context. Additional tests are needed that test how the ACL impact key creation, EZ creation, file creation in an EZ, etc.

      1. HDFS-9295.003.patch
        23 kB
        Daniel Templeton
      2. HDFS-9295.002.patch
        54 kB
        Daniel Templeton
      3. HDFS-9295.001.patch
        54 kB
        Daniel Templeton

        Activity

        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        -1 pre-patch 8m 31s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        -1 javac 8m 32s The applied patch generated 22 additional warning messages.
        +1 release audit 0m 21s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 33s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 36s mvn install still works.
        +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse.
        +1 findbugs 2m 40s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 1m 9s Pre-build of native portion
        -1 hdfs tests 66m 51s Tests failed in hadoop-hdfs.
            91m 52s  



        Reason Tests
        Failed unit tests hadoop.hdfs.TestSafeMode
          hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12768294/HDFS-9295.001.patch
        Optional Tests javac unit findbugs checkstyle
        git revision trunk / 124a412
        Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
        javac https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/diffJavacWarnings.txt
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13152/testReport/
        Java 1.7.0_55
        uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13152/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 8m 31s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. -1 javac 8m 32s The applied patch generated 22 additional warning messages. +1 release audit 0m 21s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 33s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 35s The patch built with eclipse:eclipse. +1 findbugs 2m 40s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 1m 9s Pre-build of native portion -1 hdfs tests 66m 51s Tests failed in hadoop-hdfs.     91m 52s   Reason Tests Failed unit tests hadoop.hdfs.TestSafeMode   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12768294/HDFS-9295.001.patch Optional Tests javac unit findbugs checkstyle git revision trunk / 124a412 Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html javac https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/diffJavacWarnings.txt hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/13152/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13152/testReport/ Java 1.7.0_55 uname Linux asf909.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13152/console This message was automatically generated.
        Hide
        templedf Daniel Templeton added a comment -

        Fixed complier warnings

        Show
        templedf Daniel Templeton added a comment - Fixed complier warnings
        Hide
        hadoopqa Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        -1 pre-patch 8m 14s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 53s There were no new javac warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 1m 25s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 30s mvn install still works.
        +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse.
        +1 findbugs 2m 30s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
        +1 native 1m 2s Pre-build of native portion
        -1 hdfs tests 52m 13s Tests failed in hadoop-hdfs.
            75m 47s  



        Reason Tests
        Failed unit tests hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes
          hadoop.hdfs.server.datanode.TestDirectoryScanner
          hadoop.hdfs.server.datanode.TestFsDatasetCache



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12768338/HDFS-9295.002.patch
        Optional Tests javac unit findbugs checkstyle
        git revision trunk / eb6379c
        Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/13160/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html
        hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/13160/artifact/patchprocess/testrun_hadoop-hdfs.txt
        Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13160/testReport/
        Java 1.7.0_55
        uname Linux asf902.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13160/console

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 pre-patch 8m 14s Pre-patch trunk has 1 extant Findbugs (version 3.0.0) warnings. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 53s There were no new javac warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 1m 25s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 30s mvn install still works. +1 eclipse:eclipse 0m 34s The patch built with eclipse:eclipse. +1 findbugs 2m 30s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 native 1m 2s Pre-build of native portion -1 hdfs tests 52m 13s Tests failed in hadoop-hdfs.     75m 47s   Reason Tests Failed unit tests hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes   hadoop.hdfs.server.datanode.TestDirectoryScanner   hadoop.hdfs.server.datanode.TestFsDatasetCache Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12768338/HDFS-9295.002.patch Optional Tests javac unit findbugs checkstyle git revision trunk / eb6379c Pre-patch Findbugs warnings https://builds.apache.org/job/PreCommit-HDFS-Build/13160/artifact/patchprocess/trunkFindbugsWarningshadoop-hdfs.html hadoop-hdfs test log https://builds.apache.org/job/PreCommit-HDFS-Build/13160/artifact/patchprocess/testrun_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/13160/testReport/ Java 1.7.0_55 uname Linux asf902.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HDFS-Build/13160/console This message was automatically generated.
        Hide
        zhz Zhe Zhang added a comment -

        Excellent work Daniel! The test is much more comprehensive and systematic than existing ones on KMS ACLs. The patch LGTM overall. Some comments below:

        1. Name clash with org.apache.hadoop.crypto.key.kms.server.TestKMSACLs. Ideally we can separate out the non-EZ tests into the original TestKMSACLs (because intuitively that should be the place to holistically test KMS ACL logics, including whitelist, blacklist, and key ACL). But I think we can do that separately. So let's just think of a different name here.
        2. I think the common parts among testGoodWithWhitelist, testGoodWithKeyAcls, and testGoodWithWhitelistWithoutBlacklist are worth separating out. This is just for better readability and can be done in a separate JIRA as well.
        3. Right now we have whitelist + blacklist, no-whitelist (only key ACLs) + blacklist, whitelist + no-blacklist. Does it make sense to complete the spectrum with no-whitelist + no-blacklist? I think that means only KEY_ACL settings? The above readability suggestion might make this easier.
        4. IIUC, the generated configs from {testGoodWithWhitelist}}, testGoodWithKeyAcls, and testGoodWithWhitelistWithoutBlacklist have the same "actual" permissions, so that they can be verified by the same doFullAclTest, right? If so it's worth a brief comment.
        5. I can see that there are 2 dimensions of tests: you first try to test a comprehensive set of operations against a certain config, and then test a comprehensive set of configs against a certain operation. I like this methodology a lot. To make the patch smaller and more tractable, I suggest we separate the 2 dimensions into 2 patches.

        Nits:

        1. Typo: Exeception – unless this is a British spelling I'm not aware of
        2. UserOp#run makes it look like a thread. Maybe something like runCommand, runOperation, or execute?
        Show
        zhz Zhe Zhang added a comment - Excellent work Daniel! The test is much more comprehensive and systematic than existing ones on KMS ACLs. The patch LGTM overall. Some comments below: Name clash with org.apache.hadoop.crypto.key.kms.server.TestKMSACLs . Ideally we can separate out the non-EZ tests into the original TestKMSACLs (because intuitively that should be the place to holistically test KMS ACL logics, including whitelist, blacklist, and key ACL). But I think we can do that separately. So let's just think of a different name here. I think the common parts among testGoodWithWhitelist , testGoodWithKeyAcls , and testGoodWithWhitelistWithoutBlacklist are worth separating out. This is just for better readability and can be done in a separate JIRA as well. Right now we have whitelist + blacklist , no-whitelist (only key ACLs) + blacklist , whitelist + no-blacklist . Does it make sense to complete the spectrum with no-whitelist + no-blacklist ? I think that means only KEY_ACL settings? The above readability suggestion might make this easier. IIUC, the generated configs from {testGoodWithWhitelist}}, testGoodWithKeyAcls , and testGoodWithWhitelistWithoutBlacklist have the same "actual" permissions, so that they can be verified by the same doFullAclTest , right? If so it's worth a brief comment. I can see that there are 2 dimensions of tests: you first try to test a comprehensive set of operations against a certain config, and then test a comprehensive set of configs against a certain operation. I like this methodology a lot. To make the patch smaller and more tractable, I suggest we separate the 2 dimensions into 2 patches. Nits: Typo: Exeception – unless this is a British spelling I'm not aware of UserOp#run makes it look like a thread. Maybe something like runCommand , runOperation , or execute ?
        Hide
        templedf Daniel Templeton added a comment -

        Zhe Zhang, I think this patch addresses all of your concerns.

        Show
        templedf Daniel Templeton added a comment - Zhe Zhang , I think this patch addresses all of your concerns.
        Hide
        zhz Zhe Zhang added a comment -

        03 patch looks great! Thanks very much for the update Daniel. +1, committing it to trunk and branch-2 now.

        Show
        zhz Zhe Zhang added a comment - 03 patch looks great! Thanks very much for the update Daniel. +1, committing it to trunk and branch-2 now.
        Hide
        zhz Zhe Zhang added a comment -

        I just committed the patch to trunk and branch-2. Thanks Daniel for the work! I look forward to the follow-on JIRA with the second set of tests

        Show
        zhz Zhe Zhang added a comment - I just committed the patch to trunk and branch-2. Thanks Daniel for the work! I look forward to the follow-on JIRA with the second set of tests
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #8721 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8721/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #8721 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8721/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #597 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/597/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #597 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/597/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2540/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2540 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2540/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #1333 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1333/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #1333 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1333/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #610 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/610/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #610 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/610/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2486 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2486/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2486 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2486/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #548 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/548/)
        HDFS-9295. Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919)

        • hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #548 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/548/ ) HDFS-9295 . Add a thorough test of the full KMS code path. Contributed by (zhz: rev 73bc65e02ba7ea8a7d622a5ba8936531b341b919) hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestAclsEndToEnd.java

          People

          • Assignee:
            templedf Daniel Templeton
            Reporter:
            templedf Daniel Templeton
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development