refactor: Implementing the YamlProvider and Cache Provider (#703)

dbt_platform_helper/constants.py

@@ -16,3 +16,12 @@
     "redis",
 ]
 CONDUIT_DOCKER_IMAGE_LOCATION = "public.ecr.aws/uktrade/tunnel"
+HYPHENATED_APPLICATION_NAME = "hyphenated-application-name"
+ALPHANUMERIC_ENVIRONMENT_NAME = "alphanumericenvironmentname123"
+ALPHANUMERIC_SERVICE_NAME = "alphanumericservicename123"
+COPILOT_IDENTIFIER = "c0PIlotiD3ntIF3r"
+CLUSTER_NAME_SUFFIX = f"Cluster-{COPILOT_IDENTIFIER}"
+SERVICE_NAME_SUFFIX = f"Service-{COPILOT_IDENTIFIER}"
+REFRESH_TOKEN_MESSAGE = (
+    "To refresh this SSO session run `aws sso login` with the corresponding profile"
+)

dbt_platform_helper/domain/config_validator.py

@@ -9,13 +9,6 @@
 from dbt_platform_helper.utils.messages import abort_with_error
 
 
-# TODO = this shouldnt live here.. should it hehe
-def get_env_deploy_account_info(config, env, key):
-    return (
-        config.get("environments", {}).get(env, {}).get("accounts", {}).get("deploy", {}).get(key)
-    )
-
-
 class ConfigValidator:
 
     def __init__(self, validations=[]):
@@ -189,8 +182,20 @@ def validate_database_copy_section(self, config):
                 from_env = section["from"]
                 to_env = section["to"]
 
-                from_account = get_env_deploy_account_info(config, from_env, "id")
-                to_account = get_env_deploy_account_info(config, to_env, "id")
+                from_account = (
+                    config.get("environments", {})
+                    .get(from_env, {})
+                    .get("accounts", {})
+                    .get("deploy", {})
+                    .get("id")
+                )
+                to_account = (
+                    config.get("environments", {})
+                    .get(to_env, {})
+                    .get("accounts", {})
+                    .get("deploy", {})
+                    .get("id")
+                )
 
                 if from_env == to_env:
                     errors.append(

dbt_platform_helper/providers/cache.py

@@ -1,17 +1,21 @@
 import os
 from datetime import datetime
-from pathlib import Path
 
-import yaml
+from dbt_platform_helper.providers.yaml_file import FileProvider
+from dbt_platform_helper.providers.yaml_file import YamlFileProvider
 
 
 class CacheProvider:
-    def __init__(self):
+    def __init__(
+        self,
+        file_provider: FileProvider = None,
+    ):
         self._cache_file = ".platform-helper-config-cache.yml"
+        self.file_provider = file_provider or YamlFileProvider
 
     def read_supported_versions_from_cache(self, resource_name):
 
-        platform_helper_config = self.__read_file_as_yaml(self._cache_file)
+        platform_helper_config = self.file_provider.load(self._cache_file)
 
         return platform_helper_config.get(resource_name).get("versions")
 
@@ -20,7 +24,7 @@ def update_cache(self, resource_name, supported_versions):
         platform_helper_config = {}
 
         if self.__cache_exists():
-            platform_helper_config = self.__read_file_as_yaml(self._cache_file)
+            platform_helper_config = self.file_provider.load(self._cache_file)
 
         cache_dict = {
             resource_name: {
@@ -31,7 +35,11 @@ def update_cache(self, resource_name, supported_versions):
 
         platform_helper_config.update(cache_dict)
 
-        self.__write_cache(platform_helper_config)
+        self.file_provider.write(
+            self._cache_file,
+            platform_helper_config,
+            "# [!] This file is autogenerated via the platform-helper. Do not edit.\n",
+        )
 
     def cache_refresh_required(self, resource_name) -> bool:
         """
@@ -47,7 +55,7 @@ def cache_refresh_required(self, resource_name) -> bool:
         if not self.__cache_exists():
             return True
 
-        platform_helper_config = self.__read_file_as_yaml(self._cache_file)
+        platform_helper_config = self.file_provider.load(self._cache_file)
 
         if platform_helper_config.get(resource_name):
             return self.__check_if_cached_datetime_is_greater_than_interval(
@@ -65,19 +73,5 @@ def __check_if_cached_datetime_is_greater_than_interval(date_retrieved, interval
 
         return delta.days > interval_in_days
 
-    # TODO - same applies here as below
-    @staticmethod
-    def __read_file_as_yaml(file_name):
-
-        return yaml.safe_load(Path(file_name).read_text())
-
-    # TODO - temp fix to the unit test coverage issue, plan is to seperate out any yaml interaction methods into a seperate 'yaml' provider
-    # should be done under a different sub-task which will need to loop back to this provider as part of that work to use the yaml provider instead
-    def __write_cache(self, contents):
-
-        with open(self._cache_file, "w") as file:
-            file.write("# [!] This file is autogenerated via the platform-helper. Do not edit.\n")
-            yaml.dump(contents, file)
-
     def __cache_exists(self):
         return os.path.exists(self._cache_file)

dbt_platform_helper/providers/cloudformation.py

@@ -93,7 +93,6 @@ def update_conduit_stack_resources(
         )
 
         params = []
-        # TODO Currently not covered by tests - see https://uktrade.atlassian.net/browse/DBTP-1582
         if "Parameters" in template_yml:
             for param in template_yml["Parameters"]:
                 params.append({"ParameterKey": param, "UsePreviousValue": True})

dbt_platform_helper/providers/yaml_file.py

@@ -59,6 +59,11 @@ def load(path: str) -> dict:
 
         return yaml_content
 
+    def write(path: str, contents: dict, comment: str = ""):
+        with open(path, "w") as file:
+            file.write(comment)
+            yaml.dump(contents, file)
+
     @staticmethod
     def lint_yaml_for_duplicate_keys(path):
         duplicate_keys = []

dbt_platform_helper/utils/aws.py

@@ -13,6 +13,7 @@
 import yaml
 from boto3 import Session
 
+from dbt_platform_helper.constants import REFRESH_TOKEN_MESSAGE
 from dbt_platform_helper.platform_exception import PlatformException
 from dbt_platform_helper.providers.aws import AWSException
 from dbt_platform_helper.providers.aws import CopilotCodebaseNotFoundException
@@ -26,9 +27,6 @@
 
 
 def get_aws_session_or_abort(aws_profile: str = None) -> boto3.session.Session:
-    REFRESH_TOKEN_MESSAGE = (
-        "To refresh this SSO session run `aws sso login` with the corresponding profile"
-    )
     aws_profile = aws_profile or os.getenv("AWS_PROFILE")
     if aws_profile in AWS_SESSION_CACHE:
         return AWS_SESSION_CACHE[aws_profile]

tests/platform_helper/providers/test_cache.py

@@ -10,7 +10,8 @@
 
 def test_cache_refresh_required_with_cached_datetime_greater_than_one_day_returns_true():
 
-    cache_provider = CacheProvider()
+    file_provider_mock = MagicMock()
+    cache_provider = CacheProvider(file_provider=file_provider_mock)
 
     read_yaml_return_value = {
         "redis": {
@@ -19,11 +20,7 @@ def test_cache_refresh_required_with_cached_datetime_greater_than_one_day_return
         }
     }
 
-    # TODO - read_file_as_yaml mocking will fall away as a result of this functionality being delegated to YamlFiles refactor
-    cache_provider._CacheProvider__read_file_as_yaml = MagicMock(
-        return_value=read_yaml_return_value
-    )
-
+    file_provider_mock.load.return_value = read_yaml_return_value
     with patch("dbt_platform_helper.providers.cache.os.path.exists", return_value=True):
 
         assert cache_provider.cache_refresh_required("redis")
@@ -35,15 +32,13 @@ def test_cache_refresh_required_with_cached_datetime_greater_less_one_day_return
     # Time range is still < 1 day so should not require refresh
     middle_of_today = today - timedelta(hours=12)
 
-    cache_provider = CacheProvider()
+    file_provider_mock = MagicMock()
+    cache_provider = CacheProvider(file_provider=file_provider_mock)
 
     read_yaml_return_value = {
         "redis": {"date-retrieved": middle_of_today.strftime("%d-%m-%y %H:%M:%S")}
     }
-    # TODO - read_file_as_yaml mocking will fall away as a result of this functionality being delegated to YamlFiles refactor
-    cache_provider._CacheProvider__read_file_as_yaml = MagicMock(
-        return_value=read_yaml_return_value
-    )
+    file_provider_mock.load.return_value = read_yaml_return_value
 
     with patch("dbt_platform_helper.providers.cache.os.path.exists", return_value=True):
         assert not cache_provider.cache_refresh_required("redis")
@@ -52,16 +47,14 @@ def test_cache_refresh_required_with_cached_datetime_greater_less_one_day_return
 @freeze_time("2024-12-09 16:00:00")
 def test_update_cache_with_existing_cache_file_expected_file():
 
-    cache_provider = CacheProvider()
+    file_provider_mock = MagicMock()
+    cache_provider = CacheProvider(file_provider=file_provider_mock)
 
     read_yaml_return_value = {
         "redis": {"date-retrieved": "09-02-02 10:35:48", "versions": ["7.1", "7.2"]}
     }
-    # TODO - read_file_as_yaml mocking will fall away as a result of this functionality being delegated to YamlFiles refactor
-    cache_provider._CacheProvider__read_file_as_yaml = MagicMock(
-        return_value=read_yaml_return_value
-    )
-    cache_provider._CacheProvider__write_cache = MagicMock()
+    file_provider_mock.load.return_value = read_yaml_return_value
+    file_provider_mock.write = MagicMock()
 
     expected_contents = {
         "redis": {"date-retrieved": "09-02-02 10:35:48", "versions": ["7.1", "7.2"]},
@@ -72,21 +65,23 @@ def test_update_cache_with_existing_cache_file_expected_file():
 
         cache_provider.update_cache("opensearch", ["6.1", "6.2"])
 
-    cache_provider._CacheProvider__write_cache.assert_called_once_with(expected_contents)
+    file_provider_mock.write.assert_called_once_with(
+        ".platform-helper-config-cache.yml",
+        expected_contents,
+        "# [!] This file is autogenerated via the platform-helper. Do not edit.\n",
+    )
 
 
 def test_read_supported_versions_from_cache_returns_correct_product():
 
-    cache_provider = CacheProvider()
+    file_provider_mock = MagicMock()
+    cache_provider = CacheProvider(file_provider=file_provider_mock)
 
     read_yaml_return_value = {
         "redis": {"date-retrieved": "09-02-02 10:35:48", "versions": ["7.1", "7.2"]},
         "opensearch": {"date-retrieved": "09-02-02 10:35:48", "versions": ["6.1", "6.2"]},
     }
-    # TODO - read_file_as_yaml mocking will fall away as a result of this functionality being delegated to YamlFiles refactor
-    cache_provider._CacheProvider__read_file_as_yaml = MagicMock(
-        return_value=read_yaml_return_value
-    )
+    file_provider_mock.load.return_value = read_yaml_return_value
 
     supported_versions = cache_provider.read_supported_versions_from_cache("opensearch")
 

tests/platform_helper/providers/test_cloudformation.py

@@ -66,6 +66,11 @@ def test_update_conduit_stack_resources(
     stack = boto3.client("cloudformation").describe_stacks(StackName=f"task-{task_name}")
     template_yml = load_yaml(template["TemplateBody"])
 
+    params = []
+    if "Parameters" in template_yml:
+        for param in template_yml["Parameters"]:
+            params.append({"ParameterKey": param, "UsePreviousValue": True})
+
     assert stack["Stacks"][0]["Parameters"][0]["ParameterValue"] == "does-not-matter"
     assert template_yml["Resources"]["LogGroup"]["DeletionPolicy"] == "Retain"
     assert template_yml["Resources"]["TaskNameParameter"]["Properties"]["Name"] == parameter_name
@@ -80,6 +85,9 @@ def test_update_conduit_stack_resources(
         template_yml["Resources"]["SubscriptionFilter"]["Properties"]["FilterName"]
         == f"/copilot/conduit/{mock_application.name}/{env}/{addon_type}/{addon_name}/{task_name.rsplit('-', 1)[1]}/read"
     )
+    assert len(params) == len(
+        template_yml.get("Parameters", [])
+    ), "The number of parameters does not match"
 
 
 @mock_aws

tests/platform_helper/providers/test_yaml_file_provider.py

@@ -40,3 +40,51 @@ def test_raises_exception_if_path_doesnt_exist(fs):
     test_path = "./test_duplicate_keys_path"
     with pytest.raises(FileNotFoundException):
         YamlFileProvider.load(test_path)
+
+
+def test_writes_with_correct_contents(fs):
+    test_path = ".platform-helper-config-cache.yml"
+    test_content = {
+        "redis": {"date-retrieved": "09-02-02 10:35:48", "versions": ["7.1", "7.2"]},
+        "opensearch": {"versions": ["6.1", "6.2"], "date-retrieved": "09-12-24 16:00:00"},
+    }
+    test_comment = "# [!] This file is autogenerated via the platform-helper. Do not edit.\n"
+    expected_test_cache_file = """# [!] This file is autogenerated via the platform-helper. Do not edit.
+opensearch:
+  date-retrieved: 09-12-24 16:00:00
+  versions:
+  - '6.1'
+  - '6.2'
+redis:
+  date-retrieved: 09-02-02 10:35:48
+  versions:
+  - '7.1'
+  - '7.2'
+"""
+
+    YamlFileProvider.write(test_path, test_content, test_comment)
+    with open(test_path, "r") as test_yaml_file:
+        assert expected_test_cache_file in test_yaml_file.read()
+
+
+def test_writes_with_no_comment(fs):
+    test_path = ".platform-helper-config-cache.yml"
+    test_content = {
+        "redis": {"date-retrieved": "09-02-02 10:35:48", "versions": ["7.1", "7.2"]},
+        "opensearch": {"versions": ["6.1", "6.2"], "date-retrieved": "09-12-24 16:00:00"},
+    }
+    expected_test_cache_file = """opensearch:
+  date-retrieved: 09-12-24 16:00:00
+  versions:
+  - '6.1'
+  - '6.2'
+redis:
+  date-retrieved: 09-02-02 10:35:48
+  versions:
+  - '7.1'
+  - '7.2'
+"""
+
+    YamlFileProvider.write(test_path, test_content)
+    with open(test_path, "r") as test_yaml_file:
+        assert expected_test_cache_file in test_yaml_file.read()

tests/platform_helper/utils/test_aws.py

@@ -7,6 +7,14 @@
 
 import boto3
 import botocore
+from dbt_platform_helper.constants import (
+    ALPHANUMERIC_ENVIRONMENT_NAME,
+    ALPHANUMERIC_SERVICE_NAME,
+    CLUSTER_NAME_SUFFIX,
+    HYPHENATED_APPLICATION_NAME,
+    REFRESH_TOKEN_MESSAGE,
+    SERVICE_NAME_SUFFIX,
+)
 import pytest
 from moto import mock_aws
 
@@ -36,16 +44,6 @@
 from tests.platform_helper.conftest import mock_ecr_public_repositories_boto_client
 from tests.platform_helper.conftest import mock_get_caller_identity
 
-HYPHENATED_APPLICATION_NAME = "hyphenated-application-name"
-ALPHANUMERIC_ENVIRONMENT_NAME = "alphanumericenvironmentname123"
-ALPHANUMERIC_SERVICE_NAME = "alphanumericservicename123"
-COPILOT_IDENTIFIER = "c0PIlotiD3ntIF3r"
-CLUSTER_NAME_SUFFIX = f"Cluster-{COPILOT_IDENTIFIER}"
-SERVICE_NAME_SUFFIX = f"Service-{COPILOT_IDENTIFIER}"
-REFRESH_TOKEN_MESSAGE = (
-    "To refresh this SSO session run `aws sso login` with the corresponding profile"
-)
-
 
 def test_get_aws_session_or_abort_profile_not_configured(clear_session_cache, capsys):
     with pytest.raises(SystemExit):