[
https://issues.apache.org/jira/browse/HADOOP-10736?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14054591#comment-14054591
]
Mike Yoder commented on HADOOP-10736:
-------------------------------------
{quote}
* Nit: reviewers appreciate it when you version the filenames of uploaded
patches, e.g. "HADOOP-10736.002.patch"
* In KeyProvider#toString, let's convert the entire thing to use StringBuilder,
no need for MessageFormat. I'd also mildly prefer to print "null" with no
period rather than "none.".
{quote}
Done and done.
{quote}
* I think KeyShell is unique in Hadoop in using two hyphens for command-line
flags. Do you mind filing a follow-on JIRA to fix this? We might also be able
to use GenericOptionsParser, which supports parsing key-value pairs.
{quote}
Huh. I do like the gnu-style double hyphens, but you're right that uniformity
trumps that. GenericOptionsParser seems to have lots of things that aren't
needed, but 'CommandLine' might be a good fit. I'll file a jira issue...but
what about backwards compatibility? If the interface is in use, aren't we
stuck with it forever?
{quote}
* Could add some quick error checking to make sure a user doesn't specify the
same key twice
{quote}
Done, and test added.
{quote}
* Is a value required? That is, specifying a null vs an empty string.
{quote}
As written, yes. It won't even allow the empty string for a value. I think
this keeps things simple.
{quote}
* Bunch of parsing questions. What if you want your key to have an equals sign
or a quote? I don't know how the commandline is parsed into a String[] either,
so some of the tests might not be executing realistic situations. Have you
tried these with actual "hadoop key" invocations? These issues might go away if
you use GenericOptionsParser; maybe we should do that conversion first if so.
{quote}
All of this is taken care of by the shell - here are some examples:
{noformat}
$ hadoop key create happykey2 --provider kms://http@localhost:16000/kms \
--attr "spaces=internal space" --attr "equals = another=here" --attr "quote=\""
--attr "ke\"y=help\"me"
$ hadoop key list --provider kms://http@localhost:16000/kms --metadata
happykey2 : cipher: AES/CTR/NoPadding, length: 256, description: null,
created: Mon Jul 07 23:36:16 PDT 2014, version: 1,
attributes: [spaces=internal space], [equals=another=here], [quote="],
[ke"y=help"me]
{noformat}
As long as the arg after --attr is quoted, you can put anything you like in
there. The only limitation is that the key itself can't have an equals sign,
since we split on the first equals. But really, there's not much you can do
about that unless you change the interface to remove the equals sign altogether
and have the attribute and value be two independent tokens, like
--attr foo bar --attr glarch baz
etc - I think this is slightly more confusing for a highly dubious gain. :-)
{quote}
* In CreateCommand, the new sentence you added has two spaces after the period
rather than one, which is the style of the rest of the paragraph.
{quote}
You just nerd-sniped me - did you know that the wikipedia entry on sentence
spacing has 96 references? Anyway I changed it to one space.
As an aside... I've seen this in several places now - commands returning -1 on
failure to the shell. It's a bug. Someone confused their posix style returns
(0 on success, < 0 on failure) with program returns, which are an unsigned
character. Thus, a return of -1 actually becomes 255 to the shell.
{noformat}
$ hadoop key create happykey2 --provider kms://http@localhost:16000/kms --attr
"a=a" --attr "a=b"
Each attribute must correspond to only one value:
atttribute "a" was repeated
...
$ echo $?
255
{noformat}
We really ought to be returning 1, not -1. But I guess it's too late to change
this code (and the other code I've seen that does this...) now.
> Add key attributes to the key shell
> -----------------------------------
>
> Key: HADOOP-10736
> URL: https://issues.apache.org/jira/browse/HADOOP-10736
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Affects Versions: 3.0.0
> Reporter: Mike Yoder
> Assignee: Mike Yoder
> Fix For: 3.0.0
>
> Attachments: HADOOP-10736.003.patch, HADOOP-10736.patch,
> HADOOP-10736.patch
>
>
> The recent work in HADOOP-10696 added attribute-value pairs to keys in the
> KMS. Now the key shell needs to be updated to set/get these attributes.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
