Repository: dcs4cop/xcube

Issue:
xcube gen2 DataStore configuration to understand env var references
I want to configure a data store which contains input files to be used xcube gen2 service generator. The data is located in an s3 bucket which is protected, so my configuration looks like the following:

```
my_data:
  title: "My Data for xcube gen2"
  store_id: s3
  store_params:
    root: "<bucket-path>"
    storage_options:
      key: "${MY_ACCESS_KEY_ID}"
      secret: "${MY_SECRET_ACCESS_KEY}"
```
This leads into an "Access denied" issue. When I put the credentials straight into the configuration, it works as expected. I wish for the same behavior when using env var references. :) 



Discussion:

Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 9c10486d..2e66417b 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,5 +1,14 @@
 ## Changes in 0.9.3 (in development)
 
+* It is now possible to use environment variables in most  
+  xcube configuration files. Unix bash syntax is used, i.e. 
+  `${ENV_VAR_NAME}` or `$ENV_VAR_NAME`. (#580)
+  
+  Supported tools include
+  - `xcube gen --config CONFIG` 
+  - `xcube gen2 --stores STORES_CONFIG --service SERVICE_CONFIG` 
+  - `xcube serve -c CONFIG` 
+
 * Changed the `xcube gen` tool to extract metadata for pre-sorting inputs
   from other than NetCDF inputs, e.g. GeoTIFF.
 
diff --git a/xcube/core/gen2/remote/config.py b/xcube/core/gen2/remote/config.py
index 0c3472e0..9e34a5dd 100644
--- a/xcube/core/gen2/remote/config.py
+++ b/xcube/core/gen2/remote/config.py
@@ -19,15 +19,10 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-import io
-import json
-import os
-import string
-from typing import Any, Union
 from typing import Dict
+from typing import Union
 
-import yaml
-
+from xcube.util.config import load_json_or_yaml_config
 from xcube.util.jsonschema import JsonObject
 from xcube.util.jsonschema import JsonObjectSchema
 from xcube.util.jsonschema import JsonStringSchema
@@ -85,7 +80,7 @@ class ServiceConfig(JsonObject):
 
     @classmethod
     def from_file(cls, service_config_file: str) -> 'ServiceConfig':
-        service_config = load_json_or_yaml_file(service_config_file)
+        service_config = load_json_or_yaml_config(service_config_file)
         cls.get_schema().validate_instance(service_config)
         return ServiceConfig(**service_config)
 
@@ -102,15 +97,3 @@ class ServiceConfig(JsonObject):
             required=[],
             factory=cls,
         )
-
-
-def load_json_or_yaml_file(config_file: str) -> Any:
-    with open(config_file, 'r') as fp:
-        file_content = fp.read()
-    template = string.Template(file_content)
-    file_content = template.safe_substitute(os.environ)
-    with io.StringIO(file_content) as fp:
-        if config_file.endswith('.json'):
-            return json.load(fp)
-        else:
-            return yaml.safe_load(fp)
diff --git a/xcube/core/store/storepool.py b/xcube/core/store/storepool.py
index 793f17b5..0383dbf6 100644
--- a/xcube/core/store/storepool.py
+++ b/xcube/core/store/storepool.py
@@ -35,6 +35,7 @@ from .assertions import assert_valid_config
 from .error import DataStoreError
 from .store import DataStore
 from .store import new_data_store
+from ...util.config import load_json_or_yaml_config
 
 
 def get_data_store_instance(store_id: str,
@@ -330,11 +331,7 @@ class DataStorePool:
     @classmethod
     def from_file(cls, path: str) -> 'DataStorePool':
         _, ext = os.path.splitext(path)
-        with open(path) as fp:
-            if ext == '.json':
-                store_configs = json.load(fp)
-            else:
-                store_configs = yaml.safe_load(fp)
+        store_configs = load_json_or_yaml_config(path)
         return cls.from_dict(store_configs or {})
 
     @classmethod
diff --git a/xcube/util/config.py b/xcube/util/config.py
index 698d557c..3ebcd8f2 100644
--- a/xcube/util/config.py
+++ b/xcube/util/config.py
@@ -21,7 +21,11 @@
 
 import datetime
 import fnmatch
+import io
+import json
+import os
 import os.path
+import string
 from typing import Any, Dict, Optional, Iterable, Tuple, List
 
 import yaml
@@ -32,7 +36,9 @@ NameAnyDict = Dict[str, Any]
 NameDictPairList = List[Tuple[str, Optional[NameAnyDict]]]
 
 
-def to_resolved_name_dict_pairs(name_dict_pairs: NameDictPairList, container, keep=False) -> NameDictPairList:
+def to_resolved_name_dict_pairs(name_dict_pairs: NameDictPairList,
+                                container,
+                                keep=False) -> NameDictPairList:
     resolved_pairs = []
     for name, value in name_dict_pairs:
         if '*' in name or '?' in name or '[' in name:
@@ -44,7 +50,8 @@ def to_resolved_name_dict_pairs(name_dict_pairs: NameDictPairList, container, ke
     return resolved_pairs
 
 
-def to_name_dict_pairs(iterable: Iterable[Any], default_key=None) -> NameDictPairList:
+def to_name_dict_pairs(iterable: Iterable[Any], default_key=None) \
+        -> NameDictPairList:
     return [to_name_dict_pair(item, parent=iterable, default_key=default_key)
             for item in iterable]
 
@@ -137,7 +144,9 @@ def merge_config(first_dict: Dict, *more_dicts):
         output_dict = dict(first_dict)
         for d in more_dicts:
             for k, v in d.items():
-                if k in output_dict and isinstance(output_dict[k], dict) and isinstance(v, dict):
+                if k in output_dict \
+                        and isinstance(output_dict[k], dict) \
+                        and isinstance(v, dict):
                     v = merge_config(output_dict[k], v)
                 output_dict[k] = v
     return output_dict
@@ -148,15 +157,36 @@ def load_configs(*config_paths: str) -> Dict[str, Any]:
     for config_path in config_paths:
         if not os.path.isfile(config_path):
             raise ValueError(f'Cannot find configuration {config_path!r}')
-        with open(config_path) as fp:
-            try:
-                config_dict = yaml.safe_load(fp)
-            except yaml.YAMLError as e:
-                raise ValueError(f'YAML in {config_path!r} is invalid: {e}') from e
-            except OSError as e:
-                raise ValueError(f'Cannot load configuration from {config_path!r}: {e}') from e
-            if not isinstance(config_dict, dict):
-                raise ValueError(f'Invalid configuration format in {config_path!r}: dictionary expected')
-            config_dicts.append(config_dict)
+        config_dict = load_json_or_yaml_config(config_path)
+        config_dicts.append(config_dict)
     config = merge_config(*config_dicts)
     return config
+
+
+def load_json_or_yaml_config(config_path: str) -> Dict[str, Any]:
+    try:
+        config_dict = _load_json_or_yaml_config(config_path)
+    except yaml.YAMLError as e:
+        raise ValueError(
+            f'YAML in {config_path!r} is invalid: {e}') from e
+    except OSError as e:
+        raise ValueError(
+            f'Cannot load configuration from'
+            f' {config_path!r}: {e}') from e
+    if not isinstance(config_dict, dict):
+        raise ValueError(
+            f'Invalid configuration format in'
+            f' {config_path!r}: dictionary expected')
+    return config_dict
+
+
+def _load_json_or_yaml_config(config_file: str) -> Any:
+    with open(config_file, 'r') as fp:
+        file_content = fp.read()
+    template = string.Template(file_content)
+    file_content = template.safe_substitute(os.environ)
+    with io.StringIO(file_content) as fp:
+        if config_file.endswith('.json'):
+            return json.load(fp)
+        else:
+            return yaml.safe_load(fp)
diff --git a/xcube/util/dask.py b/xcube/util/dask.py
index 5373da2b..55d5370f 100644
--- a/xcube/util/dask.py
+++ b/xcube/util/dask.py
@@ -36,9 +36,9 @@ def compute_array_from_func(func: Callable[..., np.ndarray],
     * ``dtype``: The array's numpy data type.
     * ``name``: The array's name. A string or ``None``.
     * ``block_id``: The block's unique ID. An integer number ranging from zero to number of blocks minus one.
-    * ``block_index``: The block's index. A tuple of ints.
-    * ``block_shape``: The block's shape. A tuple of ints.
-    * ``block_slices``: The block's shape. A tuple of int pair tuples.
+    * ``block_index``: The block's index as a tuple of ints.
+    * ``block_shape``: The block's shape as a tuple of ints.
+    * ``block_slices``: The block's shape as a tuple of int pair tuples.
 
     :param func: User function that is called for each block of the array using arguments specified by
         *ctx_arg_names*, *args*, and *kwargs*. It must return a numpy array of shape "block_shape" and type
diff --git a/xcube/webapi/service.py b/xcube/webapi/service.py
index 5235e002..3d0d2d60 100644
--- a/xcube/webapi/service.py
+++ b/xcube/webapi/service.py
@@ -34,10 +34,10 @@ from typing import Optional, Dict, List, Tuple, Mapping
 
 import tornado.escape
 import tornado.options
-import yaml
 from tornado.ioloop import IOLoop
 from tornado.log import enable_pretty_logging
-from tornado.web import RequestHandler, Application
+from tornado.web import Application
+from tornado.web import RequestHandler
 
 from xcube.constants import LOG
 from xcube.core.dsio import is_s3_url
@@ -45,11 +45,17 @@ from xcube.core.mldataset import guess_ml_dataset_format
 from xcube.util.cache import parse_mem_size
 from xcube.util.caseless import caseless_dict
 from xcube.util.config import load_configs
+from xcube.util.config import load_json_or_yaml_config
 from xcube.util.undefined import UNDEFINED
 from xcube.version import version
 from xcube.webapi.context import ServiceContext
-from xcube.webapi.defaults import DEFAULT_ADDRESS, DEFAULT_PORT, DEFAULT_UPDATE_PERIOD, DEFAULT_LOG_PREFIX, \
-    DEFAULT_TILE_CACHE_SIZE, DEFAULT_TRACE_PERF, DEFAULT_TILE_COMP_MODE
+from xcube.webapi.defaults import DEFAULT_ADDRESS
+from xcube.webapi.defaults import DEFAULT_LOG_PREFIX
+from xcube.webapi.defaults import DEFAULT_PORT
+from xcube.webapi.defaults import DEFAULT_TILE_CACHE_SIZE
+from xcube.webapi.defaults import DEFAULT_TILE_COMP_MODE
+from xcube.webapi.defaults import DEFAULT_TRACE_PERF
+from xcube.webapi.defaults import DEFAULT_UPDATE_PERIOD
 from xcube.webapi.errors import ServiceBadRequestError
 from xcube.webapi.reqparams import RequestParams
 
@@ -233,11 +239,10 @@ class Service:
         if self.context.config_mtime != stat.st_mtime:
             self.context.config_mtime = stat.st_mtime
             try:
-                with open(config_file, encoding='utf-8') as stream:
-                    self.context.config = yaml.safe_load(stream)
+                self.context.config = load_json_or_yaml_config(config_file)
                 self.config_error = None
                 LOG.info(f'configuration file {config_file!r} successfully loaded')
-            except (yaml.YAMLError, OSError) as e:
+            except ValueError as e:
                 if self.config_error is None:
                     LOG.error(f'configuration file {config_file!r}: {e}')
                     self.config_error = e
@@ -255,6 +260,7 @@ class ServiceRequestHandler(RequestHandler):
 
     @property
     def service_context(self) -> ServiceContext:
+        # noinspection PyUnresolvedReferences
         return self.application.service_context
 
     @property
@@ -271,7 +277,8 @@ class ServiceRequestHandler(RequestHandler):
         self.set_header('Access-Control-Allow-Methods',
                         'GET,PUT,DELETE,OPTIONS')
         self.set_header('Access-Control-Allow-Headers',
-                        'x-requested-with,access-control-allow-origin,authorization,content-type')
+                        'x-requested-with,access-control-allow-origin,'
+                        'authorization,content-type')
 
     # noinspection PyUnusedLocal
     def options(self, *args, **kwargs):


Test patch:
diff --git a/test/core/gen/test_config.py b/test/core/gen/test_config.py
index 7a344a7c..1e545dcf 100644
--- a/test/core/gen/test_config.py
+++ b/test/core/gen/test_config.py
@@ -8,8 +8,8 @@ from xcube.core.gen.config import get_config_dict
 
 TEMP_PATH_FOR_YAML = './temp_test_data_for_xcube_tests'
 
-CONFIG_1_NAME = 'config_1.json'
-CONFIG_2_NAME = 'config_2.json'
+CONFIG_1_NAME = 'config_1.yaml'
+CONFIG_2_NAME = 'config_2.yaml'
 CONFIG_1_FILE_LIST = [(os.path.join(TEMP_PATH_FOR_YAML, CONFIG_1_NAME)),
                       (os.path.join(TEMP_PATH_FOR_YAML, CONFIG_2_NAME))]
 CONFIG_1_YAML = """
@@ -21,8 +21,8 @@ output_variables:
   - z*
 """
 
-CONFIG_3_NAME = 'config_3.json'
-CONFIG_4_NAME = 'config_4.json'
+CONFIG_3_NAME = 'config_3.yaml'
+CONFIG_4_NAME = 'config_4.yaml'
 CONFIG_2_FILE_LIST = [(os.path.join(TEMP_PATH_FOR_YAML, CONFIG_3_NAME)),
                       (os.path.join(TEMP_PATH_FOR_YAML, CONFIG_4_NAME))]
 CONFIG_2_YAML = """
diff --git a/test/core/store/test_storepool.py b/test/core/store/test_storepool.py
index 3b7ccc1b..b042b986 100644
--- a/test/core/store/test_storepool.py
+++ b/test/core/store/test_storepool.py
@@ -190,7 +190,9 @@ class DataStorePoolTest(unittest.TestCase):
         }
         with self.assertRaises(jsonschema.exceptions.ValidationError) as cm:
             DataStorePool.from_dict(store_configs)
-        self.assertTrue("'store_id' is a required property" in f'{cm.exception}', msg=f'{cm.exception}')
+        self.assertTrue(
+            "'store_id' is a required property" in f'{cm.exception}',
+            msg=f'{cm.exception}')
 
         store_configs = {
             "dir": {
@@ -199,47 +201,88 @@ class DataStorePoolTest(unittest.TestCase):
         }
         with self.assertRaises(jsonschema.exceptions.ValidationError) as cm:
             DataStorePool.from_dict(store_configs)
-        self.assertTrue("Failed validating 'type' in schema" in f'{cm.exception}', msg=f'{cm.exception}')
+        self.assertTrue(
+            "Failed validating 'type' in schema" in f'{cm.exception}',
+            msg=f'{cm.exception}')
 
     def test_from_json_file(self):
-        store_configs = {
-            "ram-1": {
-                "store_id": "memory"
-            },
-            "ram-2": {
-                "store_id": "memory"
-            }
-        }
-        path = 'test-store-configs.json'
+        self._assert_file_ok('json')
+
+    def test_from_yaml_file(self):
+        self._assert_file_ok('yaml')
+
+    def test_from_json_file_env(self):
+        self._assert_file_ok('json', use_env_vars=True)
+        self._assert_file_ok('json',
+                             root_1='/test_1',
+                             root_2='/test_2',
+                             use_env_vars=True)
+
+    def test_from_yaml_file_env(self):
+        self._assert_file_ok('yaml', use_env_vars=True)
+        self._assert_file_ok('yaml',
+                             root_1='/test_1',
+                             root_2='/test_2',
+                             use_env_vars=True)
+
+    def _assert_file_ok(self,
+                        format_name: str,
+                        root_1="/root1",
+                        root_2="/root2",
+                        use_env_vars=False):
+        if use_env_vars:
+            store_configs = self._get_test_config(
+                root_1='${_TEST_ROOT_1}',
+                root_2='${_TEST_ROOT_2}'
+            )
+            import os
+            os.environ['_TEST_ROOT_1'] = root_1
+            os.environ['_TEST_ROOT_2'] = root_2
+        else:
+            store_configs = self._get_test_config(
+                root_1=root_1,
+                root_2=root_2
+            )
+        path = 'test-store-configs.' + format_name
         with open(path, 'w') as fp:
-            json.dump(store_configs, fp, indent=2)
+            mod = yaml if format_name == 'yaml' else json
+            mod.dump(store_configs, fp, indent=2)
         try:
             pool = DataStorePool.from_file(path)
             self.assertIsInstance(pool, DataStorePool)
             self.assertEqual(['ram-1', 'ram-2'], pool.store_instance_ids)
+            config_1 = pool.get_store_config('ram-1')
+            self.assertIsInstance(config_1, DataStoreConfig)
+            self.assertEqual(
+                {'store_id': 'memory',
+                 'store_params': {'root': root_1}},
+                config_1.to_dict())
+            config_2 = pool.get_store_config('ram-2')
+            self.assertIsInstance(config_2, DataStoreConfig)
+            self.assertEqual(
+                {'store_id': 'memory',
+                 'store_params': {'root': root_2}},
+                config_2.to_dict())
         finally:
             import os
             os.remove(path)
 
-    def test_from_yaml_file(self):
-        store_configs = {
+    @staticmethod
+    def _get_test_config(root_1: str, root_2: str):
+        return {
             "ram-1": {
-                "store_id": "memory"
+                "store_id": "memory",
+                "store_params": {
+                    "root": root_1
+                },
             },
             "ram-2": {
-                "store_id": "memory"
+                "store_id": "memory",
+                "store_params": {
+                    "root": root_2
+                },
             }
         }
-        path = 'test-store-configs.yaml'
-        with open(path, 'w') as fp:
-            yaml.dump(store_configs, fp, indent=2)
-        try:
-            pool = DataStorePool.from_file(path)
-            self.assertIsInstance(pool, DataStorePool)
-            self.assertEqual(['ram-1', 'ram-2'], pool.store_instance_ids)
-        finally:
-            import os
-            os.remove(path)
 
     def test_get_store(self):
         store_configs = {
diff --git a/test/webapi/test_auth.py b/test/webapi/test_auth.py
index 19406823..8a9401f3 100644
--- a/test/webapi/test_auth.py
+++ b/test/webapi/test_auth.py
@@ -79,6 +79,8 @@ class AuthMixinIdTokenTest(unittest.TestCase):
         auth_mixin.request = RequestMock(headers={})
         self.assertEqual(None, auth_mixin.get_id_token())
 
+    @unittest.skipUnless(XCUBE_TEST_CLIENT_ID and XCUBE_TEST_CLIENT_SECRET,
+                         'XCUBE_TEST_CLIENT_ID and XCUBE_TEST_CLIENT_SECRET must be set')
     def test_expired_access_token(self):
         auth_mixin = AuthMixin()
         auth_mixin.service_context = ServiceContextMock(config=dict(