Closed Bug 1335370 Opened 7 years ago Closed 7 years ago

[tc-migration] Cannot upload APKs onto Google Play Store: Signed APKs are not Zip aligned

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jlorenzo, Assigned: jlorenzo)

References

Details

Attachments

(2 files)

When uploading APKs[1] onto Google Play, this error occurs:

> 2017-01-31 13:45:20,010 - discovery.py - INFO - URL being requested: POST https://www.googleapis.com/upload/androidpublisher/v2/applications/org.mozilla.fennec_aurora/edits/12605206835735614028/apks?uploadType=media&alt=json
> 2017-01-31 13:45:34,837 - http.py - WARNING - Encountered 403 Forbidden with reason "apkNotZipAligned"
> Traceback (most recent call last):
>   File "./mozapkpublisher/push_apk.py", line 119, in <module>
>     main(__name__)
>   File "./mozapkpublisher/push_apk.py", line 112, in main
>     PushAPK().run()
>   File "./mozapkpublisher/push_apk.py", line 84, in run
>     self.upload_apks(apks)
>   File "./mozapkpublisher/push_apk.py", line 65, in upload_apks
>     apk_response = edit_service.upload_apk(apk_file_name)
>   File "/home/jlorenzo/.virtualenvs/mozapkpublisher/lib/python3.5/site-packages/mozapkpublisher-0.1.4-py3.5.egg/mozapkpublisher/googleplay.py", line 68, in _transaction_required
>     return method(*args, **kwargs)
>   File "/home/jlorenzo/.virtualenvs/mozapkpublisher/lib/python3.5/site-packages/mozapkpublisher-0.1.4-py3.5.egg/mozapkpublisher/googleplay.py", line 88, in upload_apk
>     media_body=apk_path
>   File "/home/jlorenzo/.virtualenvs/mozapkpublisher/lib/python3.5/site-packages/oauth2client-3.0.0-py3.5.egg/oauth2client/util.py", line 137, in positional_wrapper
>   File "/home/jlorenzo/.virtualenvs/mozapkpublisher/lib/python3.5/site-packages/google_api_python_client-1.5.3-py3.5.egg/googleapiclient/http.py", line 838, in execute
> googleapiclient.errors.HttpError: <HttpError 403 when requesting https://www.googleapis.com/upload/androidpublisher/v2/applications/org.mozilla.fennec_aurora/edits/12605206835735614028/apks?uploadType=media&alt=json returned "APK is not zip aligned.">

This can be reproduce when APK are sent via mozapkpublisher[2] or manually via the webapp[3]. Based on the Android documentation, it seems one step is missing after jarsigner has been executed: We need to run zipalign[4]

As a short term solution, I'm going to run it manually for the APK at [1].

[1] For instance https://queue.taskcluster.net/v1/task/UJTNCy_QQmW-LmO52x1eYw/runs/0/artifacts/public/build/target.apk and https://queue.taskcluster.net/v1/task/MKmgFx3BSCO4tFJmAirr_A/runs/0/artifacts/public/build/target.apk
[2] https://github.com/mozilla-releng/mozapkpublisher
[3] https://play.google.com/apps/publish#ApkPlace:p=org.mozilla.fennec_aurora
[4] https://developer.android.com/studio/command-line/zipalign.html
Summary: [tc-migration] Signed APKs are not Zip aligned → [tc-migration] Cannot upload APKs onto Google Play Store: Signed APKs are not Zip aligned
Like the Android documentation reads, we require one more step in our signing jobs:
> zipalign -v 4 inbound.apk final.apk 

For (personal) reference, I had to install android-sdk-build-tools[1] on Arch Linux, in order to be able to use zipalign. That software wasn't present in my $PATH after installation, it lives at /opt/android-sdk/build-tools/25.0.2/zipalign 

[1] https://aur.archlinux.org/packages/android-sdk-build-tools/
:Callek, :jlund and I discussed about one possible solution on IRC. The takeaway of it is:

> we add a function like post_sign_steps() in signingsript,
> that zipaligns if filename.ext == "apk"? 

However, none of us knows if that's in line with the signingscript's design plans. What do you think Aki? 

For the record, :catlee pointed out: zipalign is done in this make file[1], in the old architecture. 

[1] https://dxr.mozilla.org/mozilla-central/rev/1fe66bd0efba89df59d2046e8c91418eb5ae10b8/config/android-common.mk#37
Flags: needinfo?(aki)
I think so. Do you know how much time the zipalign takes? (<1 second, several seconds, many seconds, ...?)

We could put that here: https://github.com/mozilla-releng/signingscript/blob/c4bed53/signingscript/script.py#L50
but we log shas here: https://github.com/mozilla-releng/signingscript/blob/c4bed53/signingscript/task.py#L97 .

It almost makes sense to do the zipalign on the signing servers, since they cache signed bits by sha, and the zipalign will change the sha.  However, we're likely going to move logic out of the signing servers onto the signing scriptworkers over time, so apks may be a good place to start.

Right now I'm thinking:

- in signingscript.script.async_main, look at the extension of the file to be signed and figure out which callback to send to sign_file (we can create a constant extension->callback map outside async_main)
- in sign_file, run the callback against the signed file if not None.
- the callback for apks can create a tmpdir and zipalign into that, then overwrite the unaligned file with the zipaligned file.
Flags: needinfo?(aki)
Zipalign takes less than a second on my local machine. More precisely, `time` returns a total time between 0.1s and 0.3s.

I'll give signingscript a try.
Assignee: nobody → jlorenzo
Attached file PR
Attachment #8832895 - Flags: review?(aki)
Depends on: 1336179
Comment on attachment 8832895 [details] [review]
PR

As noted in the PR, we'll need to install zipalign and point to it in a puppet patch.
Attachment #8832895 - Flags: review?(aki) → review+
Comment on attachment 8835051 [details]
Bug 1335370 - signing_scriptworker: install Android build tools

https://reviewboard.mozilla.org/r/109198/#review112130

as mentioned in person, it's not great we have to update 2 places to upgrade this, but we don't foresee needing to upgrade this often, or possibly at all.
Attachment #8835051 - Flags: review?(aki) → review+
I agree. It's a compromise due to the fact that we can use a variable defined in the class. I could have made a "define" instead, but that duplicated the logic, which is worse, IMO.

Signingscript patch landed at https://github.com/mozilla-releng/signingscript/commit/f8f0e21aa945326a97ef5e2ee575f098f7242009
Signingscript v0.10.0 released: https://github.com/mozilla-releng/signingscript/releases/tag/0.10.0

Puppet patch landed at https://hg.mozilla.org/build/puppet/rev/4a3ee9ace2d2
Pushed to production at https://hg.mozilla.org/build/puppet/rev/ea6e8331dcdf
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Signingscript failed, like shown in this task: https://tools.taskcluster.net/task-inspector/#axv1P7vPSoqM5Vt6PzGKdQ/0

I prefer to roll back the changes in production: https://hg.mozilla.org/build/puppet/rev/cb1329fb277bb5bdb133f81bbfbbecd2548a2fb2 and in default: https://hg.mozilla.org/build/puppet/rev/e2a394ea0eca1538b434d5cc4256ac404597e382
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 12 failed because of a nit fixed in [1]. There were also some issues related to file ownership that I fixed in the puppet patch. 

Based on Aki's suggestions, I pined all 4 signing-linux on my personal environment for a short period of time. I ran an APK signing task [2] and a regular linux task [3]. Both succeed and the APK one did produce a zipaligned APK (I downloaded the APK and verified it locally). 

I released Signingscript v0.10.1 [4].

I re-landed patches on default [5] and on production [6]

[1] https://github.com/mozilla-releng/signingscript/pull/23
[2] https://tools.taskcluster.net/task-inspector/#E1n3fTjnRgmKyj94INPBmw/0
[3] https://tools.taskcluster.net/task-inspector/#U2QZcFreR12N1EY2clHgWQ/0
[4] https://github.com/mozilla-releng/signingscript/releases/tag/0.10.1
[5] https://hg.mozilla.org/build/puppet/rev/8d7a3bf3cebc4dc304ecf98f9f5105224e509102
[6] https://hg.mozilla.org/build/puppet/rev/b964478fe8e18023939cd515d1dc8ded7e183000
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
I forced the run of Puppet agent in Puppet. This allowed me to verify:
* android https://tools.taskcluster.net/task-inspector/#JbIJHAMWTe-Cug1EzYt0kQ/0
* linux https://tools.taskcluster.net/task-inspector/#IU0un7tHR9ih9hlkJVZVig/0 

I had to manually update the ownership of /tools/android-sdk in signing-linux-1 and 3 (2 and 4 weren't impatcted). They had picked up the changes, but the puppet patch didn't update the ownwership after the untar operation. This shouldn't be an issue anymore.
Status: RESOLVED → VERIFIED
Blocks: 1328263
Product: TaskCluster → Firefox Build System
Blocks: 1807317
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: