Add argument validator to Agent attachment options

genie-agent/src/main/java/com/netflix/genie/agent/cli/ArgumentValidators.java

@@ -20,10 +20,18 @@
 
 import com.amazonaws.services.s3.AmazonS3URI;
 import com.beust.jcommander.IParameterValidator;
+import com.beust.jcommander.IValueValidator;
 import com.beust.jcommander.ParameterException;
 import com.beust.jcommander.validators.PositiveInteger;
 import org.apache.commons.lang3.StringUtils;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.List;
+
 /**
  * Parameter validators for command-line arguments.
  *
@@ -78,4 +86,55 @@ public void validate(final String name, final String value) throws ParameterExce
      */
     public static class PortValidator extends PositiveInteger {
     }
+
+    /**
+     * Validates an URI parameter can be parsed as URI.
+     * If the resource type is local file, validate it exists, it is readable, etc.
+     */
+    public static class URIValidator implements IValueValidator<String> {
+
+        /**
+         * {@inheritDoc}
+         */
+        @Override
+        public void validate(final String name, final String value) throws ParameterException {
+            // Make sure the value is a valid URI
+            final URI uri;
+            try {
+                uri = new URI(value);
+            } catch (URISyntaxException e) {
+                throw new ParameterException("Invalid URI " + value + " (for option: " + name + ")", e);
+            }
+
+            // If it's a file, make sure it exists, it's a file, it's readable, etc.
+            if (uri.getScheme().equals("file")) {
+                final Path path = Paths.get(uri.getPath());
+
+                if (!Files.exists(path)) {
+                    throw new ParameterException("File " + value + " does not exist (for option: " + name + ")");
+                } else if (!Files.isRegularFile(path)) {
+                    throw new ParameterException("File " + value + " is not a file (for option: " + name + ")");
+                } else if (!Files.isReadable(path)) {
+                    throw new ParameterException("File " + value + " is not readable (for option: " + name + ")");
+                }
+            }
+        }
+    }
+
+    /**
+     * Validates a URI collection parameter by delegating validation to {@link URIValidator}.
+     */
+    public static class URIListValidator implements IValueValidator<List<String>> {
+
+        /**
+         * {@inheritDoc}
+         */
+        @Override
+        public void validate(final String name, final List<String> values) throws ParameterException {
+            final URIValidator validator = new URIValidator();
+            for (final String value : values) {
+                validator.validate(name, value);
+            }
+        }
+    }
 }

genie-agent/src/main/java/com/netflix/genie/agent/cli/JobRequestArgumentsImpl.java

@@ -166,6 +166,7 @@ class JobRequestArgumentsImpl implements ArgumentDelegates.JobRequestArguments {
         names = {"--jobConfiguration"},
         description = "URI or path of a job-level configuration file to attach, can be repeated",
         converter = ArgumentConverters.UriOrLocalPathConverter.class,
+        validateValueWith = ArgumentValidators.URIListValidator.class,
         splitter = NoopParameterSplitter.class
     )
     private List<String> jobConfigurations = Lists.newArrayList();
@@ -174,14 +175,16 @@ class JobRequestArgumentsImpl implements ArgumentDelegates.JobRequestArguments {
         names = {"--jobDependency"},
         description = "URI or path of a job-level dependency file to attach, can be repeated",
         converter = ArgumentConverters.UriOrLocalPathConverter.class,
+        validateValueWith = ArgumentValidators.URIListValidator.class,
         splitter = NoopParameterSplitter.class
     )
     private List<String> jobDependencies = Lists.newArrayList();
 
     @Parameter(
         names = {"--jobSetup"},
         description = "URI or path of a job-level setup file to attach. The file is sourced during job setup",
-        converter = ArgumentConverters.UriOrLocalPathConverter.class
+        converter = ArgumentConverters.UriOrLocalPathConverter.class,
+        validateValueWith = ArgumentValidators.URIValidator.class
     )
     private String jobSetup;
 

genie-agent/src/test/groovy/com/netflix/genie/agent/cli/JobRequestArgumentsImplSpec.groovy

@@ -22,12 +22,16 @@ import com.beust.jcommander.ParameterException
 import com.beust.jcommander.ParametersDelegate
 import com.netflix.genie.common.internal.dto.v4.Criterion
 import com.netflix.genie.common.util.GenieObjectMapper
+import org.junit.Rule
+import org.junit.rules.TemporaryFolder
 import spock.lang.Specification
 
 import java.nio.file.Paths
 
 class JobRequestArgumentsImplSpec extends Specification {
 
+    @Rule
+    TemporaryFolder temporaryFolder
     MainCommandArguments commandArguments
     TestOptions options
     JCommander jCommander
@@ -76,7 +80,14 @@ class JobRequestArgumentsImplSpec extends Specification {
     }
 
     def "Parse"() {
+        setup:
         def archiveLocationPrefix = "s3://bucket/" + UUID.randomUUID().toString()
+        File cfg1 = temporaryFolder.newFile("cfg1.cfg")
+        File cfg2 = temporaryFolder.newFile("cfg2.cfg")
+        File dep1 = temporaryFolder.newFile("dep1.bin")
+        File dep2 = temporaryFolder.newFile("dep2.jar")
+        File setup = temporaryFolder.newFile("setup.sh")
+
 
         when:
         jCommander.parse(
@@ -100,11 +111,11 @@ class JobRequestArgumentsImplSpec extends Specification {
             "--jobVersion", "1.0",
             "--jobMetadata", "{\"foo\": false}",
             "--api-job",
-            "--jobConfiguration", "cfg1.cfg",
-            "--jobConfiguration", "cfg2.cfg",
-            "--jobDependency", "dep1.bin",
-            "--jobDependency", "dep2.bin",
-            "--jobSetup", "setup.sh",
+            "--jobConfiguration", cfg1.getPath().toString(),
+            "--jobConfiguration", cfg2.getPath().toString(),
+            "--jobDependency", dep1.getPath().toString(),
+            "--jobDependency", dep2.getPath().toString(),
+            "--jobSetup", setup.getPath().toString(),
         )
 
         then:
@@ -132,13 +143,13 @@ class JobRequestArgumentsImplSpec extends Specification {
         options.jobRequestArguments.getJobVersion() == "1.0"
         options.jobRequestArguments.getJobMetadata() == GenieObjectMapper.getMapper().createObjectNode().put("foo", false)
         options.jobRequestArguments.isJobRequestedViaAPI()
-        options.jobRequestArguments.getJobConfigurations().containsAll([fileResource("cfg1.cfg"), fileResource("cfg2.cfg")])
-        options.jobRequestArguments.getJobDependencies().containsAll([fileResource("dep1.bin"), fileResource("dep2.bin")])
-        options.jobRequestArguments.getJobSetup() == fileResource("setup.sh")
+        options.jobRequestArguments.getJobConfigurations().containsAll([fileResource(cfg1), fileResource(cfg2)])
+        options.jobRequestArguments.getJobDependencies().containsAll([fileResource(dep1), fileResource(dep2)])
+        options.jobRequestArguments.getJobSetup() == fileResource(setup)
     }
 
-    String fileResource(final String filename) {
-        return "file:" + Paths.get(filename).toAbsolutePath().toString()
+    String fileResource(final File file) {
+        return "file:" + file.toPath().toAbsolutePath().toString()
     }
 
     def "Unknown parameters throw"() {
@@ -181,6 +192,24 @@ class JobRequestArgumentsImplSpec extends Specification {
 
     }
 
+    def "Invalid file references throw ParameterException"() {
+
+        setup:
+        File folder = temporaryFolder.newFolder()
+
+        when:
+        jCommander.parse("--jobDependency", folder.toPath().toString())
+
+        then:
+        thrown(ParameterException)
+
+        when:
+        jCommander.parse("--jobConfiguration", "/file/does/not/exist")
+
+        then:
+        thrown(ParameterException)
+    }
+
     class TestOptions {
 
         @ParametersDelegate