diff --git a/doc/source/exceptions.rst b/doc/source/exceptions.rst index c79dc063..e79e2b57 100644 --- a/doc/source/exceptions.rst +++ b/doc/source/exceptions.rst @@ -45,8 +45,8 @@ Git Exceptions :members: :show-inheritance: :undoc-members: - * - GitDirtyRepoException - - .. autoexception:: pegleg.engine.exceptions.GitDirtyRepoException + * - GitConfigException + - .. autoexception:: pegleg.engine.exceptions.GitConfigException :members: :show-inheritance: :undoc-members: diff --git a/src/bin/pegleg/pegleg/engine/exceptions.py b/src/bin/pegleg/pegleg/engine/exceptions.py index 7c65a2a5..808aa4e2 100644 --- a/src/bin/pegleg/pegleg/engine/exceptions.py +++ b/src/bin/pegleg/pegleg/engine/exceptions.py @@ -51,7 +51,7 @@ class GitAuthException(BaseGitException): self._ssh_key_path = ssh_key_path self._message = ('Failed to authenticate for repo %s with ssh-key at ' - 'path %s.' % (self._repo_url, self._ssh_key_path)) + 'path %s' % (self._repo_url, self._ssh_key_path)) super(GitAuthException, self).__init__(self._message) @@ -62,7 +62,7 @@ class GitProxyException(BaseGitException): def __init__(self, location): self._location = location - self._message = ('Could not resolve proxy [%s].' % self._location) + self._message = ('Could not resolve proxy [%s]' % self._location) super(GitProxyException, self).__init__(self._message) @@ -73,14 +73,12 @@ class GitSSHException(BaseGitException): def __init__(self, ssh_key_path): self._ssh_key_path = ssh_key_path - self._message = ('Failed to find specified SSH key: %s.' % + self._message = ('Failed to find specified SSH key: %s' % (self._ssh_key_path)) super(GitSSHException, self).__init__(self._message) -class GitDirtyRepoException(BaseGitException): - """Exception that occurs when trying to checkout ref from dirty repo.""" - message = ("Failed to checkout ref=%(ref)s from repo_url=%(repo_url)s. " - "Please manually clean all tracked/untracked files from repo " - "before proceeding.") +class GitConfigException(BaseGitException): + """Exception that occurs when reading Git repo config fails.""" + message = ("Failed to read Git config file for repo_url=%(repo_url)s") diff --git a/src/bin/pegleg/pegleg/engine/util/git.py b/src/bin/pegleg/pegleg/engine/util/git.py index 344a190b..b80a9f94 100644 --- a/src/bin/pegleg/pegleg/engine/util/git.py +++ b/src/bin/pegleg/pegleg/engine/util/git.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import atexit import logging import os import tempfile @@ -27,11 +26,7 @@ from pegleg.engine import exceptions LOG = logging.getLogger(__name__) -__all__ = [ - 'git_handler', -] - -__MODIFIED_REPOS = [] +__all__ = ('git_handler', ) def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None): @@ -98,11 +93,15 @@ def git_handler(repo_url, ref=None, proxy_server=None, auth_key=None): repo = Repo(repo_url, search_parent_directories=True) if repo.is_dirty(untracked_files=True): - LOG.error('The locally cloned repo_url=%s is dirty. Manual clean ' - 'up of tracked/untracked files required.', repo_url) - # Raise an exception and force the user to clean up the repo. - # This is the safest approach to avoid data loss/corruption. - raise exceptions.GitDirtyRepoException(ref=ref, repo_url=repo_url) + # NOTE(felipemonteiro): This code should never be executed on a + # real local repository. Wrapper logic around this module will + # only perform this functionality against a temporary replica of + # local repositories, making the below operations safe. + LOG.info('Replica of local repo=%s is dirty. Committing all ' + 'tracked/untracked changes to ref=%s', + repo_name(repo_url), ref) + repo.git.add(all=True) + repo.index.commit('Temporary Pegleg commit') try: # Check whether the ref exists locally. @@ -365,8 +364,8 @@ def repo_name(repo_url_or_path): if config_reader.has_section(section): repo_url = config_reader.get_value(section, option) return repo_url.split('/')[-1].split('.git')[0] - raise click.ClickException( - "Repo=%s is not a valid Git repository" % repo_url_or_path) + + raise exceptions.GitConfigException(repo_url=repo_url_or_path) def normalize_repo_path(repo_path): @@ -403,12 +402,3 @@ def normalize_repo_path(repo_path): "repository" % orig_repo_path) return repo_path, sub_path - - -@atexit.register -def clean_repo(): - global __MODIFIED_REPOS - - for r in __MODIFIED_REPOS: - repo = Repo(r) - repo.head.reset(index=True, working_tree=True) diff --git a/src/bin/pegleg/tests/unit/engine/util/test_git.py b/src/bin/pegleg/tests/unit/engine/util/test_git.py index 80559017..d4b46f05 100644 --- a/src/bin/pegleg/tests/unit/engine/util/test_git.py +++ b/src/bin/pegleg/tests/unit/engine/util/test_git.py @@ -19,6 +19,7 @@ import requests import tempfile import fixtures +from git import Repo import mock import pytest @@ -337,6 +338,61 @@ def test_git_checkout_none_ref_checks_out_master(mock_log): _validate_git_clone(git_dir, 'master') +@pytest.mark.skipif( + not is_connected(), reason='git clone requires network connectivity.') +@mock.patch.object(git, 'LOG', autospec=True) +def test_git_checkout_dirty_repo_tracked_file_committed(mock_log): + """Validate a dirty tracked file is committed.""" + # Clone the openstack-helm repo and automatically checkout patch 73. + ref = 'refs/changes/54/457754/73' + repo_url = 'https://github.com/openstack/openstack-helm' + git_dir = git.git_handler(repo_url, ref) + _validate_git_clone(git_dir, ref) + + # Simulate a dirty repo by removing the first file in it we encounter. + dirty_file = next( + os.path.join(git_dir, x) for x in os.listdir(git_dir) + if os.path.isfile(os.path.join(git_dir, x))) + os.remove(dirty_file) + + # Sanity check that the repo has a dirty tracked file. + repo = Repo(git_dir) + assert repo.is_dirty() + + git.git_handler(git_dir, ref) + + # Validate that the tracked file is committed. + repo = Repo(git_dir) + assert not repo.is_dirty() + + +@pytest.mark.skipif( + not is_connected(), reason='git clone requires network connectivity.') +@mock.patch.object(git, 'LOG', autospec=True) +def test_git_checkout_dirty_repo_untracked_file_committed(mock_log): + """Validate a dirty untracked file is committed.""" + # Clone the openstack-helm repo and automatically checkout patch 73. + ref = 'refs/changes/54/457754/73' + repo_url = 'https://github.com/openstack/openstack-helm' + git_dir = git.git_handler(repo_url, ref) + _validate_git_clone(git_dir, ref) + + # Simulate a dirty repo by adding in a new untracked file. + with open(os.path.join(git_dir, test_utils.rand_name('test_file')), + 'w') as f: + f.write('whatever') + + # Sanity check that the repo has an untracked file. + repo = Repo(git_dir) + assert len(repo.untracked_files) + + git.git_handler(git_dir, ref) + + # Validate that the untracked file is committed. + assert not len(repo.untracked_files) + assert not repo.is_dirty(untracked_files=True) + + @pytest.mark.skipif( not is_connected(), reason='git clone requires network connectivity.') @mock.patch.object(git, 'LOG', autospec=True) @@ -357,51 +413,6 @@ def test_git_clone_existing_directory_raises_exc_for_invalid_ref(mock_log): _assert_check_out_from_local_repo(mock_log, git_dir) -@pytest.mark.skipif( - not is_connected(), reason='git clone requires network connectivity.') -@mock.patch.object(git, 'LOG', autospec=True) -def test_git_checkout_dirty_repo_tracked_file_raises_exception(mock_log): - """Validate Git throws an error when attempting to checkout a ref from - a dirty repo, with modified tracked file. - """ - # Clone the openstack-helm repo and automatically checkout patch 73. - ref = 'refs/changes/54/457754/73' - repo_url = 'https://github.com/openstack/openstack-helm' - git_dir = git.git_handler(repo_url, ref) - _validate_git_clone(git_dir, ref) - - # Simulate a dirty repo by removing the first file in it we encounter. - dirty_file = next( - os.path.join(git_dir, x) for x in os.listdir(git_dir) - if os.path.isfile(os.path.join(git_dir, x))) - os.remove(dirty_file) - - with pytest.raises(exceptions.GitDirtyRepoException): - git.git_handler(git_dir, ref) - - -@pytest.mark.skipif( - not is_connected(), reason='git clone requires network connectivity.') -@mock.patch.object(git, 'LOG', autospec=True) -def test_git_checkout_dirty_repo_untracked_file_raises_exception(mock_log): - """Validate Git throws an error when attempting to checkout a ref from - a dirty repo, with untracked file. - """ - # Clone the openstack-helm repo and automatically checkout patch 73. - ref = 'refs/changes/54/457754/73' - repo_url = 'https://github.com/openstack/openstack-helm' - git_dir = git.git_handler(repo_url, ref) - _validate_git_clone(git_dir, ref) - - # Simulate a dirty repo by adding in a new untracked file. - with open(os.path.join(git_dir, test_utils.rand_name('test_file')), - 'w') as f: - f.write('whatever') - - with pytest.raises(exceptions.GitDirtyRepoException): - git.git_handler(git_dir, ref) - - @pytest.mark.skipif( not is_connected(), reason='git clone requires network connectivity.') def test_git_clone_empty_url_raises_value_error():