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)

enhancement
Not set
normal

Tracking

(firefox56 fixed, firefox57 fixed)

RESOLVED FIXED
Tracking Status
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: sfraser, Assigned: sfraser)

Details

Attachments

(4 files)

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: nobody → sfraser
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)
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)
Attachment #8892081 - Flags: review?(catlee) → review+
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 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+
(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.
(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
With suggested updates, and new keys
Attachment #8892423 - Flags: review?(catlee)
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 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+
Pushed by sfraser@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e1b93d2abdb2
Final update to funsize docker images r=catlee
https://hg.mozilla.org/mozilla-central/rev/e1b93d2abdb2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.