Skip to content

Commit 8cc3634

Browse files
Copilotl0lawrence
andcommitted
Add GitHub Action workflow to block strict version pins without architect approval
Co-authored-by: l0lawrence <[email protected]>
1 parent 2f19d56 commit 8cc3634

File tree

3 files changed

+401
-0
lines changed

3 files changed

+401
-0
lines changed

.github/CODEOWNERS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,3 +871,8 @@
871871

872872
/pylintrc @l0lawrence @scbedd @mccoyp
873873
/sdk/**/ci.yml @msyyc @lmazuel @scbedd
874+
875+
# Require architect approval for dependency changes (setup.py and pyproject.toml)
876+
# to prevent strict version pins without proper review
877+
/sdk/**/setup.py @kashifkhan @annatisch @johanste
878+
/sdk/**/pyproject.toml @kashifkhan @annatisch @johanste
Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Script to detect strict version pins (==) in Python package dependencies.
4+
5+
This script checks for new strict version pins introduced in setup.py and pyproject.toml
6+
files within the sdk directory, focusing on main runtime dependencies (install_requires
7+
for setup.py, [project] dependencies for pyproject.toml).
8+
9+
It ignores:
10+
- dev/test/extras dependencies
11+
- comments
12+
- Changes outside of main dependency sections
13+
"""
14+
15+
import os
16+
import re
17+
import subprocess
18+
import sys
19+
from typing import Dict, List, Set, Tuple
20+
import json
21+
import urllib.request
22+
import urllib.parse
23+
24+
25+
def run_git_command(cmd: List[str]) -> str:
26+
"""Run a git command and return its output."""
27+
try:
28+
result = subprocess.run(
29+
cmd,
30+
capture_output=True,
31+
text=True,
32+
check=True
33+
)
34+
return result.stdout
35+
except subprocess.CalledProcessError as e:
36+
print(f"Error running git command: {e}")
37+
print(f"stderr: {e.stderr}")
38+
return ""
39+
40+
41+
def get_changed_files(base_ref: str, head_ref: str) -> List[str]:
42+
"""Get list of changed setup.py and pyproject.toml files in sdk directory."""
43+
diff_output = run_git_command([
44+
"git", "diff", "--name-only", "--diff-filter=AM",
45+
base_ref, head_ref
46+
])
47+
48+
files = []
49+
for line in diff_output.strip().split('\n'):
50+
if line:
51+
if (line.startswith('sdk/') and
52+
(line.endswith('/setup.py') or line.endswith('/pyproject.toml'))):
53+
files.append(line)
54+
55+
return files
56+
57+
58+
def extract_strict_pins_from_setup_py_diff(diff_content: str) -> List[str]:
59+
"""
60+
Extract strict version pins from a setup.py diff.
61+
Only considers additions in install_requires section.
62+
"""
63+
strict_pins = []
64+
in_install_requires = False
65+
bracket_depth = 0
66+
67+
for line in diff_content.split('\n'):
68+
# Skip the +++ file marker
69+
if line.startswith('+++') or line.startswith('---'):
70+
continue
71+
72+
# Process all lines to track context, but only extract from added lines
73+
actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else line.strip()
74+
75+
# Detect start of install_requires in any line
76+
if 'install_requires' in actual_line and '=' in actual_line:
77+
in_install_requires = True
78+
bracket_depth = 0
79+
# Check if the array starts on the same line
80+
if '[' in actual_line:
81+
bracket_depth = actual_line.count('[') - actual_line.count(']')
82+
83+
# Detect end of install_requires or start of other sections
84+
if in_install_requires:
85+
if 'extras_require' in actual_line or 'tests_require' in actual_line:
86+
in_install_requires = False
87+
continue
88+
89+
# Track brackets in all lines
90+
bracket_depth += actual_line.count('[') - actual_line.count(']')
91+
92+
# If we close all brackets, we're done with install_requires
93+
if bracket_depth <= 0 and (']' in actual_line or '),' in actual_line):
94+
# Check current line before exiting if it's an added line
95+
if line.startswith('+') and '==' in actual_line and not actual_line.strip().startswith('#'):
96+
match = re.search(r'["\']([^"\']+==[\d\.]+[^"\']*)["\']', actual_line)
97+
if match:
98+
strict_pins.append(match.group(1))
99+
in_install_requires = False
100+
continue
101+
102+
# Look for strict pins in added lines within install_requires
103+
if in_install_requires and line.startswith('+'):
104+
if '==' in actual_line and not actual_line.strip().startswith('#'):
105+
# Match package==version pattern
106+
match = re.search(r'["\']([^"\']+==[\d\.]+[^"\']*)["\']', actual_line)
107+
if match:
108+
strict_pins.append(match.group(1))
109+
110+
return strict_pins
111+
112+
113+
def extract_strict_pins_from_pyproject_diff(diff_content: str) -> List[str]:
114+
"""
115+
Extract strict version pins from a pyproject.toml diff.
116+
Only considers additions in [project] dependencies section.
117+
"""
118+
strict_pins = []
119+
in_project_dependencies = False
120+
in_other_section = False
121+
122+
for line in diff_content.split('\n'):
123+
# Skip the +++ and --- file markers
124+
if line.startswith('+++') or line.startswith('---'):
125+
continue
126+
127+
# Process all lines to track context
128+
actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else line.strip()
129+
130+
# Detect [project] section markers in any line (context or changes)
131+
if actual_line.startswith('['):
132+
if actual_line.startswith('[project]'):
133+
in_other_section = False
134+
elif actual_line.startswith('['):
135+
in_other_section = True
136+
in_project_dependencies = False
137+
138+
# Detect start of dependencies in [project] section
139+
if not in_other_section and 'dependencies' in actual_line and '=' in actual_line:
140+
if not ('optional-dependencies' in actual_line or
141+
'dev-dependencies' in actual_line):
142+
in_project_dependencies = True
143+
continue
144+
145+
# Detect end of dependencies array
146+
if in_project_dependencies and ']' in actual_line and '==' not in actual_line:
147+
in_project_dependencies = False
148+
continue
149+
150+
# Look for strict pins in added lines within [project] dependencies
151+
if in_project_dependencies and line.startswith('+'):
152+
if '==' in actual_line and not actual_line.strip().startswith('#'):
153+
# Match package==version pattern
154+
match = re.search(r'["\']([^"\']+==[\d\.]+[^"\']*)["\']', actual_line)
155+
if match:
156+
strict_pins.append(match.group(1))
157+
158+
return strict_pins
159+
160+
161+
def check_file_for_strict_pins(filepath: str, base_ref: str, head_ref: str) -> List[str]:
162+
"""Check a specific file for new strict version pins."""
163+
# Get the diff for this file
164+
diff_output = run_git_command([
165+
"git", "diff", base_ref, head_ref, "--", filepath
166+
])
167+
168+
if not diff_output:
169+
return []
170+
171+
if filepath.endswith('setup.py'):
172+
return extract_strict_pins_from_setup_py_diff(diff_output)
173+
elif filepath.endswith('pyproject.toml'):
174+
return extract_strict_pins_from_pyproject_diff(diff_output)
175+
176+
return []
177+
178+
179+
def check_architect_approval(pr_number: str, repo: str, github_token: str) -> bool:
180+
"""
181+
Check if any of the architects have approved the PR.
182+
Architects: kashifkhan, annatisch, johanste
183+
"""
184+
architects = {'kashifkhan', 'annatisch', 'johanste'}
185+
186+
# GitHub API to get PR reviews
187+
url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}/reviews"
188+
189+
headers = {
190+
'Authorization': f'token {github_token}',
191+
'Accept': 'application/vnd.github.v3+json'
192+
}
193+
194+
try:
195+
req = urllib.request.Request(url, headers=headers)
196+
with urllib.request.urlopen(req) as response:
197+
reviews = json.loads(response.read())
198+
199+
for review in reviews:
200+
if review['state'] == 'APPROVED':
201+
reviewer = review['user']['login']
202+
if reviewer in architects:
203+
print(f"✅ Architect {reviewer} has approved this PR")
204+
return True
205+
206+
print("❌ No architect approval found")
207+
return False
208+
209+
except Exception as e:
210+
print(f"Error checking PR reviews: {e}")
211+
# In case of error, we should fail open to not block legitimate PRs
212+
# if the API is down
213+
return False
214+
215+
216+
def set_output(name: str, value: str):
217+
"""Set GitHub Actions output."""
218+
github_output = os.getenv('GITHUB_OUTPUT')
219+
if github_output:
220+
with open(github_output, 'a') as f:
221+
# Escape newlines and special characters
222+
escaped_value = value.replace('%', '%25').replace('\n', '%0A').replace('\r', '%0D')
223+
f.write(f"{name}={escaped_value}\n")
224+
else:
225+
print(f"::set-output name={name}::{value}")
226+
227+
228+
def main():
229+
base_ref = os.getenv('BASE_REF', 'origin/main')
230+
head_ref = os.getenv('HEAD_REF', 'HEAD')
231+
pr_number = os.getenv('PR_NUMBER')
232+
repo = os.getenv('REPO')
233+
github_token = os.getenv('GITHUB_TOKEN')
234+
235+
print(f"Checking for strict version pins...")
236+
print(f"Base: {base_ref}, Head: {head_ref}")
237+
238+
# Get changed files
239+
changed_files = get_changed_files(base_ref, head_ref)
240+
241+
if not changed_files:
242+
print("No relevant files changed")
243+
set_output('strict_pins_found', 'false')
244+
set_output('architect_approved', 'false')
245+
set_output('strict_pins_details', '')
246+
return 0
247+
248+
print(f"Checking {len(changed_files)} file(s):")
249+
for f in changed_files:
250+
print(f" - {f}")
251+
252+
# Check each file for strict pins
253+
all_strict_pins = {}
254+
for filepath in changed_files:
255+
strict_pins = check_file_for_strict_pins(filepath, base_ref, head_ref)
256+
if strict_pins:
257+
all_strict_pins[filepath] = strict_pins
258+
259+
if not all_strict_pins:
260+
print("✅ No new strict version pins found")
261+
set_output('strict_pins_found', 'false')
262+
set_output('architect_approved', 'false')
263+
set_output('strict_pins_details', '')
264+
return 0
265+
266+
# Format the findings
267+
details = []
268+
for filepath, pins in all_strict_pins.items():
269+
details.append(f"{filepath}:")
270+
for pin in pins:
271+
details.append(f" - {pin}")
272+
273+
details_str = '\n'.join(details)
274+
print(f"\n⚠️ Strict version pins found:\n{details_str}")
275+
276+
# Check for architect approval
277+
architect_approved = False
278+
if pr_number and repo and github_token:
279+
architect_approved = check_architect_approval(pr_number, repo, github_token)
280+
281+
set_output('strict_pins_found', 'true')
282+
set_output('architect_approved', 'true' if architect_approved else 'false')
283+
set_output('strict_pins_details', details_str)
284+
285+
return 0
286+
287+
288+
if __name__ == '__main__':
289+
sys.exit(main())

0 commit comments

Comments
 (0)