Closed
Bug 1385946
Opened 7 years ago
Closed 7 years ago
Update funsize to cope with migration to lzma and sha384
Categories
(Release Engineering :: Release Automation: Other, enhancement)
Release Engineering
Release Automation: Other
Tracking
(firefox56 fixed, firefox57 fixed)
RESOLVED
FIXED
People
(Reporter: sfraser, Assigned: sfraser)
Details
Attachments
(4 files)
2.73 KB,
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
9.30 KB,
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
16.75 KB,
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
59 bytes,
text/x-review-board-request
|
catlee
:
review+
|
Details |
mar tools will behave differently if MAR_OLD_FORMAT environment variable is set, now that they support lzma compression. Adjust funsize scheduler and in-tree to cope with this change, and to support sha384 signing.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sfraser
Assignee | ||
Comment 1•7 years ago
|
||
Update funsize scheduler to look at dependencies to work out the signing format, using the same method as the signing scriptworkers. Ideally this should be added to balrog_props.json, but that's created before the signing workers, and the task that creates it has no knowledge of the signing formats. Having multiple different balrog_props may lead to confusion about which one to check.
Attachment #8892081 -
Flags: review?(catlee)
Assignee | ||
Comment 2•7 years ago
|
||
This feels awkward, with the signature files, but should otherwise catch both old and new formats and work properly with both.
Attachment #8892082 -
Flags: review?(catlee)
Updated•7 years ago
|
Attachment #8892081 -
Flags: review?(catlee) → review+
Comment 3•7 years ago
|
||
Comment on attachment 8892081 [details] [diff] [review] funsize_scheduler_lzma.diff Review of attachment 8892081 [details] [diff] [review]: ----------------------------------------------------------------- ::: funsize/tasks/funsize.yml @@ +70,5 @@ > - /runme.sh > > env: > + SHA1_SIGNING_CERT: 'nightly' > + SHA384_SIGNING_CERT: 'nightly' I think we need different certs for each format, don't we?
Comment 4•7 years ago
|
||
Comment on attachment 8892082 [details] [diff] [review] m-c-lzma_funsize.diff Review of attachment 8892082 [details] [diff] [review]: ----------------------------------------------------------------- ::: taskcluster/docker/funsize-update-generator/scripts/funsize.py @@ +70,4 @@ > unwrap_cmd = sh.Command(os.path.join(work_env.workdir, > "unwrap_full_update.pl")) > log.debug("Unwrapping %s", mar) > + env = work_env.env need a .copy() here? @@ +72,5 @@ > log.debug("Unwrapping %s", mar) > + env = work_env.env > + if not is_lzma_compressed_mar(mar): > + env['MAR_OLD_FORMAT'] = 1 > + elif env.get('MAR_OLD_FORMAT'): what's the purpose of this block? just making sure that MAR_OLD_FORMAT is unset? should be `elif 'MAR_OLD_FORMAT in env:` in that case. @@ +109,5 @@ > env["MOZ_CHANNEL_ID"] = channel_ids > + if not is_lzma_compressed_mar(dest_mar): > + env['MAR_OLD_FORMAT'] = 1 > + elif env.get('MAR_OLD_FORMAT'): > + del env['MAR_OLD_FORMAT'] same comments here. @@ +201,5 @@ > > + signing_certs = { > + 'sha1': args.sha1_signing_cert, > + 'sha384': args.sha384_signing_cert, > + } I wonder if we can test loading the certs on start, just to make sure they're correctly formatted. e.g. assert that mardor.signing.get_keysize(open(args.sha1_signing_cert).read()) == 2048 mardor.signing.get_keysize(open(args.sha384_signing_cert).read()) == 4096 or raise an error.
Attachment #8892082 -
Flags: review?(catlee) → review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Chris AtLee [:catlee] from comment #3) > Comment on attachment 8892081 [details] [diff] [review] > funsize_scheduler_lzma.diff > > Review of attachment 8892081 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: funsize/tasks/funsize.yml > @@ +70,5 @@ > > - /runme.sh > > > > env: > > + SHA1_SIGNING_CERT: 'nightly' > > + SHA384_SIGNING_CERT: 'nightly' > > I think we need different certs for each format, don't we? We do, they just haven't been deployed yet, so I don't know what the new one is called.
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Chris AtLee [:catlee] from comment #4) > Comment on attachment 8892082 [details] [diff] [review] > m-c-lzma_funsize.diff > > Review of attachment 8892082 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: taskcluster/docker/funsize-update-generator/scripts/funsize.py > @@ +70,4 @@ > > unwrap_cmd = sh.Command(os.path.join(work_env.workdir, > > "unwrap_full_update.pl")) > > log.debug("Unwrapping %s", mar) > > + env = work_env.env > > need a .copy() here? .env is a property that does `os.environ.copy()` and adds a few values to it, so it'll always be a fresh dict > > @@ +72,5 @@ > > log.debug("Unwrapping %s", mar) > > + env = work_env.env > > + if not is_lzma_compressed_mar(mar): > > + env['MAR_OLD_FORMAT'] = 1 > > + elif env.get('MAR_OLD_FORMAT'): > > what's the purpose of this block? just making sure that MAR_OLD_FORMAT is > unset? > > should be `elif 'MAR_OLD_FORMAT in env:` in that case. Fair comment! > @@ +109,5 @@ > > env["MOZ_CHANNEL_ID"] = channel_ids > > + if not is_lzma_compressed_mar(dest_mar): > > + env['MAR_OLD_FORMAT'] = 1 > > + elif env.get('MAR_OLD_FORMAT'): > > + del env['MAR_OLD_FORMAT'] > > same comments here. > > @@ +201,5 @@ > > > > + signing_certs = { > > + 'sha1': args.sha1_signing_cert, > > + 'sha384': args.sha384_signing_cert, > > + } > > I wonder if we can test loading the certs on start, just to make sure > they're correctly formatted. > > e.g. assert that > > mardor.signing.get_keysize(open(args.sha1_signing_cert).read()) == 2048 > mardor.signing.get_keysize(open(args.sha384_signing_cert).read()) == 4096 > > or raise an error. Sure, sounds good
Assignee | ||
Comment 7•7 years ago
|
||
With suggested updates, and new keys
Attachment #8892423 -
Flags: review?(catlee)
Updated•7 years ago
|
Attachment #8892423 -
Flags: review?(catlee) → review+
Pushed by sfraser@mozilla.com: https://hg.mozilla.org/mozilla-central/rev/ef9a0f01e4f6 Update funsize to enable both sha1 and sha384 signing formats a=catlee DONTBUILD
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8892647 [details] bug 1385946 Final update to funsize docker images https://reviewboard.mozilla.org/r/163636/#review168996
Attachment #8892647 -
Flags: review?(catlee) → review+
Comment 11•7 years ago
|
||
Pushed by sfraser@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e1b93d2abdb2 Final update to funsize docker images r=catlee
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e1b93d2abdb2
Comment 13•7 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/dcf3c9806427
status-firefox56:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•