diff --git a/scripts/releases/sequencer.py b/scripts/releases/sequencer.py index c715bf55d9..671c76ae3c 100644 --- a/scripts/releases/sequencer.py +++ b/scripts/releases/sequencer.py @@ -388,9 +388,10 @@ def reconcile(env: dict, *, run=output, read_head=None, advance_head=None, # the publication pass with the feed pointer, never in the green # build that pushed the image under the attempt ref. docker.promote_stable(record["claim_tag"], record["docker_manifest_digest"]) - # The Store release joins the pass here (after the feeds and - # aliases move), not before. A failure leaves the run red. - store.release_from_env(env) + # The Store check joins the pass here (after the feeds and aliases + # move). It never releases the held submission: the API cannot, + # so it prints the Publish now step. A failed submission is red. + store.check_from_env(env) advance_head = production_advance for step in steps: diff --git a/scripts/releases/store.py b/scripts/releases/store.py index 6637e4a1ec..2f347165a4 100644 --- a/scripts/releases/store.py +++ b/scripts/releases/store.py @@ -1,9 +1,10 @@ """Store submission control for the release pipeline. The green run submits the verified ``.msixbundle`` with auto-publish off -(``targetPublishMode: "Manual"``); the publication pass releases it (or turns -auto-publish on while it is still in certification). Both entry points act on -the submission's current state, so both are safe to rerun. +(``targetPublishMode: "Manual"``). The submission API has no call that +releases a held submission, so publication never edits it: the publication +pass only checks its state and tells a person to click Publish now in Partner +Center once it is certified. Both entry points are safe to rerun. Green run — ``python -m scripts.releases.store submit `` (Windows runner, ``msstore`` CLI configured beforehand by @@ -25,25 +26,18 @@ runner, ``msstore`` CLI configured beforehand by the complete submission JSON, so it sets packages and publish mode at once. 6. ``msstore submission publish `` — commits; certification starts. -Publication pass — ``scripts.releases.store.release(...)`` (Ubuntu runner, -Partner Center submission REST API, stdlib ``urllib`` only): +Publication pass — ``scripts.releases.store.check(...)`` (Ubuntu runner, +Partner Center submission REST API, stdlib ``urllib`` only, read-only): 1. ``POST https://login.microsoftonline.com/{tenant}/oauth2/v2.0/token`` with ``grant_type=client_credentials`` and scope ``https://manage.devcenter.microsoft.com/.default`` (the same MS_STORE_* credentials the CLI uses). -2. ``POST https://manage.devcenter.microsoft.com/v1.0/my/applications/{id}/submissions`` - to find the in-flight submission: ``201`` means none existed (the fresh - probe draft is deleted immediately with ``DELETE .../submissions/{id}``, - nothing else is touched); ``409`` means one exists and its id is in the - error body. -3. ``GET .../submissions/{id}/status``. Terminal or already-live states - (``PendingPublication, Publishing, Published``) are a no-op ("already-live"). -4. Otherwise the held submission is released by rewriting it with - ``targetPublishMode: "Immediate"`` (``PUT .../submissions/{id}``) and - committing it (``POST .../submissions/{id}/commit``). A certified - submission (status ``Release``) goes live now ("released"); one still in - certification goes live when certification passes ("auto-publish"). +2. ``GET https://manage.devcenter.microsoft.com/v1.0/my/applications/{id}`` — + ``pendingApplicationSubmission.id`` names the held submission. +3. ``GET .../submissions/{id}/status``. ``Release`` (certified, held by + Manual) and the certification states print a Publish now instruction as + a GitHub warning; the live states are a no-op; a failed state raises. Sources for the commands and fields above: - msstore CLI commands and options (``submission status/get/update/delete @@ -57,18 +51,13 @@ Sources for the commands and fields above: (``PendingCommit, CommitStarted, PreProcessing, Certification, CertificationFailed, Release, PendingPublication, Publishing, Published, ...``): https://learn.microsoft.com/en-us/windows/uwp/monetize/manage-app-submissions -- REST methods (create/update/commit/delete/status): - https://learn.microsoft.com/en-us/windows/uwp/monetize/manage-app-submissions#methods-for-managing-app-submissions - (individual pages: create-an-app-submission, update-an-app-submission, - commit-an-app-submission, delete-an-app-submission, - get-status-for-an-app-submission) +- REST methods (get an app, get submission status): + https://learn.microsoft.com/en-us/windows/uwp/monetize/get-an-app + https://learn.microsoft.com/en-us/windows/uwp/monetize/get-status-for-an-app-submission + The documented methods are get, create, update, commit, delete and status; + none releases a held submission, and update refuses a committed one (409). - Azure AD client-credentials token: https://learn.microsoft.com/en-us/windows/uwp/monetize/create-and-manage-submissions-using-windows-store-services#obtain-an-azure-ad-access-token - -Unverified on paper but guarded in code: whether the API accepts a ``PUT`` -update on a submission that is committed and held (certified, waiting for -release). If it refuses, the release step fails and the publication run stays -red; rerunning after a fix from Partner Center is safe. """ from __future__ import annotations @@ -135,7 +124,7 @@ def _http_run(request: dict) -> dict: """Default injected runner: one HTTP request, stdlib only.""" url = request["url"] data = None - headers = {"Content-Type": "application/json"} + headers = {"Content-Type": "application/json", **request.get("headers", {})} body = request.get("body") if body is not None: data = json.dumps(body).encode() @@ -171,71 +160,65 @@ def _token(tenant_id: str, client_id: str, client_secret: str, run) -> str: return response["body"]["access_token"] -def _in_progress_id(conflict_body) -> str: - """The 409 body names the in-progress submission; pull its id out.""" - text = json.dumps(conflict_body) if not isinstance(conflict_body, str) \ - else conflict_body - digits = "".join(character if character.isdigit() or character == " " - else " " for character in text).split() - long_ids = [token for token in digits if len(token) >= 10] - if not long_ids: - raise StoreError(f"cannot find the in-flight submission id in {text!r}") - return long_ids[0] +# Statuses of a committed submission that has not been decided yet. +_IN_CERTIFICATION = {"CommitStarted", "PreProcessing", "Certification"} +_FAILED = {"CommitFailed", "PreProcessingFailed", "CertificationFailed", "PublishFailed", + "Canceled"} -def release(*, product_id: str, tenant_id: str, client_id: str, - client_secret: str, run=_http_run) -> str: - """Release the held submission, or let it go live when certified.""" +def check(*, product_id: str, tenant_id: str, client_id: str, client_secret: str, + run=_http_run) -> str: + """Report the held submission's state and what a person must do next. + + The submission API has no call that releases a held (Manual) submission, + so the publication pass never edits or commits it: a certified submission + is put live by "Publish now" in Partner Center. A failed submission leaves + the run red. + """ token = _token(tenant_id, client_id, client_secret, run) - def call(method: str, path: str, body: dict | None = None) -> dict: - request = {"method": method, "url": API_ROOT + path} - if body is not None: - request["body"] = body - request["headers"] = {"Authorization": f"Bearer {token}"} - return run(request) + def get(path: str) -> dict: + response = run({"method": "GET", "url": API_ROOT + path, + "headers": {"Authorization": f"Bearer {token}"}}) + if response["status"] != 200: + raise StoreError(f"GET {path} failed: {response['status']}") + return response["body"] - submissions_path = f"/applications/{product_id}/submissions" - probe = call("POST", submissions_path) - if probe["status"] == 201: - # Nothing was in flight; do not leave the probe draft behind. - probe_id = probe["body"].get("id") - if probe_id: - call("DELETE", f"{submissions_path}/{probe_id}") + app = get(f"/applications/{product_id}") + pending = (app.get("pendingApplicationSubmission") or {}).get("id") + if not pending: + _notice(f"No Store submission is pending for {product_id}. The green run submits " + "one; check its stable-store job.") return "no-submission" - if probe["status"] != 409: - raise StoreError(f"unexpected submission probe reply: {probe['status']}") - submission_id = _in_progress_id(probe["body"]) - - status = call("GET", f"{submissions_path}/{submission_id}/status") - if status["status"] != 200: - raise StoreError(f"submission status failed: {status['status']}") - state = status["body"]["status"] + state = get(f"/applications/{product_id}/submissions/{pending}/status").get("status") + if state in _FAILED: + raise StoreError(f"Store submission {pending} is {state}; resubmit from a green run") if state in _ALREADY_LIVE: return "already-live" - - submission = call("GET", f"{submissions_path}/{submission_id}") - if submission["status"] != 200: - raise StoreError(f"submission read failed: {submission['status']}") - held = submission["body"] - held["targetPublishMode"] = "Immediate" - updated = call("PUT", f"{submissions_path}/{submission_id}", held) - if updated["status"] != 200: - raise StoreError(f"submission update failed: {updated['status']}") - committed = call("POST", f"{submissions_path}/{submission_id}/commit") - if committed["status"] != 200: - raise StoreError(f"submission commit failed: {committed['status']}") - return "released" if state == _CERTIFIED_HELD else "auto-publish" + if state == _CERTIFIED_HELD: + _notice(f"Store submission {pending} passed certification and is held. " + "Open it in Partner Center and click Publish now.") + return "needs-publish-now" + if state in _IN_CERTIFICATION: + _notice(f"Store submission {pending} is still in certification ({state}). When it " + "passes, open it in Partner Center and click Publish now.") + return "in-certification" + raise StoreError(f"Store submission {pending} has an unexpected status: {state!r}") -def release_from_env(env: dict, run=_http_run) -> str: - """Release the Store submission when the environment configures it.""" +def _notice(text: str) -> None: + # A GitHub annotation, so the manual step shows on the run's summary page. + print(f"::warning title=Microsoft Store::{text}") + + +def check_from_env(env: dict, run=_http_run) -> str: + """Check the Store submission when the environment configures it.""" product_id = env.get("MS_STORE_PRODUCT_ID") if not product_id: - print("Store release skipped: MS_STORE_PRODUCT_ID is not configured.", + print("Store check skipped: MS_STORE_PRODUCT_ID is not configured.", file=sys.stderr) return "not-configured" - return release( + return check( product_id=product_id, tenant_id=env["MS_STORE_TENANT_ID"], client_id=env["MS_STORE_CLIENT_ID"], @@ -251,11 +234,11 @@ def main(argv: list[str]) -> int: result = submit(package, product_id=product_id) print(json.dumps(result, sort_keys=True)) return 0 - if argv[:1] == ["release"]: - print(json.dumps({"result": release_from_env(dict(os.environ))}, + if argv[:1] == ["check"]: + print(json.dumps({"result": check_from_env(dict(os.environ))}, sort_keys=True)) return 0 - print("usage: python -m scripts.releases.store submit | release", + print("usage: python -m scripts.releases.store submit | check", file=sys.stderr) return 2 diff --git a/tests/scripts/test_release_sequencer.py b/tests/scripts/test_release_sequencer.py index b898e98666..a9f97871c2 100644 --- a/tests/scripts/test_release_sequencer.py +++ b/tests/scripts/test_release_sequencer.py @@ -411,7 +411,7 @@ def test_reconcile_discovers_custody_flips_then_advances_oldest_first(): assert release["tag_name"] == f"v{version}" and release["draft"] is False -def test_the_store_release_joins_the_pass_after_the_aliases_move(monkeypatch): +def test_the_store_check_joins_the_pass_after_the_aliases_move(monkeypatch): from scripts.releases import channel_releases, docker, sequencer, store manifest_digest = hashlib.sha256(b"m").hexdigest() @@ -428,7 +428,7 @@ def test_the_store_release_joins_the_pass_after_the_aliases_move(monkeypatch): lambda claim_tag, digest: events.append(("aliases", claim_tag))) seen_env = [] monkeypatch.setattr( - store, "release_from_env", + store, "check_from_env", lambda env: seen_env.append(env) or events.append(("store",))) steps = sequencer.reconcile( diff --git a/tests/scripts/test_release_store.py b/tests/scripts/test_release_store.py index f726b26047..2520850d14 100644 --- a/tests/scripts/test_release_store.py +++ b/tests/scripts/test_release_store.py @@ -99,109 +99,138 @@ def test_submit_keeps_the_submission_in_draft_until_the_mode_is_set(): class _Api: - """Stands in for the Partner Center submission REST API.""" + """Stands in for the Partner Center submission REST API (read-only use).""" - def __init__(self, status, in_flight=True, fail_update=False): + def __init__(self, status, pending="1152921504621243540"): self.requests = [] self.status = status - self.in_flight = in_flight - self.fail_update = fail_update - self.updated = None - self.committed = False + self.pending = pending def __call__(self, request): self.requests.append(request) url, method = request["url"], request["method"] if url.endswith("/token"): return {"status": 200, "body": {"access_token": "tok"}} - if url.endswith("/submissions") and method == "POST": - if self.in_flight: - return { - "status": 409, - "body": {"code": "InvalidOperation", "message": - "The app already has an in-progress submission: " - "1152921504621243540"}, - } - return {"status": 201, "body": {"id": "probe", "status": "PendingCommit"}} - if url.endswith("/status") and method == "GET": + if method != "GET": + raise AssertionError(f"the check must never write: {method} {url}") + if request.get("headers", {}).get("Authorization") != "Bearer tok": + return {"status": 401, "body": {"code": "Unauthorized"}} + if url.endswith("/applications/9NTEST"): + app = {"id": "9NTEST"} + if self.pending: + app["pendingApplicationSubmission"] = {"id": self.pending} + return {"status": 200, "body": app} + if url.endswith(f"/submissions/{self.pending}/status"): return {"status": 200, "body": {"status": self.status}} - if "/submissions/" in url and method == "GET": - return {"status": 200, "body": { - "id": "1152921504621243540", "targetPublishMode": "Manual", - "friendlyName": "Submission 2"}} - if "/submissions/" in url and method == "DELETE": - return {"status": 204, "body": ""} - if "/submissions/" in url and method == "PUT": - if self.fail_update: - return {"status": 500, "body": {"code": "ServiceError"}} - self.updated = request["body"] - return {"status": 200, "body": {"id": "1152921504621243540"}} - if url.endswith("/commit") and method == "POST": - self.committed = True - return {"status": 200, "body": {"status": "CommitStarted"}} raise AssertionError(request) -def _release(status, **kwargs): - from scripts.releases.store import release +def _check(status, **kwargs): + from scripts.releases.store import check api = _Api(status, **kwargs) - result = release(product_id="9NTEST", tenant_id="T", client_id="C", - client_secret="S", run=api) + result = check(product_id="9NTEST", tenant_id="T", client_id="C", + client_secret="S", run=api) return api, result -def test_release_publishes_a_certified_submission(): - api, result = _release("Release") - assert result == "released" - assert api.updated["targetPublishMode"] == "Immediate" - assert api.committed is True +def test_a_certified_held_submission_asks_for_publish_now(capsys): + _api, result = _check("Release") + assert result == "needs-publish-now" + out = capsys.readouterr().out + assert out.startswith("::warning title=Microsoft Store::") and "Publish now" in out -def test_release_turns_auto_publish_on_when_not_certified(): - api, result = _release("Certification") - assert result == "auto-publish" - assert api.updated["targetPublishMode"] == "Immediate" - assert api.committed is True +@pytest.mark.parametrize("status", ["CommitStarted", "PreProcessing", "Certification"]) +def test_a_submission_in_certification_says_what_comes_next(status, capsys): + _api, result = _check(status) + assert result == "in-certification" + out = capsys.readouterr().out + assert "Publish now" in out and status in out -def test_release_is_a_noop_when_the_submission_already_goes_live(): - api, result = _release("PendingPublication") +@pytest.mark.parametrize("status", ["PendingPublication", "Publishing", "Published"]) +def test_a_live_submission_is_a_noop(status, capsys): + _api, result = _check(status) assert result == "already-live" - assert api.updated is None and api.committed is False + assert capsys.readouterr().out == "" -def test_release_without_an_in_flight_submission_deletes_its_probe_draft(): - api, result = _release("Certification", in_flight=False) - assert result == "no-submission" - deleted = [r for r in api.requests - if r["method"] == "DELETE" and "/submissions/" in r["url"]] - assert len(deleted) == 1 - assert api.committed is False - - -def test_a_failed_store_call_leaves_the_run_red(): +@pytest.mark.parametrize("status", ["CertificationFailed", "CommitFailed", "PublishFailed", + "Canceled"]) +def test_a_failed_submission_leaves_the_run_red_and_says_to_resubmit(status): from scripts.releases.store import StoreError - with pytest.raises(StoreError): - _release("Certification", fail_update=True) -def test_release_from_env_skips_when_the_store_is_not_configured(capsys): - from scripts.releases.store import release_from_env - - assert release_from_env({}) == "not-configured" + with pytest.raises(StoreError, match="resubmit from a green run"): + _check(status) -def test_release_from_env_passes_the_configured_credentials(): - from scripts.releases.store import release_from_env +def test_an_unknown_submission_status_leaves_the_run_red(): + from scripts.releases.store import StoreError - api = _Api("Release") + with pytest.raises(StoreError, match="unexpected status"): + _check("SomethingNew") + + +def test_no_pending_submission_is_reported_not_invented(capsys): + _api, result = _check("Release", pending=None) + assert result == "no-submission" + assert "::warning" in capsys.readouterr().out + + +def test_the_check_never_writes_to_the_store(): + # _Api raises on any non-GET after the token; every state must pass. + for status in ("Release", "Certification", "Published"): + api, _ = _check(status) + assert all(r["method"] == "GET" for r in api.requests[1:]) + + +def test_http_run_sends_the_request_headers(monkeypatch): + import urllib.request + + from scripts.releases.store import _http_run + + sent = {} + + class _Response: + status = 200 + + def read(self): + return b"{}" + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + def fake_urlopen(request): + sent.update({key.lower(): value for key, value in request.header_items()}) + return _Response() + + monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + _http_run({"method": "GET", "url": "https://example.test/x", + "headers": {"Authorization": "Bearer tok"}}) + assert sent["authorization"] == "Bearer tok" + + +def test_check_from_env_skips_when_the_store_is_not_configured(): + from scripts.releases.store import check_from_env + + assert check_from_env({}) == "not-configured" + + +def test_check_from_env_passes_the_configured_credentials(): + from scripts.releases.store import check_from_env + + api = _Api("Published") env = { "MS_STORE_PRODUCT_ID": "9NTEST", "MS_STORE_TENANT_ID": "TENANT", "MS_STORE_CLIENT_ID": "CLIENT", "MS_STORE_CLIENT_SECRET": "SECRET", } - assert release_from_env(env, run=api) == "released" + assert check_from_env(env, run=api) == "already-live" token = next(r for r in api.requests if r["url"].endswith("/token")) assert token["form"]["client_id"] == "CLIENT" assert token["form"]["client_secret"] == "SECRET"