Skip to content
This repository was archived by the owner on Jul 21, 2021. It is now read-only.

Allow setting chroot for a connection #74

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

spenczar
Copy link
Contributor

Added simple chroot support, requested in #32.

Conn gets a SetChroot function which fiddles with the path of any requests which use a path.

It has a known bug: if the Chroot'd connection is asked to work on paths that look like the chroot itself, it will do things to the wrong directory. In particular, if you do this:

conn.SetChroot("/a/b")
conn.Create("/a/b/c", []byte{}, 0, WorldACL(PermAll))

Then you will not get a node written to "/a/b/a/b/c", as you might expect - it will go to "/a/b/c". This is unfortunate, but hard to avoid.

Also, we don't currently fiddle with response Paths, which is probably flat-out wrong... but I'll wait to hear whether this is a direction worth moving in before adding that.

@spenczar
Copy link
Contributor Author

I've added fiddling of response paths, and quite a few tests. The build failure is just from unsafe startup of the test cluster. I'll submit a fix for that in a separate PR.

@thorhs
Copy link

thorhs commented Aug 25, 2015

I would like this feature very much. To me, the client should not be aware that it has been chrooted, that is, all operations should return paths relative to the chroot, not the actual root.

Also, the library should accept connection strings in the same format as the normal ZK library:

connectString - comma separated host:port pairs, each corresponding to a zk server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002" If the optional chroot suffix is used the example would look like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a" where the client would be rooted at "/app/a" and all paths would be relative to this root - ie getting/setting/etc... "/foo/bar" would result in operations being run on "/app/a/foo/bar" (from the server perspective)

@spenczar
Copy link
Contributor Author

@thorhs , just to be clear - the current diff successfully hides chrooting from the client. It doesn't parse connection strings in any particularly clever way, though. I can add that, although I want to wait for #76 to get in so that I can run tests.

@zellyn
Copy link
Contributor

zellyn commented Dec 1, 2015

I've since added options to the Connect(...) method, so you should probably just make a WithChroot(string) connOption method for setting this.

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

Successfully merging this pull request may close these issues.

3 participants