Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14520 )

Change subject: KUDU-2979: Add wrapper function of krb5_parse_name to be used 
in Impala
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14520/6/src/kudu/security/init.cc
File src/kudu/security/init.cc:

http://gerrit.cloudera.org:8080/#/c/14520/6/src/kudu/security/init.cc@464
PS6, Line 464: "bad principal format");
             :   }
Could you also add the original principal string into the error message?  That 
could help in troubleshooting if this error is ever triggered.

That would be something like:

  return Status::InvalidArgument(Substitute("$0: principal should include 
service name, hostname and realm", principal));


http://gerrit.cloudera.org:8080/#/c/14520/5/src/kudu/security/test/mini_kdc-test.cc
File src/kudu/security/test/mini_kdc-test.cc:

http://gerrit.cloudera.org:8080/#/c/14520/5/src/kudu/security/test/mini_kdc-test.cc@78
PS5, Line 78:   // Test parse krb5 principal.
> So the requirements is that principal must have service_name, hostname and
Thanks!

Then maybe add the case of `kudu/foo.example.com` as a 'happy' case as well, 
please.



--
To view, visit http://gerrit.cloudera.org:8080/14520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifddafa7aae25d66ed7d9fa0306f17501a191cdac
Gerrit-Change-Number: 14520
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaomeng Zhang <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Sherman <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Xiaomeng Zhang <[email protected]>
Gerrit-Comment-Date: Fri, 25 Oct 2019 23:12:20 +0000
Gerrit-HasComments: Yes

Reply via email to