From 781e71e49fe543b1d8791750a0a6cd870ef7fdef Mon Sep 17 00:00:00 2001 From: Mun Yong Jang Date: Wed, 25 Oct 2017 15:46:20 -0700 Subject: [PATCH] [presubmit] Add check to call validation endpoint for config files Bug:509672 Change-Id: I74bbaa725be829c625cd617c58b3e849acb18102 Reviewed-on: https://chromium-review.googlesource.com/726847 Commit-Queue: Mun Yong Jang Reviewed-by: Andrii Shyshkalov Reviewed-by: Nodir Turakulov --- git_common.py | 7 +++ presubmit_canned_checks.py | 86 +++++++++++++++++++++++++++++++++++++ tests/presubmit_unittest.py | 57 ++++++++++++++++++++++++ 3 files changed, 150 insertions(+) diff --git a/git_common.py b/git_common.py index 1bb59375f..59a904b93 100644 --- a/git_common.py +++ b/git_common.py @@ -954,6 +954,13 @@ def tree(treeref, recurse=False): return ret +def get_remote_url(remote='origin'): + try: + return run('config', 'remote.%s.url' % remote) + except subprocess2.CalledProcessError: + return None + + def upstream(branch): try: return run('rev-parse', '--abbrev-ref', '--symbolic-full-name', diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 102129295..ad472ece0 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -39,6 +39,92 @@ BLACKLIST_LINT_FILTERS = [ ### Description checks +def CheckChangedConfigs(input_api, output_api): + import urllib2 + import json + import collections + + import auth + import git_common as git + import git_cl + + LUCI_CONFIG_HOST_NAME = 'luci-config.appspot.com' + + cl = git_cl.Changelist() + remote, remote_branch = cl.FetchUpstreamTuple(cl.GetBranch()) + if not remote_branch: + return [output_api.PresubmitError('Upstream branch has not been set')] + remote_host_url = git.get_remote_url(remote=remote) + if not remote_host_url: + return [output_api.PresubmitError( + 'Remote host url for git has not been defined')] + if remote_host_url.endswith('.git'): + remote_host_url = remote_host_url[:-len('.git')] + + # authentication + try: + authenticator = auth.get_authenticator_for_host( + LUCI_CONFIG_HOST_NAME, auth.make_auth_config()) + acc_tkn = authenticator.get_access_token(allow_user_interaction=True).token + except auth.AuthenticationError as e: + return [output_api.PresubmitError( + 'Error in authenticating user.', long_text=str(e))] + + def request(endpoint, body=None): + api_url = ('https://%s/_ah/api/config/v1/%s' + % (LUCI_CONFIG_HOST_NAME, endpoint)) + req = urllib2.Request(api_url) + req.add_header('Authorization', 'Bearer %s' % acc_tkn) + if body is not None: + req.add_header('Content-Type', 'application/json') + req.add_data(json.dumps(body)) + return json.load(urllib2.urlopen(req)) + + try: + config_sets = request('config-sets').get('config_sets') + except urllib2.HTTPError as e: + return [output_api.PresubmitError( + 'Config set request to luci-config failed', long_text=str(e))] + if not config_sets: + return [output_api.PresubmitWarning('No config_sets were returned')] + loc_pref = '%s/+/%s/' % (remote_host_url, remote_branch) + dir_to_config_set = { + '%s/' % cs['location'][len(loc_pref):].rstrip('/'): cs['config_set'] + for cs in config_sets + if cs['location'].startswith(loc_pref) or + ('%s/' % cs['location']) == loc_pref + } + cs_to_files = collections.defaultdict(list) + for f in input_api.AffectedFiles(): + # windows + file_path = f.LocalPath().replace(_os.sep, '/') + for dr, cs in dir_to_config_set.iteritems(): + if dr == '/' or file_path.startswith(dr): + cs_to_files[cs].append({ + 'path': file_path[len(dr):] if dr != '/' else file_path, + 'content': '\n'.join(f.NewContents()) + }) + outputs = [] + for cs, f in cs_to_files.iteritems(): + try: + # TODO(myjang): parallelize + res = request( + 'validate-config', body={'config_set': cs, 'files': f}) + except urllib2.HTTPError as e: + return [output_api.PresubmitError( + 'Validation request to luci-config failed', long_text=str(e))] + for msg in res.get('messages', []): + sev = msg['severity'] + if sev == 'WARNING': + out_f = output_api.PresubmitPromptWarning + elif sev == 'ERROR' or sev == 'CRITICAL': + out_f = output_api.PresubmitError + else: + out_f = output_api.PresubmitNotifyResult + outputs.append(out_f('Config validation: %s' % msg['text'])) + return outputs + + def CheckChangeHasBugField(input_api, output_api): """Requires that the changelist have a Bug: field.""" if input_api.change.BugsFromDescription(): diff --git a/tests/presubmit_unittest.py b/tests/presubmit_unittest.py index 3c95eccf6..41800b3fa 100755 --- a/tests/presubmit_unittest.py +++ b/tests/presubmit_unittest.py @@ -16,6 +16,7 @@ import os import sys import time import unittest +import urllib2 _ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, _ROOT) @@ -27,6 +28,10 @@ import owners_finder import subprocess2 as subprocess import presubmit_support as presubmit import rietveld +import auth +import git_cl +import git_common as git +import json # Shortcut. presubmit_canned_checks = presubmit.presubmit_canned_checks @@ -1708,6 +1713,7 @@ class CannedChecksUnittest(PresubmitTestsBase): 'GetPythonUnitTests', 'GetPylint', 'GetUnitTests', 'GetUnitTestsInDirectory', 'GetUnitTestsRecursively', 'CheckCIPDManifest', 'CheckCIPDPackages', + 'CheckChangedConfigs', ] # If this test fails, you should add the relevant test. self.compareMembers(presubmit_canned_checks, members) @@ -1870,6 +1876,57 @@ class CannedChecksUnittest(PresubmitTestsBase): "TODO(foo): bar", None, "TODO: bar", None, presubmit.OutputApi.PresubmitPromptWarning) + def testCannedCheckChangedConfigs(self): + affected_file1 = self.mox.CreateMock(presubmit.GitAffectedFile) + affected_file1.LocalPath().AndReturn('foo.cfg') + affected_file1.NewContents().AndReturn(['test', 'foo']) + affected_file2 = self.mox.CreateMock(presubmit.GitAffectedFile) + affected_file2.LocalPath().AndReturn('bar.cfg') + affected_file2.NewContents().AndReturn(['test', 'bar']) + + token_mock = self.mox.CreateMock(auth.AccessToken) + token_mock.token = 123 + auth_mock = self.mox.CreateMock(auth.Authenticator) + auth_mock.get_access_token( + allow_user_interaction=True).AndReturn(token_mock) + self.mox.StubOutWithMock(auth, 'get_authenticator_for_host') + auth.get_authenticator_for_host( + mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(auth_mock) + + host = 'https://host.com' + branch = 'branch' + http_resp = { + 'messages': [{'severity': 'ERROR', 'text': 'deadbeef'}], + 'config_sets': [{'config_set': 'deadbeef', + 'location': '%s/+/%s' % (host, branch)}] + } + self.mox.StubOutWithMock(urllib2, 'urlopen') + urllib2.urlopen(mox.IgnoreArg()).MultipleTimes().AndReturn(http_resp) + self.mox.StubOutWithMock(json, 'load') + json.load(http_resp).MultipleTimes().AndReturn(http_resp) + + mock_cl = self.mox.CreateMock(git_cl.Changelist) + mock_cl.GetBranch().AndReturn('test') + mock_cl.FetchUpstreamTuple('test').AndReturn((host, branch)) + self.mox.StubOutWithMock(git_cl, 'Changelist', use_mock_anything=True) + git_cl.Changelist().AndReturn(mock_cl) + + self.mox.StubOutWithMock(git, 'get_remote_url') + git.get_remote_url(remote=host).AndReturn(host) + + change1 = presubmit.Change( + 'foo', 'foo1', self.fake_root_dir, None, 0, 0, None) + input_api = self.MockInputApi(change1, False) + affected_files = (affected_file1, affected_file2) + + input_api.AffectedFiles = lambda: affected_files + + self.mox.ReplayAll() + + results = presubmit_canned_checks.CheckChangedConfigs( + input_api, presubmit.OutputApi) + self.assertEquals(len(results), 1) + def testCannedCheckChangeHasNoTabs(self): self.ContentTest(presubmit_canned_checks.CheckChangeHasNoTabs, 'blah blah', None, 'blah\tblah', None,