Repository: jcrist/skein (python)

Issue:
Validate file destinations are names not paths
Localized file destinations must be file names only, and thus can't contain any `'/'` characters. Currently this is only validated on the application master, but should be validated on the client beforehand.

See https://github.com/dask/dask-yarn/issues/49#issuecomment-456923658.

Pull request description:
Validate targets in `files` in the client
Keys in the `files` field must be filenames only (no directories
allowed). Previously this was checked only in the server, now we
validate it in the client as well.

Fixes #139.

Patch:
diff --git a/skein/model.py b/skein/model.py
index 5184209..c2cf798 100644
--- a/skein/model.py
+++ b/skein/model.py
@@ -19,6 +19,18 @@ __all__ = ('ApplicationSpec', 'Service', 'Resources', 'File', 'FileType',
            'ContainerState', 'Container', 'LogLevel')
 
 
+def _check_is_filename(target):
+    orig = target
+    # Allow local relative paths
+    if target.startswith("./"):
+        target = target[2:]
+    if "/" in target:
+        raise context.ValueError(
+            "Keys in `files` must be filenames only (no directories allowed), "
+            "found %r" % orig
+        )
+
+
 def _pop_origin(kwargs):
     _origin = kwargs.pop('_origin', None)
     if kwargs:
@@ -794,7 +806,8 @@ class Service(Specification):
         self.resources._validate(is_request=True)
 
         self._check_is_dict_of('files', string, File)
-        for f in self.files.values():
+        for target, f in self.files.items():
+            _check_is_filename(target)
             f._validate()
 
         self._check_is_dict_of('env', string, string)
@@ -1053,7 +1066,8 @@ class Master(Specification):
         self.resources._validate(is_request=True)
 
         self._check_is_dict_of('files', string, File)
-        for f in self.files.values():
+        for target, f in self.files.items():
+            _check_is_filename(target)
             f._validate()
 
         self._check_is_dict_of('env', string, string)


Test patch:
diff --git a/skein/test/test_model.py b/skein/test/test_model.py
index e1f2ec1..5591c75 100644
--- a/skein/test/test_model.py
+++ b/skein/test/test_model.py
@@ -318,6 +318,12 @@ def test_master_invariants():
     assert m.files['target'].type == 'archive'
     assert m.files['target2'].type == 'file'
 
+    # File targets are checked
+    with pytest.raises(ValueError):
+        Master(files={'foo/bar': '/source.zip'})
+    # Local relative paths are fine
+    Master(files={'./bar': '/source.zip'})
+
 
 def test_service():
     r = Resources(memory=1024, vcores=1)
@@ -372,6 +378,14 @@ def test_service_invariants():
     assert s.files['target'].type == 'archive'
     assert s.files['target2'].type == 'file'
 
+    # File targets are checked
+    with pytest.raises(ValueError):
+        Service(script="script", resources=r,
+                files={'foo/bar': '/source.zip'})
+    # Local relative paths are fine
+    Service(script="script", resources=r,
+            files={'./bar': '/source.zip'})
+
 
 def test_application_spec():
     r = Resources(memory=1024, vcores=1)