[
https://issues.apache.org/jira/browse/HDFS-10450?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15474617#comment-15474617
]
James Clampffer commented on HDFS-10450:
----------------------------------------
bq. per_connection_callbacks is required by the Cyrus SASL library to be a
specially-terminated array of structs. We're (without heroic effort) limited by
the public interface of the library we're using.
Oh, that makes much more sense. Thanks for the explanation.
Thanks for fixing the rest of those things, I just have a few more minor ones
and then I think it's good to go.
1) Still a bunch of places with "using namespace hdfs".
2) Still places with the "// end <scopename>" at the back of fairly small
blocks e.g. srrStr in cyrus_sasl_engine.cc. This isn't really a blocker for me
though.
3) In sasl_protocol.cc:
{code}
- LOG_DEBUG(kRPC, << "Tried sending an AuthComplete but the RPC connection
was gone: " << status.ToString());
+ LOG_DEBUG(kRPC, << "Tried sending an AuthComplete but the RPC connection
was gone");
{code}
Any reason not to keep the status in the output here? Same thing in
SaslProtocol::OnServerResponse:
{code}
- LOG_TRACE(kRPC, << "Received SASL response: " << status.ToString());
+ LOG_TRACE(kRPC, << "Received SASL response");
{code}
In general I wonder if it's worth keeping a file around as a staging area for
holding things like getsecret in cyrus_sasl_engine.cc (it's currently ~40 lines
commented out). I've wanted to do that before for things like debugging macros
like MEMCHECKED_CLASS that weren't ready to be part of the core library but
would be nice to keep under version control. Any thoughts about that?
The underlying code for the sasl engines looks safe in terms of
memory/threading and to the best of my understanding is interacting with gsasl
and cyrus correctly. I'll +1 once there's a passing run and the minor
issues/nits above are addressed.
> libhdfs++: Implement Cyrus SASL implementation in sasl_enigne.cc
> ----------------------------------------------------------------
>
> Key: HDFS-10450
> URL: https://issues.apache.org/jira/browse/HDFS-10450
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Reporter: Bob Hansen
> Assignee: Bob Hansen
> Attachments: HDFS-10450.HDFS-8707.000.patch,
> HDFS-10450.HDFS-8707.001.patch, HDFS-10450.HDFS-8707.002.patch,
> HDFS-10450.HDFS-8707.003.patch
>
>
> The current sasl_engine implementation was proven out using GSASL, which is
> does not have an ASF-approved license. It included a framework to use Cyrus
> SASL (libsasl2.so) instead; we should complete that implementation.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
