Skip to content

Commit 4ba2e5a

Browse files
subinz1subinz1
committed
[CRCR] Move all OIDC provider config to ci_providers.yml
Move GitHub Actions OIDC configuration (issuer URL, JWKS endpoint) from hardcoded constants in jwt_helper.py to ci_providers.yml, so all CI provider authentication is configured in a single file. The new providers section defines issuer, jwks_uri, and optional repo_map for each provider. JWKS clients are created dynamically from the config. The legacy flat buildkite section is still supported for backward compatibility. Preserves all security features from #8453: immutable Buildkite IDs, required_claims enforcement, and post-decode issuer mismatch check.
1 parent 6f37b78 commit 4ba2e5a

3 files changed

Lines changed: 226 additions & 59 deletions

File tree

‎aws/lambda/cross_repo_ci_relay/config/ci_providers.yml‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,46 @@
1-
# External CI Provider → GitHub Repo Mapping for CRCR OIDC
1+
# CI Provider Configuration for CRCR OIDC
22
#
3-
# CI platforms that don't carry a native "repository" OIDC claim need an
4-
# explicit mapping from their pipeline identity to a GitHub owner/repo.
3+
# Defines all supported CI providers: their OIDC issuer URLs, JWKS
4+
# endpoints, and (for providers without a native "repository" claim)
5+
# pipeline-to-repo identity mappings.
56
#
67
# IMPORTANT: Buildkite mappings use immutable organization_id/pipeline_id
78
# (UUIDs) rather than slugs. Slugs are renameable and a released slug
89
# can be claimed by a different organization; IDs are permanent.
910
# Find IDs via: buildkite-agent oidc request-token --audience <aud>
1011
#
11-
# SECURITY: By default, any build in a mapped pipeline can authenticate.
12-
# If a pipeline runs fork-PR builds, use required_claims to restrict
13-
# which builds are authorized (e.g. only builds on specific branches).
14-
# Pipelines that do NOT expose OIDC to fork builds are safe without
15-
# required_claims.
16-
#
1712
# The Lambda fetches this file at runtime (via CI_PROVIDERS_URL) and
1813
# caches it in Redis, so changes here take effect without redeployment.
1914

20-
buildkite:
21-
# Simple format (pipeline does not run fork builds):
22-
# organization_id/pipeline_id: owner/repo
23-
#
24-
# Constrained format (pipeline may run fork builds):
25-
# organization_id/pipeline_id:
26-
# repo: owner/repo
27-
# required_claims:
28-
# build_branch: [main, nightly] # only these branches can auth
29-
# cluster_id: specific-cluster-uuid
30-
31-
# Example entries:
32-
# 018e4f2a-1b2c-3d4e/018e5a6b-7c8d-9e0f: vllm-project/vllm
33-
#
34-
# 018e4f2a-1b2c-3d4e/018e9a0b-1c2d-3e4f:
35-
# repo: vllm-project/vllm
36-
# required_claims:
37-
# build_branch: [main, release]
15+
providers:
16+
github:
17+
issuer: https://token.actions.githubusercontent.com
18+
jwks_uri: https://token.actions.githubusercontent.com/.well-known/jwks
3819

39-
# gitlab:
40-
# group/project: owner/repo
20+
buildkite:
21+
issuer: https://agent.buildkite.com
22+
jwks_uri: https://agent.buildkite.com/.well-known/jwks
23+
# Pipeline-to-repo mapping (Buildkite OIDC has no "repository" claim).
24+
# Keys are immutable organization_id/pipeline_id UUIDs.
25+
#
26+
# Simple format:
27+
# org_id/pipeline_id: owner/repo
28+
#
29+
# Constrained format (for pipelines that run fork builds):
30+
# org_id/pipeline_id:
31+
# repo: owner/repo
32+
# required_claims:
33+
# build_branch: [main, nightly]
34+
repo_map:
35+
# 018e4f2a-1b2c-3d4e/018e5a6b-7c8d-9e0f: vllm-project/vllm
36+
#
37+
# 018e4f2a-1b2c-3d4e/018e9a0b-1c2d-3e4f:
38+
# repo: vllm-project/vllm
39+
# required_claims:
40+
# build_branch: [main, release]
4141

42-
# jenkins:
43-
# job-name: owner/repo
42+
# gitlab:
43+
# issuer: https://gitlab.com
44+
# jwks_uri: https://gitlab.com/oauth/discovery/keys
45+
# repo_map:
46+
# group/project: owner/repo

‎aws/lambda/cross_repo_ci_relay/tests/test_jwt_helper.py‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
from unittest.mock import MagicMock, patch
33

44
from utils.jwt_helper import (
5+
_jwks_clients,
6+
_PROVIDER_ISSUERS,
57
AUDIENCE,
68
BUILDKITE_ISSUER,
79
BUILDKITE_REPO_MAP,
@@ -254,12 +256,15 @@ class TestLoadCIProviderMappings(unittest.TestCase):
254256

255257
def setUp(self):
256258
self._orig_map = BUILDKITE_REPO_MAP.copy()
259+
self._orig_issuers = _PROVIDER_ISSUERS.copy()
257260

258261
def tearDown(self):
259262
BUILDKITE_REPO_MAP.clear()
260263
BUILDKITE_REPO_MAP.update(self._orig_map)
264+
_PROVIDER_ISSUERS.clear()
265+
_PROVIDER_ISSUERS.update(self._orig_issuers)
261266

262-
def test_loads_valid_buildkite_entries(self):
267+
def test_loads_valid_buildkite_entries_legacy(self):
263268
raw = {
264269
"buildkite": {
265270
"org-id-1/pipe-id-1": "vllm-project/vllm",
@@ -275,7 +280,7 @@ def test_loads_valid_buildkite_entries(self):
275280
BUILDKITE_REPO_MAP[("org-id-2", "pipe-id-2")]["repo"], "acme/repo"
276281
)
277282

278-
def test_loads_constrained_entries(self):
283+
def test_loads_constrained_entries_legacy(self):
279284
raw = {
280285
"buildkite": {
281286
"org-id/pipe-id": {
@@ -293,6 +298,79 @@ def test_loads_constrained_entries(self):
293298
self.assertEqual(entry["required_claims"]["build_branch"], ["main", "nightly"])
294299
self.assertEqual(entry["required_claims"]["cluster_id"], ["cluster-uuid"])
295300

301+
def test_providers_section_loads_issuers(self):
302+
raw = {
303+
"providers": {
304+
"github": {
305+
"issuer": GITHUB_ISSUER,
306+
"jwks_uri": f"{GITHUB_ISSUER}/.well-known/jwks",
307+
},
308+
"buildkite": {
309+
"issuer": BUILDKITE_ISSUER,
310+
"jwks_uri": f"{BUILDKITE_ISSUER}/.well-known/jwks",
311+
},
312+
}
313+
}
314+
load_ci_provider_mappings(raw)
315+
self.assertEqual(_PROVIDER_ISSUERS["github"], GITHUB_ISSUER)
316+
self.assertEqual(_PROVIDER_ISSUERS["buildkite"], BUILDKITE_ISSUER)
317+
318+
def test_providers_section_creates_jwks_clients(self):
319+
fake_issuer = "https://fake.issuer.example.com"
320+
raw = {
321+
"providers": {
322+
"fakeci": {
323+
"issuer": fake_issuer,
324+
"jwks_uri": f"{fake_issuer}/.well-known/jwks",
325+
},
326+
}
327+
}
328+
load_ci_provider_mappings(raw)
329+
self.assertIn(fake_issuer, _jwks_clients)
330+
# Cleanup
331+
del _jwks_clients[fake_issuer]
332+
333+
def test_providers_section_loads_repo_map(self):
334+
raw = {
335+
"providers": {
336+
"github": {
337+
"issuer": GITHUB_ISSUER,
338+
"jwks_uri": f"{GITHUB_ISSUER}/.well-known/jwks",
339+
},
340+
"buildkite": {
341+
"issuer": BUILDKITE_ISSUER,
342+
"jwks_uri": f"{BUILDKITE_ISSUER}/.well-known/jwks",
343+
"repo_map": {
344+
"org-id/pipe-id": "vllm-project/vllm",
345+
},
346+
},
347+
}
348+
}
349+
load_ci_provider_mappings(raw)
350+
self.assertEqual(
351+
BUILDKITE_REPO_MAP[("org-id", "pipe-id")]["repo"], "vllm-project/vllm"
352+
)
353+
354+
def test_providers_section_loads_constrained_repo_map(self):
355+
raw = {
356+
"providers": {
357+
"buildkite": {
358+
"issuer": BUILDKITE_ISSUER,
359+
"jwks_uri": f"{BUILDKITE_ISSUER}/.well-known/jwks",
360+
"repo_map": {
361+
"org-id/pipe-id": {
362+
"repo": "myorg/myrepo",
363+
"required_claims": {"build_branch": ["main"]},
364+
},
365+
},
366+
},
367+
}
368+
}
369+
load_ci_provider_mappings(raw)
370+
entry = BUILDKITE_REPO_MAP[("org-id", "pipe-id")]
371+
self.assertEqual(entry["repo"], "myorg/myrepo")
372+
self.assertEqual(entry["required_claims"]["build_branch"], ["main"])
373+
296374
def test_empty_config_clears_map(self):
297375
BUILDKITE_REPO_MAP[("old", "entry")] = {
298376
"repo": "old/repo",
@@ -317,6 +395,15 @@ def test_skips_invalid_entries(self):
317395
self.assertNotIn(("noslash", ""), BUILDKITE_REPO_MAP)
318396
self.assertEqual(BUILDKITE_REPO_MAP[("ok-id", "pipe-id")]["repo"], "ok/repo")
319397

398+
def test_providers_skips_missing_issuer(self):
399+
raw = {
400+
"providers": {
401+
"bad": {"jwks_uri": "https://x/.well-known/jwks"},
402+
}
403+
}
404+
load_ci_provider_mappings(raw)
405+
self.assertNotIn("bad", _PROVIDER_ISSUERS)
406+
320407

321408
class TestUnsupportedIssuer(unittest.TestCase):
322409
"""Tests for tokens from unsupported issuers."""

‎aws/lambda/cross_repo_ci_relay/utils/jwt_helper.py‎

Lines changed: 103 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
"""JWT utilities for the cross-repo CI relay.
22
3-
Supports multiple OIDC issuers (GitHub Actions, Buildkite) so downstream
4-
repos running on any supported CI can authenticate callbacks.
3+
Supports multiple OIDC issuers so downstream repos running on any
4+
supported CI can authenticate callbacks. Provider configuration
5+
(issuer URLs, JWKS endpoints, repo mappings) is loaded from
6+
ci_providers.yml at runtime.
57
"""
68

79
from __future__ import annotations
@@ -17,21 +19,24 @@
1719

1820
logger = logging.getLogger(__name__)
1921

22+
AUDIENCE = "pytorch-cross-repo-ci-relay"
23+
2024
GITHUB_ISSUER = "https://token.actions.githubusercontent.com"
2125
BUILDKITE_ISSUER = "https://agent.buildkite.com"
22-
AUDIENCE = "pytorch-cross-repo-ci-relay"
2326

24-
_ISSUER_CONFIG: Dict[str, dict] = {
25-
GITHUB_ISSUER: {
26-
"jwks_uri": f"{GITHUB_ISSUER}/.well-known/jwks",
27-
},
28-
BUILDKITE_ISSUER: {
29-
"jwks_uri": f"{BUILDKITE_ISSUER}/.well-known/jwks",
30-
},
27+
_DEFAULT_PROVIDERS: Dict[str, dict] = {
28+
GITHUB_ISSUER: {"jwks_uri": f"{GITHUB_ISSUER}/.well-known/jwks"},
29+
BUILDKITE_ISSUER: {"jwks_uri": f"{BUILDKITE_ISSUER}/.well-known/jwks"},
3130
}
3231

3332
_jwks_clients: Dict[str, jwt.PyJWKClient] = {
34-
issuer: jwt.PyJWKClient(cfg["jwks_uri"]) for issuer, cfg in _ISSUER_CONFIG.items()
33+
iss: jwt.PyJWKClient(cfg["jwks_uri"]) for iss, cfg in _DEFAULT_PROVIDERS.items()
34+
}
35+
36+
# Mapping from provider name to its issuer URL, populated by config.
37+
_PROVIDER_ISSUERS: Dict[str, str] = {
38+
"github": GITHUB_ISSUER,
39+
"buildkite": BUILDKITE_ISSUER,
3540
}
3641

3742
# Runtime-populated mapping from Buildkite (organization_id, pipeline_id) to
@@ -46,15 +51,89 @@
4651

4752

4853
def load_ci_provider_mappings(raw: dict) -> None:
49-
"""Populate provider repo maps from parsed ci_providers.yml content.
50-
51-
Buildkite entries support two formats:
52-
org_id/pipeline_id: owner/repo # simple
53-
org_id/pipeline_id: # with constraints
54-
repo: owner/repo
55-
required_claims:
56-
build_branch: [main, nightly]
54+
"""Populate provider configs and repo maps from ci_providers.yml.
55+
56+
The ``providers`` section defines issuer URLs, JWKS endpoints, and
57+
optional repo mappings for each CI platform. JWKS clients are
58+
rebuilt whenever the config is reloaded.
59+
60+
Falls back to the legacy flat ``buildkite:`` section for backward
61+
compatibility.
5762
"""
63+
providers = raw.get("providers")
64+
if not providers or not isinstance(providers, dict):
65+
_load_legacy_buildkite(raw)
66+
return
67+
68+
BUILDKITE_REPO_MAP.clear()
69+
_PROVIDER_ISSUERS.clear()
70+
71+
for name, cfg in providers.items():
72+
if not isinstance(cfg, dict):
73+
continue
74+
issuer = cfg.get("issuer")
75+
jwks_uri = cfg.get("jwks_uri")
76+
if not issuer or not jwks_uri:
77+
logger.warning("Provider %s missing issuer or jwks_uri, skipping", name)
78+
continue
79+
80+
_PROVIDER_ISSUERS[name] = issuer
81+
82+
if issuer not in _jwks_clients:
83+
_jwks_clients[issuer] = jwt.PyJWKClient(jwks_uri)
84+
85+
repo_map = cfg.get("repo_map")
86+
if name == "buildkite" and repo_map and isinstance(repo_map, dict):
87+
for bk_key, value in repo_map.items():
88+
bk_key_str = str(bk_key).strip()
89+
if "/" not in bk_key_str:
90+
logger.warning("Skipping buildkite entry without /: %s", bk_key)
91+
continue
92+
org, pipeline = bk_key_str.split("/", 1)
93+
94+
if isinstance(value, str):
95+
repo_str = value.strip()
96+
if "/" not in repo_str:
97+
logger.warning(
98+
"Skipping invalid buildkite entry: %s -> %s",
99+
bk_key,
100+
value,
101+
)
102+
continue
103+
BUILDKITE_REPO_MAP[(org, pipeline)] = {
104+
"repo": repo_str,
105+
"required_claims": {},
106+
}
107+
elif isinstance(value, dict):
108+
repo_str = str(value.get("repo", "")).strip()
109+
if "/" not in repo_str:
110+
logger.warning(
111+
"Skipping buildkite entry with invalid repo: %s", bk_key
112+
)
113+
continue
114+
required = value.get("required_claims", {})
115+
if not isinstance(required, dict):
116+
required = {}
117+
normalized = {}
118+
for k, v in required.items():
119+
if isinstance(v, list):
120+
normalized[k] = [str(x) for x in v]
121+
else:
122+
normalized[k] = [str(v)]
123+
BUILDKITE_REPO_MAP[(org, pipeline)] = {
124+
"repo": repo_str,
125+
"required_claims": normalized,
126+
}
127+
128+
logger.info(
129+
"Loaded %d provider(s), %d Buildkite mapping(s) from ci_providers",
130+
len(_PROVIDER_ISSUERS),
131+
len(BUILDKITE_REPO_MAP),
132+
)
133+
134+
135+
def _load_legacy_buildkite(raw: dict) -> None:
136+
"""Backward compat: load flat buildkite section without providers key."""
58137
BUILDKITE_REPO_MAP.clear()
59138
bk_section = raw.get("buildkite")
60139
if bk_section and isinstance(bk_section, dict):
@@ -98,7 +177,7 @@ def load_ci_provider_mappings(raw: dict) -> None:
98177
}
99178
if BUILDKITE_REPO_MAP:
100179
logger.info(
101-
"Loaded %d Buildkite repo mapping(s) from ci_providers",
180+
"Loaded %d Buildkite repo mapping(s) from ci_providers (legacy)",
102181
len(BUILDKITE_REPO_MAP),
103182
)
104183

@@ -124,11 +203,7 @@ def _fetch_github_file(url: str) -> str:
124203

125204

126205
def load_ci_providers(config) -> None:
127-
"""Load CI provider mappings from the configured URL, with Redis caching.
128-
129-
``config`` is a ``RelayConfig`` instance (imported lazily to avoid
130-
pulling heavy dependencies at module-import time during testing).
131-
"""
206+
"""Load CI provider config from the configured URL, with Redis caching."""
132207
if not config.ci_providers_url:
133208
return
134209

@@ -233,7 +308,9 @@ def verify_oidc_token(token: str) -> dict:
233308
f"but was routed as '{detected_issuer}'",
234309
)
235310

236-
extractor = _REPO_EXTRACTORS[detected_issuer]
311+
extractor = _REPO_EXTRACTORS.get(detected_issuer)
312+
if not extractor:
313+
raise HTTPException(401, f"No repo extractor for issuer: {detected_issuer}")
237314
repo = extractor(claims)
238315
claims["repository"] = repo
239316

0 commit comments

Comments
 (0)