Skip to content

Bug 2037926 - landoscript: introduce file-specific version_bump scopes #1448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohanLorenzo wants to merge 2 commits into
base: master
Choose a base branch
from
+59 −7
Open

Bug 2037926 - landoscript: introduce file-specific version_bump scopes #1448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcc9481
bug 2037926 - landoscript: accept file-specific version_bump scopes
JohanLorenzo May 25, 2026
94037ce
style: pre-commit.ci auto fixes [...]
pre-commit-ci[bot] May 25, 2026
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
14 changes: 7 additions & 7 deletions landoscript/src/landoscript/script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ 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: list[str] | None = None):
expected_scopes = {f"project:releng:lando:repo:{lando_repo}"}
expected_scopes.update(f"project:releng:lando:action:{a}" for a in actions if a != "version_bump")
if "version_bump" in actions and "project:releng:lando:action:version_bump" not in scopes:
expected_scopes.update(f"project:releng:lando:action:version_bump:file:{f}" for f in (version_bump_files or []))
Copy link
Copy Markdown
Contributor

@jcristau jcristau May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of this ugly (version_bump_files or []), consider having the caller pass an empty list, or adding and version_bump_files to the if?

Copy link
Copy Markdown
Contributor

@bhearsum bhearsum May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree; this is a case where more verbose/simpler code would be beneficial now that the logic is less than trivial. I think it also has a bug in it as currently written? (I don't see any way that project:releng:lando:action:version_bump is ever in expected_scopes?)

How about something like this?

def validate_scopes(scopes, lando_repo, actions, version_bump_files=[]):
  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:
        # It's superfluous to add this to the expected scopes, but a bit more clear/correct to do so? This block goes away when we stop supporting this legacy scope anyways.
        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)}")


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 = payload.get("version_bump_info", {}).get("files") if "version_bump" in payload["actions"] else None
validate_scopes(scopes, lando_repo, payload["actions"], version_bump_files)
if len(payload["actions"]) < 1:
raise TaskVerificationError("must provide at least one action!")

Expand Down
52 changes: 52 additions & 0 deletions landoscript/tests/test_script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,58 @@ async def test_missing_scopes(aioresponses, github_installation_responses, conte
assert m in e.args[0]


@pytest.mark.asyncio
async def test_file_specific_scope_allows_permitted_file(aioresponses, github_installation_responses, context):
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)


@pytest.mark.asyncio
async def test_file_specific_scope_rejects_non_permitted_file(aioresponses, github_installation_responses, context):
payload = {
"actions": ["version_bump"],
"lando_repo": "repo_name",
"version_bump_info": {
"files": ["config/milestone.txt"],
"next_version": "135.0",
},
}
setup_test(aioresponses, github_installation_responses, context, payload, ["version_bump"])

context.task = {
"payload": payload,
"scopes": [
"project:releng:lando:repo:repo_name",
"project:releng:lando:action:version_bump:file:browser/config/version.txt",
],
}
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]
assert "project:releng:lando:action:version_bump:file:config/milestone.txt" in e.args[0]


def test_task_schema(context):
payload = {
"actions": ["tag", "version_bump"],
Expand Down