Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Kerberos Support via TSaslClientTransport #100

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rickysaltzer
Copy link

  • Added a sasl dependency to requirements
  • Added working TSaslClientTransport
  • 2 optional arguments were added to the Connection class:
    :use_kerberos   | signals to use secure authentication
    :sasl_service   | name of the SASL service (default: hbase)

If you enable Kerberos by using use_kerberos=True, then you must first remember to kinit before running your Python code.

One thing still up in the air, is the requirement for sasl. Perhaps I should leave it out of the requirements and only import if the user has it installed. That way it doesn't force people who have no desire to connect to secure HBase clusters to have sasl installed.

I have tested this successfully against an HBase cluster secured with Kerberos.

@landscape-bot
Copy link

Code Health
Repository health increased by 1% when pulling a18f12d on rickysaltzer:kerberos into 9cbd718 on wbolster:master.

@rickysaltzer
Copy link
Author

After doing some testing, I found that the keytab is actually unnecessary since you must kinit before running Python. I'll fix this and not require it.

- Added a "sasl" dependency to requirements
- Added working TSaslClientTransport

- 2 (optional) arguments were added to the Connection class:
	:use_kerberos		| signals to use secure authentication
	:sasl_service		| name of the SASL service (default: hbase)
@landscape-bot
Copy link

Code Health
Repository health increased by 0.86% when pulling 1c1e507 on rickysaltzer:kerberos into 9cbd718 on wbolster:master.

@@ -9,11 +9,14 @@
from thrift.transport.TSocket import TSocket
from thrift.transport.TTransport import TBufferedTransport, TFramedTransport
from thrift.protocol import TBinaryProtocol, TCompactProtocol
import sasl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to install a C extension. Which system dependencies does that one have? Will it compile on any system?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependencies are the cyrus-sasl extension. Those are available on every reasonable recent *nix system (ie. year 2000+ or so)

@wbolster
Copy link
Member

hi, thanks for your pr. i've added a bunch of (sometimes nitpicky) comments.

@gglanzani
Copy link

Hi @wbolster and @rickysaltzer what's the status on this one? It looks really neat so I'd love to see in the official release (not that I cannot use it right now)

@bolkedebruin
Copy link

Overall I would make the dependency optional, as py3 sasl support isn't really there yet.

@rickysaltzer
Copy link
Author

really sorry I haven't been able to devote much time to getting this release worthy. I'll try and find time as soon as I can.

@gglanzani
Copy link

@rickysaltzer Any updates on this one?

@gglanzani gglanzani mentioned this pull request Mar 8, 2016
@gglanzani
Copy link

I've made a new PR, as this branch stopped working for me in a kerberized cluster.

@wbolster
Copy link
Member

also, happybase is migrating to thriftpy (instead of using thrift).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants