Repository: JonathonReinhart/scuba

Issue:
Scuba Can't Override Problematic ENTRYPOINT Values
When using `docker run`, if an image has an ENTRYPOINT value that the user does not wish to use, [they can pass](https://medium.com/@oprearocks/how-to-properly-override-the-entrypoint-using-docker-run-2e081e5feb9d) the `--entrypoint=""` flag. This is possible with scuba using the `--docker` option to pass a docker argument. However, because scuba currently [writes the `command.sh` using the entrypoint value](https://github.com/JonathonReinhart/scuba/blob/daf9a9680c81985b9671381db7ad57fb5faf91f2/scuba/__main__.py#L302), there is no way to override this value without creating a new docker image.

I have created a POC Docker image that can be used to replicate the issue with the following command:
```
scuba --image wills64/scubaentrypointpoc:latest -d="--entrypoint=''" echo foo
```

The Dockerfile used to generate the image is simply:
```
FROM debian:9
ENTRYPOINT "/bin/cat"
```

Currently, the scuba command above hangs infinitely. With the resolution of this issue, the entrypoint command should never run, `foo` should be printed to the screen, and the container and scuba should exit.

The real-world impetus for this was the [mutliarch/crossbuild](https://hub.docker.com/r/multiarch/crossbuild/) image, which wants to run its [crossbuild](https://github.com/multiarch/crossbuild/blob/master/assets/crossbuild) script before running the specified commands, which can cause issues when using builtin shell commands like `export`.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 009cfa1..90668f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Added
+- Enable scuba to override entrypoint via `--entrypoint` or `.scuba.yml` (#125)
+
 ### Changed
 - Don't run image entrypoint for each line in a mult-line alias (#121)
 
diff --git a/scuba/__main__.py b/scuba/__main__.py
index bd65dca..329e7e2 100644
--- a/scuba/__main__.py
+++ b/scuba/__main__.py
@@ -53,6 +53,8 @@ def parse_scuba_args(argv):
     ap.add_argument('-e', '--env', dest='env_vars', action='append',
             type=parse_env_var, default=[],
             help='Environment variables to pass to docker')
+    ap.add_argument('--entrypoint',
+            help='Override the default ENTRYPOINT of the image')
     ap.add_argument('--list-aliases', action='store_true',
             help=argparse.SUPPRESS)
     ap.add_argument('--list-available-options', action=ListOptsAction,
@@ -92,7 +94,7 @@ class ScubaError(Exception):
 
 class ScubaDive(object):
     def __init__(self, user_command, docker_args=None, env=None, as_root=False, verbose=False,
-            image_override=None):
+            image_override=None, entrypoint=None):
 
         env = env or {}
         if not isinstance(env, collections.Mapping):
@@ -102,6 +104,7 @@ class ScubaDive(object):
         self.as_root = as_root
         self.verbose = verbose
         self.image_override = image_override
+        self.entrypoint_override = entrypoint
 
         # These will be added to docker run cmdline
         self.env_vars = env
@@ -293,9 +296,22 @@ class ScubaDive(object):
 
         # Make scubainit the real entrypoint, and use the defined entrypoint as
         # the docker command (if it exists)
-        self.docker_cmd = get_image_entrypoint(context.image) or []
         self.add_option('--entrypoint={}'.format(scubainit_cpath))
 
+        self.docker_cmd = []
+        if self.entrypoint_override is not None:
+            # --entrypoint takes precedence
+            if self.entrypoint_override != '':
+                self.docker_cmd = [self.entrypoint_override]
+        elif context.entrypoint is not None:
+            # then .scuba.yml
+            if context.entrypoint != '':
+                self.docker_cmd = [context.entrypoint]
+        else:
+            ep = get_image_entrypoint(context.image)
+            if ep:
+                self.docker_cmd = ep
+
         # The user command is executed via a generated shell script
         with self.open_scubadir_file('command.sh', 'wt') as f:
             self.docker_cmd += ['/bin/sh', f.container_path]
@@ -395,6 +411,7 @@ def run_scuba(scuba_args):
         as_root = scuba_args.root,
         verbose = scuba_args.verbose,
         image_override = scuba_args.image,
+        entrypoint = scuba_args.entrypoint,
         )
 
     if scuba_args.list_aliases:
diff --git a/scuba/config.py b/scuba/config.py
index 812fce4..01570e9 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -151,28 +151,55 @@ def _process_environment(node, name):
 
     return result
 
+def _get_entrypoint(data):
+    # N.B. We can't use data.get() here, because that might return
+    # None, leading to ambiguity between entrypoint being absent or set
+    # to a null value.
+    #
+    # "Note that a null is different from an empty string and that a
+    # mapping entry with some key and a null value is valid and
+    # different from not having that key in the mapping."
+    #   - http://yaml.org/type/null.html
+    key = 'entrypoint'
+
+    if not key in data:
+        return None
+
+    ep = data[key]
+
+    # We represent a null value as an empty string.
+    if ep is None:
+        ep = ''
+
+    if not isinstance(ep, basestring):
+        raise ConfigError("'{}' must be a string, not {}".format(
+                key, type(ep).__name__))
+    return ep
 
 
 class ScubaAlias(object):
-    def __init__(self, name, script, image, environment):
+    def __init__(self, name, script, image, entrypoint, environment):
         self.name = name
         self.script = script
         self.image = image
+        self.entrypoint = entrypoint
         self.environment = environment
 
     @classmethod
     def from_dict(cls, name, node):
         script = _process_script_node(node, name)
         image = None
+        entrypoint = None
         environment = None
 
         if isinstance(node, dict):  # Rich alias
             image = node.get('image')
+            entrypoint = _get_entrypoint(node)
             environment = _process_environment(
                     node.get('environment'),
                     '{}.{}'.format(name, 'environment'))
 
-        return cls(name, script, image, environment)
+        return cls(name, script, image, entrypoint, environment)
 
 class ScubaContext(object):
     pass
@@ -180,7 +207,7 @@ class ScubaContext(object):
 class ScubaConfig(object):
     def __init__(self, **data):
         required_nodes = ('image',)
-        optional_nodes = ('aliases','hooks','environment')
+        optional_nodes = ('aliases','hooks','entrypoint','environment')
 
         # Check for missing required nodes
         missing = [n for n in required_nodes if not n in data]
@@ -195,7 +222,7 @@ class ScubaConfig(object):
                     's' if len(extra) > 1 else '', ', '.join(extra)))
 
         self._image = data['image']
-
+        self._entrypoint = _get_entrypoint(data)
         self._load_aliases(data)
         self._load_hooks(data)
         self._environment = self._load_environment(data)
@@ -229,6 +256,10 @@ class ScubaConfig(object):
     def image(self):
         return self._image
 
+    @property
+    def entrypoint(self):
+        return self._entrypoint
+
     @property
     def aliases(self):
         return self._aliases
@@ -255,6 +286,7 @@ class ScubaConfig(object):
         result = ScubaContext()
         result.script = None
         result.image = self.image
+        result.entrypoint = self.entrypoint
         result.environment = self.environment.copy()
 
         if command:
@@ -264,9 +296,11 @@ class ScubaConfig(object):
                 result.script = [shell_quote_cmd(command)]
             else:
                 # Using an alias
-                # Does this alias override the image?
+                # Does this alias override the image and/or entrypoint?
                 if alias.image:
                     result.image = alias.image
+                if alias.entrypoint is not None:
+                    result.entrypoint = alias.entrypoint
 
                 # Merge/override the environment
                 if alias.environment:
diff --git a/scubainit/scubainit.c b/scubainit/scubainit.c
index 5929128..c8b23c6 100644
--- a/scubainit/scubainit.c
+++ b/scubainit/scubainit.c
@@ -619,6 +619,6 @@ main(int argc, char **argv)
     verbose("execvp(\"%s\", ...)\n", new_argv[0]);
     execvp(new_argv[0], new_argv);
 
-    errmsg("execv() failed: %m\n");
+    errmsg("execvp(\"%s\", ...) failed: %m\n", new_argv[0]);
     exit(99);
 }


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 912f86f..b6a7a6c 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -216,6 +216,19 @@ class TestConfig(TmpDirTestCase):
     ######################################################################
     # process_command
 
+    def test_process_command_image(self):
+        '''process_command returns the image and entrypoint'''
+        image_name = 'test_image'
+        entrypoint = 'test_entrypoint'
+
+        cfg = scuba.config.ScubaConfig(
+                image = image_name,
+                entrypoint = entrypoint,
+                )
+        result = cfg.process_command([])
+        assert_equal(result.image, image_name)
+        assert_equal(result.entrypoint, entrypoint)
+
     def test_process_command_empty(self):
         '''process_command handles no aliases and an empty command'''
         cfg = scuba.config.ScubaConfig(
@@ -321,6 +334,46 @@ class TestConfig(TmpDirTestCase):
         result = cfg.process_command(['apple'])
         assert_equal(result.image, 'overridden')
 
+    def test_process_command_alias_overrides_image_and_entrypoint(self):
+        '''aliases can override the image and entrypoint'''
+        cfg = scuba.config.ScubaConfig(
+                image = 'default',
+                entrypoint = 'default_entrypoint',
+                aliases = dict(
+                    apple = dict(
+                        script = [
+                            'banana cherry "pie is good"',
+                            'so is peach',
+                        ],
+                        image = 'overridden',
+                        entrypoint = 'overridden_entrypoint',
+                    ),
+                ),
+            )
+        result = cfg.process_command(['apple'])
+        assert_equal(result.image, 'overridden')
+        assert_equal(result.entrypoint, 'overridden_entrypoint')
+
+    def test_process_command_alias_overrides_image_and_empty_entrypoint(self):
+        '''aliases can override the image and empty/null entrypoint'''
+        cfg = scuba.config.ScubaConfig(
+                image = 'default',
+                entrypoint = 'default_entrypoint',
+                aliases = dict(
+                    apple = dict(
+                        script = [
+                            'banana cherry "pie is good"',
+                            'so is peach',
+                        ],
+                        image = 'overridden',
+                        entrypoint = '',
+                    ),
+                ),
+            )
+        result = cfg.process_command(['apple'])
+        assert_equal(result.image, 'overridden')
+        assert_equal(result.entrypoint, '')
+
 
     ############################################################################
     # Hooks
@@ -479,3 +532,98 @@ class TestConfig(TmpDirTestCase):
                 BAR = "42",
                 MORE = "Hello world",
             ))
+
+
+    ############################################################################
+    # Entrypoint
+
+    def test_entrypoint_not_set(self):
+        '''Entrypoint can be missing'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertIsNone(config.entrypoint)
+
+    def test_entrypoint_null(self):
+        '''Entrypoint can be set to null'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint:
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.entrypoint, '')     # Null => empty string
+
+    def test_entrypoint_emptry_string(self):
+        '''Entrypoint can be set to an empty string'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint: ""
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.entrypoint, '')
+
+    def test_entrypoint_set(self):
+        '''Entrypoint can be set'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint: my_ep
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.entrypoint, 'my_ep')
+
+    def test_alias_entrypoint_null(self):
+        '''Entrypoint can be set to null via alias'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint: na_ep
+                aliases:
+                  testalias:
+                    entrypoint:
+                    script:
+                      - ugh
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.aliases['testalias'].entrypoint, '')    # Null => empty string
+
+    def test_alias_entrypoint_empty_string(self):
+        '''Entrypoint can be set to an empty string via alias'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint: na_ep
+                aliases:
+                  testalias:
+                    entrypoint: ""
+                    script:
+                      - ugh
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.aliases['testalias'].entrypoint, '')
+
+    def test_alias_entrypoint(self):
+        '''Entrypoint can be set via alias'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                entrypoint: na_ep
+                aliases:
+                  testalias:
+                    entrypoint: use_this_ep
+                    script:
+                      - ugh
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+        self.assertEqual(config.aliases['testalias'].entrypoint, 'use_this_ep')
diff --git a/tests/test_main.py b/tests/test_main.py
index 141508e..44aa173 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -358,6 +358,9 @@ class TestMain(TmpDirTestCase):
         assert_str_equalish(out, data)
 
 
+    ############################################################################
+    # Entrypoint
+
     def test_image_entrypoint(self):
         '''Verify scuba doesn't interfere with the configured image ENTRYPOINT'''
 
@@ -384,6 +387,101 @@ class TestMain(TmpDirTestCase):
         assert_str_equalish('\n'.join(['success']*2), out)
 
 
+    def test_entrypoint_override(self):
+        '''Verify --entrypoint override works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: scuba/entrypoint-test
+                aliases:
+                  testalias:
+                    script:
+                      - echo $ENTRYPOINT_WORKS
+                ''')
+
+        test_str = 'This is output from the overridden entrypoint'
+
+        with open('new.sh', 'w') as f:
+            f.write('#!/bin/sh\n')
+            f.write('echo "{}"\n'.format(test_str))
+        make_executable('new.sh')
+
+        args = [
+            '--entrypoint', os.path.abspath('new.sh'),
+            'true',
+        ]
+        out, _ = self.run_scuba(args)
+        assert_str_equalish(test_str, out)
+
+
+    def test_entrypoint_override_none(self):
+        '''Verify --entrypoint override (to nothing) works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: scuba/entrypoint-test
+                aliases:
+                  testalias:
+                    script:
+                      - echo $ENTRYPOINT_WORKS
+                ''')
+
+        args = [
+            '--entrypoint', '',
+            'testalias',
+        ]
+        out, _ = self.run_scuba(args)
+
+        # Verify that ENTRYPOINT_WORKS wasn not set by the entrypoint
+        # (because it didn't run)
+        self.assertNotIn('success', out)
+
+
+    def test_yaml_entrypoint_override(self):
+        '''Verify entrypoint in .scuba.yml works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: scuba/entrypoint-test
+                entrypoint: "./new.sh"
+                ''')
+
+        test_str = 'This is output from the overridden entrypoint'
+
+        with open('new.sh', 'w') as f:
+            f.write('#!/bin/sh\n')
+            f.write('echo "{}"\n'.format(test_str))
+        make_executable('new.sh')
+
+        args = [
+            'true',
+        ]
+        out, _ = self.run_scuba(args)
+        assert_str_equalish(test_str, out)
+
+
+    def test_entrypoint_override_none(self):
+        '''Verify "none" entrypoint in .scuba.yml works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('''
+                image: scuba/entrypoint-test
+                entrypoint:
+                aliases:
+                  testalias:
+                    script:
+                      - echo $ENTRYPOINT_WORKS
+                ''')
+
+        args = [
+            'testalias',
+        ]
+        out, _ = self.run_scuba(args)
+
+        # Verify that ENTRYPOINT_WORKS wasn not set by the entrypoint
+        # (because it didn't run)
+        self.assertNotIn('success', out)
+
+
+    ############################################################################
+    # Image override
+
     def test_image_override(self):
         '''Verify --image works'''