Skip to content

Commit

Permalink
feat: Pull and rebase before push
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
christiansiegel committed Jul 10, 2024
1 parent 458dca6 commit 0585f73
Show file tree
Hide file tree
Showing 11 changed files with 129 additions and 22 deletions.
21 changes: 0 additions & 21 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions gitopscli/commands/create_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions gitopscli/commands/delete_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions gitopscli/commands/deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions gitopscli/commands/sync_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
12 changes: 12 additions & 0 deletions gitopscli/git_api/git_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions tests/commands/test_create_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
]

Expand Down Expand Up @@ -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(),
]

Expand Down Expand Up @@ -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(),
]

Expand Down
2 changes: 2 additions & 0 deletions tests/commands/test_delete_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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(),
]

Expand Down
8 changes: 8 additions & 0 deletions tests/commands/test_deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"

Expand Down Expand Up @@ -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(),
]

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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(),
]

Expand Down Expand Up @@ -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(),
]

Expand Down Expand Up @@ -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(),
]

Expand Down
2 changes: 2 additions & 0 deletions tests/commands/test_sync_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand Down
98 changes: 97 additions & 1 deletion tests/git_api/test_git_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 <[email protected]>")

# 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 <[email protected]>")

# 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 <[email protected]>")

# 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 <[email protected]>")

# 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:
Expand Down

0 comments on commit 0585f73

Please sign in to comment.