[
https://issues.apache.org/jira/browse/HDFS-9295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14974779#comment-14974779
]
Zhe Zhang commented on HDFS-9295:
---------------------------------
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}}?
> Add a thorough test of the full KMS code path
> ---------------------------------------------
>
> Key: HDFS-9295
> URL: https://issues.apache.org/jira/browse/HDFS-9295
> Project: Hadoop HDFS
> Issue Type: Test
> Components: security, test
> Affects Versions: 2.7.1
> Reporter: Daniel Templeton
> Assignee: Daniel Templeton
> Priority: Critical
> Attachments: HDFS-9295.001.patch, HDFS-9295.002.patch
>
>
> 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
