From 0585f738fc09f526e9ae25abd80e3b2ed343f9c2 Mon Sep 17 00:00:00 2001 From: Christian Siegel Date: Wed, 10 Jul 2024 19:03:37 +0200 Subject: [PATCH] feat: Pull and rebase before push Resolves #229 If another user pushes changes to the remote repo after gitopscli has cloned it, the subsequent push will fail. Alliviate this problem by pulling and rebasing local changes on top of any remote changes just before pushing.: For the case of creating a new branch (for a PR), the pull is skipped because it would fail due to the missing remote branch. --- Dockerfile | 21 ------ gitopscli/commands/create_preview.py | 1 + gitopscli/commands/delete_preview.py | 1 + gitopscli/commands/deploy.py | 1 + gitopscli/commands/sync_apps.py | 1 + gitopscli/git_api/git_repo.py | 12 ++++ tests/commands/test_create_preview.py | 4 ++ tests/commands/test_delete_preview.py | 2 + tests/commands/test_deploy.py | 8 +++ tests/commands/test_sync_apps.py | 2 + tests/git_api/test_git_repo.py | 98 ++++++++++++++++++++++++++- 11 files changed, 129 insertions(+), 22 deletions(-) diff --git a/Dockerfile b/Dockerfile index d3f827ca..dde8db76 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,27 +21,6 @@ FROM dev AS deps RUN poetry install --only main -# ========= -FROM deps AS test - -RUN poetry install --with test -COPY . . -RUN pip install . -RUN make checks - -# ========= -FROM deps AS docs - -RUN poetry install --with docs -COPY docs ./docs -COPY CONTRIBUTING.md mkdocs.yml ./ -RUN mkdocs build - -# ========= -FROM scratch AS docs-site - -COPY --from=docs /app/site /site - # ========= FROM deps AS install diff --git a/gitopscli/commands/create_preview.py b/gitopscli/commands/create_preview.py index ba88d5fa..3b4c0b2e 100644 --- a/gitopscli/commands/create_preview.py +++ b/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: diff --git a/gitopscli/commands/delete_preview.py b/gitopscli/commands/delete_preview.py index 04be1fd2..4b1deee0 100644 --- a/gitopscli/commands/delete_preview.py +++ b/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 diff --git a/gitopscli/commands/deploy.py b/gitopscli/commands/deploy.py index d72324f9..705717e1 100644 --- a/gitopscli/commands/deploy.py +++ b/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: diff --git a/gitopscli/commands/sync_apps.py b/gitopscli/commands/sync_apps.py index 0bd30c39..5cff0948 100644 --- a/gitopscli/commands/sync_apps.py +++ b/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() diff --git a/gitopscli/git_api/git_repo.py b/gitopscli/git_api/git_repo.py index 6b413416..b3b5f94d 100644 --- a/gitopscli/git_api/git_repo.py +++ b/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) diff --git a/tests/commands/test_create_preview.py b/tests/commands/test_create_preview.py index ac066efc..06c5ad4f 100644 --- a/tests/commands/test_create_preview.py +++ b/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(), ] diff --git a/tests/commands/test_delete_preview.py b/tests/commands/test_delete_preview.py index d4a33083..d263c7e4 100644 --- a/tests/commands/test_delete_preview.py +++ b/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(), ] diff --git a/tests/commands/test_deploy.py b/tests/commands/test_deploy.py index 97d24777..e68d546c 100644 --- a/tests/commands/test_deploy.py +++ b/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(), ] diff --git a/tests/commands/test_sync_apps.py b/tests/commands/test_sync_apps.py index 86835b66..3153fef3 100644 --- a/tests/commands/test_sync_apps.py +++ b/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), diff --git a/tests/git_api/test_git_repo.py b/tests/git_api/test_git_repo.py index 2f3a86bb..ec2d896f 100644 --- a/tests/git_api/test_git_repo.py +++ b/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,102 @@ 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.git.commit("-m", "local commit", "--author", "local ") + + # 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.git.commit("-m", "origin commit", "--author", "origin ") + + # 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.git.commit("-m", "local branch commit", "--author", "local ") + + # 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.git.commit("-m", "origin branch commit", "--author", "origin ") + + # 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: