Xiaomeng Zhang 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 6:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/14520/4/src/kudu/security/init.cc@466
PS4, Line 466:   }
             :   realm->assign(princ->realm.data, princ->realm.length);
             :   servi
> What the SCOPED_CLEANUP block is doing is saying "when I go out of scope, r
Done


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

http://gerrit.cloudera.org:8080/#/c/14520/5/src/kudu/security/init.cc@464
PS5, Line 464: "bad principal format,
> Does it make sense to add information on what was the input string principa
Done


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

http://gerrit.cloudera.org:8080/#/c/14520/4/src/kudu/security/test/mini_kdc-test.cc@79
PS4, Line 79:   string principal = "kudu/[email protected]";
            :   string service_name;
            :   string hostname;
            :   string realm;
> In Kudu, only class members are suffixed with an underscore. Regular locals
Done


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.
> This is a good test for the 'happy' or 'positive' case.
So the requirements is that principal must have service_name, hostname and 
realm. I find that mini_kdc has default realm. So what we can test is empty 
string, and missing service_name or hostname.



--
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: 6
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 22:15:45 +0000
Gerrit-HasComments: Yes

Reply via email to