[Gluster-devel] What would be ideal option for 'auth.allow' to support subdir mount?

[Amar Tumballi]

On Thu, Jul 20, 2017 at 9:21 PM, Niels de Vos <ndevos at redhat.com> wrote:

> On Thu, Jul 20, 2017 at 08:25:23PM +0530, Amar Tumballi wrote:
> > On Thu, Jul 20, 2017 at 7:36 PM, Niels de Vos <ndevos at redhat.com> wrote:
> >
> > > On Thu, Jul 20, 2017 at 07:11:29PM +0530, Amar Tumballi wrote:
> > > > Hi,
> > > >
> > > > I was working on subdir mount for fuse clients [1], and was able to
> > > handle
> > > > pieces just fine in filesystem part of gluster. [2]
> > > >
> > > > What is pending is, how will we handle the authentication options for
> > > this
> > > > at each subdir level?
> > > >
> > > > I propose to keep the current option and extending it to handle new
> > > feature
> > > > with proper backward compatibility.
> > > >
> > > > Currently, the option auth.allow (and auth.reject) are of the type
> > > > GF_OPTION_TYPE_INTERNET_ADDRESS_LIST. Which expects valid internet
> > > > addresses with comma separation.
> > > >
> > > > For example the current option looks likes this:
> > > >
> > > >  'option auth.addr.brick-name.allow *' OR 'option
> > > > auth.addr.brick-name.allow "192.168.*.* ,10.10.*.*"'.
> > > >
> > > > In future, it may look like:
> > > >
> > > > `option auth.addr.brick-name.allow "10.0.1.13;192.168.1.*
> > > > =/subdir1;192.168.10.* ,192.168.11.104 =/subdir2"`
> > > >
> > > > so each entries will be separated by ';'. And in each entry, first
> part
> > > ("
> > > > =") is address list and second part is directory. If directory is
> empty,
> > > > its assumed as '/'. (Handles the backward compatibility). And if
> there is
> > > > no entry for a $subdir here, that $subdir won't be mountable.
> > >
> > > IIRC Gluster/NFS allows you to set permissions for subdir mounting with
> > > a format like this:
> > >
> > >   /subdir/next/dir(IP,IP-range,...) /subdir2(IP)
> > >
> > > This is good, but would currently break the compatibility with existing
> > auth.allow of gluster.
> >
> > Backward compatibility was the main reason for me to consider the above
> > approach.
> >
> > It would be best to use the existing format if we can to prevent
> > > confusion among our users.
> > >
> > > Currently existing gluster's option is not same as NFS in my opinion.
> How
> > do you want to handle it?
>
> I'm wondering if the current format that us used for NFS is not
> sufficient? Some defaults and quircks that would apply:
>
>
Should be sufficient. Earlier I was not sure of which option you were
talking.

For everyone's clarity, I assume Niels is talking about 'nfs3.*.export-dir'
option in xlators/nfs/server/src/nfs.c.

It is of the form:  /foo(192.168.1.0/24|host1|10.1.1.8),/host2.

<dir>[(hostspec[|hostspec|...])][,...]

But point to note here, it is of form GF_OPTION_STR, which means there
won't be any validation for this key, unline current gluster's
server-protocol auth.allow, which check for valid_hostname during gluster
volume set itself.

I am fine to support this format for auth-allow too, by handling current
format as special case for backward compatibility. I will give others time
till Monday before confirming this and going ahead with implementation.
Suggest other valid options and reason if this is not enough.

Regards,
Amar


>  - if an entry does not start with "/", assume it is an IP/host/... and
>    apply the restriction to the whole volume
>  - separator between entries can be either " " or "," or a combination
>
> It would be good not to break any of the current accepted formats, and
> make them equal if we can.
>
> Do you see a problem with this that I might have missed?
> Niels
>
>
> >
> > -Amar
> >
> >
> > > Thanks,
> > > Niels
> > >
> > >
> > > >
> > > > (The above format is handled properly already at [2] in addr.c, the
> > > pending
> > > > thing is to handle the option properly in options.c's validate).
> > > >
> > > > [1] - https://github.com/gluster/glusterfs/issues/175
> > > > [2] - https://review.gluster.org/17141
> > > >
> > > > If everyone agrees to this, I guess we can pull it off before
> absolute
> > > > feature freeze date for 3.12 branch.
> > > >
> > > > Let me know the feedback. (I am updating the same content in github,
> so
> > > > feel free to comment there too).
> > > >
> > > > NOTE: I thought of using ':' (colon) as field separator between
> addr_list
> > > > and subdir entry, but with IPv6 ':' is valid character in string.
> Hence
> > > > using ' ='.
> > > > --
> > > > Amar Tumballi (amarts)
> > >
> > > > _______________________________________________
> > > > Gluster-devel mailing list
> > > > Gluster-devel at gluster.org
> > > > http://lists.gluster.org/mailman/listinfo/gluster-devel
> > >
> > >
> >
> >
> > --
> > Amar Tumballi (amarts)
>



-- 
Amar Tumballi (amarts)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gluster.org/pipermail/gluster-devel/attachments/20170720/cefe721c/attachment.html>