Skip to content

acl for zookeeper is added#2190

Closed
genevien wants to merge 1 commit intoapache:masterfrom
genevien:feature-zk-acl
Closed

acl for zookeeper is added#2190
genevien wants to merge 1 commit intoapache:masterfrom
genevien:feature-zk-acl

Conversation

@genevien
Copy link
Copy Markdown

@genevien genevien commented Jan 4, 2016

With this patch you can enable ACL security for zookeeper. By default it will be disabled. In order to enable it you should add this line in your druid properties file:
druid.zk.service.acl=true

@drcrallen
Copy link
Copy Markdown
Contributor

Looks reasonable. Thanks for the patch!

Can you add some basic testing in io.druid.curator.CuratorConfigTest ?

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 4, 2016

👍

Do you mind signing the CLA here? http://druid.io/community/cla.html

@genevien
Copy link
Copy Markdown
Author

genevien commented Jan 5, 2016

@fjy just signed.

@genevien
Copy link
Copy Markdown
Author

genevien commented Jan 5, 2016

@drcrallen Do you mean something like testHostName()? I'm confused because I can't see any tests for zkSessionTimeoutMs and enableCompression parameters.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 5, 2016

@drcrallen Same thoughts here. We have to create the framework so it is easy for contributors to add their UT.

@himanshug
Copy link
Copy Markdown
Contributor

@genevien with just this, how is zookeeper server going to identify the connecting user to be able to enforce ACL ?

@genevien
Copy link
Copy Markdown
Author

genevien commented Jan 6, 2016

@himanshug on my local environment I have configured zookeeper server according to https://cwiki.apache.org/confluence/display/ZOOKEEPER/Zookeeper+and+SASL and I have run druid process setting "-Djava.security.auth.login.config=/path/to/client/jaas/file.conf"

@himanshug
Copy link
Copy Markdown
Contributor

@genevien thanks, can you also update https://github.com/druid-io/druid/blob/master/server/src/test/java/io/druid/curator/CuratorConfigTest.java to check that serde for acl attribute is working as expected.

@xvrl
Copy link
Copy Markdown
Member

xvrl commented Jan 6, 2016

@genevien can you update the documentation to reflect this new setting?

@genevien
Copy link
Copy Markdown
Author

genevien commented Jan 8, 2016

@himanshug @xvrl sure, I will do it after the weekends.

@fjy fjy mentioned this pull request Jan 13, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Jan 13, 2016

@genevien I added the tests and docs for you here: #2258

Closing this PR.

@fjy fjy closed this Jan 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants