From ab620e95af1472637366ead91d056ec93f8fcdb6 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Sun, 19 Dec 2021 00:35:12 +0100 Subject: [PATCH] [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 - ingestion/setup.py | 4 +- ingestion/src/metadata/config/common.py | 4 +- .../tests/unit/resources/config/basic.json | 20 +++++ .../tests/unit/resources/config/basic.random | 20 +++++ .../tests/unit/resources/config/dollar.json | 21 +++++ .../tests/unit/resources/config/env_ok.json | 21 +++++ ingestion/tests/unit/test_config.py | 76 +++++++++++++++++++ profiler/requirements.txt | 1 - profiler/setup.py | 8 +- profiler/src/openmetadata/common/config.py | 4 +- 11 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 ingestion/tests/unit/resources/config/basic.json create mode 100644 ingestion/tests/unit/resources/config/basic.random create mode 100644 ingestion/tests/unit/resources/config/dollar.json create mode 100644 ingestion/tests/unit/resources/config/env_ok.json create mode 100644 ingestion/tests/unit/test_config.py diff --git a/ingestion/requirements.txt b/ingestion/requirements.txt index 873fbae5d55..967052ce29f 100644 --- a/ingestion/requirements.txt +++ b/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 diff --git a/ingestion/setup.py b/ingestion/setup.py index 2bbdc779120..e0ae69c7c57 100644 --- a/ingestion/setup.py +++ b/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", diff --git a/ingestion/src/metadata/config/common.py b/ingestion/src/metadata/config/common.py index 7e311156087..2637787eb19 100644 --- a/ingestion/src/metadata/config/common.py +++ b/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) diff --git a/ingestion/tests/unit/resources/config/basic.json b/ingestion/tests/unit/resources/config/basic.json new file mode 100644 index 00000000000..9653237544c --- /dev/null +++ b/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" + } + } +} diff --git a/ingestion/tests/unit/resources/config/basic.random b/ingestion/tests/unit/resources/config/basic.random new file mode 100644 index 00000000000..9653237544c --- /dev/null +++ b/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" + } + } +} diff --git a/ingestion/tests/unit/resources/config/dollar.json b/ingestion/tests/unit/resources/config/dollar.json new file mode 100644 index 00000000000..5d09bbc9350 --- /dev/null +++ b/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" + } + } +} diff --git a/ingestion/tests/unit/resources/config/env_ok.json b/ingestion/tests/unit/resources/config/env_ok.json new file mode 100644 index 00000000000..3dd7d2a1e96 --- /dev/null +++ b/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" + } + } +} diff --git a/ingestion/tests/unit/test_config.py b/ingestion/tests/unit/test_config.py new file mode 100644 index 00000000000..cff8826db73 --- /dev/null +++ b/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" diff --git a/profiler/requirements.txt b/profiler/requirements.txt index 0c82a7ac1f0..aa661007c2d 100644 --- a/profiler/requirements.txt +++ b/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 \ No newline at end of file diff --git a/profiler/setup.py b/profiler/setup.py index 4cd78a67de6..f8774fbe74c 100644 --- a/profiler/setup.py +++ b/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", diff --git a/profiler/src/openmetadata/common/config.py b/profiler/src/openmetadata/common/config.py index 2bcc4ac4e38..654174c6195 100644 --- a/profiler/src/openmetadata/common/config.py +++ b/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)