1346015 - Support DMG input in signingscriptworker

Status: RESOLVED FIXED

(Release Engineering :: General, enhancement, P1)

Description

[Justin Wood (:Callek)]

Over in https://bugzilla.mozilla.org/show_bug.cgi?id=1324834 we identified the commands we need for OSX unpacking on linux...
'
these amount to:

dmg extract "${DMG_PATH}" firefox.hfs
hfsplus firefox.hfs extractall / "${TARGETPATH}"


both binaries are in tooltool https://dxr.mozilla.org/mozilla-central/source/browser/config/tooltool-manifests/macosx64/cross-releng.manifest#42  (aka: https://api.pub.build.mozilla.org/tooltool/sha512/9073c41034784eb8823ec817aed42bbc65c8da63ad3fac572726fa48b36320ee302ca8f51b23576e7fdbeec6ab300610d0c58bbd9c52024577dfdb13d95aa2ec )

`firefox.hfs` can be any tempfile (unsure if extension matters) and removable after this executes.

$DMG_PATH and $TARGETPATH are as they sound.

Comment 2

[Aki Sasaki (not active)]

Attachment: puppet: dmgv2-to-dmg.diff

Since we're using a standalone version of signtool, we can get rid of the dmgv2 vs dmg split, which was only put in place to specify different pools of mac signing servers.

Comment 3

[Aki Sasaki (not active)]

https://github.com/escapewindow/signtool/tree/mac for py3 signtool to take a tarball and return a tarball.

- update the puppet patch to install the dmg extraction tools onto signing scriptworkers
- update signingscript to use the dmg extraction tool, and signtool/upload a tarball, if format is dmg

We can probably deploy to prod, since nothing is using signing scriptworkers for dmg.  Otherwise we'd need to spin up a dmg signing scriptworker for testing.

Comment 4

[Aki Sasaki (not active)]

https://github.com/mozilla-releng/signingscript/pull/25
https://github.com/mozilla-releng/signtool/pull/2

Comment 5

[Aki Sasaki (not active)]

https://github.com/escapewindow/build-puppet/tree/macsigning . I think this is ready for testing.

Comment 6

[Aki Sasaki (not active)]

Attachment: py3 signtool dmg support

:catlee, do you want to review this py3 signtool patch, or do you want me to send it elsewhere?

Sending it your way first since you helped me with this in Toronto IIRC, but I'm happy to redirect if wanted.

Comment 7

[Aki Sasaki (not active)]

Attachment: signingscript dmg support

- adds an _explode_dmg call in a new _execute_pre_signing_steps
- sign_file now returns the source path; this allows us to rewrite the path/to/dmg with a path/to/tarball for async_main to copy to the artifact_dir
- moves _execute_subprocess to utils
- maintains 100% code coverage

Comment 8

[Aki Sasaki (not active)]

Attachment: bug 1346015 - support dmg signing in signing scriptworkers.

Review commit: https://reviewboard.mozilla.org/r/122890/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122890/

Comment 9

[Aki Sasaki (not active)]

Attachment: bug 1346015 - allow for signing scriptworker dev instances

Review commit: https://reviewboard.mozilla.org/r/122898/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122898/

Comment 10

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8850264 [details]
bug 1346015 - allow for signing scriptworker dev instances

https://reviewboard.mozilla.org/r/122898/#review125302

::: modules/signing_scriptworker/manifests/init.pp:65
(Diff revision 1)
>              ];
>      }
>  
>      scriptworker::instance {
>          "${signing_scriptworker::settings::root}":
> -            instance_name            => $module_name,
> +            instance_name            => "${module_name}",

What's the advantage of quoting single variables in this case?

For reference, in the puppet training I attended, the trainer told us to avoid using them whenever that's possible. It saves the interpolation step, which (apparently) can save several seconds when a manifest is compiled on master.

That said, I haven't managed to find these explanation in the official documentation:

* https://docs.puppet.com/puppet/3.7/lang_variables.html#interpolation

::: modules/signing_scriptworker/templates/dev-passwords.json.erb:3
(Diff revision 1)
> +{
> +    "project:releng:signing:cert:dep-signing": [
> +        ["signing4.srv.releng.scl3.mozilla.com:9110", "<%= scope.function_secret(["signing_server_username"]) %>", "<%= scope.function_secret(["signing_server_dep_password"]) %>", ["gpg", "sha2signcode", "sha2signcodestub", "osslsigncode", "signcode", "mar", "mar_sha384", "mar_sha384", "jar", "emevoucher"]],

Are these instances meant to become production at some point? If so, I'd add a comment, maybe in settings.pp. If not, could we add "-dev" on the hostname?

Comment 11

[Johan Lorenzo [:jlorenzo]]

(In reply to Johan Lorenzo [:jlorenzo] from comment #10)
The puppet patch looks good to me, modulo a couple of nits. These can be handled in a follow up.

Comment 12

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8850241 [details] [review]
signingscript dmg support

https://github.com/mozilla-releng/signingscript/pull/26#pullrequestreview-28605039

Comment 13

[Justin Wood (:Callek)]

Comment on attachment 8850243 [details]
bug 1346015 - support dmg signing in signing scriptworkers.

https://reviewboard.mozilla.org/r/122890/#review125454

I'd be much happier with the binaries placed into the puppetmasters /data https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Data#.._add_a_data_file even if its a zip/compressed format that the instance decompresses.

I'm not going to hard block on that though.

Comment 14

[Aki Sasaki (not active)]

(In reply to Justin Wood (:Callek) from comment #13)
> Comment on attachment 8850243 [details]
> bug 1346015 - support dmg signing in signing scriptworkers.
> 
> https://reviewboard.mozilla.org/r/122890/#review125454
> 
> I'd be much happier with the binaries placed into the puppetmasters /data
> https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Data#..
> _add_a_data_file even if its a zip/compressed format that the instance
> decompresses.
> 
> I'm not going to hard block on that though.

That makes sense. What's the best way to tell puppet to overwrite the files if there's something newer?
If the answer is "ssh in and blow away the previous file and rerun puppet", then I'd prefer this route until the dmg tools are more settled... they're ~260KB total, not too onerous atm.

Comment 15

[Aki Sasaki (not active)]

(In reply to Johan Lorenzo [:jlorenzo] from comment #10)
> Comment on attachment 8850264 [details]
> bug 1346015 - allow for signing scriptworker dev instances
> 
> https://reviewboard.mozilla.org/r/122898/#review125302
> 
> ::: modules/signing_scriptworker/manifests/init.pp:65
> (Diff revision 1)
> >              ];
> >      }
> >  
> >      scriptworker::instance {
> >          "${signing_scriptworker::settings::root}":
> > -            instance_name            => $module_name,
> > +            instance_name            => "${module_name}",
> 
> What's the advantage of quoting single variables in this case?

Explicitly stating they're strings?  I was just going for consistency.

> 
> For reference, in the puppet training I attended, the trainer told us to
> avoid using them whenever that's possible. It saves the interpolation step,
> which (apparently) can save several seconds when a manifest is compiled on
> master.
> 
> That said, I haven't managed to find these explanation in the official
> documentation:
> 
> * https://docs.puppet.com/puppet/3.7/lang_variables.html#interpolation

Hm, good to know.  Maybe I'll revert these changes.

> ::: modules/signing_scriptworker/templates/dev-passwords.json.erb:3
> (Diff revision 1)
> > +{
> > +    "project:releng:signing:cert:dep-signing": [
> > +        ["signing4.srv.releng.scl3.mozilla.com:9110", "<%= scope.function_secret(["signing_server_username"]) %>", "<%= scope.function_secret(["signing_server_dep_password"]) %>", ["gpg", "sha2signcode", "sha2signcodestub", "osslsigncode", "signcode", "mar", "mar_sha384", "mar_sha384", "jar", "emevoucher"]],
> 
> Are these instances meant to become production at some point? If so, I'd add
> a comment, maybe in settings.pp. If not, could we add "-dev" on the hostname?

No, these are the production signing servers. They each run 3 ports: release, nightly, dep, so renaming them isn't really an option.  Moving the dep signing servers onto different instances is an option, but isn't a short-term thing.  For the long term fix, we're leaning towards somehow being able to sign locally -- docker-signing-server might be a good option, or tc-secrets, or somehow follow how android creates local signing keys for dev signing.  Our current thinking is dep/CI signing keys should essentially be untrusted and shared widely.

Comment 16

[Aki Sasaki (not active)]

https://hg.mozilla.org/build/puppet/rev/2579c487584788d39c81ed7b1ddb8ddca23b8b44
bug 1346015 - support dmg signing in signing scriptworkers. r=callek

https://hg.mozilla.org/build/puppet/rev/c28f866ce0bb4aba88c556b17c8db89b84997b36
bug 1346015 - allow for signing scriptworker dev instances r=jlorenzo

https://hg.mozilla.org/build/puppet/rev/6745cb294e553005eb087d49b2fdd31f9c58e93f
bug 1346015 - bump signing scriptworker versions. r=callek

Comment 17

[Aki Sasaki (not active)]

https://hg.mozilla.org/build/puppet/rev/ced6ac1f76d64b5db1cd5a41b8f133d65eb2318c
bug 1346015 - allow for signing scriptworker dev instances r=jlorenzo

https://hg.mozilla.org/build/puppet/rev/d5e86776f695142bc821db7138a4fc605d62c538
bug 1346015 - bump signing scriptworker versions. r=callek

https://hg.mozilla.org/build/puppet/rev/94a44004e74f2ae79f2a05d8806c081ae5e7a5a8
bug 1346015 - support dmg signing in signing scriptworkers. r=callek

Comment 18

[Aki Sasaki (not active)]

Looks like the dmg signing on date is working, and I was able to launch the app from the signed tarball.