feat(pushapk): Implement huawei store support - #1516
Open
hneiva wants to merge 8 commits into
Open
Conversation
publish() passes `huawei_credentials` to push_apk() unconditionally, and that argument only exists as of mozapkpublisher 12.0.0. The dependency was left unpinned pending that release, so the lockfile still resolved 11.0.2 - under which every publication, google included, would have raised TypeError. The tests mock push_apk, so none of them caught it. 12.0.0 is on PyPI now, so pin it and drop the TODO. It also pulls in pyjwt[crypto], which the AppGallery client needs to sign its PS256 JWTs.
The huawei store support added `test_var_set 'HUAWEI_SERVICE_ACCOUNT'` to pushapkscript's prod init_worker.sh, but not the matching entry to the cloudops-infra variables the init script test simulates, so test_init_script[pushapk-firefox-prod] exited 1 instead of 0.
_log_warning_forewords only had a message for `target_store == "google"`, so a samsung or huawei publication logged nothing at all - not even the "this action is irreversible" warning that precedes a committed upload. Drive the messages off a store name table instead, so every store the script can target gets one. This also fixes two typos in the Google wording: a missing space after "irreversible," and the double negative in "no change will not be committed".
The example's samsung block used `sgs_service_account_id`/`sgs_access_token` - the names publish_config emits for mozapkpublisher - rather than the `service_account_id`/`access_token` the config schema requires. Since the samsung block sets `additionalProperties: false`, the example did not validate against config_schema.json; it does now.
`PublishTest` matched neither pytest's `Test*` class pattern nor unittest.TestCase, so tests/test_publish.py collected zero tests and had been silently dead. Rename it and swap the unused `setUp` for `setup_method`, which completes the refactor the file's TODO asked for. Two problems surfaced once the tests ran: a missing `await` on a publish_aab call, and no coverage of the huawei arguments publish() now passes through. Both are fixed here.
Contributor
Author
|
Note: https://github.com/mozilla-services/cloudops-infra/pull/7000 has to be merged before this. This is now done. Ok to merge. |
bhearsum
reviewed
Sep 11, 2026
bhearsum
left a comment
Contributor
There was a problem hiding this comment.
What's the testing/rollout plan for this? IIRC, we have had manual runs of either this or mozapkpublisher that were coordinated with RelMan, and then close monitoring of the first automated ones?
Note: https://github.com/mozilla-services/cloudops-infra/pull/7000 has to be merged before this
I see you added some secrets already as well. You'll also need to update https://github.com/mozilla-releng/k8s-autoscale, but only after the pools actually exist. (Which you probably already know!)
| rollout_percentage = task.get("rollout_percentage") | ||
|
|
||
| if target_store == "samsung": | ||
| if target_store in _NON_GOOGLE_STORE_CREDENTIALS: |
Contributor
There was a problem hiding this comment.
This might be a case where verbosity is preferable; adding a mostly duplicated branch for huawei would probably read better (after all, it's what we already do between google and samsung?).
Not a blocker, just a mild suggestion.
Contributor
Author
There was a problem hiding this comment.
We are planning on adding more stores, I'll keep a mental note on this; it's likely we'll need a refactor since other stores might have different config models.
| # mocks the API when this instance may not contact the server. The other stores | ||
| # have no transaction to leave uncommitted, so mozapkpublisher skips the upload | ||
| # outright instead of performing or mocking it. | ||
| reason = "this is a dry run" if contact_server else "this pushapk instance is not allowed to talk to it" |
Contributor
There was a problem hiding this comment.
This indirection hurts readability IMO, and it's also not equivalent to the old code? In particular, we lose the "not allowed to talk to" messages for non-google stores?
~/tmp/2026-09-11 ❯ cat foo.py
def test(contact_server, dry_run, target_store):
if contact_server:
if target_store == "google":
if not dry_run:
return "you will publish APKs"
else:
return "APKs will be submitted"
else:
return "not allowed to talk to"
def test2(contact_server, dry_run, target_store):
if contact_server and not dry_run:
return "you will publish APKs"
elif target_store != "google":
pass
elif contact_server:
return "APKs will be submitted"
else:
return "not allowed to talk to"
results = {"test": {}, "test2": {}}
for contact_server in (True, False):
for dry_run in (True, False):
for target_store in ("google", "samsung", "huawei"):
r1 = test(contact_server, dry_run, target_store)
r2 = test2(contact_server, dry_run, target_store)
key = f"{contact_server}-{dry_run}-{target_store}"
results["test"][key] = r1
results["test2"][key] = r2
import pprint
pprint.pprint(results)
~/tmp/2026-09-11 ❯ python3 foo.py
{'test': {'False-False-google': 'not allowed to talk to',
'False-False-huawei': 'not allowed to talk to',
'False-False-samsung': 'not allowed to talk to',
'False-True-google': 'not allowed to talk to',
'False-True-huawei': 'not allowed to talk to',
'False-True-samsung': 'not allowed to talk to',
'True-False-google': 'you will publish APKs',
'True-False-huawei': None,
'True-False-samsung': None,
'True-True-google': 'APKs will be submitted',
'True-True-huawei': None,
'True-True-samsung': None},
'test2': {'False-False-google': 'not allowed to talk to',
'False-False-huawei': None,
'False-False-samsung': None,
'False-True-google': 'not allowed to talk to',
'False-True-huawei': None,
'False-True-samsung': None,
'True-False-google': 'you will publish APKs',
'True-False-huawei': 'you will publish APKs',
'True-False-samsung': 'you will publish APKs',
'True-True-google': 'APKs will be submitted',
'True-True-huawei': None,
'True-True-samsung': None}}
Contributor
Author
There was a problem hiding this comment.
For the record, I don't think pushapkscript should have both dryrun and contact_server settings; I think that's an implementation detail on the google side and we should stick to either using their dryrun, or just never contact a store in such cases.
I'll update it to:
# $ cat foo.py
STORE_NAMES = {
"google": "Google Play",
"samsung": "the Samsung Galaxy Store",
"huawei": "the Huawei AppGallery",
}
# old code, before the PR
def test(contact_server, dry_run, target_store):
if contact_server:
if target_store == "google":
if not dry_run:
return "You will publish APKs to Google Play. This action is irreversible,if no error is detected either by this script or by Google Play."
else:
return "APKs will be submitted, but no change will not be committed."
else:
return "This pushapk instance is not allowed to talk to Google Play. *All* requests will be mocked."
# new code, as it stands now
def test2(contact_server, dry_run, target_store):
store_name = STORE_NAMES.get(target_store, target_store)
if contact_server and not dry_run:
return "You will publish APKs to {}. This action is irreversible, if no error is detected either by this script or by {}.".format(store_name, store_name)
elif target_store == "google":
if contact_server:
return "APKs will be submitted to {}, but no change will be committed.".format(store_name)
else:
return "This pushapk instance is not allowed to talk to {}. *All* requests will be mocked.".format(store_name)
else:
if contact_server:
return "Nothing will be uploaded to {}, since this is a dry run.".format(store_name)
else:
return "Nothing will be uploaded to {}, since this pushapk instance is not allowed to talk to it.".format(store_name)
results = {"test": {}, "test2": {}}
for contact_server in (True, False):
for dry_run in (True, False):
for target_store in ("google", "samsung", "huawei"):
key = f"{contact_server}-{dry_run}-{target_store}"
results["test"][key] = test(contact_server, dry_run, target_store)
results["test2"][key] = test2(contact_server, dry_run, target_store)
print(f"{'cell':24s} {'old':6s} {'new':6s}")
for key in sorted(results["test"]):
old, new = results["test"][key], results["test2"][key]
print(f"{key:24s} {'None' if old is None else 'msg':6s} {'None' if new is None else 'msg':6s} {'<-- old said nothing' if old is None else ''}")
print()
print("cells where the old code emitted no message at all:", sum(1 for v in results["test"].values() if v is None))
print("cells where the new code emitted no message at all:", sum(1 for v in results["test2"].values() if v is None))
print()
for key in sorted(results["test2"]):
print(f"{key:24s} {results['test2'][key]}")$ python foo.py
cell old new
False-False-google msg msg
False-False-huawei msg msg
False-False-samsung msg msg
False-True-google msg msg
False-True-huawei msg msg
False-True-samsung msg msg
True-False-google msg msg
True-False-huawei None msg <-- old said nothing
True-False-samsung None msg <-- old said nothing
True-True-google msg msg
True-True-huawei None msg <-- old said nothing
True-True-samsung None msg <-- old said nothing
cells where the old code emitted no message at all: 4
cells where the new code emitted no message at all: 0
False-False-google This pushapk instance is not allowed to talk to Google Play. *All* requests will be mocked.
False-False-huawei Nothing will be uploaded to the Huawei AppGallery, since this pushapk instance is not allowed to talk to it.
False-False-samsung Nothing will be uploaded to the Samsung Galaxy Store, since this pushapk instance is not allowed to talk to it.
False-True-google This pushapk instance is not allowed to talk to Google Play. *All* requests will be mocked.
False-True-huawei Nothing will be uploaded to the Huawei AppGallery, since this pushapk instance is not allowed to talk to it.
False-True-samsung Nothing will be uploaded to the Samsung Galaxy Store, since this pushapk instance is not allowed to talk to it.
True-False-google You will publish APKs to Google Play. This action is irreversible, if no error is detected either by this script or by Google Play.
True-False-huawei You will publish APKs to the Huawei AppGallery. This action is irreversible, if no error is detected either by this script or by the Huawei AppGallery.
True-False-samsung You will publish APKs to the Samsung Galaxy Store. This action is irreversible, if no error is detected either by this script or by the Samsung Galaxy Store.
True-True-google APKs will be submitted to Google Play, but no change will be committed.
True-True-huawei Nothing will be uploaded to the Huawei AppGallery, since this is a dry run.
True-True-samsung Nothing will be uploaded to the Samsung Galaxy Store, since this is a dry run.
Contributor
There was a problem hiding this comment.
For the record, I don't think pushapkscript should have both dryrun and contact_server settings; I think that's an implementation detail on the google side and we should stick to either using their dryrun, or just never contact a store in such cases.
Consider collapsing them to one, in that case? (Not required as part of this change obviously...)
Generalising these warnings across stores reused wording that is only true of Google Play. Google uploads inside an edit transaction it then declines to commit, and mocks the API when the instance may not contact the server - so "will be submitted, but no change will be committed" and "*All* requests will be mocked" describe it accurately. The Samsung and Huawei clients have no transaction to leave uncommitted. mozapkpublisher collapses dry_run and contact_server into one flag for them and returns from upload_apks before uploading anything, so both messages promised an upload that never happens. Give those stores their own message, naming which of the two reasons applies. Adds the missing parametrized cases: every branch for each store, plus an unrecognised store to pin the STORE_NAMES fallback.
…ture All ten push_apk/push_aab patches were bare mocks, which accept any keyword whatsoever, so nothing checked that publish.py's call matches the library it is pinned against. That is not hypothetical. Shadowing the venv with mozapkpublisher 11.0.2 - the version uv.lock actually resolved when huawei support landed, and whose push_apk has no `huawei_credentials` parameter - the suite reports 109 passed, while a real publish() call raises TypeError. Every publication, Google Play included, was broken and fully green. autospec=True turns each existing assert_called_with into a signature check: the same shadowed run now fails 16 tests, naming the unexpected keyword.
hneiva
force-pushed
the
hneiva/huawei
branch
from
September 11, 2026 19:01
12f707f to
20f3b1f
Compare
Contributor
Author
Updated the PR description to reference the testing that was done on the mozapkpublisher side.
🤔 I don't think we'll need to update the autoscaler. AIUI pushapk workers are already the same worker for both google and samsung. Unless if I missed something? |
Contributor
You're right, my bad! I went full-on into "new scriptworker mode" 🤦 |
bhearsum
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1475
Manual Validation
The Huawei store integration validation was done as part of the mozapkpublisher implementation: mozilla-releng/mozapkpublisher#333