From 06d52f2c838deab749f3624befade9b1390b887b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 26 Jul 2016 13:41:13 -0700 Subject: [PATCH] Fix handling of multi-part branches in the build triggers Fixes #1360 --- data/database.py | 6 + endpoints/api/build.py | 46 +++--- endpoints/building.py | 2 +- static/js/directives/ui/source-ref-link.js | 2 +- test/test_prepare_trigger.py | 21 +++ test/test_trigger.py | 23 +++ .../github_webhook_slash_branch.json | 153 ++++++++++++++++++ 7 files changed, 232 insertions(+), 21 deletions(-) create mode 100644 test/triggerjson/github_webhook_slash_branch.json diff --git a/data/database.py b/data/database.py index a00b8a139..a866aa161 100644 --- a/data/database.py +++ b/data/database.py @@ -665,6 +665,12 @@ class BUILD_PHASE(object): WAITING = 'waiting' COMPLETE = 'complete' + @classmethod + def is_terminal_phase(cls, phase): + return (phase == cls.COMPLETE or + phase == cls.ERROR or + phase == cls.INTERNAL_ERROR) + class QueueItem(BaseModel): queue_name = CharField(index=True, max_length=1024) diff --git a/endpoints/api/build.py b/endpoints/api/build.py index f2481e52d..e43c253d5 100644 --- a/endpoints/api/build.py +++ b/endpoints/api/build.py @@ -87,36 +87,44 @@ def trigger_view(trigger, can_read=False, can_admin=False, for_build=False): return None -def build_status_view(build_obj): +def _get_build_status(build_obj): + """ Returns the updated build phase, status and (if any) error for the build object. """ phase = build_obj.phase status = {} error = None - try: - status = build_logs.get_status(build_obj.uuid) - except BuildStatusRetrievalError as bsre: - phase = 'cannot_load' - if SuperUserPermission().can(): - error = str(bsre) - else: - error = 'Redis may be down. Please contact support.' + # If the build is currently running, then load its "real-time" status from Redis. + if not database.BUILD_PHASE.is_terminal_phase(phase): + try: + status = build_logs.get_status(build_obj.uuid) + except BuildStatusRetrievalError as bsre: + phase = 'cannot_load' + if SuperUserPermission().can(): + error = str(bsre) + else: + error = 'Redis may be down. Please contact support.' - if phase != 'cannot_load': - # If the status contains a heartbeat, then check to see if has been written in the last few - # minutes. If not, then the build timed out. - if phase != database.BUILD_PHASE.COMPLETE and phase != database.BUILD_PHASE.ERROR: + if phase != 'cannot_load': + # If the status contains a heartbeat, then check to see if has been written in the last few + # minutes. If not, then the build timed out. if status is not None and 'heartbeat' in status and status['heartbeat']: heartbeat = datetime.datetime.utcfromtimestamp(status['heartbeat']) if datetime.datetime.utcnow() - heartbeat > datetime.timedelta(minutes=1): phase = database.BUILD_PHASE.INTERNAL_ERROR - # If the phase is internal error, return 'error' instead if the number of retries - # on the queue item is 0. - if phase == database.BUILD_PHASE.INTERNAL_ERROR: - retry = build_obj.queue_id and dockerfile_build_queue.has_retries_remaining(build_obj.queue_id) - if not retry: - phase = database.BUILD_PHASE.ERROR + # If the phase is internal error, return 'error' instead if the number of retries + # on the queue item is 0. + if phase == database.BUILD_PHASE.INTERNAL_ERROR: + retry = (build_obj.queue_id and + dockerfile_build_queue.has_retries_remaining(build_obj.queue_id)) + if not retry: + phase = database.BUILD_PHASE.ERROR + return (phase, status, error) + + +def build_status_view(build_obj): + phase, status, error = _get_build_status(build_obj) repo_namespace = build_obj.repository.namespace_user.username repo_name = build_obj.repository.name diff --git a/endpoints/building.py b/endpoints/building.py index b6d2f4904..93961bfc8 100644 --- a/endpoints/building.py +++ b/endpoints/building.py @@ -98,7 +98,7 @@ class PreparedBuild(object): return sha[0:7] def tags_from_ref(self, ref, default_branch=None): - branch = ref.split('/')[-1] + branch = ref.split('/', 2)[-1] tags = {branch} if branch == default_branch: diff --git a/static/js/directives/ui/source-ref-link.js b/static/js/directives/ui/source-ref-link.js index d8ee3d4c3..5a42a620a 100644 --- a/static/js/directives/ui/source-ref-link.js +++ b/static/js/directives/ui/source-ref-link.js @@ -29,7 +29,7 @@ angular.module('quay').directive('sourceRefLink', function () { return ''; } - return parts[2]; + return parts.slice(2).join('/'); }; $scope.getUrl = function(ref, template, kind) { diff --git a/test/test_prepare_trigger.py b/test/test_prepare_trigger.py index e47e7f859..f4e79f1cc 100644 --- a/test/test_prepare_trigger.py +++ b/test/test_prepare_trigger.py @@ -207,6 +207,27 @@ class TestPrepareTrigger(unittest.TestCase): self.assertSchema('bitbucket_webhook', expected, bb_webhook) + def test_github_webhook_payload_slash_branch(self): + expected = { + 'commit': u'410f4cdf8ff09b87f245b13845e8497f90b90a4c', + 'ref': u'refs/heads/slash/branch', + 'git_url': u'git@github.com:josephschorr/anothertest.git', + 'commit_info': { + 'url': u'https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c', + 'date': u'2015-09-11T14:26:16-04:00', + 'message': u'Update Dockerfile', + 'committer': { + 'username': u'josephschorr', + }, + 'author': { + 'username': u'josephschorr', + }, + }, + } + + self.assertSchema('github_webhook_slash_branch', expected, gh_webhook) + + def test_github_webhook_payload(self): expected = { 'commit': u'410f4cdf8ff09b87f245b13845e8497f90b90a4c', diff --git a/test/test_trigger.py b/test/test_trigger.py index 8cdb8921a..81970c338 100644 --- a/test/test_trigger.py +++ b/test/test_trigger.py @@ -16,6 +16,8 @@ class TestRegex(unittest.TestCase): self.assertMatches('ref/heads/master', '.+') self.assertMatches('ref/heads/master', 'heads/.+') self.assertMatches('ref/heads/master', 'heads/master') + self.assertMatches('ref/heads/slash/branch', 'heads/slash/branch') + self.assertMatches('ref/heads/slash/branch', 'heads/.+') self.assertDoesNotMatch('ref/heads/foobar', 'heads/master') self.assertDoesNotMatch('ref/heads/master', 'tags/master') @@ -28,6 +30,27 @@ class TestRegex(unittest.TestCase): self.assertDoesNotMatch('ref/heads/delta', '(((heads/alpha)|(heads/beta))|(heads/gamma))|(heads/master)') +class TestTags(unittest.TestCase): + def assertTagsForRef(self, ref, tags): + prepared = PreparedBuild() + prepared.tags_from_ref(ref, default_branch='master') + self.assertEquals(tags, prepared._tags) + + def test_normal(self): + self.assertTagsForRef('ref/heads/somebranch', ['somebranch']) + self.assertTagsForRef('ref/heads/master', ['master', 'latest']) + + self.assertTagsForRef('ref/tags/somebranch', ['somebranch']) + self.assertTagsForRef('ref/tags/master', ['master', 'latest']) + + def test_slash(self): + self.assertTagsForRef('ref/heads/slash/branch', ['slash_branch']) + self.assertTagsForRef('ref/tags/slash/tag', ['slash_tag']) + + def test_unusual(self): + self.assertTagsForRef('ref/heads/foobar#2', ['foobar_2']) + + class TestSkipBuild(unittest.TestCase): def testSkipNoMetadata(self): prepared = PreparedBuild() diff --git a/test/triggerjson/github_webhook_slash_branch.json b/test/triggerjson/github_webhook_slash_branch.json new file mode 100644 index 000000000..13e89361d --- /dev/null +++ b/test/triggerjson/github_webhook_slash_branch.json @@ -0,0 +1,153 @@ +{ + "ref": "refs/heads/slash/branch", + "before": "9ea43cab474709d4a61afb7e3340de1ffc405b41", + "after": "410f4cdf8ff09b87f245b13845e8497f90b90a4c", + "created": false, + "deleted": false, + "forced": false, + "base_ref": null, + "compare": "https://github.com/josephschorr/anothertest/compare/9ea43cab4747...410f4cdf8ff0", + "commits": [ + { + "id": "410f4cdf8ff09b87f245b13845e8497f90b90a4c", + "distinct": true, + "message": "Update Dockerfile", + "timestamp": "2015-09-11T14:26:16-04:00", + "url": "https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c", + "author": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com", + "username": "josephschorr" + }, + "committer": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com", + "username": "josephschorr" + }, + "added": [], + "removed": [], + "modified": [ + "Dockerfile" + ] + } + ], + "head_commit": { + "id": "410f4cdf8ff09b87f245b13845e8497f90b90a4c", + "distinct": true, + "message": "Update Dockerfile", + "timestamp": "2015-09-11T14:26:16-04:00", + "url": "https://github.com/josephschorr/anothertest/commit/410f4cdf8ff09b87f245b13845e8497f90b90a4c", + "author": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com", + "username": "josephschorr" + }, + "committer": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com", + "username": "josephschorr" + }, + "added": [], + "removed": [], + "modified": [ + "Dockerfile" + ] + }, + "repository": { + "id": 34876107, + "name": "anothertest", + "full_name": "josephschorr/anothertest", + "owner": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com" + }, + "private": false, + "html_url": "https://github.com/josephschorr/anothertest", + "description": "", + "fork": false, + "url": "https://github.com/josephschorr/anothertest", + "forks_url": "https://api.github.com/repos/josephschorr/anothertest/forks", + "keys_url": "https://api.github.com/repos/josephschorr/anothertest/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/josephschorr/anothertest/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/josephschorr/anothertest/teams", + "hooks_url": "https://api.github.com/repos/josephschorr/anothertest/hooks", + "issue_events_url": "https://api.github.com/repos/josephschorr/anothertest/issues/events{/number}", + "events_url": "https://api.github.com/repos/josephschorr/anothertest/events", + "assignees_url": "https://api.github.com/repos/josephschorr/anothertest/assignees{/user}", + "branches_url": "https://api.github.com/repos/josephschorr/anothertest/branches{/branch}", + "tags_url": "https://api.github.com/repos/josephschorr/anothertest/tags", + "blobs_url": "https://api.github.com/repos/josephschorr/anothertest/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/josephschorr/anothertest/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/josephschorr/anothertest/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/josephschorr/anothertest/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/josephschorr/anothertest/statuses/{sha}", + "languages_url": "https://api.github.com/repos/josephschorr/anothertest/languages", + "stargazers_url": "https://api.github.com/repos/josephschorr/anothertest/stargazers", + "contributors_url": "https://api.github.com/repos/josephschorr/anothertest/contributors", + "subscribers_url": "https://api.github.com/repos/josephschorr/anothertest/subscribers", + "subscription_url": "https://api.github.com/repos/josephschorr/anothertest/subscription", + "commits_url": "https://api.github.com/repos/josephschorr/anothertest/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/josephschorr/anothertest/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/josephschorr/anothertest/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/josephschorr/anothertest/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/josephschorr/anothertest/contents/{+path}", + "compare_url": "https://api.github.com/repos/josephschorr/anothertest/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/josephschorr/anothertest/merges", + "archive_url": "https://api.github.com/repos/josephschorr/anothertest/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/josephschorr/anothertest/downloads", + "issues_url": "https://api.github.com/repos/josephschorr/anothertest/issues{/number}", + "pulls_url": "https://api.github.com/repos/josephschorr/anothertest/pulls{/number}", + "milestones_url": "https://api.github.com/repos/josephschorr/anothertest/milestones{/number}", + "notifications_url": "https://api.github.com/repos/josephschorr/anothertest/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/josephschorr/anothertest/labels{/name}", + "releases_url": "https://api.github.com/repos/josephschorr/anothertest/releases{/id}", + "created_at": 1430426945, + "updated_at": "2015-04-30T20:49:05Z", + "pushed_at": 1441995976, + "git_url": "git://github.com/josephschorr/anothertest.git", + "ssh_url": "git@github.com:josephschorr/anothertest.git", + "clone_url": "https://github.com/josephschorr/anothertest.git", + "svn_url": "https://github.com/josephschorr/anothertest", + "homepage": null, + "size": 144, + "stargazers_count": 0, + "watchers_count": 0, + "language": null, + "has_issues": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": false, + "forks_count": 0, + "mirror_url": null, + "open_issues_count": 0, + "forks": 0, + "open_issues": 0, + "watchers": 0, + "default_branch": "master", + "stargazers": 0, + "master_branch": "master" + }, + "pusher": { + "name": "josephschorr", + "email": "josephschorr@users.noreply.github.com" + }, + "sender": { + "login": "josephschorr", + "id": 4073002, + "avatar_url": "https://avatars.githubusercontent.com/u/4073002?v=3", + "gravatar_id": "", + "url": "https://api.github.com/users/josephschorr", + "html_url": "https://github.com/josephschorr", + "followers_url": "https://api.github.com/users/josephschorr/followers", + "following_url": "https://api.github.com/users/josephschorr/following{/other_user}", + "gists_url": "https://api.github.com/users/josephschorr/gists{/gist_id}", + "starred_url": "https://api.github.com/users/josephschorr/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/josephschorr/subscriptions", + "organizations_url": "https://api.github.com/users/josephschorr/orgs", + "repos_url": "https://api.github.com/users/josephschorr/repos", + "events_url": "https://api.github.com/users/josephschorr/events{/privacy}", + "received_events_url": "https://api.github.com/users/josephschorr/received_events", + "type": "User", + "site_admin": false + } +} \ No newline at end of file