I was being stupid and didn’t do something right in my IDE; hardcoding the “ip” ACL as I described in my last email does actually cause the test to pass. So CURATOR-58 is the problem. I’ll try to take a stab at a proper fix for it tomorrow.
- Robert On Thu, Nov 7, 2013 at 5:46 PM, Robert Kanter <[email protected]> wrote: > I tried changing the line mentioned in CURATOR-58 to simply always set the > “ip” ACL that I was using in my test: > - zookeeper.create(subPath, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE, > CreateMode.PERSISTENT); > + zookeeper.create(subPath, new byte[0], Collections.singletonList(new > ACL(ZooDefs.Perms.ALL, new Id("ip", "127.0.0.1"))), CreateMode.PERSISTENT); > > This caused my test to get stuck on trying to acquire the lock, which > makes me think the above change is working (presumably, ZK doesn’t think my > IP is “127.0.0.1”). However, when I added a timeout to acquiring the lock > (i.e. writeLock.acquire(3, TimeUnit.SECONDS);) so it could continue with > the test, it still shows that the znode has world ACLs. So it doesn’t look > like CURATOR-58 is the problem :( > > Any other ideas? > > Would it be better for me to file a JIRA with my test code and continue > the conversation there? > > thanks > - Robert > > > > On Thu, Nov 7, 2013 at 4:29 PM, Robert Kanter <[email protected]>wrote: > >> That could be it. I’ll try to take a look and see if I can figure out a >> patch :) >> >> thanks >> - Robert >> >> >> On Thu, Nov 7, 2013 at 4:16 PM, Jordan Zimmerman < >> [email protected]> wrote: >> >>> OK - I think this is the problem: >>> >>> https://issues.apache.org/jira/browse/CURATOR-58 >>> >>> Curator is creating parent nodes without going through the ACLProvider. >>> Please watch CURATOR-58. If you can, please submit a patch. >>> >>> -JZ >>> >>> On Nov 7, 2013, at 3:51 PM, Robert Kanter <[email protected]> wrote: >>> >>> Are you sure that this is something my ACLProvider has to handle? My >>> understanding is that the ACLProvider is a fairly “dumb” object that other >>> code is supposed to simply use to get the ACLs to apply when creating >>> znodes. The ACLProvider interface only has two methods: >>> public List<ACL> getDefaultAcl(); >>> public List<ACL> getAclForPath(String path); >>> So there’s nothing in here about protection mode. >>> >>> I think whatever code is changing the GUID-prefixed path into the >>> originally-named path (this does happen because when I browse the znodes >>> using ZKCli.sh it has “/foo” and not the prefixed one) needs to be checking >>> with the ACLProvider, and its not. I tried looking around the Curator >>> code, but I wasn’t able to find what does this. Do you know? >>> >>> >>> - Robert >>> >>> >>> >>> On Thu, Nov 7, 2013 at 3:40 PM, Jordan Zimmerman < >>> [email protected]> wrote: >>> >>>> Yes, this is called "protection" mode. I'll paste the details from the >>>> Javadoc below. The TL;DR is that this is required when using >>>> sequential/ephemeral with ZooKeeper. So, your ACLProvider needs to be able >>>> to handle this. >>>> >>>> -Jordan >>>> >>>> from create().withProtection(): >>>> >>>> It turns out there is an edge case that exists when creating >>>> sequential-ephemeral nodes. The creation can succeed on the server, but the >>>> server can crash before the created node name is returned to the client. >>>> However, the ZK session is still valid so the ephemeral node is not >>>> deleted. Thus, there is no way for the client to determine what node was >>>> created for them. >>>> Even without sequential-ephemeral, however, the create can succeed on >>>> the sever but the client (for various reasons) will not know it. >>>> >>>> Putting the create builder into protection mode works around this. The >>>> name of the node that is created is prefixed with a GUID. If node creation >>>> fails the normal retry mechanism will occur. On the retry, the parent path >>>> is first searched for a node that has the GUID in it. If that node is >>>> found, it is assumed to be the lost node that was successfully created on >>>> the first try and is returned to the caller. >>>> >>>> On Nov 7, 2013, at 1:13 PM, Robert Kanter <[email protected]> wrote: >>>> >>>> I did some more investigating on this issue. It looks like when you >>>> acquire a lock, it creates the path by doing: >>>> ourPath = >>>> client.create().creatingParentsIfNeeded().withProtection().withMode(CreateMode.EPHEMERAL_SEQUENTIAL).forPath(path); >>>> When I do that manually where path is “/foo”, it actually doesn’t >>>> create “/foo” yet and instead creates something like >>>> "/_c_2235fc7d-f5e8-4c3a-bb24-27c610022aaa-foo0000000000”. >>>> >>>> From what I can tell, this has to do with the withprotection() and >>>> EPHEMERAL_SEQUENTIAL mode and is to prevent some kind of problem I >>>> don’t quite understand from the javadocs. In any case, I believe that >>>> something eventually must rename >>>> "/_c_2235fc7d-f5e8-4c3a-bb24-27c610022aaa-foo0000000000” to “/foo”. When I >>>> checked, "/_c_2235fc7d-f5e8-4c3a-bb24-27c610022aaa-foo0000000000” has the >>>> ACLs I set in the ACLProvider, so I’m thinking the problem must be >>>> happening when the znode is renamed. I’m not sure where/when that happens, >>>> but I’d guess its not using the ACLProvider and that’s why “/foo" has >>>> the default ACLs. >>>> >>>> Do you think this is the cause? Any idea on how to fix it or >>>> workaround it? >>>> >>>> thanks >>>> - Robert >>>> >>>> >>>> >>>> On Tue, Nov 5, 2013 at 1:58 PM, Robert Kanter <[email protected]>wrote: >>>> >>>>> I created the below test class using JUnit. It starts a TestingServerand >>>>> connects to it; then it creates a path directly to verify that the >>>>> custom ACLProvider is being applied. Then it tries to do the same >>>>> with an InterProcessReadWriteLock and fails the test because its >>>>> using the default ACLs. I used “ip” instead of “sasl” to keep things >>>>> simpler. >>>>> >>>>> I did take a quick look at the Curator code and it seemed to be using >>>>> the ACLProvider through the CuratorFramework when using locks, but >>>>> perhaps I missed something (and I’m not super familiar with the codebase). >>>>> >>>>> Please take a look; thanks! >>>>> - Robert >>>>> >>>>> >>>>> import java.util.Collections; >>>>> import java.util.List; >>>>> import junit.framework.TestCase; >>>>> import org.apache.curator.RetryPolicy; >>>>> import org.apache.curator.framework.CuratorFramework; >>>>> import org.apache.curator.framework.CuratorFrameworkFactory; >>>>> import org.apache.curator.framework.api.ACLProvider; >>>>> import org.apache.curator.framework.recipes.locks.InterProcessMutex; >>>>> import >>>>> org.apache.curator.framework.recipes.locks.InterProcessReadWriteLock; >>>>> import org.apache.curator.retry.ExponentialBackoffRetry; >>>>> import org.apache.curator.test.TestingServer; >>>>> import org.apache.zookeeper.ZooDefs; >>>>> import org.apache.zookeeper.data.ACL; >>>>> import org.apache.zookeeper.data.Id<http://org.apache.zookeeper.data.id/> >>>>> ; >>>>> >>>>> public class TestLockACLs extends TestCase { >>>>> private TestingServer zkServer; >>>>> private CuratorFramework client; >>>>> private final List<ACL> acls = Collections.singletonList(new >>>>> ACL(ZooDefs.Perms.ALL, new Id("ip", "127.0.0.1"))); >>>>> >>>>> @Override >>>>> protected void setUp() throws Exception { >>>>> super.setUp(); >>>>> zkServer = new TestingServer(); >>>>> createClient(); >>>>> } >>>>> >>>>> @Override >>>>> protected void tearDown() throws Exception { >>>>> super.tearDown(); >>>>> client.close(); >>>>> zkServer.stop(); >>>>> zkServer.close(); >>>>> } >>>>> >>>>> private void createClient() throws Exception { >>>>> RetryPolicy retryPolicy = new ExponentialBackoffRetry(1000, 3); >>>>> String zkConnectionString = zkServer.getConnectString(); >>>>> String zkNamespace = "ns"; >>>>> client = CuratorFrameworkFactory.builder() >>>>> .namespace(zkNamespace) >>>>> >>>>> .connectString(zkConnectionString) >>>>> .retryPolicy(retryPolicy) >>>>> .aclProvider(new >>>>> MyACLProvider()) >>>>> .build(); >>>>> client.start(); >>>>> } >>>>> >>>>> public void testLockACLs() throws Exception { >>>>> // Create a path directly and verify that MyACLProvider is >>>>> being used >>>>> client.create().forPath("/foo"); >>>>> assertNotNull(client.checkExists().forPath("/foo")); >>>>> assertEquals(ZooDefs.Perms.ALL, >>>>> client.getACL().forPath("/foo").get(0).getPerms()); >>>>> assertEquals("ip", >>>>> client.getACL().forPath("/foo").get(0).getId().getScheme()); >>>>> assertEquals("127.0.0.1", >>>>> client.getACL().forPath("/foo").get(0).getId().getId()); >>>>> >>>>> // Now try creating a lock and we'll see that it incorrectly >>>>> has the default world ACLs >>>>> // and doesn't seem to be using MyACLProvider >>>>> InterProcessReadWriteLock lock = new >>>>> InterProcessReadWriteLock(client, "/bar"); >>>>> InterProcessMutex writeLock = lock.writeLock(); >>>>> writeLock.acquire(); >>>>> assertNotNull(client.checkExists().forPath("/bar")); >>>>> assertEquals(ZooDefs.Perms.ALL, >>>>> client.getACL().forPath("/bar").get(0).getPerms()); >>>>> assertEquals("ip", >>>>> client.getACL().forPath("/bar").get(0).getId().getScheme()); >>>>> assertEquals("127.0.0.1", >>>>> client.getACL().forPath("/bar").get(0).getId().getId()); >>>>> } >>>>> >>>>> public class MyACLProvider implements ACLProvider { >>>>> >>>>> @Override >>>>> public List<ACL> getDefaultAcl() { >>>>> return acls; >>>>> } >>>>> >>>>> @Override >>>>> public List<ACL> getAclForPath(String path) { >>>>> return acls; >>>>> } >>>>> } >>>>> } >>>>> >>>>> >>>>> On Mon, Nov 4, 2013 at 6:10 PM, Jordan Zimmerman < >>>>> [email protected]> wrote: >>>>> >>>>>> The ACLProvider should be called for every node created. It’s not >>>>>> getting called? Can you produce a test that shows this? >>>>>> >>>>>> -Jordan >>>>>> >>>>>> On Nov 4, 2013, at 5:57 PM, Robert Kanter <[email protected]> >>>>>> wrote: >>>>>> >>>>>> I have everything working now except for one thing: >>>>>> The ACLProvider doesn’t seem to be used for the locks (Curator’s >>>>>> InterProcessReadWriteLock); they are always created with the default >>>>>> fully open ACLs. I know the ACLProvider is correct now because the >>>>>> service discovery is using it and znodes created by it have the correct >>>>>> ACLs. InterProcessReadWriteLock’s constructor takes in the >>>>>> CuratorFramework object, which has the ACLProvider set. >>>>>> >>>>>> Any ideas? >>>>>> This sounds like it could be a Curator bug :( >>>>>> I’m not familiar with Curator’s codebase, but I’ll try to take a look >>>>>> and see if I can figure it out. >>>>>> >>>>>> thanks >>>>>> - Robert >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Nov 4, 2013 at 1:09 PM, Robert Kanter >>>>>> <[email protected]>wrote: >>>>>> >>>>>>> I don’t have it 100% working yet, but I’ve figured out a lot more, >>>>>>> so I thought I’d share in case anyone else runs into this: >>>>>>> >>>>>>> The ZooDefs.Ids.CREATOR_ALL_ACL predefined ACL that I was trying to >>>>>>> use is for the “auth” scheme. For SASL/Kerberos, we want “sasl”. >>>>>>> The javadoc for the predefined one wasn’t very clear on that; I had to >>>>>>> look at the code. Using this is working: >>>>>>> Collections.singletonList(new ACL(Perms.ALL, new Id("sasl", >>>>>>> principal))); >>>>>>> >>>>>>> I was also able to find answers to the three questions I asked: >>>>>>> 1) Yes; looking through the code, its definitely grabbing the >>>>>>> ACLProvider and using it. >>>>>>> 2) Yes; I think the only way to do this is to recursively travel >>>>>>> through the znodes under /oozie and apply the ACL on starting up Oozie. >>>>>>> We should only have to do this if previously it was setup without >>>>>>> security and has since been reconfigured to use security; so we should >>>>>>> only >>>>>>> have to do this once. I can probably have a znode as a flag that >>>>>>> states if everything has ACLs or not to make it more efficient >>>>>>> 3) It doesn’t look like it; I’ll have to get the ZK client and do >>>>>>> it from outside Curator >>>>>>> >>>>>>> >>>>>>> - Robert >>>>>>> >>>>>>> >>>>>>> On Mon, Oct 28, 2013 at 5:47 PM, Jordan Zimmerman < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> I don’t have any experience with this. Curator doesn’t do much - it >>>>>>>> sets up the ACL as the CLI options dictate. I do know that you also >>>>>>>> have to >>>>>>>> do work on the server side to make this work. >>>>>>>> >>>>>>>> -JZ >>>>>>>> >>>>>>>> On Oct 24, 2013, at 4:58 PM, Robert Kanter <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Is there any documentation on using an ACLProvider and/or >>>>>>>> Kerberos? >>>>>>>> >>>>>>>> From what I gathered at various sites, to use Kerberos, all I have >>>>>>>> to do is set the following properties before building the >>>>>>>> CuratorFramework >>>>>>>> client: >>>>>>>> System.setProperty("java.security.auth.login.config", >>>>>>>> "/path/to/jaasConfFile"); >>>>>>>> >>>>>>>> System.setProperty("zookeeper.authProvider.1","org.apache.zookeeper.server.auth.SASLAuthenticationProvider"); >>>>>>>> System.setProperty(ZooKeeperSaslClient.LOGIN_CONTEXT_NAME_KEY, >>>>>>>> "Client"); >>>>>>>> Looking at the logs for the client and server, this appears to be >>>>>>>> working properly and my program is connecting to ZooKeeper using >>>>>>>> Kerberos. >>>>>>>> >>>>>>>> The problem I'm having is with the ACLs. >>>>>>>> >>>>>>>> I'd like to set the ACLs so that only the Kerberos user running the >>>>>>>> program can do anything. From what I can tell, if I specify an >>>>>>>> ACLProvider, then Curator will automatically use it for setting >>>>>>>> ACLs on all paths. So, an ACLProvider like the following should >>>>>>>> do what I want: >>>>>>>> public class CreatorACLProvider implements ACLProvider { >>>>>>>> @Override >>>>>>>> public List<ACL> getDefaultAcl() { >>>>>>>> return ZooDefs.Ids.CREATOR_ALL_ACL; >>>>>>>> } >>>>>>>> @Override >>>>>>>> public List<ACL> getAclForPath(String path) { >>>>>>>> return ZooDefs.Ids.CREATOR_ALL_ACL; >>>>>>>> } >>>>>>>> } >>>>>>>> Then I would just do this: >>>>>>>> client = CuratorFrameworkFactory.builder() >>>>>>>> .namespace(zkNamespace) >>>>>>>> .connectString(zkConnectionString) >>>>>>>> .retryPolicy(retryPolicy) >>>>>>>> .aclProvider(new >>>>>>>> CreatorACLProvider()) >>>>>>>> .build(); >>>>>>>> client.start(); >>>>>>>> >>>>>>>> However, this doesn't seem to be working. The zkcli returns this >>>>>>>> (on a newly created znode): >>>>>>>> [zk: localhost:2181(CONNECTED) 8] getAcl >>>>>>>> /oozie/locks/0000000-131024162150146-oozie-oozi-W >>>>>>>> 'world,'anyone >>>>>>>> : Cdr. >>>>>>>> Is there something that I missed? >>>>>>>> >>>>>>>> A few other questions: >>>>>>>> 1) Will the ACLProvider cause the ACLs to be applied to znodes >>>>>>>> created by the Curator recipes? (e.g. InterProcessReadWriteLock, >>>>>>>> ServiceDiscovery, etc). If not, then how should I go about >>>>>>>> setting the ACLs for these znodes? >>>>>>>> 2) I'm guessing that the ACLProvider is only applied when creating >>>>>>>> the znode, right; so existing znodes from before I added the >>>>>>>> ACLProvider won't have the ACLs I want, right? What would be the >>>>>>>> best way to apply the ACLs to any existing znodes that don't have it >>>>>>>> set? >>>>>>>> (My goal is to have all znodes under /oozie have the >>>>>>>> CREATOR_ALL_ACL) >>>>>>>> 3) Is there a way to set the ACLs on the namespace itself (i.e. >>>>>>>> /oozie)? The methods that take a path (and automatically prepend >>>>>>>> the namespace) don't allow simply "/", so it seems like I'd have >>>>>>>> to use the ZooKeeper client directly to set ACLs manually on the >>>>>>>> namespace. >>>>>>>> Or would simply passing an empty string "" work? >>>>>>>> >>>>>>>> thanks >>>>>>>> - Robert >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >>> >> >
