From e465667e78b3f898f25b64a3ec4f5c003f7554e7 Mon Sep 17 00:00:00 2001 From: hinoka Date: Mon, 19 Sep 2016 15:28:19 -0700 Subject: [PATCH] bot_update: add --auth-refresh-token-json passthrough for apply_issue BUG=642150 Review-Url: https://codereview.chromium.org/2294413002 --- recipe_modules/bot_update/api.py | 17 ++++-- .../trychange_oauth2_json.json | 52 +++++++++++++++++++ .../trychange_oauth2_json_win.json | 52 +++++++++++++++++++ recipe_modules/bot_update/example.py | 15 ++++++ .../bot_update/resources/bot_update.py | 26 +++++++--- tests/bot_update_coverage_test.py | 1 + 6 files changed, 151 insertions(+), 12 deletions(-) create mode 100644 recipe_modules/bot_update/example.expected/trychange_oauth2_json.json create mode 100644 recipe_modules/bot_update/example.expected/trychange_oauth2_json_win.json diff --git a/recipe_modules/bot_update/api.py b/recipe_modules/bot_update/api.py index f1f31bd1c..512a89e41 100644 --- a/recipe_modules/bot_update/api.py +++ b/recipe_modules/bot_update/api.py @@ -66,7 +66,8 @@ class BotUpdateApi(recipe_api.RecipeApi): patch=True, update_presentation=True, patch_root=None, no_shallow=False, with_branch_heads=False, refs=None, - patch_oauth2=False, use_site_config_creds=True, + patch_oauth2=False, oauth2_json=False, + use_site_config_creds=True, output_manifest=True, clobber=False, root_solution_revision=None, rietveld=None, issue=None, patchset=None, gerrit_no_reset=False, @@ -91,6 +92,9 @@ class BotUpdateApi(recipe_api.RecipeApi): assert cfg is not None, ( 'missing gclient_config or forgot api.gclient.set_config(...) before?') + # Only one of these should exist. + assert not (oauth2_json and patch_oauth2) + # Construct our bot_update command. This basically be inclusive of # everything required for bot_update to know: root = patch_root @@ -123,7 +127,13 @@ class BotUpdateApi(recipe_api.RecipeApi): # Point to the oauth2 auth files if specified. # These paths are where the bots put their credential files. - if patch_oauth2: + oauth2_json_file = email_file = key_file = None + if oauth2_json: + if self.m.platform.is_win: + oauth2_json_file = 'C:\\creds\\refresh_tokens\\rietveld.json' + else: + oauth2_json_file = '/creds/refresh_tokens/rietveld.json' + elif patch_oauth2: # TODO(martiniss): remove this hack :(. crbug.com/624212 if use_site_config_creds: email_file = self.m.path['build'].join( @@ -134,8 +144,6 @@ class BotUpdateApi(recipe_api.RecipeApi): #TODO(martiniss): make this use path.join, so it works on windows email_file = '/creds/rietveld/client_email' key_file = '/creds/rietveld/secret_key' - else: - email_file = key_file = None # Allow patch_project's revision if necessary. # This is important for projects which are checked out as DEPS of the @@ -160,6 +168,7 @@ class BotUpdateApi(recipe_api.RecipeApi): ['--gerrit_ref', gerrit_ref], ['--apply_issue_email_file', email_file], ['--apply_issue_key_file', key_file], + ['--apply_issue_oauth2_file', oauth2_json_file], # Hookups to JSON output back into recipes. ['--output_json', self.m.json.output()],] diff --git a/recipe_modules/bot_update/example.expected/trychange_oauth2_json.json b/recipe_modules/bot_update/example.expected/trychange_oauth2_json.json new file mode 100644 index 000000000..0c0277779 --- /dev/null +++ b/recipe_modules/bot_update/example.expected/trychange_oauth2_json.json @@ -0,0 +1,52 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::bot_update]/resources/bot_update.py", + "--spec", + "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]", + "--patch_root", + "src", + "--revision_mapping_file", + "{\"src\": \"got_cr_revision\"}", + "--git-cache-dir", + "[GIT_CACHE]", + "--apply_issue_oauth2_file", + "/creds/refresh_tokens/rietveld.json", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@HEAD" + ], + "env": { + "PATH": "%(PATH)s:RECIPE_PACKAGE_REPO[depot_tools]" + }, + "name": "bot_update", + "~followup_annotations": [ + "@@@STEP_TEXT@Some step text@@@", + "@@@STEP_LOG_LINE@json.output@{@@@", + "@@@STEP_LOG_LINE@json.output@ \"did_run\": true, @@@", + "@@@STEP_LOG_LINE@json.output@ \"fixed_revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"src\": \"HEAD\"@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_failure\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/master@{#170242}\"@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@" + ] + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/bot_update/example.expected/trychange_oauth2_json_win.json b/recipe_modules/bot_update/example.expected/trychange_oauth2_json_win.json new file mode 100644 index 000000000..d084143b6 --- /dev/null +++ b/recipe_modules/bot_update/example.expected/trychange_oauth2_json_win.json @@ -0,0 +1,52 @@ +[ + { + "cmd": [ + "python", + "-u", + "RECIPE_MODULE[depot_tools::bot_update]\\resources\\bot_update.py", + "--spec", + "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]", + "--patch_root", + "src", + "--revision_mapping_file", + "{\"src\": \"got_cr_revision\"}", + "--git-cache-dir", + "[GIT_CACHE]", + "--apply_issue_oauth2_file", + "C:\\creds\\refresh_tokens\\rietveld.json", + "--output_json", + "/path/to/tmp/json", + "--revision", + "src@HEAD" + ], + "env": { + "PATH": "%(PATH)s;RECIPE_PACKAGE_REPO[depot_tools]" + }, + "name": "bot_update", + "~followup_annotations": [ + "@@@STEP_TEXT@Some step text@@@", + "@@@STEP_LOG_LINE@json.output@{@@@", + "@@@STEP_LOG_LINE@json.output@ \"did_run\": true, @@@", + "@@@STEP_LOG_LINE@json.output@ \"fixed_revisions\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"src\": \"HEAD\"@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_failure\": false, @@@", + "@@@STEP_LOG_LINE@json.output@ \"patch_root\": \"src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"properties\": {@@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision\": \"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"got_cr_revision_cp\": \"refs/heads/master@{#170242}\"@@@", + "@@@STEP_LOG_LINE@json.output@ }, @@@", + "@@@STEP_LOG_LINE@json.output@ \"root\": \"src\", @@@", + "@@@STEP_LOG_LINE@json.output@ \"step_text\": \"Some step text\"@@@", + "@@@STEP_LOG_LINE@json.output@}@@@", + "@@@STEP_LOG_END@json.output@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision@\"f27fede2220bcd326aee3e86ddfd4ebd0fe58cb9\"@@@", + "@@@SET_BUILD_PROPERTY@got_cr_revision_cp@\"refs/heads/master@{#170242}\"@@@" + ] + }, + { + "name": "$result", + "recipe_result": null, + "status_code": 0 + } +] \ No newline at end of file diff --git a/recipe_modules/bot_update/example.py b/recipe_modules/bot_update/example.py index 362d1707b..d95fc4d48 100644 --- a/recipe_modules/bot_update/example.py +++ b/recipe_modules/bot_update/example.py @@ -5,6 +5,7 @@ DEPS = [ 'bot_update', 'gclient', + 'recipe_engine/platform', 'recipe_engine/path', 'recipe_engine/properties', ] @@ -30,6 +31,7 @@ def RunSteps(api): with_branch_heads = api.properties.get('with_branch_heads', False) refs = api.properties.get('refs', []) oauth2 = api.properties.get('oauth2', False) + oauth2_json = api.properties.get('oauth2_json', False) root_solution_revision = api.properties.get('root_solution_revision') suffix = api.properties.get('suffix') gerrit_no_reset = True if api.properties.get('gerrit_no_reset') else False @@ -48,6 +50,7 @@ def RunSteps(api): with_branch_heads=with_branch_heads, output_manifest=output_manifest, refs=refs, patch_oauth2=oauth2, + oauth2_json=oauth2_json, clobber=clobber, root_solution_revision=root_solution_revision, suffix=suffix, @@ -78,6 +81,18 @@ def GenTests(api): yield api.test('trychange_oauth2') + api.properties( oauth2=True, ) + yield api.test('trychange_oauth2_json') + api.properties( + mastername='tryserver.chromium.linux', + buildername='linux_rel', + slavename='totallyaslave-c4', + oauth2_json=True, + ) + yield api.test('trychange_oauth2_json_win') + api.properties( + mastername='tryserver.chromium.win', + buildername='win_rel', + slavename='totallyaslave-c4', + oauth2_json=True, + ) + api.platform('win', 64) yield api.test('tryjob_fail') + api.properties( issue=12345, patchset=654321, diff --git a/recipe_modules/bot_update/resources/bot_update.py b/recipe_modules/bot_update/resources/bot_update.py index ab71dd4a9..2323f4c4b 100755 --- a/recipe_modules/bot_update/resources/bot_update.py +++ b/recipe_modules/bot_update/resources/bot_update.py @@ -555,7 +555,8 @@ def _download(url): def apply_rietveld_issue(issue, patchset, root, server, _rev_map, _revision, - email_file, key_file, whitelist=None, blacklist=None): + email_file, key_file, oauth2_file, + whitelist=None, blacklist=None): apply_issue_bin = ('apply_issue.bat' if sys.platform.startswith('win') else 'apply_issue') cmd = [apply_issue_bin, @@ -570,8 +571,10 @@ def apply_rietveld_issue(issue, patchset, root, server, _rev_map, _revision, # Don't run gclient sync when it sees a DEPS change. '--ignore_deps', ] - # Use an oauth key file if specified. - if email_file and key_file: + # Use an oauth key or json file if specified. + if oauth2_file: + cmd.extend(['--auth-refresh-token-json', oauth2_file]) + elif email_file and key_file: cmd.extend(['--email-file', email_file, '--private-key-file', key_file]) else: cmd.append('--no-auth') @@ -714,8 +717,9 @@ def ensure_deps_revisions(deps_url_mapping, solutions, revisions): def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, patch_root, issue, patchset, rietveld_server, gerrit_repo, gerrit_ref, gerrit_rebase_patch_ref, revision_mapping, - apply_issue_email_file, apply_issue_key_file, shallow, refs, - git_cache_dir, gerrit_reset): + apply_issue_email_file, apply_issue_key_file, + apply_issue_oauth2_file, shallow, refs, git_cache_dir, + gerrit_reset): # Get a checkout of each solution, without DEPS or hooks. # Calling git directly because there is no way to run Gclient without # invoking DEPS. @@ -738,7 +742,8 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, if issue: apply_rietveld_issue(issue, patchset, patch_root, rietveld_server, revision_mapping, git_ref, apply_issue_email_file, - apply_issue_key_file, whitelist=[target]) + apply_issue_key_file, apply_issue_oauth2_file, + whitelist=[target]) already_patched.append(target) elif gerrit_ref: apply_gerrit_ref(gerrit_repo, gerrit_ref, patch_root, gerrit_reset, @@ -765,8 +770,9 @@ def ensure_checkout(solutions, revisions, first_sln, target_os, target_os_only, # Apply the rest of the patch here (sans DEPS) if issue: apply_rietveld_issue(issue, patchset, patch_root, rietveld_server, - revision_mapping, git_ref, apply_issue_email_file, - apply_issue_key_file, blacklist=already_patched) + revision_mapping, apply_issue_oauth2_file, git_ref, + apply_issue_email_file, apply_issue_key_file, + blacklist=already_patched) elif gerrit_ref and not applied_gerrit_patch: # If gerrit_ref was for solution's main repository, it has already been # applied above. This chunk is executed only for patches to DEPS-ed in @@ -835,6 +841,9 @@ def parse_args(): parse.add_option('--apply_issue_key_file', help='--private-key-file option passthrough for ' 'apply_patch.py.') + parse.add_option('--apply_issue_oauth2_file', + help='--auth-refresh-token-json option passthrough for ' + 'apply_patch.py.') parse.add_option('--root', dest='patch_root', help='DEPRECATED: Use --patch_root.') parse.add_option('--patch_root', help='Directory to patch on top of.') @@ -972,6 +981,7 @@ def checkout(options, git_slns, specs, revisions, step_text, shallow): revision_mapping=options.revision_mapping, apply_issue_email_file=options.apply_issue_email_file, apply_issue_key_file=options.apply_issue_key_file, + apply_issue_oauth2_file=options.apply_issue_oauth2_file, # Finally, extra configurations such as shallowness of the clone. shallow=shallow, diff --git a/tests/bot_update_coverage_test.py b/tests/bot_update_coverage_test.py index efbc8e538..f055a9ba6 100755 --- a/tests/bot_update_coverage_test.py +++ b/tests/bot_update_coverage_test.py @@ -37,6 +37,7 @@ DEFAULT_PARAMS = { 'revision_mapping': {}, 'apply_issue_email_file': None, 'apply_issue_key_file': None, + 'apply_issue_oauth2_file': None, 'shallow': False, 'refs': [], 'git_cache_dir': '',