Add support for specifying uid, gid, filemode and dirmode #44
Conversation
|
One more note. The defaults are still 0777 and uid/gid 0. |
| files and directories. See the `mount.cifs(8)` man page more details on these options. | ||
|
|
||
| ```shell | ||
| $ docker volume create --name rabbitmq -d azurefile -o share=rabbitmq -o uid=999 -o gid=999 -o filemode=0600 -o dirmode=0755 |
There was a problem hiding this comment.
this is a bit long, maybe separate into new line with \
$ docker volume create --name rabbitmq -d azurefile \
-o share=rabbitmq \
-o uid=999 \
-o gid=999 \
-o filemode=0600 \
-o dirmode=0755
and it would be great if you could avoid the rabbitmq name. It looks confusing.
|
@jgreat thanks for contributing. This generally looks good to me. I added a minor comment about the docs changes and have a couple of asks:
Thanks! |
|
@jgreat pinging if you are still interested. |
| options.UID, | ||
| options.GID, | ||
| ) | ||
| opts = append(opts, "vers=3.0") |
There was a problem hiding this comment.
I am not sure append is a good way to construct templates. I think the old version was more readable?
There was a problem hiding this comment.
maybe do it like
opts := []string{
fmt.Sprintf(...),
fmt.Sprintf(...),
fmt.Sprintf(...),
fmt.Sprintf(...),
}
There was a problem hiding this comment.
Sorry for the delay on this, I've been super busy.
We found that we needed to add in the boolean nolock option for some things. For mount, boolean options either exist or are left off. Adding the options to a slice if they are true and joining that into a string seems to be a good way to do it.
I can format the "required" options like your above comment, but any optional or boolean ones I'd still do as an append to the slice.
As a side note, I've tested this and seems to be working reliably so far.
There was a problem hiding this comment.
As a side note, I've tested this and seems to be working reliably so far.
@jgreat sounds great , thank you!
I can format the "required" options like your above comment, but any optional or boolean ones I'd still do as an append to the slice.
yep you only append nolock anyway. let's just append that.
For mount, boolean options either exist or are left off.
ok we can leave it as is.
happy to merge once this is fixed.
| opts.GID = meta["gid"] | ||
| opts.UID = meta["uid"] | ||
|
|
||
| if len(meta["nolock"]) > 0 { |
There was a problem hiding this comment.
can we make this a meta["nolock"] == "1" or =="true" kind of comparison?
|
@jgreat thanks for your contribution and carrying this out. it's a bit late for that but can you please sign the Contributors License Agreement (CLA) at https://cla.azure.com/ if you already have not? |
|
Signed the cla, or at least thought I did. |
|
@jgreat thanks, released this as 0.3.0 in case you want to pick up https://github.com/Azure/azurefile-dockervolumedriver/releases/tag/0.3.0 |
There are quite a few docker library images that run their services as a non-privileged user instead of root and have security checks that fail if files/directories are world read/writable. library/rabbitmq is an example.
I've added in the ability to pass in the gid, uid, filemode and dirmode as options to volume create.