diff --git a/contrib/buildbot/server.py b/contrib/buildbot/server.py --- a/contrib/buildbot/server.py +++ b/contrib/buildbot/server.py @@ -450,16 +450,31 @@ } return next_token[current_token] if current_token in next_token else "PHID-TOKN-like-1" - def is_user_allowed_to_trigger_builds(user_PHID, current_token): + def is_user_allowed_to_trigger_builds( + user_PHID, current_token, comment_builds, build_configs): if current_token not in [ "", "PHID-TOKN-coin-1", "PHID-TOKN-coin-2", "PHID-TOKN-coin-3"]: return False - return all(role in phab.get_user_roles(user_PHID) for role in [ + if not all(role in phab.get_user_roles(user_PHID) for role in [ "verified", "approved", "activated", - ]) + ]): + return False + + for build_name in comment_builds: + build_config = build_configs.get(build_name, None) + if build_config is None: + # If one of the build doesn't exist, reject them all. + return False + + if "docker" in build_config: + # If one of the build contain a docker configuration, reject + # them all. + return False + + return True # Anti DoS filter # @@ -470,6 +485,8 @@ # | AND # | - The maximum number of requests for this revision has not been # | reached yet. + # | AND + # | - The build doesn't contain a `docker` configuration. # # The number of requests is tracked by awarding a coin token to the # revision each time a build request is submitted (the number of build @@ -482,6 +499,8 @@ abc_members = phab.get_project_members(BITCOIN_ABC_PROJECT_PHID) current_token = phab.get_object_token(revision_PHID) + build_configs = get_master_build_configurations() + builds = [] for comment in comments: comment_builds = get_builds_from_comment(comment["content"]["raw"]) @@ -498,7 +517,8 @@ builds += comment_builds continue - if is_user_allowed_to_trigger_builds(user, current_token): + if is_user_allowed_to_trigger_builds( + user, current_token, comment_builds, build_configs): builds += comment_builds # If there is no build provided, this request is not what we are after, diff --git a/contrib/buildbot/test/test_endpoint_triggerCI.py b/contrib/buildbot/test/test_endpoint_triggerCI.py --- a/contrib/buildbot/test/test_endpoint_triggerCI.py +++ b/contrib/buildbot/test/test_endpoint_triggerCI.py @@ -5,6 +5,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. import itertools +import json import test.mocks.phabricator import test.mocks.teamcity import unittest @@ -43,11 +44,47 @@ } }]) + self.phab.user.search.return_value = test.mocks.phabricator.Result([ + { + "id": 1, + "type": "USER", + "phid": "PHID-AUTHORIZED-USER", + "fields": { + "roles": [ + "verified", + "approved", + "activated", + ], + }, + }, + ]) + # Phabricator returns the default diff ID self.phab.differential.diff.search.return_value = test.mocks.phabricator.Result([{ "id": self.diff_id, }]) + config = { + "builds": { + "build-1": {}, + "build-11": {}, + "build-12": {}, + "build-13": {}, + "build-2": {}, + "build-21": {}, + "build-22": {}, + "build-23": {}, + "build-3": {}, + "build-31": {}, + "build-32": {}, + "build-33": {}, + "build-docker": {"docker": {}}, + }, + } + self.phab.get_file_content_from_master = mock.Mock() + self.phab.get_file_content_from_master.return_value = json.dumps( + config) + # Transaction webhook on diff update def call_endpoint(self): webhook_transaction = { @@ -253,8 +290,36 @@ self.teamcity.session.send.assert_not_called() self.assertEqual(response.status_code, 200) + # Authorized but non-ABC user, running at least one non-existent build + self.set_transaction_return_value( + [ + # Build 4 doesn't exist + "@bot build-4", + "@bot build-4 build-11 build-12 build-13 build-2 build-3", + "@bot build-11 build-12 build-13 build-2 build-3 build-4", + ], + "PHID-AUTHORIZED-USER" + ) + response = self.call_endpoint() + self.teamcity.session.send.assert_not_called() + self.assertEqual(response.status_code, 200) + + # Authorized but non-ABC user, running at least one docker build + self.set_transaction_return_value( + [ + "@bot build-docker", + "@bot build-docker build-11 build-12 build-13 build-2 build-3", + "@bot build-11 build-12 build-13 build-2 build-3 build-docker", + ], + "PHID-AUTHORIZED-USER" + ) + response = self.call_endpoint() + self.teamcity.session.send.assert_not_called() + self.assertEqual(response.status_code, 200) + def test_triggerCI_some_build_queued(self): def assert_teamcity_queued_builds(comments, queued_builds): + # Default user is an ABC member in set_transaction_return_value self.set_transaction_return_value(comments) response = self.call_endpoint() expected_calls = [ @@ -268,12 +333,11 @@ ) for build_id in queued_builds ] - print(expected_calls) self.teamcity.trigger_build.assert_has_calls( expected_calls, any_order=True) self.assertEqual(response.status_code, 200) - # Authorized user, 1 comment targeting the bot with 1 build + # ABC user, 1 comment targeting the bot with 1 build assert_teamcity_queued_builds( [ "@bot build-1", @@ -283,7 +347,7 @@ ] ) - # Authorized user, 1 comment targeting the bot with 3 builds + # ABC user, 1 comment targeting the bot with 3 builds assert_teamcity_queued_builds( [ "@bot build-1 build-2 build-3", @@ -295,7 +359,7 @@ ] ) - # Authorized user, 3 comments targeting the bot with 3 builds each + # ABC user, 3 comments targeting the bot with 3 builds each assert_teamcity_queued_builds( [ "@bot build-11 build-12 build-13", @@ -309,7 +373,7 @@ ] ) - # Authorized user, 1 comment targeting the bot with duplicated builds + # ABC user, 1 comment targeting the bot with duplicated builds assert_teamcity_queued_builds( [ "@bot build-1 build-2 build-1 build-3 build-2", @@ -321,6 +385,40 @@ ] ) + # ABC user, some comments targeting the bot with 3 builds involving docker + assert_teamcity_queued_builds( + [ + "@bot build-docker build-1 build-2 build-3", + ], + [ + "build-docker", "build-1", "build-2", "build-3", + ] + ) + assert_teamcity_queued_builds( + [ + "@bot build-1 build-2 build-docker build-3", + ], + [ + "build-1", "build-2", "build-docker", "build-3", + ] + ) + assert_teamcity_queued_builds( + [ + "@bot build-1 build-2 build-3 build-docker", + ], + [ + "build-1", "build-2", "build-3", "build-docker", + ] + ) + assert_teamcity_queued_builds( + [ + "@bot build-docker build-1 build-docker build-2 build-docker build-3 build-docker", + ], + [ + "build-docker", "build-1", "build-2", "build-3", + ] + ) + def test_triggerCI_check_user_roles(self): user_PHID = "PHID-USER-notabc"