-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1889223 - Submit all files to notarization before polling #957
Changes from all commits
ef6d833
f69f61e
a1e0009
c26bafa
df02a5c
7925b96
12d3b77
9eac5fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
import scriptworker.client | ||
|
||
from signingscript.exceptions import SigningScriptError | ||
from signingscript.task import build_filelist_dict, sign, task_cert_type, task_signing_formats | ||
from signingscript.task import apple_notarize_stacked, build_filelist_dict, sign, task_cert_type, task_signing_formats | ||
from signingscript.utils import copy_to_dir, load_apple_notarization_configs, load_autograph_configs | ||
|
||
log = logging.getLogger(__name__) | ||
|
@@ -24,6 +24,7 @@ async def async_main(context): | |
context (Context): the signing context. | ||
|
||
""" | ||
work_dir = context.config["work_dir"] | ||
async with aiohttp.ClientSession() as session: | ||
all_signing_formats = task_signing_formats(context) | ||
if "gpg" in all_signing_formats or "autograph_gpg" in all_signing_formats: | ||
|
@@ -36,17 +37,24 @@ async def async_main(context): | |
if not context.config.get("widevine_cert"): | ||
raise Exception("Widevine format is enabled, but widevine_cert is not defined") | ||
|
||
if "apple_notarization" in all_signing_formats or "apple_notarization_geckodriver" in all_signing_formats: | ||
if {"apple_notarization", "apple_notarization_geckodriver", "apple_notarization_stacked"}.intersection(all_signing_formats): | ||
if not context.config.get("apple_notarization_configs", False): | ||
raise Exception("Apple notarization is enabled but apple_notarization_configs is not defined") | ||
setup_apple_notarization_credentials(context) | ||
|
||
context.session = session | ||
context.autograph_configs = load_autograph_configs(context.config["autograph_configs"]) | ||
|
||
work_dir = context.config["work_dir"] | ||
# TODO: Make task.sign take in the whole filelist_dict and return a dict of output files. | ||
# That would likely mean changing all behaviors to accept and deal with multiple files at once. | ||
|
||
filelist_dict = build_filelist_dict(context) | ||
for path, path_dict in filelist_dict.items(): | ||
if path_dict["formats"] == ["apple_notarization_stacked"]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this actually function correctly if some files are not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I'll disallow mixed formats when stacked is present There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did disallowing mixed formats get implemented? |
||
# Skip if only format is notarization_stacked - handled below | ||
continue | ||
if "apple_notarization_stacked" in path_dict["formats"]: | ||
raise SigningScriptError("apple_notarization_stacked cannot be mixed with other signing types") | ||
copy_to_dir(path_dict["full_path"], context.config["work_dir"], target=path) | ||
log.info("signing %s", path) | ||
output_files = await sign(context, os.path.join(work_dir, path), path_dict["formats"], authenticode_comment=path_dict.get("comment")) | ||
|
@@ -55,6 +63,16 @@ async def async_main(context): | |
copy_to_dir(os.path.join(work_dir, source), context.config["artifact_dir"], target=source) | ||
if "gpg" in path_dict["formats"] or "autograph_gpg" in path_dict["formats"]: | ||
copy_to_dir(context.config["gpg_pubkey"], context.config["artifact_dir"], target="public/build/KEY") | ||
|
||
# notarization_stacked is a special format that takes in all files at once instead of sequentially like other formats | ||
# Should be fixed in https://github.com/mozilla-releng/scriptworker-scripts/issues/980 | ||
notarization_dict = {path: path_dict for path, path_dict in filelist_dict.items() if "apple_notarization_stacked" in path_dict["formats"]} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is using a new format for this just to make it safer to roll out? If so, what are the plans for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I initially replaced There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll leave geckodriver alone for now, and refactor that as we move things around later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's get an issue on file to remove this later, too. |
||
if notarization_dict: | ||
output_files = await apple_notarize_stacked(context, notarization_dict) | ||
for source in output_files: | ||
source = os.path.relpath(source, work_dir) | ||
copy_to_dir(os.path.join(work_dir, source), context.config["artifact_dir"], target=source) | ||
|
||
log.info("Done!") | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1604,7 +1604,11 @@ async def _notarize_geckodriver(context, path, workdir): | |
|
||
|
||
async def _notarize_all(context, path, workdir): | ||
"""Notarizes all files in a tarball""" | ||
""" | ||
Notarizes all files in a tarball | ||
@Deprecated: This function is deprecated and will be removed in the future. Use apple_notarize_stacked instead. | ||
""" | ||
_, extension = os.path.splitext(path) | ||
# Attempt extracting | ||
await _extract_tarfile(context, path, extension, tmp_dir=workdir) | ||
|
@@ -1634,10 +1638,11 @@ async def _notarize_all(context, path, workdir): | |
async def apple_notarize(context, path, *args, **kwargs): | ||
""" | ||
Notarizes given package(s) using rcodesign. | ||
@Deprecated: This function is deprecated and will be removed in the future. Use apple_notarize_stacked instead. | ||
""" | ||
# Setup workdir | ||
notarization_workdir = os.path.join(context.config["work_dir"], "apple_notarize") | ||
shutil.rmtree(notarization_workdir, ignore_errors=True) | ||
utils.mkdir(notarization_workdir) | ||
|
||
_, extension = os.path.splitext(path) | ||
|
@@ -1658,3 +1663,88 @@ async def apple_notarize_geckodriver(context, path, *args, **kwargs): | |
utils.mkdir(notarization_workdir) | ||
|
||
return await _notarize_geckodriver(context, path, notarization_workdir) | ||
|
||
|
||
@time_async_function | ||
async def apple_notarize_stacked(context, filelist_dict): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a lot of overlap between this function and |
||
""" | ||
Notarizes multiple packages using rcodesign. | ||
Submits everything before polling for status. | ||
""" | ||
ATTEMPTS = 5 | ||
|
||
relpath_index_map = {} | ||
paths_to_notarize = [] | ||
task_index = 0 | ||
|
||
# Create list of files to be notarized + check for potential problems | ||
for relpath, path_dict in filelist_dict.items(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I'm wrong, but I seem to recall that we can upload a zipfile with multiple files we want notarized inside. Assuming that's true, I suggest we construct a new zipfile and make a single request rather than making a bunch of smaller ones. (This would avoid potential issues with rate limiting or similar problems.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For what I've gathered it's supported - but on the apple side it will check the apps sequentially, making things slower. |
||
task_index += 1 | ||
relpath_index_map[relpath] = task_index | ||
notarization_workdir = os.path.join(context.config["work_dir"], f"apple_notarize-{task_index}") | ||
shutil.rmtree(notarization_workdir, ignore_errors=True) | ||
utils.mkdir(notarization_workdir) | ||
_, extension = os.path.splitext(relpath) | ||
if extension == ".pkg": | ||
path = os.path.join(notarization_workdir, relpath) | ||
utils.copy_to_dir(path_dict["full_path"], notarization_workdir, target=relpath) | ||
paths_to_notarize.append(path) | ||
elif extension == ".gz": | ||
await _extract_tarfile(context, path_dict["full_path"], extension, notarization_workdir) | ||
workdir_files = os.listdir(notarization_workdir) | ||
supported_files = [filename for filename in workdir_files if _can_notarize(filename, (".app", ".pkg"))] | ||
if not supported_files: | ||
raise SigningScriptError(f"No supported files found for file {relpath}") | ||
for file in supported_files: | ||
path = os.path.join(notarization_workdir, file) | ||
paths_to_notarize.append(path) | ||
else: | ||
raise SigningScriptError(f"Unsupported file extension: {extension} for file {relpath}") | ||
|
||
# notarization submissions map (path -> submission_id) | ||
submissions_map = {} | ||
# Submit to notarization one by one | ||
for path in paths_to_notarize: | ||
submissions_map[path] = await retry_async( | ||
func=rcodesign_notarize, | ||
args=(path, context.apple_credentials_path), | ||
attempts=ATTEMPTS, | ||
retry_exceptions=RCodesignError, | ||
) | ||
|
||
# Notary wait all files | ||
for path, submission_id in submissions_map.items(): | ||
await retry_async( | ||
func=rcodesign_notary_wait, | ||
args=(submission_id, context.apple_credentials_path), | ||
attempts=ATTEMPTS, | ||
retry_exceptions=RCodesignError, | ||
) | ||
|
||
# Staple files | ||
for path in submissions_map.keys(): | ||
await retry_async( | ||
func=rcodesign_staple, | ||
args=[path], | ||
attempts=ATTEMPTS, | ||
retry_exceptions=RCodesignError, | ||
) | ||
|
||
# Wrap up | ||
stapled_files = [] | ||
for relpath, path_dict in filelist_dict.items(): | ||
task_index = relpath_index_map[relpath] | ||
notarization_workdir = os.path.join(context.config["work_dir"], f"apple_notarize-{task_index}") | ||
target_path = os.path.join(context.config["work_dir"], relpath) | ||
_, extension = os.path.splitext(relpath) | ||
# Pkgs don't need to be tarred | ||
if extension == ".pkg": | ||
utils.copy_to_dir(os.path.join(notarization_workdir, relpath), os.path.dirname(target_path)) | ||
else: | ||
all_files = [] | ||
for root, _, files in os.walk(notarization_workdir): | ||
for f in files: | ||
all_files.append(os.path.join(root, f)) | ||
await _create_tarfile(context, target_path, all_files, extension, notarization_workdir) | ||
stapled_files.append(target_path) | ||
return stapled_files |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ | |
from signingscript.sign import ( | ||
apple_notarize, | ||
apple_notarize_geckodriver, | ||
apple_notarize_stacked, # noqa: F401 | ||
sign_authenticode, | ||
sign_debian_pkg, | ||
sign_file, | ||
|
@@ -33,6 +34,7 @@ | |
|
||
log = logging.getLogger(__name__) | ||
|
||
|
||
FORMAT_TO_SIGNING_FUNCTION = immutabledict( | ||
{ | ||
"autograph_hash_only_mar384": sign_mar384_with_autograph_hash, | ||
|
@@ -58,6 +60,9 @@ | |
"autograph_rsa": sign_file_detached, | ||
"apple_notarization": apple_notarize, | ||
"apple_notarization_geckodriver": apple_notarize_geckodriver, | ||
# This format is handled in script.py | ||
# Should be refactored in https://github.com/mozilla-releng/scriptworker-scripts/issues/980 | ||
# "apple_notarization_stacked": apple_notarize_stacked, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be able to map this format to a function once the first todo above (about |
||
"default": sign_file, | ||
} | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,6 @@ def mkdir(path): | |
Args: | ||
path (str): the path to mkdir | ||
""" | ||
try: | ||
os.makedirs(path) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get an issue on file for this, please.