Michael Smith has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20269 )

Change subject: IMPALA-12318: Add a flag option for http spnego dedicated 
keytab file.
......................................................................


Patch Set 7:

(4 comments)

The ticket sounds like a reasonable improvement, just have some questions and 
clarification about the changes.

I see you put up a similar Kudu change - https://gerrit.cloudera.org/c/20278/ - 
and be/src/kudu should be kept in-sync with its changes.

http://gerrit.cloudera.org:8080/#/c/20269/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20269/7//COMMIT_MSG@7
PS7, Line 7: IMPALA-12318: Add a flag option for http spnego dedicated keytab 
file.
Please describe what led to this change in the commit message.


http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/kudu/security/gssapi.h
File be/src/kudu/security/gssapi.h:

http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/kudu/security/gssapi.h@22
PS7, Line 22: #include <gssapi/gssapi_krb5.h>
Why is this needed here? Could it be added in gssapi.cc?


http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/kudu/security/gssapi.cc
File be/src/kudu/security/gssapi.cc:

http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/kudu/security/gssapi.cc@131
PS7, Line 131:     
krb5_gss_register_acceptor_identity(FLAGS_spnego_keytab_file.c_str());
I'm not clear how this avoids overriding keytab_file globally.


http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/util/webserver.cc
File be/src/util/webserver.cc:

http://gerrit.cloudera.org:8080/#/c/20269/7/be/src/util/webserver.cc@479
PS7, Line 479:     const char* kt_file = FLAGS_spnego_keytab_file.empty() ?
Can you add to the above comment explaining the conditional behavior?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4794ca97316c63a0e6fef9f7428fc05dd9904b0
Gerrit-Change-Number: 20269
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Fri, 28 Jul 2023 21:35:53 +0000
Gerrit-HasComments: Yes

Reply via email to