1239778 - release sanity: check en-US binaries

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[Rail Aliiev [:rail]]

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

Comment 1

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Puppet changes to include gpg key path

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

Comment 2

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Puppet changes to include gpg key path

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

Comment 3

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Add release sanity for en-US binaries

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

Comment 4

[Rail Aliiev [:rail]]

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 5

[Mihai Tabara [:mtabara]⌚️GMT]

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

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

Attachment: Add release sanity for en-US binaries

Refactoring upon Rail's review.

Comment 7

[Mihai Tabara [:mtabara]⌚️GMT]

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

Comment 8

[Mihai Tabara [:mtabara]⌚️GMT]

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.