Merge pull request #231 from baloise/feat/pull-rebase

feat: Pull and rebase before push

Author: christiansiegel

gitopscli/commands/create_preview.py

@@ -101,6 +101,7 @@ def __commit_and_push(self, git_repo: GitRepo, message: str) -> None:
             self.__args.git_author_email,
             message,
         )
+        git_repo.pull_rebase()
         git_repo.push()
 
     def __get_gitops_config(self) -> GitOpsConfig:

gitopscli/commands/delete_preview.py

@@ -74,6 +74,7 @@ def __commit_and_push(self, git_repo: GitRepo, message: str) -> None:
             self.__args.git_author_email,
             message,
         )
+        git_repo.pull_rebase()
         git_repo.push()
 
     @staticmethod

gitopscli/commands/deploy.py

@@ -55,6 +55,7 @@ def execute(self) -> None:
                 logging.info("All values already up-to-date. I'm done here.")
                 return
 
+            git_repo.pull_rebase()
             git_repo.push()
 
         if self.__args.create_pr:

gitopscli/commands/sync_apps.py

@@ -103,4 +103,5 @@ def __commit_and_push(
         git_author_email,
         f"{author} updated " + app_file_name,
     )
+    root_config_git_repo.pull_rebase()
     root_config_git_repo.push()

gitopscli/git_api/git_repo.py

@@ -105,6 +105,14 @@ def __validate_git_author(self, name: str | None, email: str | None) -> None:
         if (name and not email) or (not name and email):
             raise GitOpsException("Please provide the name and email address of the Git author or provide neither!")
 
+    def pull_rebase(self) -> None:
+        repo = self.__get_repo()
+        branch = repo.git.branch("--show-current")
+        if not self.__remote_branch_exists(branch):
+            return
+        logging.info("Pull and rebase: %s", branch)
+        repo.git.pull("--rebase")
+
     def push(self, branch: str | None = None) -> None:
         repo = self.__get_repo()
         if not branch:
@@ -122,6 +130,10 @@ def get_author_from_last_commit(self) -> str:
         last_commit = repo.head.commit
         return str(repo.git.show("-s", "--format=%an <%ae>", last_commit.hexsha))
 
+    def __remote_branch_exists(self, branch: str) -> bool:
+        repo = self.__get_repo()
+        return bool(repo.git.ls_remote("--heads", "origin", f"refs/heads/{branch}").strip() != "")
+
     def __delete_tmp_dir(self) -> None:
         if self.__tmp_dir:
             delete_tmp_dir(self.__tmp_dir)

tests/commands/test_create_preview.py

@@ -107,6 +107,7 @@ def git_repo_api_factory_create_mock(_: GitApiConfig, organisation: str, reposit
         self.template_git_repo_mock.get_full_file_path.side_effect = lambda x: f"/tmp/template-repo/{x}"
         self.template_git_repo_mock.clone.return_value = None
         self.template_git_repo_mock.commit.return_value = None
+        self.template_git_repo_mock.pull_rebase.return_value = None
         self.template_git_repo_mock.push.return_value = None
 
         self.target_git_repo_mock = self.create_mock(GitRepo)
@@ -208,6 +209,7 @@ def test_create_new_preview(self):
                 "GIT_AUTHOR_EMAIL",
                 "Create new preview environment for 'my-app' and git hash '3361723dbd91fcfae7b5b8b8b7d462fbc14187a9'.",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 
@@ -312,6 +314,7 @@ def test_create_new_preview_from_same_template_target_repo(self):
                 "GIT_AUTHOR_EMAIL",
                 "Create new preview environment for 'my-app' and git hash '3361723dbd91fcfae7b5b8b8b7d462fbc14187a9'.",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 
@@ -381,6 +384,7 @@ def test_update_existing_preview(self):
                 "GIT_AUTHOR_EMAIL",
                 "Update preview environment for 'my-app' and git hash '3361723dbd91fcfae7b5b8b8b7d462fbc14187a9'.",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 

tests/commands/test_delete_preview.py

@@ -63,6 +63,7 @@ def setUp(self):
         self.git_repo_mock.get_full_file_path.side_effect = lambda x: f"/tmp/created-tmp-dir/{x}"
         self.git_repo_mock.clone.return_value = None
         self.git_repo_mock.commit.return_value = None
+        self.git_repo_mock.pull_rebase.return_value = None
         self.git_repo_mock.push.return_value = None
 
         self.seal_mocks()
@@ -100,6 +101,7 @@ def test_delete_existing_happy_flow(self):
                 "GIT_AUTHOR_EMAIL",
                 "Delete preview environment for 'APP' and preview id 'PREVIEW_ID'.",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 

tests/commands/test_deploy.py

@@ -48,6 +48,7 @@ def setUp(self):
         self.git_repo_mock.new_branch.return_value = None
         self.example_commit_hash = "5f3a443e7ecb3723c1a71b9744e2993c0b6dfc00"
         self.git_repo_mock.commit.return_value = self.example_commit_hash
+        self.git_repo_mock.pull_rebase.return_value = None
         self.git_repo_mock.push.return_value = None
         self.git_repo_mock.get_full_file_path.side_effect = lambda x: f"/tmp/created-tmp-dir/{x}"
 
@@ -101,6 +102,7 @@ def test_happy_flow(self, mock_print):
                 "GIT_AUTHOR_EMAIL",
                 "changed 'a.b.d' to 'bar' in test/file.yml",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 
@@ -142,6 +144,7 @@ def test_create_pr_single_value_change_happy_flow_with_output(self, mock_print):
             call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.c", "foo"),
             call.logging.info("Updated yaml property %s to %s", "a.b.c", "foo"),
             call.GitRepo.commit("GIT_USER", "GIT_EMAIL", None, None, "changed 'a.b.c' to 'foo' in test/file.yml"),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
             call.GitRepoApi.create_pull_request_to_default_branch(
                 "gitopscli-deploy-b973b5bb",
@@ -199,6 +202,7 @@ def test_create_pr_multiple_value_changes_happy_flow_with_output(self, mock_prin
             call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.d", "bar"),
             call.logging.info("Updated yaml property %s to %s", "a.b.d", "bar"),
             call.GitRepo.commit("GIT_USER", "GIT_EMAIL", None, None, "changed 'a.b.d' to 'bar' in test/file.yml"),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
             call.GitRepoApi.create_pull_request_to_default_branch(
                 "gitopscli-deploy-b973b5bb",
@@ -259,6 +263,7 @@ def test_create_pr_and_merge_happy_flow(self, mock_print):
             call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.d", "bar"),
             call.logging.info("Updated yaml property %s to %s", "a.b.d", "bar"),
             call.GitRepo.commit("GIT_USER", "GIT_EMAIL", None, None, "changed 'a.b.d' to 'bar' in test/file.yml"),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
             call.GitRepoApi.create_pull_request_to_default_branch(
                 "gitopscli-deploy-b973b5bb",
@@ -313,6 +318,7 @@ def test_single_commit_happy_flow(self, mock_print):
                 None,
                 "updated 2 values in test/file.yml\n\na.b.c: foo\na.b.d: bar",
             ),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 
@@ -352,6 +358,7 @@ def test_single_commit_single_value_change_happy_flow(self, mock_print):
             call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.c", "foo"),
             call.logging.info("Updated yaml property %s to %s", "a.b.c", "foo"),
             call.GitRepo.commit("GIT_USER", "GIT_EMAIL", None, None, "changed 'a.b.c' to 'foo' in test/file.yml"),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 
@@ -393,6 +400,7 @@ def test_commit_message_multiple_value_changes_happy_flow(self, mock_print):
             call.update_yaml_file("/tmp/created-tmp-dir/test/file.yml", "a.b.d", "bar"),
             call.logging.info("Updated yaml property %s to %s", "a.b.d", "bar"),
             call.GitRepo.commit("GIT_USER", "GIT_EMAIL", None, None, "testcommit"),
+            call.GitRepo.pull_rebase(),
             call.GitRepo.push(),
         ]
 

tests/commands/test_sync_apps.py

@@ -73,6 +73,7 @@ def setUp(self):
         self.root_config_git_repo_mock.get_clone_url.return_value = "https://repository.url/root/root-config.git"
         self.root_config_git_repo_mock.clone.return_value = None
         self.root_config_git_repo_mock.commit.return_value = None
+        self.root_config_git_repo_mock.pull_rebase.return_value = None
         self.root_config_git_repo_mock.push.return_value = None
 
         self.git_repo_api_factory_mock = self.monkey_patch(GitRepoApiFactory)
@@ -166,6 +167,7 @@ def test_sync_apps_happy_flow(self):
                 "GIT_AUTHOR_EMAIL",
                 "author updated /tmp/root-config-repo/apps/team-non-prod.yaml",
             ),
+            call.GitRepo_root.pull_rebase(),
             call.GitRepo_root.push(),
             call.GitRepo_root.__exit__(None, None, None),
             call.GitRepo_team.__exit__(None, None, None),

tests/git_api/test_git_repo.py

@@ -49,7 +49,7 @@ def __create_origin(self):
         with Path(f"{repo_dir}/README.md").open("w") as readme:
             readme.write("xyz branch readme")
         repo.git.add("--all")
-        repo.git.commit("-m", "xyz brach commit", "--author", f"{git_user} <{git_email}>")
+        repo.git.commit("-m", "initial xyz branch commit", "--author", f"{git_user} <{git_email}>")
 
         repo.git.checkout("master")  # master = default branch
         repo.git.config("receive.denyCurrentBranch", "ignore")
@@ -313,6 +313,106 @@ def test_commit_nothing_to_commit(self, logging_mock):
             self.assertEqual("initial commit\n", commits[0].message)
         logging_mock.assert_not_called()
 
+    @patch("gitopscli.git_api.git_repo.logging")
+    def test_pull_rebase_master_single_commit(self, logging_mock):
+        origin_repo = self.__origin
+        with GitRepo(self.__mock_repo_api) as testee:
+            testee.clone()
+
+            # local commit
+            with Path(testee.get_full_file_path("local.md")).open("w") as outfile:
+                outfile.write("local file")
+            local_repo = Repo(testee.get_full_file_path("."))
+            local_repo.git.add("--all")
+            local_repo.config_writer().set_value("user", "email", "[email protected]").release()
+            local_repo.git.commit("-m", "local commit")
+
+            # origin commit
+            with Path(f"{origin_repo.working_dir}/origin.md").open("w") as readme:
+                readme.write("origin file")
+            origin_repo.git.add("--all")
+            origin_repo.config_writer().set_value("user", "email", "[email protected]").release()
+            origin_repo.git.commit("-m", "origin commit")
+
+            # pull and rebase from remote
+            logging_mock.reset_mock()
+
+            testee.pull_rebase()
+
+            logging_mock.info.assert_called_once_with("Pull and rebase: %s", "master")
+
+            # then push should work
+            testee.push()
+
+            commits = list(self.__origin.iter_commits("master"))
+            self.assertEqual(3, len(commits))
+            self.assertEqual("initial commit\n", commits[2].message)
+            self.assertEqual("origin commit\n", commits[1].message)
+            self.assertEqual("local commit\n", commits[0].message)
+
+    @patch("gitopscli.git_api.git_repo.logging")
+    def test_pull_rebase_remote_branch_single_commit(self, logging_mock):
+        origin_repo = self.__origin
+        origin_repo.git.checkout("xyz")
+        with GitRepo(self.__mock_repo_api) as testee:
+            testee.clone(branch="xyz")
+
+            # local commit
+            with Path(testee.get_full_file_path("local.md")).open("w") as outfile:
+                outfile.write("local file")
+            local_repo = Repo(testee.get_full_file_path("."))
+            local_repo.git.add("--all")
+            local_repo.config_writer().set_value("user", "email", "[email protected]").release()
+            local_repo.git.commit("-m", "local branch commit")
+
+            # origin commit
+            with Path(f"{origin_repo.working_dir}/origin.md").open("w") as readme:
+                readme.write("origin file")
+            origin_repo.git.add("--all")
+            origin_repo.config_writer().set_value("user", "email", "[email protected]").release()
+            origin_repo.git.commit("-m", "origin branch commit")
+
+            # pull and rebase from remote
+            logging_mock.reset_mock()
+
+            testee.pull_rebase()
+
+            logging_mock.info.assert_called_once_with("Pull and rebase: %s", "xyz")
+
+            # then push should work
+            testee.push()
+
+            commits = list(self.__origin.iter_commits("xyz"))
+            self.assertEqual(4, len(commits))
+            self.assertEqual("local branch commit\n", commits[0].message)
+            self.assertEqual("origin branch commit\n", commits[1].message)
+            self.assertEqual("initial xyz branch commit\n", commits[2].message)
+
+    @patch("gitopscli.git_api.git_repo.logging")
+    def test_pull_rebase_without_new_commits(self, logging_mock):
+        with GitRepo(self.__mock_repo_api) as testee:
+            testee.clone()
+
+            # pull and rebase from remote
+            logging_mock.reset_mock()
+
+            testee.pull_rebase()
+
+            logging_mock.info.assert_called_once_with("Pull and rebase: %s", "master")
+
+    @patch("gitopscli.git_api.git_repo.logging")
+    def test_pull_rebase_if_no_remote_branch_is_noop(self, logging_mock):
+        with GitRepo(self.__mock_repo_api) as testee:
+            testee.clone()
+            testee.new_branch("new-branch-only-local")
+
+            # pull and rebase from remote
+            logging_mock.reset_mock()
+
+            testee.pull_rebase()
+
+            logging_mock.assert_not_called()
+
     @patch("gitopscli.git_api.git_repo.logging")
     def test_push(self, logging_mock):
         with GitRepo(self.__mock_repo_api) as testee: