From 425768ae096c75eaeed787bd5951dacf8dc34494 Mon Sep 17 00:00:00 2001
From: Albin Vass <albin.vass@volvocars.com>
Date: Fri, 12 Feb 2021 10:15:52 +0100
Subject: [PATCH] Fix executor errors on faulty .gitmodules file.

A badly configured .gitmodules committed to a repo can currently break
zuul by not allowing it to git fetch [1]. There is already logic handling
merge conflicts in .gitmodules by resetting the repo but this is not
enough if the current commit is faulty. Instead fix this by trying to
reset to a commit before .gitmodules was introduced.

[1] Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/zuul/executor/server.py", line 2935, in _innerUpdateLoop
    self.merger.updateRepo(
  File "/usr/local/lib/python3.8/site-packages/zuul/merger/merger.py", line 805, in updateRepo
    repo.reset(zuul_event_id=zuul_event_id, build=build,
  File "/usr/local/lib/python3.8/site-packages/zuul/merger/merger.py", line 397, in reset
    self.update(zuul_event_id=zuul_event_id, build=build)
  File "/usr/local/lib/python3.8/site-packages/zuul/merger/merger.py", line 601, in update
    self._git_fetch(repo, 'origin', zuul_event_id, tags=True, prune=True)
  File "/usr/local/lib/python3.8/site-packages/zuul/merger/merger.py", line 265, in _git_fetch
    repo.git.fetch(remote, ref_to_fetch,
  File "/usr/local/lib/python3.8/site-packages/git/cmd.py", line 542, in <lambda>
    return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
  File "/usr/local/lib/python3.8/site-packages/git/cmd.py", line 1006, in _call_process
    return self.execute(call, **exec_kwargs)
  File "/usr/local/lib/python3.8/site-packages/git/cmd.py", line 823, in execute
    raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
  cmdline: git fetch -f --prune --tags origin
  stderr: 'fatal: bad config line 9 in file /var/lib/zuul/executor-git/.../.gitmodules'

Change-Id: I33f8f5905167ebb95ab58f9ef192359573495927
---
 tests/unit/test_merger_repo.py | 18 ++++++++++++++++--
 zuul/merger/merger.py          | 19 +++++++++++++++++--
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py
index e4cc34e5a2..d73def8123 100644
--- a/tests/unit/test_merger_repo.py
+++ b/tests/unit/test_merger_repo.py
@@ -312,7 +312,7 @@ class TestMergerRepo(ZuulTestCase):
                          'none@example.org', 'User Name', '0', '0')
         self.waitUntilSettled()
 
-        # Break the gitmodules
+        # Break the gitmodules with uncommited changes
         path = work_repo.local_path
         with open(os.path.join(path, '.gitmodules'), 'w') as f:
             f.write('[submodule "libfoo"]\n'
@@ -320,7 +320,21 @@ class TestMergerRepo(ZuulTestCase):
                     '---\n'
                     'url = git://example.com/git/lib.git')
 
-        # And now reset the repo again. This should not crash
+        # And now reset the repo. This should not crash
+        work_repo.reset()
+
+        # Break the gitmodules with a commit
+        path = work_repo.local_path
+        with open(os.path.join(path, '.gitmodules'), 'w') as f:
+            f.write('[submodule "libfoo"]\n'
+                    'path = include/foo\n'
+                    '---\n'
+                    'url = git://example.com/git/lib.git')
+        git_repo = work_repo._createRepoObject(work_repo.local_path,
+                                               work_repo.env)
+        git_repo.git.add('.gitmodules')
+        git_repo.index.commit("Broken gitmodule")
+        # And now reset the repo. This should not crash
         work_repo.reset()
 
     def test_files_changes(self):
diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py
index 81114179d0..c1875bd989 100644
--- a/zuul/merger/merger.py
+++ b/zuul/merger/merger.py
@@ -270,11 +270,26 @@ class Repo(object):
                 if attempt < self.retry_attempts:
                     if 'fatal: bad config' in e.stderr.lower():
                         # This error can be introduced by a merge conflict
+                        # or someone committing faulty configuration
                         # in the .gitmodules which was left by the last
                         # merge operation. In this case reset and clean
                         # the repo and try again immediately.
-                        reset_repo_to_head(repo)
-                        repo.git.clean('-x', '-f', '-d')
+                        reset_ref = 'HEAD'
+                        try:
+                            if not repo.is_dirty():
+                                reset_ref = "{}^".format(repo.git.log(
+                                    '--diff-filter=A',
+                                    '-n', '1',
+                                    '--pretty=format:%H',
+                                    '--', '.gitmodules'))
+                            repo.head.reset(reset_ref, working_tree=True)
+                            repo.git.clean('-x', '-f', '-d')
+                        except Exception:
+                            # If we get here there probably isn't
+                            # a valid commit we can easily find so
+                            # delete the repo to make sure it doesn't
+                            # get stuck in a broken state.
+                            shutil.rmtree(self.local_path)
                     elif 'fatal: not a git repository' in e.stderr.lower():
                         # If we get here the git.Repo object was happy with its
                         # lightweight way of checking if this is a valid git