Skip to content

Commit f3080c5

Browse files
authored
Be stricter in index routes verifications (#924)
Some verification was added in #722 but for slashes only. Taskcluster is actually way stricter than that with what it accepts as index keys and it doesn't prevent task creations when index routes are invalid (taskcluster/taskcluster#6266). The regex comes from the service iteself at https://github.com/taskcluster/taskcluster/blob/62655d84d55680ffb8faa35e7deadee69f389efa/services/index/src/helpers.js#L8 Fixes #883
1 parent 01028b2 commit f3080c5

2 files changed

Lines changed: 45 additions & 6 deletions

File tree

src/taskgraph/util/verify.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,14 @@ def verify_routes_notification_filters(
235235
)
236236

237237

238+
# https://docs.taskcluster.net/docs/reference/core/index#valid-characters
239+
INDEX_NAMESPACE_RE = re.compile(r"^([a-zA-Z0-9_!~*'()%-]+\.)*[a-zA-Z0-9_!~*'()%-]+$")
240+
241+
238242
@verifications.add("full_task_graph")
239243
def verify_index_route(task, taskgraph, scratch_pad, graph_config, parameters):
240244
"""
241-
This function ensures that routes do not contain forward slashes.
245+
This function ensures that index routes would create valid taskcluster index paths
242246
"""
243247
if task is None:
244248
return
@@ -247,11 +251,13 @@ def verify_index_route(task, taskgraph, scratch_pad, graph_config, parameters):
247251
route_prefix = "index."
248252

249253
for route in routes:
250-
# Check for invalid / in the index route
251-
if route.startswith(route_prefix) and "/" in route:
252-
raise Exception(
253-
f"{task.label} has invalid route with forward slash: {route}"
254-
)
254+
if route.startswith(route_prefix):
255+
namespace = route[len(route_prefix) :]
256+
if not INDEX_NAMESPACE_RE.match(namespace):
257+
raise Exception(
258+
f"{task.label} has invalid index route namespace: {route}. "
259+
f"Namespace must match {INDEX_NAMESPACE_RE.pattern}"
260+
)
255261

256262

257263
@verifications.add("full_task_graph")

test/test_util_verify.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,17 @@ def make_task_treeherder(label, symbol, platform="linux/opt"):
211211
pytest.raises(DeprecationWarning),
212212
id="routes_notfication_filter: deprecated",
213213
),
214+
pytest.param(
215+
"verify_index_route",
216+
make_graph(
217+
make_task(
218+
"valid_route",
219+
task_def={"routes": ["index.example.v2.latest.taskgraph.decision"]},
220+
),
221+
),
222+
does_not_raise(),
223+
id="verify_index_route: valid route",
224+
),
214225
pytest.param(
215226
"verify_index_route",
216227
make_graph(
@@ -226,6 +237,28 @@ def make_task_treeherder(label, symbol, platform="linux/opt"):
226237
pytest.raises(Exception),
227238
id="verify_index_route: invalid slash in route",
228239
),
240+
pytest.param(
241+
"verify_index_route",
242+
make_graph(
243+
make_task(
244+
"invalid_plus",
245+
task_def={"routes": ["index.example.1.0+hotfix1.latest"]},
246+
),
247+
),
248+
pytest.raises(Exception),
249+
id="verify_index_route: invalid plus in route",
250+
),
251+
pytest.param(
252+
"verify_index_route",
253+
make_graph(
254+
make_task(
255+
"invalid_space",
256+
task_def={"routes": ["index.example.some namespace.latest"]},
257+
),
258+
),
259+
pytest.raises(Exception),
260+
id="verify_index_route: invalid space in route",
261+
),
229262
pytest.param(
230263
"verify_task_dependencies",
231264
make_graph(

0 commit comments

Comments
 (0)