Skip to content
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.

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

Support multiple scope prefixes in bitrisescript #953

Merged
merged 3 commits into from
Mar 28, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion bitrisescript/docker.d/worker.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
work_dir: { "$eval": "WORK_DIR" }
artifact_dir: { "$eval": "ARTIFACTS_DIR" }
verbose: { "$eval": "VERBOSE == 'true'" }
bitrise:
access_token: { "$eval": "BITRISE_ACCESS_TOKEN" }

taskcluster_scope_prefix: "project:${COT_PRODUCT}:bitrise:"
trust_domain: "${COT_PRODUCT}"
taskcluster_scope_prefixes:
$switch:
'COT_PRODUCT == "mobile"':
["project:mobile:firefox-ios:releng:bitrise:"]
2 changes: 1 addition & 1 deletion bitrisescript/examples/config.example.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"bitrise": {
"access_token": "<token>"
},
"taskcluster_scope_prefix": ["project:mobile:firefox-ios:bitrise:"],
"taskcluster_scope_prefixes": ["project:mobile:firefox-ios:bitrise:"],

"verbose": true
}
6 changes: 4 additions & 2 deletions bitrisescript/src/bitrisescript/bitrise.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ async def request(self, endpoint: str, method: str = "get", **kwargs: Any) -> An
while True:
r = await self._client.request(method, url, **kwargs)
log.debug(f"{method_and_url} returned HTTP code {r.status}")
if r.status >= 400:
log.debug(f"{method_and_url} returned JSON:\n{pformat(data)}")
r.raise_for_status()

response = await r.json()

if "data" not in response:
Expand All @@ -104,8 +108,6 @@ async def request(self, endpoint: str, method: str = "get", **kwargs: Any) -> An
break
kwargs.setdefault("params", {})["next"] = next

log.debug(f"{method_and_url} returned JSON {pformat(data)}")

return data


Expand Down
9 changes: 6 additions & 3 deletions bitrisescript/src/bitrisescript/data/config_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"type": "object",
"required": [
"bitrise",
"taskcluster_scope_prefix",
"taskcluster_scope_prefixes",
"trust_domain"
],
"properties": {
Expand All @@ -16,8 +16,11 @@
}
}
},
"taskcluster_scope_prefix": {
"type": "string"
"taskcluster_scope_prefixes": {
"type": "array",
"items": {
"type": "string"
}
},
"trust_domain": {
"type": "string"
Expand Down
12 changes: 4 additions & 8 deletions bitrisescript/src/bitrisescript/script.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
#!/usr/bin/env python3
""" Github main script
""" Bitrise main script
"""
import asyncio
import logging
import os

from bitrisescript.bitrise import BitriseClient, run_build
from bitrisescript.task import get_bitrise_app, get_bitrise_workflows, get_build_params, validate_scope_prefixes
from bitrisescript.task import get_bitrise_app, get_bitrise_workflows, get_build_params
from scriptworker_client.client import sync_main

log = logging.getLogger(__name__)


async def async_main(config, task):
validate_scope_prefixes(config, task)

artifact_dir = os.path.join(config["work_dir"], "artifacts")

app = get_bitrise_app(config, task)
log.info(f"Bitrise app: '{app}'")

Expand All @@ -25,12 +21,12 @@ async def async_main(config, task):
futures = []
for workflow in get_bitrise_workflows(config, task):
build_params["workflow_id"] = workflow
futures.append(run_build(artifact_dir, **build_params))
futures.append(run_build(config["artifact_dir"], **build_params))

client = None
try:
client = BitriseClient()
client.set_auth(config["bitrise"]["token"])
client.set_auth(config["bitrise"]["access_token"])
await client.set_app_prefix(app)
await asyncio.gather(*futures)
finally:
Expand Down
44 changes: 24 additions & 20 deletions bitrisescript/src/bitrisescript/task.py
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
from textwrap import dedent
from typing import Any

from scriptworker_client.exceptions import TaskVerificationError
from scriptworker_client.utils import get_single_item_from_sequence


def validate_scope_prefixes(config, task):
"""Validates scope prefixes.
def _get_allowed_scope_prefixes(config):
prefixes = config["taskcluster_scope_prefixes"]
return [prefix if prefix.endswith(":") else "{}:".format(prefix) for prefix in prefixes]


def extract_common_scope_prefix(config: dict[str, Any], task: dict[str, Any]) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

are these scope resolution functions generic enough to move to something like scriptworker_client.task?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, good call! I copy/pasted it from another script and it looks like a bunch of them have something very similar.. Should be a follow-up though.

"""Extract common scope prefix.

Args:
config (dict): The bitrisescript config.
task (dict): The task definition.
"""
prefix = config["taskcluster_scope_prefix"]
prefixes = _get_allowed_scope_prefixes(config)
scopes = task["scopes"]

invalid_scopes = {s for s in task["scopes"] if not s.startswith(prefix)}
if invalid_scopes:
invalid_scopes = "\n".join(sorted(invalid_scopes))
raise TaskVerificationError(
dedent(
f"""
The following scopes have an invalid prefix:
{invalid_scopes}
found_prefixes = {prefix for prefix in prefixes for scope in scopes if scope.startswith(prefix)}

Expected prefix:
{prefix}
""".lstrip()
)
)
return get_single_item_from_sequence(
sequence=found_prefixes,
condition=lambda _: True,
ErrorClass=TaskVerificationError,
no_item_error_message=f"No scope starting with any of these prefixes {prefixes} found",
too_many_item_error_message="Too many prefixes found",
)


def _extract_last_chunk_of_scope(scope: str, prefix: str) -> str:
Expand All @@ -51,7 +51,7 @@ def get_bitrise_app(config: dict[str, Any], task: dict[str, Any]) -> str:
TaskVerificationError: If task is missing the app scope or has more
than one app scope.
"""
prefix = config["taskcluster_scope_prefix"]
prefix = extract_common_scope_prefix(config, task)
app_prefix = f"{prefix}app:"
scope = get_single_item_from_sequence(
sequence=task["scopes"],
Expand All @@ -76,9 +76,13 @@ def get_bitrise_workflows(config: dict[str, Any], task: dict[str, Any]) -> list[
Returns:
list: A list of bitrise workflows that should be scheduled.
"""
prefix = config["taskcluster_scope_prefix"]
prefix = extract_common_scope_prefix(config, task)
workflow_prefix = f"{prefix}workflow:"
return [_extract_last_chunk_of_scope(scope, workflow_prefix) for scope in task["scopes"] if scope.startswith(workflow_prefix)]
workflows = [_extract_last_chunk_of_scope(scope, workflow_prefix) for scope in task["scopes"] if scope.startswith(workflow_prefix)]

if not workflows:
raise TaskVerificationError(f"No workflow scopes starting with '{prefix}' found")
return workflows


def get_build_params(task: dict[str, Any]) -> dict[str, Any]:
Expand Down
2 changes: 1 addition & 1 deletion bitrisescript/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ def responses():

@pytest.fixture
def config():
return {"bitrise": {"token": "abc"}, "taskcluster_scope_prefix": "test:prefix:", "work_dir": "work"}
return {"bitrise": {"access_token": "abc"}, "taskcluster_scope_prefixes": ["test:prefix:"], "artifact_dir": "work/artifacts", "work_dir": "work"}
6 changes: 4 additions & 2 deletions bitrisescript/tests/test_bitrise.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,13 @@ async def test_bitrise_client_set_app_prefix(mocker, client, app, response, expe
)
async def test_bitrise_client_request(config, mocker, client, prefix, endpoint, method, kwargs, raises, expected_args, expected_kwargs):
expected_kwargs.setdefault("headers", {}).update(
{"Authorization": config["bitrise"]["token"]},
{"Authorization": config["bitrise"]["access_token"]},
)

response = mocker.AsyncMock()
response.status = 200
m = mocker.patch.object(client._client, "request", return_value=Future())
m.return_value.set_result(mocker.AsyncMock())
m.return_value.set_result(response)

if prefix:
client.prefix = prefix
Expand Down
8 changes: 6 additions & 2 deletions bitrisescript/tests/test_integration.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from pathlib import Path
import pytest

from bitrisescript.bitrise import BITRISE_API_URL
Expand All @@ -8,7 +9,10 @@
async def test_main_run_workflow(responses, tmp_path, config):
work_dir = tmp_path / "work"
work_dir.mkdir()

artifact_dir = work_dir / "artifacts"
config["work_dir"] = str(work_dir)
config["artifact_dir"] = str(artifact_dir)

app = "project"
app_slug = "abc"
Expand Down Expand Up @@ -49,10 +53,10 @@ async def test_main_run_workflow(responses, tmp_path, config):
await async_main(config, task)

for workflow in workflows:
artifact = work_dir / "artifacts" / workflow / f"{workflow}.zip"
artifact = artifact_dir / workflow / f"{workflow}.zip"
assert artifact.is_file()
assert artifact.read_text() == workflow

log = work_dir / "artifacts" / workflow / "bitrise.log"
log = artifact_dir / workflow / "bitrise.log"
assert log.is_file()
assert log.read_text() == "log"
6 changes: 3 additions & 3 deletions bitrisescript/tests/test_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
@pytest.mark.parametrize(
"task, expectation, expected_num_futures",
(
pytest.param({"scopes": ["test:prefix:app:bar"]}, does_not_raise(), 0, id="no_workflow"),
pytest.param({"scopes": ["test:prefix:app:bar"]}, pytest.raises(TaskVerificationError), 0, id="no_workflow"),
pytest.param({"scopes": ["test:prefix:app:bar", "test:prefix:workflow:baz"]}, does_not_raise(), 1, id="single_workflow"),
pytest.param({"scopes": ["test:prefix:app:bar", "test:prefix:workflow:baz", "test:prefix:workflow:other"]}, does_not_raise(), 2, id="two_workflows"),
pytest.param({"scopes": ["bad:app:bar"]}, pytest.raises(TaskVerificationError), 1, id="invalid_prefix_app"),
pytest.param({"scopes": ["test:prefix:app:bar", "bad:workflow:baz"]}, pytest.raises(TaskVerificationError), 1, id="invalid_prefix_workflow"),
pytest.param({"scopes": ["bad:app:bar"]}, pytest.raises(TaskVerificationError), 0, id="invalid_prefix_app"),
pytest.param({"scopes": ["test:prefix:app:bar", "bad:workflow:baz"]}, pytest.raises(TaskVerificationError), 0, id="invalid_prefix_workflow"),
),
)
async def test_async_main(mocker, config, task, expectation, expected_num_futures):
Expand Down
50 changes: 40 additions & 10 deletions bitrisescript/tests/test_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,43 @@


@pytest.mark.parametrize(
"task, expectation",
"config, task, expectation, expected_result",
(
pytest.param({"scopes": ["test:prefix:app:foo", "test:prefix:workflow:bar"]}, does_not_raise(), id="valid"),
pytest.param({"scopes": ["bad:prefix:app:foo", "test:prefix:workflow:bar"]}, pytest.raises(TaskVerificationError), id="invalid"),
pytest.param({"scopes": ["test:prefix:app:foo", "bad:prefix:workflow:bar"]}, pytest.raises(TaskVerificationError), id="invalid"),
(
{"taskcluster_scope_prefixes": ["some:prefix"]},
{"scopes": ["some:prefix:project:someproject"]},
does_not_raise(),
"some:prefix:",
),
(
{"taskcluster_scope_prefixes": ["some:prefix:"]},
{"scopes": ["some:prefix:project:someproject"]},
does_not_raise(),
"some:prefix:",
),
(
{"taskcluster_scope_prefixes": ["some:prefix"]},
{"scopes": ["some:prefix:project:someproject", "some:prefix:action:someaction"]},
does_not_raise(),
"some:prefix:",
),
(
{"taskcluster_scope_prefixes": ["another:prefix"]},
{"scopes": ["some:prefix:project:someproject", "some:prefix:action:someaction"]},
pytest.raises(TaskVerificationError),
None,
),
(
{"taskcluster_scope_prefixes": ["some:prefix", "another:prefix"]},
{"scopes": ["some:prefix:project:someproject", "another:prefix:action:someaction"]},
pytest.raises(TaskVerificationError),
None,
),
),
)
def test_validate_scope_prefixes(config, task, expectation):
def test_extract_common_scope_prefix(config, task, expectation, expected_result):
with expectation:
task_mod.validate_scope_prefixes(config, task)
assert task_mod.extract_common_scope_prefix(config, task) == expected_result


@pytest.mark.parametrize(
Expand Down Expand Up @@ -65,20 +92,23 @@ def test_get_bitrise_app(config, task, expected):


@pytest.mark.parametrize(
"task, expected",
"task, expectation, expected",
(
(
{"scopes": ["test:prefix:app:foo", "test:prefix:workflow:bar", "test:prefix:workflow:baz"]},
does_not_raise(),
["bar", "baz"],
),
(
{"scopes": ["test:prefix:app:foo"]},
[],
pytest.raises(TaskVerificationError),
None,
),
),
)
def test_get_bitrise_workflows(config, task, expected):
assert task_mod.get_bitrise_workflows(config, task) == expected
def test_get_bitrise_workflows(config, task, expectation, expected):
with expectation:
assert task_mod.get_bitrise_workflows(config, task) == expected


@pytest.mark.parametrize(
Expand Down