diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index 99f90bdd5..a51526507 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -261,13 +261,14 @@ class GithubBuildTrigger(BuildTriggerHandler): raise TriggerDeactivationException(msg) # Remove the webhook. - try: - hook = repo.get_hook(config['hook_id']) - hook.delete() - except GithubException as ghe: - default_msg = 'Unable to remove hook: %s' % config['hook_id'] - msg = GithubBuildTrigger._get_error_message(ghe, default_msg) - raise TriggerDeactivationException(msg) + if 'hook_id' in config: + try: + hook = repo.get_hook(config['hook_id']) + hook.delete() + except GithubException as ghe: + default_msg = 'Unable to remove hook: %s' % config['hook_id'] + msg = GithubBuildTrigger._get_error_message(ghe, default_msg) + raise TriggerDeactivationException(msg) config.pop('hook_id', None) self.config = config @@ -315,12 +316,14 @@ class GithubBuildTrigger(BuildTriggerHandler): gh_client = self._get_client() usr = gh_client.get_user() - if namespace == usr.login: return [repo_view(repo) for repo in usr.get_repos() if repo.owner.login == namespace] - org = gh_client.get_organization(namespace) - if org is None: + try: + org = gh_client.get_organization(namespace) + if org is None: + return [] + except GithubException: return [] return [repo_view(repo) for repo in org.get_repos(type='member')] @@ -479,8 +482,11 @@ class GithubBuildTrigger(BuildTriggerHandler): raise TriggerStartException(msg) def get_branch_sha(branch_name): - branch = repo.get_branch(branch_name) - return branch.commit.sha + try: + branch = repo.get_branch(branch_name) + return branch.commit.sha + except GithubException: + raise TriggerStartException('Could not find branch in repository') def get_tag_sha(tag_name): tags = {tag.name: tag for tag in repo.get_tags()} @@ -515,9 +521,21 @@ class GithubBuildTrigger(BuildTriggerHandler): # This is for GitHub's probing/testing. if 'zen' in payload: - raise ValidationRequestException() + raise SkipRequestException() # Lookup the default branch for the repository. + if 'repository' not in payload: + raise ValidationRequestException("Missing 'repository' on request") + + if 'owner' not in payload['repository']: + raise ValidationRequestException("Missing 'owner' on repository") + + if 'name' not in payload['repository']['owner']: + raise ValidationRequestException("Missing owner 'name' on repository") + + if 'name' not in payload['repository']: + raise ValidationRequestException("Missing 'name' on repository") + default_branch = None lookup_user = None try: diff --git a/buildtrigger/test/test_githubhandler.py b/buildtrigger/test/test_githubhandler.py new file mode 100644 index 000000000..3d6555f81 --- /dev/null +++ b/buildtrigger/test/test_githubhandler.py @@ -0,0 +1,361 @@ +import json +import pytest + +from github import GithubException +from mock import Mock +from datetime import datetime + +from buildtrigger.githubhandler import GithubBuildTrigger +from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, + TriggerStartException, ValidationRequestException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict + +def get_mock_github(): + def get_commit_mock(commit_sha): + if commit_sha == 'aaaaaaa': + commit_mock = Mock() + commit_mock.sha = commit_sha + commit_mock.html_url = 'http://url/to/commit' + commit_mock.last_modified = 'now' + + commit_mock.commit = Mock() + commit_mock.commit.message = 'some cool message' + + commit_mock.committer = Mock() + commit_mock.committer.login = 'someuser' + commit_mock.committer.avatar_url = 'avatarurl' + commit_mock.committer.html_url = 'htmlurl' + + commit_mock.author = Mock() + commit_mock.author.login = 'someuser' + commit_mock.author.avatar_url = 'avatarurl' + commit_mock.author.html_url = 'htmlurl' + return commit_mock + + raise GithubException(None, None) + + def get_branch_mock(branch_name): + if branch_name == 'master': + branch_mock = Mock() + branch_mock.commit = Mock() + branch_mock.commit.sha = 'aaaaaaa' + return branch_mock + + raise GithubException(None, None) + + def get_repo_mock(namespace, name): + repo_mock = Mock() + repo_mock.owner = Mock() + repo_mock.owner.login = namespace + + repo_mock.full_name = '%s/%s' % (namespace, name) + repo_mock.name = name + repo_mock.description = 'some %s repo' % (name) + repo_mock.pushed_at = datetime.utcfromtimestamp(0) + repo_mock.html_url = 'http://some/url' + repo_mock.private = name == 'somerepo' + repo_mock.permissions = Mock() + repo_mock.permissions.admin = namespace == 'knownuser' + return repo_mock + + def get_user_repos_mock(): + return [get_repo_mock('knownuser', 'somerepo')] + + def get_org_repos_mock(type='all'): + return [get_repo_mock('someorg', 'somerepo'), get_repo_mock('someorg', 'anotherrepo')] + + def get_orgs_mock(): + return [get_org_mock('someorg')] + + def get_user_mock(username='knownuser'): + if username == 'knownuser': + user_mock = Mock() + user_mock.name = username + user_mock.plan = Mock() + user_mock.plan.private_repos = 1 + user_mock.login = username + user_mock.html_url = 'htmlurl' + user_mock.avatar_url = 'avatarurl' + user_mock.get_repos = Mock(side_effect=get_user_repos_mock) + user_mock.get_orgs = Mock(side_effect=get_orgs_mock) + return user_mock + + raise GithubException(None, None) + + def get_org_mock(namespace): + if namespace == 'someorg': + org_mock = Mock() + org_mock.get_repos = Mock(side_effect=get_org_repos_mock) + org_mock.login = namespace + org_mock.html_url = 'htmlurl' + org_mock.avatar_url = 'avatarurl' + org_mock.name = namespace + org_mock.plan = Mock() + org_mock.plan.private_repos = 2 + return org_mock + + raise GithubException(None, None) + + def get_tags_mock(): + sometag = Mock() + sometag.name = 'sometag' + sometag.commit = get_commit_mock('aaaaaaa') + + someothertag = Mock() + someothertag.name = 'someothertag' + someothertag.commit = get_commit_mock('aaaaaaa') + return [sometag, someothertag] + + def get_branches_mock(): + master = Mock() + master.name = 'master' + master.commit = get_commit_mock('aaaaaaa') + + otherbranch = Mock() + otherbranch.name = 'otherbranch' + otherbranch.commit = get_commit_mock('aaaaaaa') + return [master, otherbranch] + + def get_file_contents_mock(filepath): + if filepath == '/Dockerfile': + m = Mock() + m.content = 'hello world' + return m + + if filepath == 'somesubdir/Dockerfile': + m = Mock() + m.content = 'hi universe' + return m + + raise GithubException(None, None) + + def get_git_tree_mock(commit_sha, recursive=False): + first_file = Mock() + first_file.type = 'blob' + first_file.path = 'Dockerfile' + + second_file = Mock() + second_file.type = 'other' + second_file.path = '/some/Dockerfile' + + third_file = Mock() + third_file.type = 'blob' + third_file.path = 'somesubdir/Dockerfile' + + t = Mock() + + if commit_sha == 'aaaaaaa': + t.tree = [ + first_file, second_file, third_file, + ] + else: + t.tree = [] + + return t + + repo_mock = Mock() + repo_mock.default_branch = 'master' + repo_mock.ssh_url = 'ssh_url' + + repo_mock.get_branch = Mock(side_effect=get_branch_mock) + repo_mock.get_tags = Mock(side_effect=get_tags_mock) + repo_mock.get_branches = Mock(side_effect=get_branches_mock) + repo_mock.get_commit = Mock(side_effect=get_commit_mock) + repo_mock.get_file_contents = Mock(side_effect=get_file_contents_mock) + repo_mock.get_git_tree = Mock(side_effect=get_git_tree_mock) + + gh_mock = Mock() + gh_mock.get_repo = Mock(return_value=repo_mock) + gh_mock.get_user = Mock(side_effect=get_user_mock) + gh_mock.get_organization = Mock(side_effect=get_org_mock) + return gh_mock + +@pytest.fixture +def github_trigger(): + return _get_github_trigger() + +def _get_github_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = GithubBuildTrigger(trigger_obj, {'build_source': 'foo', 'subdir': subdir}) + trigger._get_client = get_mock_github + return trigger + +@pytest.mark.parametrize('payload, expected_error, expected_message', [ + ('{"zen": true}', SkipRequestException, ""), + + ('{}', ValidationRequestException, "Missing 'repository' on request"), + ('{"repository": "foo"}', ValidationRequestException, "Missing 'owner' on repository"), + + # Valid payload: + ('''{ + "repository": { + "owner": { + "name": "someguy" + }, + "name": "somerepo", + "ssh_url": "someurl" + }, + "ref": "refs/tags/foo", + "head_commit": { + "id": "11d6fbc", + "url": "http://some/url", + "message": "some message", + "timestamp": "NOW" + } + }''', None, None), + + # Skip message: + ('''{ + "repository": { + "owner": { + "name": "someguy" + }, + "name": "somerepo", + "ssh_url": "someurl" + }, + "ref": "refs/tags/foo", + "head_commit": { + "id": "11d6fbc", + "url": "http://some/url", + "message": "[skip build]", + "timestamp": "NOW" + } + }''', SkipRequestException, ''), +]) +def test_handle_trigger_request(github_trigger, payload, expected_error, expected_message): + def get_payload(): + return json.loads(payload) + + request = AttrDict(dict(get_json=get_payload)) + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + github_trigger.handle_trigger_request(request) + assert ipe.value.message == expected_message + else: + assert isinstance(github_trigger.handle_trigger_request(request), PreparedBuild) + + +@pytest.mark.parametrize('run_parameters, expected_error, expected_message', [ + # No branch or tag specified: use the commit of the default branch. + ({}, None, None), + + # Invalid branch. + ({'refs': {'kind': 'branch', 'name': 'invalid'}}, TriggerStartException, + 'Could not find branch in repository'), + + # Invalid tag. + ({'refs': {'kind': 'tag', 'name': 'invalid'}}, TriggerStartException, + 'Could not find tag in repository'), + + # Valid branch. + ({'refs': {'kind': 'branch', 'name': 'master'}}, None, None), + + # Valid tag. + ({'refs': {'kind': 'tag', 'name': 'sometag'}}, None, None), +]) +def test_manual_start(run_parameters, expected_error, expected_message, github_trigger): + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + github_trigger.manual_start(run_parameters) + assert ipe.value.message == expected_message + else: + assert isinstance(github_trigger.manual_start(run_parameters), PreparedBuild) + + +@pytest.mark.parametrize('username, expected_response', [ + ('unknownuser', None), + ('knownuser', {'html_url': 'htmlurl', 'avatar_url': 'avatarurl'}), +]) +def test_lookup_user(username, expected_response, github_trigger): + assert github_trigger.lookup_user(username) == expected_response + + +@pytest.mark.parametrize('name, expected', [ + ('refs', [ + {'kind': 'branch', 'name': 'master'}, + {'kind': 'branch', 'name': 'otherbranch'}, + {'kind': 'tag', 'name': 'sometag'}, + {'kind': 'tag', 'name': 'someothertag'}, + ]), + ('tag_name', ['sometag', 'someothertag']), + ('branch_name', ['master', 'otherbranch']), + ('invalid', None) +]) +def test_list_field_values(name, expected, github_trigger): + assert github_trigger.list_field_values(name) == expected + + +@pytest.mark.parametrize('subdir, contents', [ + ('', 'hello world'), + ('somesubdir', 'hi universe'), + ('unknownpath', None), +]) +def test_load_dockerfile_contents(subdir, contents): + trigger = _get_github_trigger(subdir) + assert trigger.load_dockerfile_contents() == contents + + +def test_list_build_subdirs(github_trigger): + assert github_trigger.list_build_subdirs() == ['', 'somesubdir'] + + +def test_list_build_source_namespaces(github_trigger): + namespaces_expected = [ + { + 'personal': True, + 'score': 1, + 'avatar_url': 'avatarurl', + 'id': 'knownuser', + 'title': 'knownuser' + }, + { + 'score': 2, + 'title': 'someorg', + 'personal': False, + 'url': 'htmlurl', + 'avatar_url': 'avatarurl', + 'id': 'someorg' + } + ] + assert github_trigger.list_build_source_namespaces() == namespaces_expected + + +@pytest.mark.parametrize('namespace, expected', [ + ('', []), + ('unknown', []), + + ('knownuser', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', 'private': True, + 'full_name': 'knownuser/somerepo', 'has_admin_permissions': True, + 'description': 'some somerepo repo' + }]), + + ('someorg', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', + 'private': True, 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, + 'description': 'some somerepo repo' + }, + { + 'last_updated': 0, 'name': 'anotherrepo', 'url': 'http://some/url', + 'private': False, 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, + 'description': 'some anotherrepo repo' + }]), +]) +def test_list_build_sources_for_namespace(namespace, expected, github_trigger): + # TODO: schema validation on the resulting namespaces. + assert github_trigger.list_build_sources_for_namespace(namespace) == expected + + +def test_activate(github_trigger): + config, private_key = github_trigger.activate('http://some/url') + assert 'deploy_key_id' in config + assert 'hook_id' in config + assert 'private_key' in private_key + + +def test_deactivate(github_trigger): + github_trigger.deactivate()