From 55b0816fe78bc25471145b8d767bc51ad9a1e5bf Mon Sep 17 00:00:00 2001 From: pcvantol Date: Thu, 17 Sep 2026 22:14:02 +0200 Subject: [PATCH] Retry transient GitHub release asset uploads --- .../test_github_draft_release_helper.py | 276 ++++++++++++++++++ tools/qualification/github_draft_release.sh | 101 ++++++- 2 files changed, 372 insertions(+), 5 deletions(-) diff --git a/tests/engineering/test_github_draft_release_helper.py b/tests/engineering/test_github_draft_release_helper.py index 11baca77..2d140be4 100644 --- a/tests/engineering/test_github_draft_release_helper.py +++ b/tests/engineering/test_github_draft_release_helper.py @@ -152,6 +152,7 @@ def test_upload_uses_the_github_uploads_endpoint_with_encoded_asset_name(self) - from pathlib import Path Path(os.environ["INVOCATION"]).write_text(json.dumps(sys.argv[1:]), encoding="utf-8") + print("201", end="") """ ), encoding="utf-8", @@ -177,9 +178,284 @@ def test_upload_uses_the_github_uploads_endpoint_with_encoded_asset_name(self) - self.assertIn("--request", arguments) self.assertIn("POST", arguments) self.assertIn("Authorization: Bearer test-token", arguments) + self.assertNotIn("--retry", arguments) + self.assertNotIn("--retry-all-errors", arguments) + self.assertIn("X-GitHub-Api-Version: 2022-11-28", arguments) self.assertIn("--data-binary", arguments) self.assertIn("@" + str(receipt), arguments) self.assertIn( "https://uploads.github.com/repos/example/repository/releases/389/assets?name=qualified%20receipt.json", arguments, ) + + def test_upload_reconciles_an_accepted_asset_after_ambiguous_502_without_reposting(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + receipt = root / "receipt.json" + receipt.write_text("exact receipt", encoding="utf-8") + counter = root / "curl-count" + fake_curl = root / "curl" + fake_curl.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import os + import sys + from pathlib import Path + + counter = Path(os.environ["COUNTER"]) + counter.write_text(str(int(counter.read_text()) + 1) if counter.exists() else "1") + print("502", end="") + raise SystemExit(22) + """ + ), + encoding="utf-8", + ) + fake_gh = root / "gh" + fake_gh.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import os + import sys + from pathlib import Path + + endpoint = sys.argv[-1] + if endpoint.endswith("/releases/389"): + print(json.dumps({"assets": [{"id": 991, "name": "receipt.json", "state": "uploaded"}]})) + elif endpoint.endswith("/releases/assets/991"): + sys.stdout.buffer.write(Path(os.environ["RECEIPT"]).read_bytes()) + else: + raise SystemExit(f"unexpected gh invocation: {sys.argv}") + """ + ), + encoding="utf-8", + ) + fake_curl.chmod(0o700) + fake_gh.chmod(0o700) + result = subprocess.run( + ["bash", "-c", f"source '{HELPER}'; ep_draft_upload 389 '{receipt}' receipt.json"], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GITHUB_REPOSITORY": "example/repository", + "GH_TOKEN": "test-token", + "COUNTER": str(counter), + "RECEIPT": str(receipt), + }, + text=True, + capture_output=True, + check=False, + ) + counter_value = counter.read_text() + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(counter_value, "1") + + def test_upload_removes_a_starter_asset_before_one_controlled_retry(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + receipt = root / "receipt.json" + receipt.write_text("exact receipt", encoding="utf-8") + counter = root / "curl-count" + state = root / "asset-state" + fake_curl = root / "curl" + fake_curl.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import os + from pathlib import Path + + counter = Path(os.environ["COUNTER"]) + count = int(counter.read_text()) + 1 if counter.exists() else 1 + counter.write_text(str(count)) + if count == 1: + Path(os.environ["STATE"]).write_text("starter") + print("502", end="") + raise SystemExit(22) + print("201", end="") + """ + ), + encoding="utf-8", + ) + fake_gh = root / "gh" + fake_gh.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import os + import sys + from pathlib import Path + + endpoint = sys.argv[-1] + state = Path(os.environ["STATE"]) + if endpoint.endswith("/releases/389"): + assets = [{"id": 992, "name": "receipt.json", "state": "starter"}] if state.exists() else [] + print(json.dumps({"assets": assets})) + elif "--method" in sys.argv and endpoint.endswith("/releases/assets/992"): + state.unlink() + else: + raise SystemExit(f"unexpected gh invocation: {sys.argv}") + """ + ), + encoding="utf-8", + ) + fake_sleep = root / "sleep" + fake_sleep.write_text("#!/usr/bin/env bash\nexit 0\n", encoding="utf-8") + for executable in (fake_curl, fake_gh, fake_sleep): + executable.chmod(0o700) + result = subprocess.run( + ["bash", "-c", f"source '{HELPER}'; ep_draft_upload 389 '{receipt}' receipt.json"], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GITHUB_REPOSITORY": "example/repository", + "GH_TOKEN": "test-token", + "COUNTER": str(counter), + "STATE": str(state), + }, + text=True, + capture_output=True, + check=False, + ) + counter_value = counter.read_text() + state_exists = state.exists() + + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(counter_value, "2") + self.assertFalse(state_exists) + + def test_upload_does_not_retry_or_read_assets_after_permanent_authorization_failure(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + receipt = root / "receipt.json" + receipt.write_text("exact receipt", encoding="utf-8") + counter = root / "curl-count" + gh_invoked = root / "gh-invoked" + fake_curl = root / "curl" + fake_curl.write_text( + "#!/usr/bin/env python3\n" + "import os\nfrom pathlib import Path\n" + "counter=Path(os.environ['COUNTER']); counter.write_text(str(int(counter.read_text())+1) if counter.exists() else '1')\n" + "print('403', end='')\n", + encoding="utf-8", + ) + fake_gh = root / "gh" + fake_gh.write_text( + "#!/usr/bin/env python3\nimport os\nfrom pathlib import Path\n" + "Path(os.environ['GH_INVOKED']).write_text('yes')\nraise SystemExit(1)\n", + encoding="utf-8", + ) + fake_curl.chmod(0o700) + fake_gh.chmod(0o700) + result = subprocess.run( + ["bash", "-c", f"source '{HELPER}'; ep_draft_upload 389 '{receipt}' receipt.json"], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GITHUB_REPOSITORY": "example/repository", + "GH_TOKEN": "test-token", + "COUNTER": str(counter), + "GH_INVOKED": str(gh_invoked), + }, + text=True, + capture_output=True, + check=False, + ) + counter_value = counter.read_text() + gh_was_invoked = gh_invoked.exists() + + self.assertNotEqual(result.returncode, 0) + self.assertEqual(counter_value, "1") + self.assertFalse(gh_was_invoked) + self.assertIn("failed permanently with HTTP 403", result.stderr) + + def test_upload_reconciles_but_rejects_an_undocumented_204_without_asset_evidence(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + receipt = root / "receipt.json" + receipt.write_text("exact receipt", encoding="utf-8") + counter = root / "curl-count" + gh_counter = root / "gh-count" + fake_curl = root / "curl" + fake_curl.write_text( + "#!/usr/bin/env python3\n" + "import os\nfrom pathlib import Path\n" + "counter=Path(os.environ['COUNTER']); counter.write_text(str(int(counter.read_text())+1) if counter.exists() else '1')\n" + "print('204', end='')\n", + encoding="utf-8", + ) + fake_gh = root / "gh" + fake_gh.write_text( + "#!/usr/bin/env python3\nimport json, os\nfrom pathlib import Path\n" + "counter=Path(os.environ['GH_COUNTER']); counter.write_text(str(int(counter.read_text())+1) if counter.exists() else '1')\n" + "print(json.dumps({'assets': []}))\n", + encoding="utf-8", + ) + fake_curl.chmod(0o700) + fake_gh.chmod(0o700) + result = subprocess.run( + ["bash", "-c", f"source '{HELPER}'; ep_draft_upload 389 '{receipt}' receipt.json"], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GITHUB_REPOSITORY": "example/repository", + "GH_TOKEN": "test-token", + "COUNTER": str(counter), + "GH_COUNTER": str(gh_counter), + }, + text=True, + capture_output=True, + check=False, + ) + counter_value = counter.read_text() + gh_counter_value = gh_counter.read_text() + + self.assertNotEqual(result.returncode, 0) + self.assertEqual(counter_value, "1") + self.assertEqual(gh_counter_value, "1") + self.assertIn("unexpected HTTP 204 without asset evidence", result.stderr) + + def test_upload_rejects_an_uploaded_asset_with_different_bytes(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + receipt = root / "receipt.json" + receipt.write_text("expected", encoding="utf-8") + fake_curl = root / "curl" + fake_curl.write_text("#!/usr/bin/env bash\nprintf 502\nexit 22\n", encoding="utf-8") + fake_gh = root / "gh" + fake_gh.write_text( + textwrap.dedent( + """\ + #!/usr/bin/env python3 + import json + import sys + endpoint = sys.argv[-1] + if endpoint.endswith("/releases/389"): + print(json.dumps({"assets": [{"id": 993, "name": "receipt.json", "state": "uploaded"}]})) + elif endpoint.endswith("/releases/assets/993"): + print("different", end="") + """ + ), + encoding="utf-8", + ) + fake_curl.chmod(0o700) + fake_gh.chmod(0o700) + result = subprocess.run( + ["bash", "-c", f"source '{HELPER}'; ep_draft_upload 389 '{receipt}' receipt.json"], + env={ + **os.environ, + "PATH": str(root) + os.pathsep + os.environ["PATH"], + "GITHUB_REPOSITORY": "example/repository", + "GH_TOKEN": "test-token", + }, + text=True, + capture_output=True, + check=False, + ) + + self.assertNotEqual(result.returncode, 0) + self.assertIn("unexpected bytes", result.stderr) diff --git a/tools/qualification/github_draft_release.sh b/tools/qualification/github_draft_release.sh index 0c628796..db2512e1 100644 --- a/tools/qualification/github_draft_release.sh +++ b/tools/qualification/github_draft_release.sh @@ -89,14 +89,105 @@ ep_draft_download() { gh api -H 'Accept: application/octet-stream' "repos/$GITHUB_REPOSITORY/releases/assets/$asset_id" > "$output" } +ep_draft_asset_records() { + local release_id="$1" asset_name="$2" + gh api "repos/$GITHUB_REPOSITORY/releases/$release_id" | ASSET_NAME="$asset_name" python3 -c ' +import json +import os +import sys + +release = json.load(sys.stdin) +assets = release.get("assets") if isinstance(release, dict) else None +if not isinstance(assets, list): + raise SystemExit("GitHub release response has no asset list") +for asset in assets: + if isinstance(asset, dict) and asset.get("name") == os.environ["ASSET_NAME"]: + asset_id, state = asset.get("id"), asset.get("state") + if not isinstance(asset_id, int) or asset_id <= 0 or not isinstance(state, str) or not state: + raise SystemExit("matching GitHub release asset has invalid identity or state") + print(f"{asset_id}\t{state}") +' +} + ep_draft_upload() { local release_id="$1" input="$2" asset_name="$3" encoded_name + local attempt=1 max_attempts=6 curl_status http_status records asset_id asset_state + local matching=() encoded_name="$(python3 -c 'from urllib.parse import quote; import sys; print(quote(sys.argv[1], safe=""))' "$asset_name")" - curl --fail --silent --show-error --request POST \ - -H "Authorization: Bearer ${GH_TOKEN:?GH_TOKEN is required for draft asset upload}" \ - -H 'Content-Type: application/octet-stream' \ - --data-binary "@$input" \ - "https://uploads.github.com/repos/$GITHUB_REPOSITORY/releases/$release_id/assets?name=$encoded_name" >/dev/null + while [ "$attempt" -le "$max_attempts" ]; do + set +e + http_status="$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' \ + --request POST \ + -H "Authorization: Bearer ${GH_TOKEN:?GH_TOKEN is required for draft asset upload}" \ + -H 'Accept: application/vnd.github+json' \ + -H 'X-GitHub-Api-Version: 2022-11-28' \ + -H 'Content-Type: application/octet-stream' \ + --data-binary "@$input" \ + "https://uploads.github.com/repos/$GITHUB_REPOSITORY/releases/$release_id/assets?name=$encoded_name")" + curl_status=$? + set -e + if [ "$curl_status" -eq 0 ] && [ "$http_status" = 201 ]; then + return 0 + fi + + # Authentication, authorization and identity failures are permanent and + # must never cause another mutating request. + if [[ "$http_status" =~ ^4[0-9][0-9]$ ]] && [ "$http_status" != 408 ] && [ "$http_status" != 422 ] && [ "$http_status" != 429 ]; then + echo "GitHub draft asset upload failed permanently with HTTP $http_status." >&2 + return 1 + fi + + # A transport error, 5xx or duplicate response is ambiguous: GitHub may + # already have accepted the bytes. Perform an authoritative read before + # deciding whether any further mutation is safe. + if ! records="$(ep_draft_asset_records "$release_id" "$asset_name")"; then + echo "GitHub draft asset upload could not reconcile the asset list." >&2 + return 1 + fi + matching=() + if [ -n "$records" ]; then + while IFS= read -r record; do matching+=("$record"); done <<< "$records" + fi + if [ "${#matching[@]}" -gt 1 ]; then + echo "GitHub draft asset upload found duplicate matching assets." >&2 + return 1 + fi + if [ "${#matching[@]}" -eq 1 ]; then + IFS=$'\t' read -r asset_id asset_state <<< "${matching[0]}" + if [ "$asset_state" = uploaded ]; then + if gh api -H 'Accept: application/octet-stream' \ + "repos/$GITHUB_REPOSITORY/releases/assets/$asset_id" | cmp -s "$input" -; then + return 0 + fi + echo "GitHub draft asset exists with unexpected bytes or is unreadable." >&2 + return 1 + fi + if [ "$asset_state" != starter ]; then + echo "GitHub draft asset is in unsupported state $asset_state." >&2 + return 1 + fi + gh api --method DELETE "repos/$GITHUB_REPOSITORY/releases/assets/$asset_id" >/dev/null + if ! records="$(ep_draft_asset_records "$release_id" "$asset_name")" || [ -n "$records" ]; then + echo "Incomplete GitHub draft asset was not authoritatively removed." >&2 + return 1 + fi + elif [ "$http_status" = 422 ]; then + echo "GitHub rejected the upload but no matching asset exists." >&2 + return 1 + fi + + if [ "$curl_status" -eq 0 ] && [ "$http_status" != 408 ] && [ "$http_status" != 429 ] && [[ ! "$http_status" =~ ^5[0-9][0-9]$ ]]; then + echo "GitHub draft asset upload returned unexpected HTTP $http_status without asset evidence." >&2 + return 1 + fi + + if [ "$attempt" -eq "$max_attempts" ]; then + echo "GitHub draft asset upload remained unavailable after reconciliation." >&2 + return 1 + fi + attempt=$((attempt + 1)) + sleep 2 + done } ep_publish_draft() {