Subdir support #40
Subdir support #40
Conversation
| @@ -11,18 +11,26 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| validHostnameRegex = `(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])` | |||
| validVolUriRegex = `([^:]+?):\/?([^\/]+)(/.+)?` | |||
There was a problem hiding this comment.
I am opting not to make the regex too strict, if the user decides to put in wrong input then it will just fail anyway. So I simplified it here
There was a problem hiding this comment.
It will also allow special internationalized characters https://en.wikipedia.org/wiki/Internationalized_domain_name
There was a problem hiding this comment.
It is better to fail at creation of volume and not mount and be strict to validate that.
It mostly important (at least for me) when using distributed docker like swarm because it will fail at creation of service and not try to mount (and remount and remount and ...) a false volume.
It allow for better error handling also otherwise it will be another case behind mount failling with just exit status 1. https://github.com/sapk/docker-volume-gluster/issues?utf8=%E2%9C%93&q=is%3Aissue+exit+status+1
I will fix this if you prefer for the rest thank you very much.
| @@ -21,6 +21,10 @@ func TestIsValidURI(t *testing.T) { | |||
| {"192.168.1.:volume", false}, | |||
| {"192.168.1.1,10.8.0.1:volume", true}, | |||
| {"192.168.1.1,test2:volume", true}, | |||
| {"192.168.1.1,test2:volume/subdir", true}, | |||
| {"192.168.1.1,test2:/volume/subdir", true}, | |||
| {"192.168.1.1,test2://volume/subdir", false}, | |||
There was a problem hiding this comment.
double slashes in the beginning is not allowed from what I can tell.
| @@ -18,9 +18,13 @@ func TestIsValidURI(t *testing.T) { | |||
| {"test,volume", false}, | |||
| {"test,test2:volume", true}, | |||
| {"192.168.1.1:volume", true}, | |||
| {"192.168.1.:volume", false}, | |||
| {"192.168.1.:volume", true}, | |||
There was a problem hiding this comment.
From previous comment, this should fail
There was a problem hiding this comment.
Feel free to update the PR. However, I still think making it loose will prevent future cases when some people have atypical but valid host names or volume names. Is there a way of extracting the error message so it can be displayed rather than status code is 1?
There was a problem hiding this comment.
Hopefully we'd get a release I can try so I don't have to spend the effort of creating extra volumes.
| @@ -18,9 +18,13 @@ func TestIsValidURI(t *testing.T) { | |||
| {"test,volume", false}, | |||
| {"test,test2:volume", true}, | |||
| {"192.168.1.1:volume", true}, | |||
| {"192.168.1.:volume", false}, | |||
| {"192.168.1.:volume", true}, | |||
There was a problem hiding this comment.
this needed to change because I made the regex looser.
| @@ -11,18 +11,26 @@ import ( | |||
| ) | |||
|
|
|||
| const ( | |||
| validHostnameRegex = `(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])` | |||
| validVolUriRegex = `([^:]+?):\/?([^\/]+)(/.+)?` | |||
There was a problem hiding this comment.
It will also allow special internationalized characters https://en.wikipedia.org/wiki/Internationalized_domain_name
|
Done thanks for the contrib. |
Fixes #35