[issue-1750] - Expandvars transforming values (#1830)

* Prepare tests for basic config cases

* Add test cases

* Use os expandvars

* Add missing commas

* Add missing commas

* Remove dataclasses backport

ingestion/requirements.txt

@@ -1,6 +1,5 @@
 click~=7.1.2
 pydantic~=1.7.4
-expandvars~=0.6.5
 requests~=2.25.1
 python-dateutil~=2.8.1
 SQLAlchemy~=1.4.5

ingestion/setup.py

@@ -26,9 +26,7 @@ base_requirements = {
     "commonregex",
     "idna<3,>=2.5",
     "click>=7.1.1",
-    "expandvars>=0.6.5"
-    "dataclasses>=0.8"
-    "typing_extensions>=3.7.4"
+    "typing_extensions>=3.7.4",
     "mypy_extensions>=0.4.3",
     "typing-inspect",
     "pydantic>=1.7.4",

ingestion/src/metadata/config/common.py

@@ -11,11 +11,11 @@
 
 import io
 import json
+import os
 import pathlib
 from abc import ABC, abstractmethod
 from typing import IO, Any, Optional
 
-from expandvars import expandvars
 from pydantic import BaseModel
 
 
@@ -57,7 +57,7 @@ def load_config_file(config_file: pathlib.Path) -> dict:
     with config_file.open() as raw_config_file:
         raw_config = raw_config_file.read()
 
-    expanded_config_file = expandvars(raw_config, nounset=True)
+    expanded_config_file = os.path.expandvars(raw_config)
     config_fp = io.StringIO(expanded_config_file)
     config = json.load(config_fp)
 

ingestion/tests/unit/resources/config/basic.json

@@ -0,0 +1,20 @@
+{
+  "source": {
+    "type": "mlflow",
+    "config": {
+      "tracking_uri": "http://localhost:5000",
+      "registry_uri": "mysql+pymysql://mlflow:password@localhost:3307/experiments"
+    }
+  },
+  "sink": {
+    "type": "metadata-rest",
+    "config": {}
+  },
+  "metadata_server": {
+    "type": "metadata-server",
+    "config": {
+      "api_endpoint": "http://localhost:8585/api",
+      "auth_provider_type": "no-auth"
+    }
+  }
+}

ingestion/tests/unit/resources/config/basic.random

@@ -0,0 +1,20 @@
+{
+  "source": {
+    "type": "mlflow",
+    "config": {
+      "tracking_uri": "http://localhost:5000",
+      "registry_uri": "mysql+pymysql://mlflow:password@localhost:3307/experiments"
+    }
+  },
+  "sink": {
+    "type": "metadata-rest",
+    "config": {}
+  },
+  "metadata_server": {
+    "type": "metadata-server",
+    "config": {
+      "api_endpoint": "http://localhost:8585/api",
+      "auth_provider_type": "no-auth"
+    }
+  }
+}

ingestion/tests/unit/resources/config/dollar.json

@@ -0,0 +1,21 @@
+{
+  "source": {
+    "type": "mlflow",
+    "config": {
+      "tracking_uri": "http://localhost:5000",
+      "registry_uri": "mysql+pymysql://mlflow:password@localhost:3307/experiments",
+      "secret": "te$t"
+    }
+  },
+  "sink": {
+    "type": "metadata-rest",
+    "config": {}
+  },
+  "metadata_server": {
+    "type": "metadata-server",
+    "config": {
+      "api_endpoint": "http://localhost:8585/api",
+      "auth_provider_type": "no-auth"
+    }
+  }
+}

ingestion/tests/unit/resources/config/env_ok.json

@@ -0,0 +1,21 @@
+{
+  "source": {
+    "type": "mlflow",
+    "config": {
+      "tracking_uri": "http://localhost:5000",
+      "registry_uri": "mysql+pymysql://mlflow:password@localhost:3307/experiments",
+      "secret": "${PASSWORD}"
+    }
+  },
+  "sink": {
+    "type": "metadata-rest",
+    "config": {}
+  },
+  "metadata_server": {
+    "type": "metadata-server",
+    "config": {
+      "api_endpoint": "http://localhost:8585/api",
+      "auth_provider_type": "no-auth"
+    }
+  }
+}

ingestion/tests/unit/test_config.py

@@ -0,0 +1,76 @@
+#  Copyright 2021 Collate
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#  http://www.apache.org/licenses/LICENSE-2.0
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+"""
+Test module for loading configs
+"""
+import json
+import os
+from pathlib import Path
+from unittest import TestCase, mock
+
+from metadata.config.common import ConfigurationError, load_config_file
+
+
+class TestConfig(TestCase):
+    """
+    Check config reading
+    """
+
+    basedir = os.path.join("resources", "config")
+
+    def test_basic(self):
+        """
+        Load basic config file
+        """
+        basic_file = Path(os.path.join(self.basedir, "basic.json"))
+        loaded = load_config_file(basic_file)
+
+        with basic_file.open() as file:
+            expected = json.loads(file.read())
+
+        assert loaded == expected
+
+    def test_invalid(self):
+        """
+        Fail with non existent file
+        """
+        no_file = Path(os.path.join(self.basedir, "random.json"))
+
+        with self.assertRaises(ConfigurationError):
+            load_config_file(no_file)
+
+    def test_bad_suffix(self):
+        """
+        Fail if not valid suffix
+        """
+        bad_suffix = Path(os.path.join(self.basedir, "basic.random"))
+
+        with self.assertRaises(ConfigurationError):
+            load_config_file(bad_suffix)
+
+    @mock.patch.dict(os.environ, {"PASSWORD": "super_safe"})
+    def test_env(self):
+        """
+        We can load env vars correctly
+        """
+        pwd_file = Path(os.path.join(self.basedir, "env_ok.json"))
+        loaded = load_config_file(pwd_file)
+
+        assert loaded["source"]["config"]["secret"] == "super_safe"
+
+    def test_dollar_string(self):
+        """
+        String with $ should not be expanded
+        """
+        dollar_file = Path(os.path.join(self.basedir, "dollar.json"))
+        loaded = load_config_file(dollar_file)
+
+        assert loaded["source"]["config"]["secret"] == "te$t"

profiler/requirements.txt

@@ -10,5 +10,4 @@ setuptools~=58.2.0
 SQLAlchemy~=1.4.26
 click~=8.0.3
 Jinja2~=3.0.2
-expandvars~=0.7.0
 pydantic~=1.8.2

profiler/setup.py

@@ -38,11 +38,11 @@ base_requirements = {
     "click>=7.1.2, <8.0",
     "cryptography==3.3.2",
     "pyyaml>=5.4.1, <6.0",
-    "requests>=2.23.0, <3.0" "idna<3,>=2.5",
+    "requests>=2.23.0, <3.0",
+    "idna<3,>=2.5",
     "click<7.2.0,>=7.1.1",
-    "expandvars>=0.6.5"
-    "dataclasses>=0.8"
-    "typing_extensions>=3.7.4"
+    "dataclasses>=0.8",
+    "typing_extensions>=3.7.4",
     "mypy_extensions>=0.4.3",
     "typing-inspect",
     "pydantic==1.7.4",

profiler/src/openmetadata/common/config.py

@@ -11,13 +11,13 @@
 
 import io
 import json
+import os
 import pathlib
 import re
 from abc import ABC, abstractmethod
 from typing import IO, Any, List, Optional
 
 import yaml
-from expandvars import expandvars
 from pydantic import BaseModel
 
 
@@ -80,7 +80,7 @@ def load_config_file(config_file: pathlib.Path) -> dict:
     with config_file.open() as raw_config_file:
         raw_config = raw_config_file.read()
 
-    expanded_config_file = expandvars(raw_config, nounset=True)
+    expanded_config_file = os.path.expandvars(raw_config)
     config_fp = io.StringIO(expanded_config_file)
     config = config_mech.load_config(config_fp)