Repository: JonathonReinhart/scuba

Issue:
Using environment variables in .scuba.yml or passing them 
Is it already possible – or do you consider it a valuable addition – to use environment variables in the `.scuba.yml` file?

For one, these could be already present in the shell where `scuba` is run. Additionally, I think some variables added by `scuba` might be helpful. (Is there a list of already used variables available somewhere, like the directory where `scuba` was started, path and directory to `.scubafile.yml` etc.)?

The goal would be do define an alias like `build: ./do-build.sh ${SCUBA_HOME} ${SOME_ENV}`.

With regard to passing environment variables into the container, what about a new configuration setting, either global or per-alias, that lists env vars to be passed?

```
image: ...
aliases:
    build: 
        command: ./do-build.sh ${TARGET}
        env:
            - THIS_VAR
            - THAT_VAR
        
env:
    - TARGET_ARCH
    - COMPILE_MODE
```

Note that we'd probably need to change the alias syntax somewhat or allow another structure to place the `env` key.

This would add `-e THIS_VAR -e THAT_VAR` to the `docker run` command when using the `build` alias, and add `-e TARGET_ARCH -e COMPILE_MODE` in any case.

Does that make sense?


Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3eefb35..3b9ff89 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 ### Added
 - Add -e/--env command-line option (#111)
+- Add support for setting environment in .scuba.yml (#120)
 
 ### Changed
 - Implemented auto-versioning using Git and Travis (#112)
diff --git a/doc/yaml-reference.md b/doc/yaml-reference.md
index 95826e8..996f554 100644
--- a/doc/yaml-reference.md
+++ b/doc/yaml-reference.md
@@ -18,6 +18,25 @@ Example:
 image: debian:8.2
 ```
 
+### `environment`
+
+The optional `environment` node allows environment variables to be specified.
+This can be either a mapping (dictionary), or a list of `KEY=VALUE` pairs.
+If a value is not specified, the value is taken from the external environment.
+
+Examples:
+```yaml
+environment:
+  FOO: "This is foo"
+  SECRET:
+```
+```yaml
+environment:
+  - FOO=This is foo
+  - SECRET
+```
+
+
 ### `aliases`
 
 The optional `aliases` node is a mapping (dictionary) of bash-like aliases,
@@ -53,6 +72,21 @@ aliases:
       - cat /etc/os-release
 ```
 
+Aliases can add to the top-level `environment` and override its values using
+the same syntax:
+```yaml
+environment:
+  FOO: "Top-level"
+aliases:
+  example:
+    environment:
+      FOO: "Override"
+      BAR: "New"
+    script:
+      - echo $FOO $BAR
+```
+
+
 ### `hooks`
 
 The optional `hooks` node is a mapping (dictionary) of "hook" scripts that run
diff --git a/example/env_vars/.scuba.yml b/example/env_vars/.scuba.yml
new file mode 100644
index 0000000..b28ca8a
--- /dev/null
+++ b/example/env_vars/.scuba.yml
@@ -0,0 +1,21 @@
+image: !from_yaml ../common.yml image
+
+environment:
+  FOO: Top-level
+  BAR: 42
+  EMPTY: ""
+  EXTERNAL_1:
+
+aliases:
+  example:
+    environment:
+      FOO: Overridden by alias
+      EXTERNAL_2:
+    script:
+      - echo "FOO=\"$FOO\""
+      - echo "BAR=\"$BAR\""
+      - echo "EMPTY=\"$EMPTY\""
+      - echo "EXTERNAL_1=\"$EXTERNAL_1\""
+      - echo "EXTERNAL_2=\"$EXTERNAL_2\""
+      - echo "CMDLINE=\"$CMDLINE\""
+
diff --git a/example/env_vars/run_example.sh b/example/env_vars/run_example.sh
new file mode 100755
index 0000000..bc742d1
--- /dev/null
+++ b/example/env_vars/run_example.sh
@@ -0,0 +1,7 @@
+#!/bin/bash
+cd $(dirname $0)
+
+export EXTERNAL_1="Value 1 taken from external environment"
+export EXTERNAL_2="Value 2 taken from external environment"
+
+scuba -e CMDLINE="This comes from the cmdline" example
diff --git a/scuba/__main__.py b/scuba/__main__.py
index 72c89d6..c3eb3e6 100644
--- a/scuba/__main__.py
+++ b/scuba/__main__.py
@@ -132,6 +132,9 @@ class ScubaDive(object):
         # Docker is running natively
         self.__setup_native_run()
 
+        # Apply environment vars from .scuba.yml
+        self.env_vars.update(self.context.environment)
+
     def __str__(self):
         s = StringIO()
         writeln(s, 'ScubaDive')
diff --git a/scuba/config.py b/scuba/config.py
index b8e0417..9b79ffd 100644
--- a/scuba/config.py
+++ b/scuba/config.py
@@ -129,17 +129,50 @@ def _process_script_node(node, name):
     raise ConfigError("{0}: must be string or dict".format(name))
 
 
+def _process_environment(node, name):
+    # Environment can be either a list of strings ("KEY=VALUE") or a mapping
+    # Environment keys and values are always strings
+    result = {}
+
+    if not node:
+        pass
+    elif isinstance(node, dict):
+        for k, v in node.items():
+            if v is None:
+                v = os.getenv(k, '')
+            result[k] = str(v)
+    elif isinstance(node, list):
+        for e in node:
+            k, v = parse_env_var(e)
+            result[k] = v
+    else:
+        raise ConfigError("'{0}' must be list or mapping, not {1}".format(
+                name, type(node).__name__))
+
+    return result
+
+
+
 class ScubaAlias(object):
-    def __init__(self, name, script, image):
+    def __init__(self, name, script, image, environment):
         self.name = name
         self.script = script
         self.image = image
+        self.environment = environment
 
     @classmethod
     def from_dict(cls, name, node):
         script = _process_script_node(node, name)
-        image = node.get('image') if isinstance(node, dict) else None
-        return cls(name, script, image)
+        image = None
+        environment = None
+
+        if isinstance(node, dict):  # Rich alias
+            image = node.get('image')
+            environment = _process_environment(
+                    node.get('environment'),
+                    '{0}.{1}'.format(name, 'environment'))
+
+        return cls(name, script, image, environment)
 
 class ScubaContext(object):
     pass
@@ -147,7 +180,7 @@ class ScubaContext(object):
 class ScubaConfig(object):
     def __init__(self, **data):
         required_nodes = ('image',)
-        optional_nodes = ('aliases','hooks',)
+        optional_nodes = ('aliases','hooks','environment')
 
         # Check for missing required nodes
         missing = [n for n in required_nodes if not n in data]
@@ -165,6 +198,7 @@ class ScubaConfig(object):
 
         self._load_aliases(data)
         self._load_hooks(data)
+        self._environment = self._load_environment(data)
 
 
 
@@ -187,6 +221,9 @@ class ScubaConfig(object):
                 hook = _process_script_node(node, name)
                 self._hooks[name] = hook
 
+    def _load_environment(self, data):
+         return _process_environment(data.get('environment'), 'environment')
+
 
     @property
     def image(self):
@@ -200,6 +237,10 @@ class ScubaConfig(object):
     def hooks(self):
         return self._hooks
 
+    @property
+    def environment(self):
+        return self._environment
+
 
     def process_command(self, command):
         '''Processes a user command using aliases
@@ -214,6 +255,7 @@ class ScubaConfig(object):
         result = ScubaContext()
         result.script = None
         result.image = self.image
+        result.environment = self.environment.copy()
 
         if command:
             alias = self.aliases.get(command[0])
@@ -226,6 +268,10 @@ class ScubaConfig(object):
                 if alias.image:
                     result.image = alias.image
 
+                # Merge/override the environment
+                if alias.environment:
+                    result.environment.update(alias.environment)
+
                 if len(alias.script) > 1:
                     # Alias is a multiline script; no additional
                     # arguments are allowed in the scuba invocation.
diff --git a/scuba/utils.py b/scuba/utils.py
index f1555fc..1af6f45 100644
--- a/scuba/utils.py
+++ b/scuba/utils.py
@@ -74,4 +74,4 @@ def parse_env_var(s):
         return (k, v)
 
     k = parts[0]
-    return (k, os.getenv(k))
+    return (k, os.getenv(k, ''))


Test patch:
diff --git a/tests/test_config.py b/tests/test_config.py
index 118379e..5d8a5b4 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -372,3 +372,110 @@ class TestConfig(TmpDirTestCase):
                 ''')
 
         assert_raises(scuba.config.ConfigError, scuba.config.load_config, '.scuba.yml')
+
+
+    ############################################################################
+    # Env
+
+    def test_env_top_dict(self):
+        '''Top-level environment can be loaded (dict)'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                environment:
+                  FOO: This is foo
+                  FOO_WITH_QUOTES: "\"Quoted foo\""    # Quotes included in value
+                  BAR: "This is bar"
+                  MAGIC: 42
+                  SWITCH_1: true        # YAML boolean
+                  SWITCH_2: "true"      # YAML string
+                  EMPTY: ""
+                  EXTERNAL:             # Comes from os env
+                  EXTERNAL_NOTSET:      # Missing in os env
+                ''')
+
+        with mocked_os_env(EXTERNAL='Outside world'):
+            config = scuba.config.load_config('.scuba.yml')
+
+        expect = dict(
+            FOO = "This is foo",
+            FOO_WITH_QUOTES = "\"Quoted foo\"",
+            BAR = "This is bar",
+            MAGIC = "42",           # N.B. string
+            SWITCH_1 = "True",      # Unfortunately this is due to str(bool(1))
+            SWITCH_2 = "true",
+            EMPTY = "",
+            EXTERNAL = "Outside world",
+            EXTERNAL_NOTSET = "",
+        )
+        self.assertEqual(expect, config.environment)
+
+
+    def test_env_top_list(self):
+        '''Top-level environment can be loaded (list)'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                environment:
+                  - FOO=This is foo                 # No quotes
+                  - FOO_WITH_QUOTES="Quoted foo"    # Quotes included in value
+                  - BAR=This is bar
+                  - MAGIC=42
+                  - SWITCH_2=true
+                  - EMPTY=
+                  - EXTERNAL                        # Comes from os env
+                  - EXTERNAL_NOTSET                 # Missing in os env
+                ''')
+
+        with mocked_os_env(EXTERNAL='Outside world'):
+            config = scuba.config.load_config('.scuba.yml')
+
+        expect = dict(
+            FOO = "This is foo",
+            FOO_WITH_QUOTES = "\"Quoted foo\"",
+            BAR = "This is bar",
+            MAGIC = "42",           # N.B. string
+            SWITCH_2 = "true",
+            EMPTY = "",
+            EXTERNAL = "Outside world",
+            EXTERNAL_NOTSET = "",
+        )
+        self.assertEqual(expect, config.environment)
+
+
+    def test_env_alias(self):
+        '''Alias can have environment which overrides top-level'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: na
+                environment:
+                  FOO: Top-level
+                  BAR: 42
+                aliases:
+                  al:
+                    script: Don't care
+                    environment:
+                      FOO: Overridden
+                      MORE: Hello world
+                ''')
+
+        config = scuba.config.load_config('.scuba.yml')
+
+        self.assertEqual(config.environment, dict(
+                FOO = "Top-level",
+                BAR = "42",
+            ))
+
+        self.assertEqual(config.aliases['al'].environment, dict(
+                FOO = "Overridden",
+                MORE = "Hello world",
+            ))
+
+        # Does the environment get overridden / merged?
+        ctx = config.process_command(['al'])
+
+        self.assertEqual(ctx.environment, dict(
+                FOO = "Overridden",
+                BAR = "42",
+                MORE = "Hello world",
+            ))
diff --git a/tests/test_main.py b/tests/test_main.py
index 0c192af..9e4641a 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -13,6 +13,7 @@ import os
 import sys
 from tempfile import TemporaryFile, NamedTemporaryFile
 import subprocess
+import shlex
 
 import scuba.__main__ as main
 import scuba.constants
@@ -356,32 +357,6 @@ class TestMain(TmpDirTestCase):
 
         assert_str_equalish(out, data)
 
-    def test_env_var_keyval(self):
-        '''Verify -e KEY=VAL works'''
-        with open('.scuba.yml', 'w') as f:
-            f.write('image: {0}\n'.format(DOCKER_IMAGE))
-        args = [
-            '-e', 'KEY=VAL',
-            '/bin/sh', '-c', 'echo $KEY',
-        ]
-        out, _ = self.run_scuba(args)
-        assert_str_equalish(out, 'VAL')
-
-    def test_env_var_key_only(self):
-        '''Verify -e KEY works'''
-        with open('.scuba.yml', 'w') as f:
-            f.write('image: {0}\n'.format(DOCKER_IMAGE))
-        args = [
-            '-e', 'KEY',
-            '/bin/sh', '-c', 'echo $KEY',
-        ]
-        def mocked_getenv(key):
-            self.assertEqual(key, 'KEY')
-            return 'mockedvalue'
-        with mock.patch('os.getenv', side_effect=mocked_getenv):
-            out, _ = self.run_scuba(args)
-        assert_str_equalish(out, 'mockedvalue')
-
 
     def test_image_entrypoint(self):
         '''Verify scuba doesn't interfere with the configured image ENTRYPOINT'''
@@ -493,6 +468,86 @@ class TestMain(TmpDirTestCase):
         self._test_one_hook('root', 0, 0)
 
 
+    ############################################################################
+    # Environment
+
+    def test_env_var_keyval(self):
+        '''Verify -e KEY=VAL works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('image: {0}\n'.format(DOCKER_IMAGE))
+        args = [
+            '-e', 'KEY=VAL',
+            '/bin/sh', '-c', 'echo $KEY',
+        ]
+        out, _ = self.run_scuba(args)
+        assert_str_equalish(out, 'VAL')
+
+    def test_env_var_key_only(self):
+        '''Verify -e KEY works'''
+        with open('.scuba.yml', 'w') as f:
+            f.write('image: {0}\n'.format(DOCKER_IMAGE))
+        args = [
+            '-e', 'KEY',
+            '/bin/sh', '-c', 'echo $KEY',
+        ]
+        with mocked_os_env(KEY='mockedvalue'):
+            out, _ = self.run_scuba(args)
+        assert_str_equalish(out, 'mockedvalue')
+
+
+    def test_env_var_sources(self):
+        '''Verify scuba handles all possible environment variable sources'''
+        with open('.scuba.yml', 'w') as f:
+            f.write(r'''
+                image: {image}
+                environment:
+                  FOO: Top-level
+                  BAR: 42
+                  EXTERNAL_2:
+                aliases:
+                  al:
+                    script:
+                      - echo "FOO=\"$FOO\""
+                      - echo "BAR=\"$BAR\""
+                      - echo "MORE=\"$MORE\""
+                      - echo "EXTERNAL_1=\"$EXTERNAL_1\""
+                      - echo "EXTERNAL_2=\"$EXTERNAL_2\""
+                      - echo "EXTERNAL_3=\"$EXTERNAL_3\""
+                      - echo "BAZ=\"$BAZ\""
+                    environment:
+                      FOO: Overridden
+                      MORE: Hello world
+                      EXTERNAL_3:
+                '''.format(image=DOCKER_IMAGE))
+
+        args = [
+            '-e', 'EXTERNAL_1',
+            '-e', 'BAZ=From the command line',
+            'al',
+        ]
+
+        m = mocked_os_env(
+                EXTERNAL_1 = "External value 1",
+                EXTERNAL_2 = "External value 2",
+                EXTERNAL_3 = "External value 3",
+                )
+        with m:
+            out, _ = self.run_scuba(args)
+
+        # Convert key/pair output to dictionary
+        result = dict( pair.split('=', 1) for pair in shlex.split(out) )
+
+        self.assertEqual(result, dict(
+                FOO = "Overridden",
+                BAR = "42",
+                MORE = "Hello world",
+                EXTERNAL_1 = "External value 1",
+                EXTERNAL_2 = "External value 2",
+                EXTERNAL_3 = "External value 3",
+                BAZ = "From the command line",
+            ))
+
+
     ############################################################################
     # Misc
     def test_list_aliases(self):
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 0e6c130..02c3aaf 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -89,10 +89,12 @@ class TestUtils(TestCase):
 
     def test_parse_env_var_no_equals(self):
         '''parse_env_var handles no equals and gets value from environment'''
-        def mocked_getenv(key):
-            self.assertEqual(key, 'KEY')
-            return 'mockedvalue'
-
-        with mock.patch('os.getenv', side_effect=mocked_getenv):
+        with mocked_os_env(KEY='mockedvalue'):
             result = scuba.utils.parse_env_var('KEY')
         self.assertEqual(result, ('KEY', 'mockedvalue'))
+
+    def test_parse_env_var_not_set(self):
+        '''parse_env_var returns an empty string if not set'''
+        with mocked_os_env():
+            result = scuba.utils.parse_env_var('NOTSET')
+        self.assertEqual(result, ('NOTSET', ''))
diff --git a/tests/utils.py b/tests/utils.py
index 1444b3d..7ca2024 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -6,6 +6,11 @@ import tempfile
 import shutil
 import unittest
 import logging
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
 
 def assert_set_equal(a, b):
     assert_equal(set(a), set(b))
@@ -49,6 +54,9 @@ def make_executable(path):
     mode |= (mode & 0o444) >> 2    # copy R bits to X
     os.chmod(path, mode)
 
+def mocked_os_env(**env):
+    return mock.patch('os.getenv', side_effect=env.get)
+
 
 # http://stackoverflow.com/a/8389373/119527
 class PseudoTTY(object):