APPSRE-11390 vcs more fetch options (#4865)

Author: fishi0x01

reconcile/aws_account_manager/merge_request_manager.py

@@ -85,7 +85,7 @@ def create_account_file(
             return None
 
         try:
-            self._vcs.get_file_content_from_app_interface_master(
+            self._vcs.get_file_content_from_app_interface_ref(
                 file_path=account_tmpl_file_path
             )
             # File already exists. nothing to do.

reconcile/aws_version_sync/merge_request_manager/merge_request_manager.py

@@ -116,7 +116,7 @@ def create_merge_request(
             return None
 
         try:
-            content = self._vcs.get_file_content_from_app_interface_master(
+            content = self._vcs.get_file_content_from_app_interface_ref(
                 file_path=namespace_file
             )
         except GitlabGetError as e:

reconcile/endpoints_discovery/merge_request_manager.py

@@ -149,7 +149,7 @@ def create_merge_request(self, apps: Sequence[App]) -> None:
         )
         for app in apps:
             try:
-                content = self._vcs.get_file_content_from_app_interface_master(
+                content = self._vcs.get_file_content_from_app_interface_ref(
                     file_path=app.path
                 )
             except GitlabGetError as e:

reconcile/saas_auto_promotions_manager/merge_request_manager/merge_request_manager_v2.py

@@ -83,7 +83,7 @@ def _render_mr(self, addition: Addition) -> None:
             if sub.target_file_path not in content_by_path:
                 try:
                     content_by_path[sub.target_file_path] = (
-                        self._vcs.get_file_content_from_app_interface_master(
+                        self._vcs.get_file_content_from_app_interface_ref(
                             file_path=sub.target_file_path
                         )
                     )

reconcile/terraform_init/merge_request_manager.py

@@ -78,7 +78,7 @@ def create_merge_request(self, data: MrData) -> None:
             return None
 
         try:
-            self._vcs.get_file_content_from_app_interface_master(file_path=data.path)
+            self._vcs.get_file_content_from_app_interface_ref(file_path=data.path)
             # the file exists, nothing to do
             return None
         except GitlabGetError as e:

reconcile/terraform_vpc_resources/merge_request_manager.py

@@ -83,7 +83,7 @@ def create_merge_request(self, data: MrData) -> None:
             return None
 
         try:
-            self._vcs.get_file_content_from_app_interface_master(file_path=data.path)
+            self._vcs.get_file_content_from_app_interface_ref(file_path=data.path)
             # the file exists, nothing to do
             return None
         except GitlabGetError as e:

reconcile/test/aws_version_sync/merge_request_manager/test_avs_merge_request_manager.py

@@ -105,7 +105,7 @@ def test_merge_request_manager_create_avs_merge_request_renderer_called(
     renderer_mock.render_merge_request_content.return_value = "new-content"
     renderer_mock.render_description.return_value = "render_description"
     renderer_mock.render_title.return_value = "title"
-    vcs_mock.get_file_content_from_app_interface_master.return_value = (
+    vcs_mock.get_file_content_from_app_interface_ref.return_value = (
         "namespace-file-content"
     )
 

reconcile/test/endpoints_discovery/test_endpoints_discovery_merge_request_manager.py

@@ -201,7 +201,7 @@ def test_endpoints_discovery_merge_request_manager_create_merge_request_app_file
 ) -> None:
     mrm, vcs_mock, _ = mrm_builder()
     # file does not exist in the repo
-    vcs_mock.get_file_content_from_app_interface_master.side_effect = GitlabGetError(
+    vcs_mock.get_file_content_from_app_interface_ref.side_effect = GitlabGetError(
         response_code=404
     )
 
@@ -222,7 +222,7 @@ def test_endpoints_discovery_merge_request_manager_create_merge_request_open_mr(
     render_mock.render_title.return_value = "title"
     render_mock.render_description.return_value = "description"
     render_mock.render_merge_request_content.return_value = "content"
-    vcs_mock.get_file_content_from_app_interface_master.return_value = "content"
+    vcs_mock.get_file_content_from_app_interface_ref.return_value = "content"
 
     mrm.create_merge_request(apps=[app])
     vcs_mock.open_app_interface_merge_request.assert_called_once_with(ANY)

reconcile/test/templating/test_renderer.py

@@ -402,7 +402,7 @@ def test_crg_file_persistence_read_found(
 def test_crg_file_persistence_read_miss(
     vcs: MagicMock, mr_manager: MagicMock, tmp_path: Path
 ) -> None:
-    vcs.get_file_content_from_app_interface_master.side_effect = GitlabGetError()
+    vcs.get_file_content_from_app_interface_ref.side_effect = GitlabGetError()
     crg = ClonedRepoGitlabPersistence(False, str(tmp_path), vcs, mr_manager)
     assert crg.read("foo") is None
 

reconcile/test/terraform_init/test_terraform_init_merge_request_manager.py

@@ -89,7 +89,7 @@ def test_merge_request_manager_create_avs_merge_request_renderer_called(
 ) -> None:
     mrm, vcs_mock, renderer_mock, _ = mrm_builder()
     # file does not exist in the repo
-    vcs_mock.get_file_content_from_app_interface_master.side_effect = GitlabGetError(
+    vcs_mock.get_file_content_from_app_interface_ref.side_effect = GitlabGetError(
         response_code=404
     )
 
@@ -108,7 +108,7 @@ def test_merge_request_manager_create_avs_merge_request_renderer_called_template
 ) -> None:
     mrm, vcs_mock, _, _ = mrm_builder()
     # file does not exist in the repo
-    vcs_mock.get_file_content_from_app_interface_master.return_value = "content"
+    vcs_mock.get_file_content_from_app_interface_ref.return_value = "content"
 
     mrm.create_merge_request(MrData(account="account", content="content", path="/path"))
     vcs_mock.open_app_interface_merge_request.assert_not_called()

reconcile/test/utils/vcs/conftest.py

@@ -7,6 +7,7 @@
 
 import pytest
 from github import Commit
+from gitlab.v4.objects import Project, ProjectFileManager
 
 from reconcile.utils.github_api import GithubRepositoryApi
 from reconcile.utils.gitlab_api import GitLabApi
@@ -27,6 +28,9 @@ def builder(data: Mapping) -> GitLabApi:
             for sha in data.get("COMMITS", [])
         ]
         gitlab_api.repository_compare.side_effect = [commits]
+        gitlab_api.project = create_autospec(spec=Project)
+        gitlab_api.project.files = create_autospec(spec=ProjectFileManager)
+
         return gitlab_api
 
     return builder

reconcile/test/utils/vcs/test_vcs.py

@@ -109,3 +109,29 @@ def test_close_mr_error(vcs_builder: Callable[[Mapping], VCS]) -> None:
         vcs.close_app_interface_mr(mr=mr, comment="test")
     vcs._app_interface_api.close.assert_not_called()  # type: ignore[attr-defined]
     vcs._app_interface_api.delete_branch.assert_not_called()  # type: ignore[attr-defined]
+
+
+def test_get_file_content_from_app_interface_ref_defaults(
+    vcs_builder: Callable[[Mapping], VCS],
+) -> None:
+    vcs = vcs_builder({})
+    vcs.get_file_content_from_app_interface_ref(file_path="/file.yaml")
+
+    vcs._app_interface_api.project.files.get.assert_called_once_with(  # type: ignore[attr-defined]
+        file_path="data/file.yaml",
+        ref="master",
+    )
+
+
+def test_get_file_content_from_app_interface_ref_overrides(
+    vcs_builder: Callable[[Mapping], VCS],
+) -> None:
+    vcs = vcs_builder({})
+    vcs.get_file_content_from_app_interface_ref(
+        file_path="/file.yaml", is_data=False, ref="ref"
+    )
+
+    vcs._app_interface_api.project.files.get.assert_called_once_with(  # type: ignore[attr-defined]
+        file_path="/file.yaml",
+        ref="ref",
+    )

reconcile/utils/vcs.py

@@ -216,14 +216,17 @@ def close_app_interface_mr(self, mr: ProjectMergeRequest, comment: str) -> None:
             self._app_interface_api.close(mr)
             self._app_interface_api.delete_branch(source_branch)
 
-    def get_file_content_from_app_interface_master(self, file_path: str) -> str:
-        file_path = (
-            f"data/{file_path.lstrip('/')}"
-            if not file_path.startswith("data")
-            else file_path
-        )
+    def get_file_content_from_app_interface_ref(
+        self, file_path: str, ref: str = "master", is_data: bool = True
+    ) -> str:
+        if is_data:
+            file_path = (
+                f"data/{file_path.lstrip('/')}"
+                if not file_path.startswith("data")
+                else file_path
+            )
         return (
-            self._app_interface_api.project.files.get(file_path=file_path, ref="master")
+            self._app_interface_api.project.files.get(file_path=file_path, ref=ref)
             .decode()
             .decode("utf-8")
         )