From 505b77ce76d3dd527a26e14f9c2d8ee9ecb27545 Mon Sep 17 00:00:00 2001 From: Johan Lorenzo Date: Mon, 25 May 2026 12:02:20 +0200 Subject: [PATCH] bug 2037926 - landoscript: accept file-specific version_bump scopes --- landoscript/src/landoscript/script.py | 20 +-- landoscript/tests/test_script.py | 174 ++++++++++++++++++++------ 2 files changed, 149 insertions(+), 45 deletions(-) diff --git a/landoscript/src/landoscript/script.py b/landoscript/src/landoscript/script.py index f6b684ad9..13a244166 100644 --- a/landoscript/src/landoscript/script.py +++ b/landoscript/src/landoscript/script.py @@ -29,11 +29,17 @@ def get_default_config(base_dir: str = "") -> dict: return default_config -def validate_scopes(scopes: set, lando_repo: str, actions: list[str]): - expected_scopes = { - f"project:releng:lando:repo:{lando_repo}", - *[f"project:releng:lando:action:{action}" for action in actions], - } +def validate_scopes(scopes: set, lando_repo: str, actions: list[str], version_bump_files: tuple[str, ...] = ()): + expected_scopes = {f"project:releng:lando:repo:{lando_repo}"} + for action in actions: + if action == "version_bump": + if "project:releng:lando:action:version_bump" in scopes: + # Legacy transition scope — remove this block once all repos generate file-specific scopes. + expected_scopes.add("project:releng:lando:action:version_bump") + else: + expected_scopes.update(f"project:releng:lando:action:version_bump:file:{f}" for f in version_bump_files) + else: + expected_scopes.add(f"project:releng:lando:action:{action}") missing = expected_scopes - scopes if missing: raise scriptworker.client.TaskVerificationError(f"required scope(s) not present: {', '.join(missing)}") @@ -41,8 +47,8 @@ def validate_scopes(scopes: set, lando_repo: str, actions: list[str]): def sanity_check_payload(payload, scopes, lando_repo): """Additional verification past what the task schema does.""" - # validate scopes - these raise if there's any scope issues - validate_scopes(scopes, lando_repo, payload["actions"]) + version_bump_files = tuple(payload.get("version_bump_info", {}).get("files", ())) if "version_bump" in payload["actions"] else () + validate_scopes(scopes, lando_repo, payload["actions"], version_bump_files) if len(payload["actions"]) < 1: raise TaskVerificationError("must provide at least one action!") diff --git a/landoscript/tests/test_script.py b/landoscript/tests/test_script.py index a31bd83b6..6c390f878 100644 --- a/landoscript/tests/test_script.py +++ b/landoscript/tests/test_script.py @@ -6,7 +6,7 @@ from pytest_scriptworker_client import get_files_payload from landoscript.errors import LandoscriptError, MergeConflictError -from landoscript.script import async_main, get_default_config +from landoscript.script import async_main, get_default_config, validate_scopes from .conftest import ( assert_lando_submission_response, assert_status_response, @@ -262,52 +262,102 @@ async def test_no_actions(aioresponses, github_installation_responses, context): ) -@pytest.mark.asyncio @pytest.mark.parametrize( - "scopes,missing", + "scopes,actions,version_bump_files,missing", ( pytest.param( - [ - "project:releng:lando:action:tag", - "project:releng:lando:action:version_bump", - ], - [ - "project:releng:lando:repo:repo_name", - ], + {"project:releng:lando:action:tag", "project:releng:lando:action:version_bump"}, + ["tag", "version_bump"], + ("browser/config/version.txt",), + {"project:releng:lando:repo:repo_name"}, id="missing_repo_scope", ), pytest.param( - [ - "project:releng:lando:repo:repo_name", - "project:releng:lando:action:tag", - ], - [ - "project:releng:lando:action:version_bump", - ], - id="missing_one_action_scope", + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:tag"}, + ["tag", "version_bump"], + ("browser/config/version.txt",), + {"project:releng:lando:action:version_bump:file:browser/config/version.txt"}, + id="missing_version_bump_scope", ), pytest.param( - [ - "project:releng:lando:repo:repo_name", - ], - [ - "project:releng:lando:action:tag", - "project:releng:lando:action:version_bump", - ], - id="missing_two_action_scopes", + {"project:releng:lando:repo:repo_name"}, + ["tag", "version_bump"], + ("browser/config/version.txt",), + {"project:releng:lando:action:tag", "project:releng:lando:action:version_bump:file:browser/config/version.txt"}, + id="missing_multiple_scopes", + ), + pytest.param( + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:version_bump:file:browser/config/version.txt"}, + ["version_bump"], + ("browser/config/version.txt", "config/milestone.txt"), + {"project:releng:lando:action:version_bump:file:config/milestone.txt"}, + id="missing_one_of_two_file_scopes", + ), + pytest.param( + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:version_bump:file:browser/config/version.txt"}, + ["version_bump"], + ("config/milestone.txt",), + {"project:releng:lando:action:version_bump:file:config/milestone.txt"}, + id="wrong_file_scope", + ), + ), +) +def test_validate_scopes_raises_on_missing(scopes, actions, version_bump_files, missing): + try: + validate_scopes(scopes, "repo_name", actions, version_bump_files) + assert False, "should've raised TaskVerificationError" + except TaskVerificationError as e: + assert "required scope(s) not present" in e.args[0] + for m in missing: + assert m in e.args[0] + + +@pytest.mark.parametrize( + "scopes,actions,version_bump_files", + ( + pytest.param( + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:version_bump"}, + ["version_bump"], + ("browser/config/version.txt",), + id="legacy_general_scope", ), pytest.param( - [], - [ + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:version_bump:file:browser/config/version.txt"}, + ["version_bump"], + ("browser/config/version.txt",), + id="file_specific_single_file", + ), + pytest.param( + { "project:releng:lando:repo:repo_name", - "project:releng:lando:action:tag", - "project:releng:lando:action:version_bump", - ], - id="no_scopes", + "project:releng:lando:action:version_bump:file:browser/config/version.txt", + "project:releng:lando:action:version_bump:file:config/milestone.txt", + }, + ["version_bump"], + ("browser/config/version.txt", "config/milestone.txt"), + id="file_specific_multi_file", + ), + pytest.param( + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:version_bump"}, + ["version_bump"], + ("browser/config/version.txt", "config/milestone.txt"), + id="legacy_general_scope_multiple_files", + ), + pytest.param( + {"project:releng:lando:repo:repo_name", "project:releng:lando:action:tag"}, + ["tag"], + (), + id="no_version_bump_action", ), ), ) -async def test_missing_scopes(aioresponses, github_installation_responses, context, scopes, missing): +def test_validate_scopes_passes(scopes, actions, version_bump_files): + validate_scopes(scopes, "repo_name", actions, version_bump_files) + + +@pytest.mark.asyncio +async def test_missing_scopes(aioresponses, github_installation_responses, context): + """Verify async_main raises when scopes are missing, exercising the full payload extraction path.""" payload = { "actions": ["tag", "version_bump"], "lando_repo": "repo_name", @@ -316,18 +366,66 @@ async def test_missing_scopes(aioresponses, github_installation_responses, conte "next_version": "135.0", }, } - setup_test(aioresponses, github_installation_responses, context, payload, ["version_bump"]) - - context.task = {"payload": payload, "scopes": scopes} + context.task = {"payload": payload, "scopes": []} try: await async_main(context) assert False, "should've raised TaskVerificationError" except TaskVerificationError as e: assert "required scope(s) not present" in e.args[0] - for m in missing: - assert m in e.args[0] + + +@pytest.mark.asyncio +async def test_legacy_scope_accepted_by_async_main(aioresponses, github_installation_responses, context): + """Verify async_main still accepts the legacy general scope during the transition period.""" + payload = { + "actions": ["version_bump"], + "lando_repo": "repo_name", + "version_bump_info": { + "files": ["browser/config/version.txt"], + "next_version": "135.0", + }, + } + submit_uri, status_uri, job_id, _ = setup_test(aioresponses, github_installation_responses, context, payload, ["version_bump"]) + setup_github_graphql_responses(aioresponses, get_files_payload({"browser/config/version.txt": "134.0"})) + aioresponses.post(submit_uri, status=202, payload={"job_id": job_id, "status_url": str(status_uri), "message": "foo", "started_at": "2025-03-08T12:25:00Z"}) + aioresponses.get(status_uri, status=200, payload={"commits": ["abcdef123"], "push_id": job_id, "status": "LANDED"}) + + context.task = { + "payload": payload, + "scopes": [ + "project:releng:lando:repo:repo_name", + "project:releng:lando:action:version_bump", + ], + } + await async_main(context) + + +@pytest.mark.asyncio +async def test_file_specific_scope_accepted_by_async_main(aioresponses, github_installation_responses, context): + """Verify async_main correctly passes version_bump_files from the payload to validate_scopes.""" + payload = { + "actions": ["version_bump"], + "lando_repo": "repo_name", + "version_bump_info": { + "files": ["browser/config/version.txt"], + "next_version": "135.0", + }, + } + submit_uri, status_uri, job_id, _ = setup_test(aioresponses, github_installation_responses, context, payload, ["version_bump"]) + setup_github_graphql_responses(aioresponses, get_files_payload({"browser/config/version.txt": "134.0"})) + aioresponses.post(submit_uri, status=202, payload={"job_id": job_id, "status_url": str(status_uri), "message": "foo", "started_at": "2025-03-08T12:25:00Z"}) + aioresponses.get(status_uri, status=200, payload={"commits": ["abcdef123"], "push_id": job_id, "status": "LANDED"}) + + context.task = { + "payload": payload, + "scopes": [ + "project:releng:lando:repo:repo_name", + "project:releng:lando:action:version_bump:file:browser/config/version.txt", + ], + } + await async_main(context) def test_task_schema(context):