* they should be accessible * the signatures should be valid * SourceRepository and SourceStamp in application.ini should match their ship-it entries

We need to toggle between production and staging GPG keys easily.

We need to toggle between production and staging GPG keys easily.

Added release sanity check for en-US binaries * gpg signatures validation * file existence tests * checksums validation per artifacts

Comment on attachment 8724102 [details] [diff] [review] Add release sanity for en-US binaries Review of attachment 8724102 [details] [diff] [review]: ----------------------------------------------------------------- In overall it looks very good. It'd be great to address the following comments to make our lives easier. ::: buildfarm/release/release-runner.py @@ +201,5 @@ > > +def validate_signatures(checksums, signature, dir_path, gpg_key_path): > + # public key must lie within release runner script > + cmd = ['gpg', '--batch', '--homedir', dir_path, '--import', gpg_key_path] > + ret = subprocess.Popen(cmd, stdout=open(os.devnull, 'wb'), using subprocess.check_call() here would be easier, to need to .communicate() @@ +213,5 @@ > + stdin=open(os.devnull, 'rb'), > + stderr=open(os.devnull, 'wb')).communicate() > + if not ret: > + log.error("gpg: failed to validate signing", exc_info=True) > + sys.exit(1) Can you use "raise SomeException" instead of exit()? It'd be better to accumulate all error somewhere up in the call stack, so you see all of them. @@ +230,5 @@ > + return _dict > + > + > +def download_all_artifacts(queue, artifacts, task_id, dir_path): > + for artifact in artifacts: If you fail to download a file, you won't be able to tell if the rest has no issues. Can you add logic which tries to download all files, reports failures and raises an exception at the end if there are errors? @@ +241,5 @@ > + if 'checksums' in name: > + continue > + > + log.debug('Downloading %s', name) > + r = requests.get(build_url) Can you explicitly set timeout here to prevent unresponsive releaserunner? @@ +255,5 @@ > + computed_hash = get_hash(filepath) > + correct_hash = _dict[name] > + if computed_hash != correct_hash: > + log.error("failed to validate checksums for %s", name, exc_info=True) > + sys.exit(1) s/exit/raise .../ @@ +268,5 @@ > + # iterate in artifacts and grab checksums and its signature only > + log.info("Retrieve the checksums file and its signature ...") > + for artifact in artifacts: > + name = os.path.basename(artifact['name']) > + if 'checksums' not in name: I'd be more explicit: if not (name.endswith(".checksums") or name.endswith(".checksums.asc")): ... It's longer, but it'd be easier to read in the future. @@ +275,5 @@ > + 'getLatestArtifact', > + task_id, > + artifact['name'] > + ) > + r = requests.get(build_url) timeout here too @@ +302,5 @@ > + log.info("Validating checksums for each artifact ...") > + validate_checksums(sha512_dict, tempdir) > + log.info("All checksums validated!") > + > + # remote entire playground before moving forward "remove"

Comment on attachment 8724101 [details] [diff] [review] Puppet changes to include gpg key path Review of attachment 8724101 [details] [diff] [review]: ----------------------------------------------------------------- http://hg.mozilla.org/build/puppet/rev/31892881348d

Refactoring upon Rail's review.

Comment on attachment 8724245 [details] [diff] [review] Add release sanity for en-US binaries Review of attachment 8724245 [details] [diff] [review]: ----------------------------------------------------------------- http://hg.mozilla.org/build/tools/rev/aa178bedc233

Pushed code to staging release-runner. All done here - will file separate bugs for any related improvements/nice-to-haves in terms or release-sanity.