From 0fd86e3550d1de99e0123c9c3c427859141319e0 Mon Sep 17 00:00:00 2001 From: Jonathan Nevelson Date: Tue, 23 Jun 2020 20:47:19 +0000 Subject: [PATCH] Fork repo (#24) * Repo forking * Only fork when dry_run=False * Bump version + changelog * Use retry lib --- CHANGELOG.md | 4 +++ gordian/gordian.py | 5 +-- gordian/repo.py | 68 +++++++++++++++++++++++------------ setup.py | 4 +-- tests/test_base_file.py | 2 +- tests/test_changelog_file.py | 2 +- tests/test_gordian.py | 30 +++++++--------- tests/test_repo.py | 41 +++++++++++++-------- tests/test_transformations.py | 14 +++++--- 9 files changed, 103 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27c7b49..d7cc573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [2.0.0] - 2020-06-23 +### Added +- Gordian now forks the repo instead of creating a branch in the specified repo so that users can run Gordian against repos that they do not have write permissions on. + ## [1.5.0] - 2020-06-23 ### Added - Added the ability to set labels to sent pull requests using a flag. diff --git a/gordian/gordian.py b/gordian/gordian.py index 53999ef..da6a777 100644 --- a/gordian/gordian.py +++ b/gordian/gordian.py @@ -140,7 +140,6 @@ def create_parser(args): raise argparse.ArgumentTypeError('Number of search and replace arguments must be the same!') return args - def apply_transformations(args, transformations, pr_created_callback=None): config = Config(args.config_file) transform(args, transformations, config.get_data(), pr_created_callback=pr_created_callback) @@ -156,8 +155,7 @@ def transform(args, transformations, repositories, pr_created_callback): repo.bump_version(args.dry_run) if not args.dry_run: try: - pull_request = repo._repo.create_pull(args.pr_message, '', args.target_branch, repo.branch_name) - pull_request.set_labels(*args.pr_labels) + pull_request = repo.create_pr(args.pr_message, '', args.target_branch, args.pr_labels) pull_request_urls.append(pull_request.html_url) if pr_created_callback is not None: logger.debug(f'Calling post pr created callback with: {pull_request}, {repo.branch_name}') @@ -175,6 +173,5 @@ def main(): args = create_parser(sys.argv[1:]) apply_transformations(args, [SearchAndReplace]) - if __name__ == '__main__': main() diff --git a/gordian/repo.py b/gordian/repo.py index 46479e6..9d28401 100644 --- a/gordian/repo.py +++ b/gordian/repo.py @@ -3,6 +3,8 @@ import datetime import logging import os +import time +from retry import retry from gordian.files import * logger = logging.getLogger(__name__) @@ -12,28 +14,29 @@ class Repo: - def __init__(self, repo_name, github_api_url=None, branch=None, git=None, files=None, semver_label=None, target_branch='master'): + def __init__(self, repo_name, github_api_url=None, branch=None, github=None, files=None, semver_label=None, target_branch='master'): if github_api_url is None: self.github_api_url = BASE_URL else: self.github_api_url = github_api_url - logger.debug(f'Github api url: {self.github_api_url}') - if git is None: + if github is not None: + self._github = github + else: if "GIT_TOKEN" in os.environ: logger.debug('Using git token') - git = Github(base_url=self.github_api_url, login_or_token=os.environ['GIT_TOKEN']) + self._github = Github(base_url=self.github_api_url, login_or_token=os.environ['GIT_TOKEN']) else: logger.debug('Using git username and password') - git = Github(base_url=self.github_api_url, login_or_token=os.environ['GIT_USERNAME'], password=os.environ['GIT_PASSWORD']) + self._github = Github(base_url=self.github_api_url, login_or_token=os.environ['GIT_USERNAME'], password=os.environ['GIT_PASSWORD']) if files is None: files = [] + self.files = files self.repo_name = repo_name - logger.debug(f'Repo name: {self.repo_name}') - self._repo = git.get_repo(repo_name) - self.files = files + self._original_repo = self._github.get_repo(repo_name) + self.version_file = None self.changelog_file = None self.branch_exists = False @@ -43,11 +46,14 @@ def __init__(self, repo_name, github_api_url=None, branch=None, git=None, files= self.target_branch = None self.set_target_branch(target_branch) - logger.debug(f'Target ref: {target_branch}') if branch: self.branch_name = f"refs/heads/{branch}" else: self.branch_name = f"refs/heads/{datetime.datetime.now().strftime('%Y-%m-%d-%H%M%S.%f')}" + + logger.debug(f'Github api url: {self.github_api_url}') + logger.debug(f'Repo name: {self.repo_name}') + logger.debug(f'Target ref: {self.target_branch}') logger.debug(f'Branch name for this changes: {self.branch_name}') def set_target_branch(self, branch): @@ -83,7 +89,7 @@ def get_objects(self, filename, klass=None): def get_files(self): if not self.files: logger.debug(f'Getting repo content') - contents = self._repo.get_contents('', self.target_ref) + contents = self._original_repo.get_contents('', self.target_ref) while contents: file = contents.pop(0) if file.path == 'version': @@ -91,7 +97,7 @@ def get_files(self): elif file.path == 'CHANGELOG.md': self.changelog = ChangelogFile(file, self) elif file.type == 'dir': - contents.extend(self._repo.get_contents(file.path, self.target_ref)) + contents.extend(self._original_repo.get_contents(file.path, self.target_ref)) else: self.files.append(file) @@ -104,15 +110,23 @@ def find_file(self, filename): if file.path == filename: return file - def make_branch(self): - sb = self._repo.get_branch(self.target_branch) + def _make_branch(self): + logger.info('Forking repo...') + self._forked_repo = self._original_repo.create_fork() + branch = self._get_branch() + logger.debug(f'Creating branch {self.branch_name}') + try: - logger.debug(f'Creating branch {self.branch_name}') - ref = self._repo.create_git_ref(ref=self.branch_name, sha=sb.commit.sha) + ref = self._forked_repo.create_git_ref(ref=self.branch_name, sha=branch.commit.sha) except GithubException as e: - print(f"Branch {self.branch_name} already exists in github") + logger.debug(f'Branch {self.branch_name} already exists in github') self.branch_exists = True + @retry(GithubException, tries=3, delay=1, backoff=2) + def _get_branch(self): + logger.debug(f'Fetching branch {self.target_branch}...') + return self._forked_repo.get_branch(self.target_branch) + def bump_version(self, dry_run=False): if self.new_version is None: return @@ -133,10 +147,10 @@ def update_file(self, repo_file, content, message, dry_run=False): return if not self.branch_exists: - self.make_branch() + self._make_branch() logger.debug(f'Updating file {repo_file.path}') - self._repo.update_file( + self._forked_repo.update_file( repo_file.path, message, content, @@ -152,10 +166,10 @@ def create_file(self, path, contents, message, dry_run=False): return if not self.branch_exists: - self.make_branch() + self._make_branch() logger.debug(f'Creating file {path}') - self._repo.create_file( + self._forked_repo.create_file( path, message, contents, @@ -170,16 +184,26 @@ def delete_file(self, file, message, dry_run=False): return if not self.branch_exists: - self.make_branch() + self._make_branch() logger.debug(f'Deleting file {file.path}') - self._repo.delete_file( + self._forked_repo.delete_file( file.path, message, file.sha, branch=self.branch_name ) + def create_pr(self, pr_message, pr_body, target_branch, labels): + pr = self._original_repo.create_pull( + pr_message, + pr_body, + target_branch, + f'{self._forked_repo.owner.login}:{self.branch_name}' + ) + pr.set_labels(*labels) + return pr + def _get_new_version(self): if self.semver_label is None: return diff --git a/setup.py b/setup.py index 5f7961b..1bbc98a 100644 --- a/setup.py +++ b/setup.py @@ -3,12 +3,12 @@ setup_reqs = ['pytest', 'pytest-cov', 'pytest-runner', 'flake8'] setuptools.setup( name="gordian", - version="1.5.0", + version="2.0.0", author="Intuit", author_email="cg-sre@intuit.com", description="A tool to search and replace files in a Git repo", url="https://github.com/argoproj-labs/gordian", - install_requires=['pygithub', 'pyyaml', 'jsonpatch', 'deepdiff'], + install_requires=['pygithub', 'pyyaml', 'jsonpatch', 'deepdiff', 'retry'], setup_requires=setup_reqs, extras_require={ 'test': setup_reqs diff --git a/tests/test_base_file.py b/tests/test_base_file.py index 3e16753..69b8d5f 100644 --- a/tests/test_base_file.py +++ b/tests/test_base_file.py @@ -9,7 +9,7 @@ class TestBaseFile(unittest.TestCase): def setUp(self): self.github_file = Utils.create_github_content_file() self.mock_git = MagicMock() - self.repo = Repo('test', git=self.mock_git) + self.repo = Repo('test', github=self.mock_git) self.base_file = YamlFile(self.github_file, self.repo) def test_iterable(self): diff --git a/tests/test_changelog_file.py b/tests/test_changelog_file.py index 6c213b0..bd376b9 100644 --- a/tests/test_changelog_file.py +++ b/tests/test_changelog_file.py @@ -10,7 +10,7 @@ class TestBaseFile(unittest.TestCase): def setUp(self): self.github_file = Utils.create_github_content_file(file='changelog_no_footer.md') self.mock_git = MagicMock() - self.repo = Repo('test', git=self.mock_git) + self.repo = Repo('test', github=self.mock_git) self.repo.new_version = '1.2.0' self.changelog = ChangelogFile(self.github_file, self.repo) diff --git a/tests/test_gordian.py b/tests/test_gordian.py index 10a0479..2327b81 100644 --- a/tests/test_gordian.py +++ b/tests/test_gordian.py @@ -28,45 +28,43 @@ def test_apply_transformations_without_changes(self): RepoMock.assert_has_calls([ call('testOrg/TestService1', github_api_url=None, branch='test', semver_label=None, target_branch='master'), call('testOrg/TestService2', github_api_url=None, branch='test', semver_label=None, target_branch='master') - ]) + ]) def test_apply_transformations_with_changes(self): - with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation', ) as TransformationMockClass: + with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation') as TransformationMockClass: instance = RepoMock.return_value instance.dirty = True apply_transformations(TestGordian.Args(), [TransformationMockClass]) RepoMock.assert_has_calls([call().bump_version(False), call().bump_version(False)], any_order=True) - RepoMock.assert_has_calls([call()._repo.create_pull('test', '', 'master', ANY), call()._repo.create_pull('test', '', 'master', ANY)], any_order=True) - RepoMock.assert_has_calls([call()._repo.create_pull().set_labels('test'), call()._repo.create_pull().set_labels('test')], any_order=True) + RepoMock.assert_has_calls([call().create_pr('test', '', 'master', ANY), call().create_pr('test', '', 'master', ANY)], any_order=True) def test_apply_transformations_with_changes_dry_run(self): - with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation', ) as TransformationMockClass: + with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation') as TransformationMockClass: instance = RepoMock.return_value instance.dirty = True apply_transformations(TestGordian.Args(dry_run=True), [TransformationMockClass]) RepoMock.assert_has_calls([call().bump_version(True), call().bump_version(True)], any_order=True) - self.assertNotIn(call().repo.create_pull('test', '', 'master', ANY), RepoMock.mock_calls) - self.assertNotIn(call()._repo.create_pull().set_labels('test'), RepoMock.mock_calls) + self.assertNotIn(call().repo.create_pr('test', '', 'master', ANY), RepoMock.mock_calls) def test_apply_transformations_with_changes_and_callback(self): - with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation', ) as TransformationMockClass: + with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation') as TransformationMockClass: instance = RepoMock.return_value instance.dirty = True callback_mock = MagicMock() args = TestGordian.Args() args.target_branch = 'target_branch' apply_transformations(args, [TransformationMockClass], callback_mock) - pull_request = RepoMock.return_value._repo.create_pull.return_value + pull_request = RepoMock.return_value.create_pr.return_value RepoMock.assert_has_calls([call().bump_version(False), call().bump_version(False)], any_order=True) - RepoMock.assert_has_calls([call()._repo.create_pull().set_labels('test'), call()._repo.create_pull().set_labels('test')], any_order=True) RepoMock.assert_has_calls([ - call()._repo.create_pull('test', '', 'target_branch', ANY), - call()._repo.create_pull('test', '', 'target_branch', ANY)], - any_order=True) + call().create_pr('test', '', 'target_branch', ANY), + call().create_pr('test', '', 'target_branch', ANY)], + any_order=True + ) callback_mock.assert_has_calls([ call('testOrg/TestService1', pull_request), call('testOrg/TestService2', pull_request)] - ) + ) def test_apply_transformations_with_changes_default_labels(self): with patch('gordian.gordian.Repo') as RepoMock, patch('gordian.transformations.Transformation', ) as TransformationMockClass: @@ -76,6 +74,4 @@ def test_apply_transformations_with_changes_default_labels(self): gordian_args.pr_labels = [] apply_transformations(gordian_args, [TransformationMockClass]) RepoMock.assert_has_calls([call().bump_version(False), call().bump_version(False)], any_order=True) - RepoMock.assert_has_calls([call()._repo.create_pull('test', '', 'master', ANY), call()._repo.create_pull('test', '', 'master', ANY)], any_order=True) - self.assertNotIn(call()._repo.create_pull().set_labels(ANY), RepoMock.mock_calls) - + RepoMock.assert_has_calls([call().create_pr('test', '', 'master', ANY), call().create_pr('test', '', 'master', ANY)], any_order=True) diff --git a/tests/test_repo.py b/tests/test_repo.py index a98335a..3b7cd69 100644 --- a/tests/test_repo.py +++ b/tests/test_repo.py @@ -12,28 +12,28 @@ def setUp(self, mock_git): self.mock_git = MagicMock() self.mock_repo = MagicMock() self.mock_branches = MagicMock() - self.repo = Repo('test', git=self.mock_git) + self.repo = Repo('test', github=self.mock_git) self.repo.files.append(Utils.create_github_content_file()) self.mock_repo.get_branches.return_value = self.mock_branches - mock_git.get_repo.return_value = self.mock_repo - self.instance = Repo(None, branch='', git=mock_git) + self.mock_git.get_repo.return_value = self.mock_repo def test_make_branch(self): - self.instance.branch_exists = False + repo = Repo(None, branch='', github=self.mock_git) + repo.branch_exists = False mock_branch = MagicMock() self.mock_repo.get_branch.return_value = mock_branch mock_branch.commit.sha = "5e69ff00a3be0a76b13356c6ff42af79ff469ef3" - self.instance.make_branch() - self.assertTrue(self.instance.branch_exists) - self.mock_repo.get_branch.assert_called_once_with('master') - self.mock_repo.create_git_ref.assert_called_once() + repo._make_branch() + self.assertTrue(repo.branch_exists) + repo._forked_repo.get_branch.assert_called_once_with('master') + repo._forked_repo.create_git_ref.assert_called_once() def test_default_github_url(self): self.assertEqual(self.repo.github_api_url, 'https://api.github.com') def test_override_github_url(self): - repo = Repo('test', github_api_url='https://test.github.com', git=self.mock_git) + repo = Repo('test', github_api_url='https://test.github.com', github=self.mock_git) self.assertEqual(repo.github_api_url, 'https://test.github.com') def test_get_object_does_not_exist(self): @@ -43,20 +43,20 @@ def test_get_object_does_not_exist(self): def test_get_existing_object(self): contents = self.repo.get_objects('/content.yaml') assert(isinstance(contents, YamlFile)) - + def test_new_files_object(self): self.assertEquals(len(self.repo.files), 1) - repo_two = Repo('test_two', github_api_url='https://test.github.com', git=self.mock_git) + repo_two = Repo('test_two', github_api_url='https://test.github.com', github=MagicMock()) self.assertEquals(len(repo_two.files), 0) - + def test_get_files(self): self.repo.set_target_branch('target') self.repo.files = [] - self.repo._repo = MagicMock() + self.repo._original_repo = MagicMock() repository_file = MagicMock(path='afile.txt', type='not_dir') - self.repo._repo.get_contents.side_effect = [[MagicMock(path='directory', type='dir')],[repository_file]] + self.repo._original_repo.get_contents.side_effect = [[MagicMock(path='directory', type='dir')],[repository_file]] self.repo.get_files() - self.repo._repo.get_contents.assert_has_calls([call('', 'refs/heads/target'), call('directory', 'refs/heads/target')]) + self.repo._original_repo.get_contents.assert_has_calls([call('', 'refs/heads/target'), call('directory', 'refs/heads/target')]) self.assertEquals(self.repo.files, [repository_file]) def test_set_target_branch(self): @@ -69,3 +69,14 @@ def test_set_target_branch(self): self.assertEqual(self.repo.files, []) self.assertEqual(self.repo.target_branch, 'Something different') self.assertEqual(self.repo.target_ref, 'refs/heads/Something different') + + def test_create_pr(self): + repo = Repo(None, branch='', github=self.mock_git) + repo._original_repo = MagicMock() + repo._forked_repo = MagicMock() + repo._forked_repo.owner.login = 'someone' + repo.branch_name = 'branch' + pr = repo.create_pr('test', '', 'target_branch', ['test']) + repo._original_repo.create_pull.assert_called_once_with('test', '', 'target_branch', 'someone:branch') + pr.set_labels.assert_called_once_with('test') + repo._forked_repo.create_pull.assert_not_called() diff --git a/tests/test_transformations.py b/tests/test_transformations.py index a1ecf9b..6f3900f 100644 --- a/tests/test_transformations.py +++ b/tests/test_transformations.py @@ -19,7 +19,7 @@ def setUp(self, mock_git): self.mock_repo.get_branches.return_value = self.mock_branches mock_git.get_repo.return_value = self.mock_repo self.mock_repo.get_contents.return_value = [] - self.instance = Repo(None, branch='', git=mock_git, files=[]) + self.instance = Repo(None, branch='', github=mock_git, files=[]) self.instance.branch_exists = False f = open('./tests/fixtures/content.yaml', 'r') contents = f.read() @@ -30,8 +30,8 @@ def setUp(self, mock_git): def test_both_false_update_files(self): assert(self.sandr.run()) - self.mock_repo.create_git_ref.assert_called_once() - self.mock_repo.update_file.assert_called_once() + self.instance._forked_repo.create_git_ref.assert_called_once() + self.instance._forked_repo.update_file.assert_called_once() def test_is_word_found(self): f = open('./tests/fixtures/content.yaml', 'r') @@ -40,9 +40,11 @@ def test_is_word_found(self): self.assertFalse(self.sandr.is_word_found(contents, 'hello')) def test_pr_false_update_files(self): + forked_repo = MagicMock() + self.instance._forked_repo = forked_repo self.instance.branch_exists = True assert(self.sandr.run()) - self.mock_repo.update_file.assert_called_once() + forked_repo.update_file.assert_called_once() def test_update_files_none(self): self.sandr.changesets = (('hello', 'iam'),) @@ -51,7 +53,9 @@ def test_update_files_none(self): self.mock_repo.create_pull.assert_not_called() def test_update_files_empty(self): - self.instance.files = [] + self.forked_repo = MagicMock() + self.instance._forked_repo = self.forked_repo + self.instance._forked_repo.files = [] self.sandr.run() self.mock_repo.update_file.assert_not_called() self.mock_repo.create_pull.assert_not_called()