From d2e47cee6491d817e9235593564ae3fff334ec4c Mon Sep 17 00:00:00 2001
From: Yuval Goldberg <yuvigoldi@gmail.com>
Date: Tue, 27 Jul 2021 20:53:58 +0300
Subject: [PATCH] Support shell interpolation

---
 README.md         | 13 +++++++++++
 skipper/config.py | 14 +++++++++---
 tests/test_cli.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index 5ecb5b6..a991766 100644
--- a/README.md
+++ b/README.md
@@ -235,6 +235,19 @@ env:
     VAR: $$VAR_NOT_INTERPOLATED
 ````
 
+### Shell Interpolation
+
+Skipper supports evaluating shell commands inside its configuration file using `$(command)` notation.
+e.g.
+
+```yaml
+env:
+    VAR: $(expr ${MY_NUMBER:-5} + 5)
+volumes:
+    - $(which myprogram):/myprogram
+```
+
+
 ### Volumes:
 Skipper can bind-mount a host directory into the container.
 you can add volumes in the configuration file:
diff --git a/skipper/config.py b/skipper/config.py
index d9f3da0..d5557bd 100644
--- a/skipper/config.py
+++ b/skipper/config.py
@@ -1,8 +1,11 @@
-from string import Template
-from collections import defaultdict
 import os
-import yaml
+from collections import defaultdict
+from re import findall
+from string import Template
+from subprocess import check_output
+
 import six
+import yaml
 
 
 def load_defaults():
@@ -32,4 +35,9 @@ def _normalize_config(config, normalized_config):
 
 
 def _interpolate_env_vars(key):
+    for match in findall(r'\$\(.+\)', key):
+        output = check_output("echo " + match, shell=True).strip()
+        if not output:
+            raise ValueError(match)
+        key = key.replace(match, output)
     return Template(key).substitute(defaultdict(lambda: "", os.environ))
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 3539929..8fbe393 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -163,6 +163,33 @@
     'container-context': SKIPPER_CONF_CONTAINER_CONTEXT
 }
 
+SKIPPER_CONF_WITH_SHELL_INTERPOLATION = {
+    'registry': REGISTRY,
+    'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE,
+    'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG,
+    'make': {
+        'makefile': SKIPPER_CONF_MAKEFILE,
+    },
+    'volumes': [
+        '$(which cat):/cat',
+    ],
+    'env': [
+        'KEY=$(expr ${MY_NUMBER:-5} + 5)'
+    ]
+}
+
+SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION = {
+    'registry': REGISTRY,
+    'build-container-image': SKIPPER_CONF_BUILD_CONTAINER_IMAGE,
+    'build-container-tag': SKIPPER_CONF_BUILD_CONTAINER_TAG,
+    'make': {
+        'makefile': SKIPPER_CONF_MAKEFILE,
+    },
+    'volumes': [
+        '$(bla bla):/cat',
+    ]
+}
+
 
 class TestCLI(unittest.TestCase):
     def setUp(self):
@@ -1572,6 +1599,37 @@ def test_run_with_defaults_from_config_file_including_volumes(self, skipper_runn
                                                         volumes=['volume1', 'volume2'], workspace=None,
                                                         workdir=None, use_cache=False, env_file=())
 
+    @mock.patch('__builtin__.open', mock.MagicMock(create=True))
+    @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True))
+    @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_SHELL_INTERPOLATION))
+    @mock.patch('subprocess.check_output', mock.MagicMock(autospec=True, return_value='1234567\n'))
+    @mock.patch('skipper.runner.run', autospec=True)
+    def test_run_with_defaults_from_config_file_including_interpolated_volumes(self, skipper_runner_run_mock):
+        command = ['ls', '-l']
+        run_params = command
+        self._invoke_cli(
+            defaults=config.load_defaults(),
+            subcmd='run',
+            subcmd_params=run_params
+        )
+        expected_fqdn_image = 'skipper-conf-build-container-image:skipper-conf-build-container-tag'
+        skipper_runner_run_mock.assert_called_once_with(command, fqdn_image=expected_fqdn_image, environment=['KEY=10'],
+                                                        interactive=False, name=None, net=None, publish=(),
+                                                        volumes=['/bin/cat:/cat'], workspace=None,
+                                                        workdir=None, use_cache=False, env_file=())
+
+    @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_INVALID_SHELL_INTERPOLATION))
+    def test_run_with_defaults_from_config_file_including_invalid_interploated_volumes_interpolated(self):
+        command = ['ls', '-l']
+        run_params = command
+
+        with self.assertRaises(ValueError):
+            self._invoke_cli(
+                defaults=config.load_defaults(),
+                subcmd='run',
+                subcmd_params=run_params
+            )
+
     @mock.patch('__builtin__.open', mock.MagicMock(create=True))
     @mock.patch('os.path.exists', mock.MagicMock(autospec=True, return_value=True))
     @mock.patch('yaml.safe_load', mock.MagicMock(autospec=True, return_value=SKIPPER_CONF_WITH_WORKDIR))