Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/odoo_forge_cli/commands/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,7 @@ def _collect_published_layer() -> dict[str, object]:
def _collect_layers() -> list[dict[str, object]]:
layers: list[dict[str, object]] = []
while typer.confirm("Add a layer?" if not layers else "Add another layer?", default=False):
layer_type = _prompt_text("Layer type (git or published)").lower()
while layer_type not in {"git", "published"}:
typer.echo("error: layer type must be 'git' or 'published'", err=True)
layer_type = _prompt_text("Layer type (git or published)").lower()
layers.append(_collect_git_layer() if layer_type == "git" else _collect_published_layer())
layers.append(_collect_git_layer())
return layers


Expand Down Expand Up @@ -162,11 +158,7 @@ def _collect_draft() -> dict[str, object]:
layers = _collect_layers()
draft["layers"] = layers
addons_path = _prompt_text("Client addons path")
requirements = _prompt_text("Python requirements path", "")
client: dict[str, object] = {"addons_path": addons_path}
if requirements:
client["python_requirements"] = requirements
draft["client"] = client
draft["client"] = {"addons_path": addons_path}
draft["overrides"] = _collect_overrides()

if typer.confirm("Configure workspace", default=False):
Expand Down
3 changes: 1 addition & 2 deletions src/odoo_forge_pipeline_github/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ def __init__(
self._ref = ref

def trigger(self, spec: PipelineRunSpec) -> PipelineRunRef:
self._transport.dispatch_workflow(spec.definition, self._ref, spec.parameters)
run_id = self._transport.latest_run_id(spec.definition, self._ref)
run_id = self._transport.dispatch_workflow(spec.definition, self._ref, spec.parameters)
return PipelineRunRef(run_id=run_id)

def status(self, ref: PipelineRunRef) -> PipelineRunStatus:
Expand Down
64 changes: 44 additions & 20 deletions src/odoo_forge_pipeline_github/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,25 @@

from __future__ import annotations

import io
import json
import urllib.request
import zipfile
import zlib
from pathlib import PurePosixPath
from typing import Protocol, runtime_checkable

GITHUB_API_BASE_URL = "https://api.github.com"
DEFAULT_TIMEOUT_SECONDS = 30.0
MAX_LOG_ARCHIVE_BYTES = 20 * 1024 * 1024
MAX_LOG_BYTES = 100 * 1024 * 1024
MAX_LOG_ENTRIES = 1000


@runtime_checkable
class GitHubActionsTransport(Protocol):
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> None:
"""Trigger a `workflow_dispatch` event for `workflow` on `ref`."""
...

def latest_run_id(self, workflow: str, ref: str) -> str:
"""Return the id of the newest run for `workflow` on `ref`."""
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> str:
"""Trigger `workflow` on `ref` and return the dispatched run id."""
...

def get_run_state(self, run_id: str) -> tuple[str, str | None]:
Expand Down Expand Up @@ -54,24 +57,20 @@ def __init__(
self._base_url = base_url.rstrip("/")
self._timeout = timeout

def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> None:
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> str:
url = (
f"{self._base_url}/repos/{self._owner}/{self._repo}/actions/"
f"workflows/{workflow}/dispatches"
)
body = json.dumps({"ref": ref, "inputs": inputs}).encode("utf-8")
self._request(url, method="POST", body=body)

def latest_run_id(self, workflow: str, ref: str) -> str:
url = (
f"{self._base_url}/repos/{self._owner}/{self._repo}/actions/workflows/"
f"{workflow}/runs?branch={ref}&per_page=1"
)
payload = json.loads(self._request(url, method="GET"))
runs = payload.get("workflow_runs", [])
if not runs:
raise RuntimeError(f"no runs found for workflow {workflow!r} on ref {ref!r}")
return str(runs[0]["id"])
try:
payload = json.loads(self._request(url, method="POST", body=body))
run_id = payload["workflow_run_id"]
except (UnicodeDecodeError, json.JSONDecodeError, KeyError, TypeError) as exc:
raise RuntimeError("workflow dispatch response has no workflow run id") from exc
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if isinstance(run_id, bool) or not isinstance(run_id, (int, str)) or not str(run_id):
raise RuntimeError("workflow dispatch response has no workflow run id")
return str(run_id)

def get_run_state(self, run_id: str) -> tuple[str, str | None]:
url = f"{self._base_url}/repos/{self._owner}/{self._repo}/actions/runs/{run_id}"
Expand All @@ -80,7 +79,31 @@ def get_run_state(self, run_id: str) -> tuple[str, str | None]:

def get_run_logs(self, run_id: str) -> str:
url = f"{self._base_url}/repos/{self._owner}/{self._repo}/actions/runs/{run_id}/logs"
return self._request(url, method="GET").decode("utf-8", errors="replace")
archive_bytes = self._request(url, method="GET")
if len(archive_bytes) > MAX_LOG_ARCHIVE_BYTES:
raise RuntimeError("log archive exceeds the compressed size limit")
try:
with zipfile.ZipFile(io.BytesIO(archive_bytes)) as archive:
entries = [entry for entry in archive.infolist() if not entry.is_dir()]
if len(entries) > MAX_LOG_ENTRIES:
raise RuntimeError("log archive contains too many entries")
if sum(entry.file_size for entry in entries) > MAX_LOG_BYTES:
raise RuntimeError("log archive exceeds the uncompressed size limit")

output: list[str] = []
for entry in sorted(entries, key=lambda item: item.filename):
normalized = entry.filename.replace("\\", "/")
path = PurePosixPath(normalized)
if path.is_absolute() or ".." in path.parts or entry.flag_bits & 0x1:
raise RuntimeError("log archive contains an unsafe entry")
try:
content = archive.read(entry)
except (zipfile.BadZipFile, EOFError, NotImplementedError, zlib.error) as exc:
raise RuntimeError("invalid log archive") from exc
output.append(content.decode("utf-8", errors="replace"))
return "".join(output)
except (zipfile.BadZipFile, UnicodeDecodeError) as exc:
raise RuntimeError("invalid log archive") from exc

def _request(self, url: str, *, method: str, body: bytes | None = None) -> bytes:
request = urllib.request.Request(
Expand All @@ -90,6 +113,7 @@ def _request(self, url: str, *, method: str, body: bytes | None = None) -> bytes
headers={
"Authorization": f"Bearer {self._token}",
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2026-03-10",
},
)
with urllib.request.urlopen(request, timeout=self._timeout) as response: # noqa: S310
Expand Down
11 changes: 1 addition & 10 deletions tests/cli/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@


def _invoke_configure(target: Path, *, name: str = "demo", edition: str = "community") -> Any:
scripted_input = f"{name}\n19.0\n{edition}\n\n\nn\nclient/addons\n\nn\nn\nn\nn\ny\n"
scripted_input = f"{name}\n19.0\n{edition}\n\n\nn\nclient/addons\nn\nn\nn\nn\ny\n"
return runner.invoke(app, ["configure", "--manifest", str(target)], input=scripted_input)


Expand Down Expand Up @@ -70,7 +70,6 @@ def test_configure_community_yaml(tmp_path: Path) -> None:

def test_configure_enterprise_collects_all_optional_branches(
monkeypatch: pytest.MonkeyPatch,
capsys: pytest.CaptureFixture[str],
) -> None:
prompts = {
"Project name": "enterprise-demo",
Expand All @@ -80,17 +79,11 @@ def test_configure_enterprise_collects_all_optional_branches(
"Core ref override": "stable",
"Enterprise URL override": "https://example.test/enterprise.git",
"Enterprise ref override": "stable",
"Layer type": ["unsupported", "git", "published"],
"Layer name": "custom",
"Layer category": "localization",
"Repository URL": "https://example.test/custom.git",
"Repository ref": "main",
"Published layer name": "published",
"Published layer source": "registry://example/addons",
"Published layer version": "1.0",
"Published layer category": "enterprise-addons",
"Client addons path": "client/addons",
"Python requirements path": "client/requirements.txt",
"Override layer": "custom",
"Override repository": "https://example.test/custom.git",
"Override fork": "https://example.test/fork.git",
Expand All @@ -104,7 +97,6 @@ def test_configure_enterprise_collects_all_optional_branches(
"Add a layer": True,
"Add another repository": False,
"Add another layer": [True, False],
"Published layer requires enterprise": True,
"Add another override": False,
"Add an override": True,
"Configure workspace": True,
Expand All @@ -119,7 +111,6 @@ def test_configure_enterprise_collects_all_optional_branches(
manifest.configure(Path("project.yaml"))

assert raised.value.exit_code == 0
assert "layer type must be 'git' or 'published'" in capsys.readouterr().err


def test_configure_rejects_existing_target_before_prompt(tmp_path: Path) -> None:
Expand Down
12 changes: 4 additions & 8 deletions tests/pipeline_github/fakes.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,20 @@ class FakeGitHubActionsTransport:
def __init__(
self,
*,
run_ids: list[str] | None = None,
run_id: str = "1",
run_state: tuple[str, str | None] = ("queued", None),
run_logs: str = "",
) -> None:
self.dispatch_calls: list[tuple[str, str, dict[str, str]]] = []
self.latest_run_id_calls: list[tuple[str, str]] = []
self.get_run_state_calls: list[str] = []
self.get_run_logs_calls: list[str] = []
self._run_ids = run_ids if run_ids is not None else ["1"]
self._run_id = run_id
self._run_state = run_state
self._run_logs = run_logs

def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> None:
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> str:
self.dispatch_calls.append((workflow, ref, inputs))

def latest_run_id(self, workflow: str, ref: str) -> str:
self.latest_run_id_calls.append((workflow, ref))
return self._run_ids[-1]
return self._run_id

def get_run_state(self, run_id: str) -> tuple[str, str | None]:
self.get_run_state_calls.append(run_id)
Expand Down
2 changes: 1 addition & 1 deletion tests/pipeline_github/test_hermetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def _raise(*args: object, **kwargs: object) -> None:

def test_trigger_status_logs_never_touch_the_network(block_network: None) -> None:
fake = FakeGitHubActionsTransport(
run_ids=["1"], run_state=("in_progress", None), run_logs="hermetic log"
run_id="1", run_state=("in_progress", None), run_logs="hermetic log"
)
provider = GitHubActionsPipelineProvider(
transport=fake, owner="acme", repo="widgets", ref="main"
Expand Down
2 changes: 1 addition & 1 deletion tests/pipeline_github/test_neutrality.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

def test_trigger_status_logs_return_exactly_the_neutral_types() -> None:
fake = FakeGitHubActionsTransport(
run_ids=["55"], run_state=("completed", "success"), run_logs="log text"
run_id="55", run_state=("completed", "success"), run_logs="log text"
)
provider = GitHubActionsPipelineProvider(
transport=fake, owner="acme", repo="widgets", ref="main"
Expand Down
140 changes: 140 additions & 0 deletions tests/pipeline_github/test_rest_transport.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import io
import json
import urllib.request
import zipfile

import pytest

from odoo_forge_pipeline_github.transport import GitHubActionsRestTransport


def _transport() -> GitHubActionsRestTransport:
return GitHubActionsRestTransport(token="token", owner="acme", repo="widgets")


def _zip_bytes(entries: dict[str, bytes]) -> bytes:
output = io.BytesIO()
with zipfile.ZipFile(output, "w") as archive:
for name, content in entries.items():
archive.writestr(name, content)
return output.getvalue()


def _corrupted_compressed_entry() -> bytes:
output = io.BytesIO()
with zipfile.ZipFile(output, "w", compression=zipfile.ZIP_DEFLATED) as archive:
archive.writestr("job.txt", b"workflow output\n" * 100)

corrupted = bytearray(output.getvalue())
with zipfile.ZipFile(io.BytesIO(corrupted)) as archive:
entry = archive.getinfo("job.txt")
data_offset = entry.header_offset + 30 + len(entry.filename.encode()) + len(entry.extra)
corrupted[data_offset] = (corrupted[data_offset] & ~0x06) | 0x06
return bytes(corrupted)


def _mock_urlopen(monkeypatch: pytest.MonkeyPatch, response: bytes) -> list[urllib.request.Request]:
requests: list[urllib.request.Request] = []

class Response:
def __enter__(self) -> "Response":
return self

def __exit__(self, *args: object) -> None:
return None

def read(self) -> bytes:
return response

def urlopen(request: urllib.request.Request, *, timeout: float) -> Response:
requests.append(request)
return Response()

monkeypatch.setattr(urllib.request, "urlopen", urlopen)
return requests


def test_requests_pin_the_github_api_version(monkeypatch: pytest.MonkeyPatch) -> None:
requests = _mock_urlopen(monkeypatch, b'{"workflow_run_id": 314}')

_transport().dispatch_workflow("ci.yml", "main", {})

assert requests[0].get_header("X-github-api-version") == "2026-03-10"


def test_dispatch_returns_the_exact_workflow_run_id(
monkeypatch: pytest.MonkeyPatch,
) -> None:
transport = _transport()
requests = _mock_urlopen(monkeypatch, json.dumps({"workflow_run_id": 314}).encode())

assert transport.dispatch_workflow("ci.yml", "main", {"env": "qa"}) == "314"
assert len(requests) == 1
assert requests[0].method == "POST"
assert requests[0].data == json.dumps({"ref": "main", "inputs": {"env": "qa"}}).encode()
Comment thread
coderabbitai[bot] marked this conversation as resolved.


@pytest.mark.parametrize("response", [b"{}", b"not-json", b"\xff"])
def test_dispatch_fails_when_the_run_id_is_missing_or_malformed(
response: bytes, monkeypatch: pytest.MonkeyPatch
) -> None:
transport = _transport()
_mock_urlopen(monkeypatch, response)

with pytest.raises(RuntimeError, match="workflow run id"):
transport.dispatch_workflow("ci.yml", "main", {})


def test_logs_render_real_zip_entries_in_deterministic_name_order(
monkeypatch: pytest.MonkeyPatch,
) -> None:
transport = _transport()
archive = _zip_bytes({"job/z.txt": b"last\n", "job/a.txt": b"first\n"})
_mock_urlopen(monkeypatch, archive)

assert transport.get_run_logs("42") == "first\nlast\n"


@pytest.mark.parametrize(
"archive",
[b"not-a-zip", _zip_bytes({"../secret.txt": b"secret"}), _zip_bytes({"/abs.txt": b"x"})],
)
def test_logs_reject_malformed_or_unsafe_archives(
archive: bytes, monkeypatch: pytest.MonkeyPatch
) -> None:
transport = _transport()
_mock_urlopen(monkeypatch, archive)

with pytest.raises(RuntimeError, match="log archive"):
transport.get_run_logs("42")


def test_logs_reject_archives_over_the_uncompressed_limit(
monkeypatch: pytest.MonkeyPatch,
) -> None:
transport = _transport()
archive = _zip_bytes({"large.txt": b"x" * 101})
_mock_urlopen(monkeypatch, archive)
monkeypatch.setattr("odoo_forge_pipeline_github.transport.MAX_LOG_BYTES", 100)

with pytest.raises(RuntimeError, match="log archive"):
transport.get_run_logs("42")


def test_logs_reject_corrupted_compressed_entries(monkeypatch: pytest.MonkeyPatch) -> None:
transport = _transport()
_mock_urlopen(monkeypatch, _corrupted_compressed_entry())

with pytest.raises(RuntimeError, match="invalid log archive"):
transport.get_run_logs("42")


def test_logs_reject_malformed_utf8_entry_names(monkeypatch: pytest.MonkeyPatch) -> None:
archive = bytearray(_zip_bytes({"job.txt": b"output"}))
central_header = archive.index(b"PK\x01\x02")
archive[central_header + 8 : central_header + 10] = (0x800).to_bytes(2, "little")
archive[central_header + 46] = 0xFF
_mock_urlopen(monkeypatch, bytes(archive))

with pytest.raises(RuntimeError, match="invalid log archive"):
_transport().get_run_logs("42")
5 changes: 1 addition & 4 deletions tests/pipeline_github/test_transport_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@


class _FakeTransportForProtocolCheck:
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> None:
return None

def latest_run_id(self, workflow: str, ref: str) -> str:
def dispatch_workflow(self, workflow: str, ref: str, inputs: dict[str, str]) -> str:
return "1"

def get_run_state(self, run_id: str) -> tuple[str, str | None]:
Expand Down
Loading
Loading