release sanity: check en-US binaries

RESOLVED FIXED

Status

Release Engineering
Release Automation
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: rail, Assigned: mtabara)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
* they should be accessible
* the signatures should be valid
* SourceRepository and SourceStamp in application.ini should match their ship-it entries
(Assignee)

Updated

a year ago
Assignee: nobody → mtabara
(Assignee)

Comment 1

a year ago
Created attachment 8724100 [details] [diff] [review]
Puppet changes to include gpg key path

We need to toggle between production and staging GPG keys easily.
Attachment #8724100 - Flags: review?(rail)
(Reporter)

Updated

a year ago
Attachment #8724100 - Flags: review?(rail) → review+
(Assignee)

Comment 2

a year ago
Created attachment 8724101 [details] [diff] [review]
Puppet changes to include gpg key path

We need to toggle between production and staging GPG keys easily.
Attachment #8724100 - Attachment is obsolete: true
Attachment #8724101 - Flags: review?(rail)
(Reporter)

Updated

a year ago
Attachment #8724101 - Flags: review?(rail) → review+
(Assignee)

Comment 3

a year ago
Created attachment 8724102 [details] [diff] [review]
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
Attachment #8724102 - Flags: review?(rail)
(Reporter)

Comment 4

a year ago
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"
Attachment #8724102 - Flags: review?(rail)
(Assignee)

Comment 5

a year ago
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
Attachment #8724101 - Flags: checked-in+
(Assignee)

Comment 6

a year ago
Created attachment 8724245 [details] [diff] [review]
Add release sanity for en-US binaries

Refactoring upon Rail's review.
Attachment #8724102 - Attachment is obsolete: true
Attachment #8724245 - Flags: review?(rail)
(Reporter)

Updated

a year ago
Attachment #8724245 - Flags: review?(rail) → review+
(Assignee)

Comment 7

a year ago
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
Attachment #8724245 - Flags: checked-in+
(Assignee)

Comment 8

a year ago
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.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.