diff --git a/.github/scripts/check_strict_pins.py b/.github/scripts/check_strict_pins.py new file mode 100755 index 000000000000..bb2c72d63997 --- /dev/null +++ b/.github/scripts/check_strict_pins.py @@ -0,0 +1,304 @@ +#!/usr/bin/env python3 +# cspell:ignore pyproject kashifkhan annatisch johanste +""" +Script to detect strict version pins (==) in Python package dependencies. + +This script checks for new strict version pins introduced in setup.py and pyproject.toml +files within the sdk directory, focusing on main runtime dependencies (install_requires +for setup.py, [project] dependencies for pyproject.toml). + +It ignores: +- dev/test/extras dependencies +- comments +- Changes outside of main dependency sections +""" + +import json +import os +import re +import subprocess +import sys +import urllib.request +import urllib.parse +from typing import Dict, List, Optional + + +def run_git_command(cmd: List[str]) -> str: + """Run a git command and return its output.""" + try: + result = subprocess.run( + cmd, + capture_output=True, + text=True, + check=True + ) + return result.stdout + except subprocess.CalledProcessError as e: + print(f"Error running git command: {e}") + print(f"stderr: {e.stderr}") + return "" + + +def get_changed_files(base_ref: str, head_ref: str) -> List[str]: + """Get list of changed setup.py and pyproject.toml files in sdk directory.""" + diff_output = run_git_command([ + "git", "diff", "--name-only", "--diff-filter=AM", + base_ref, head_ref + ]) + + files = [] + for line in diff_output.strip().split('\n'): + if line and line.startswith('sdk/'): + if line.endswith('/setup.py') or line.endswith('/pyproject.toml'): + files.append(line) + + return files + + +def extract_strict_pins_from_setup_py_diff(diff_content: str) -> List[str]: + """ + Extract strict version pins from a setup.py diff. + Only considers additions in install_requires section. + """ + strict_pins = [] + in_install_requires = False + bracket_depth = 0 + + for line in diff_content.split('\n'): + # Skip the +++ and --- file markers + if line.startswith('+++') or line.startswith('---'): + continue + + # Process all lines to track context, but only extract from added lines + is_added = line.startswith('+') + is_removed = line.startswith('-') + is_context = line.startswith(' ') + + if is_added or is_removed or is_context: + actual_line = line[1:].strip() + else: + actual_line = line.strip() + + # Detect start of install_requires in any line + if 'install_requires' in actual_line and '=' in actual_line: + in_install_requires = True + bracket_depth = 0 + # Check if the array starts on the same line + if '[' in actual_line: + bracket_depth = actual_line.count('[') - actual_line.count(']') + + # Detect end of install_requires or start of other sections + if in_install_requires: + if 'extras_require' in actual_line or 'tests_require' in actual_line: + in_install_requires = False + continue + + # Track brackets in all lines + bracket_depth += actual_line.count('[') - actual_line.count(']') + + # If we close all brackets, we're done with install_requires + if bracket_depth <= 0 and (']' in actual_line or '),' in actual_line): + # Check current line before exiting if it's an added line + if is_added and '==' in actual_line and not actual_line.startswith('#'): + match = re.search(r'["\']([^"\']+==[\d.]+[^"\']*)["\']', actual_line) + if match: + strict_pins.append(match.group(1)) + in_install_requires = False + continue + + # Look for strict pins in added lines within install_requires + if in_install_requires and is_added: + if '==' in actual_line and not actual_line.startswith('#'): + # Match package==version pattern + match = re.search(r'["\']([^"\']+==[\d.]+[^"\']*)["\']', actual_line) + if match: + strict_pins.append(match.group(1)) + + return strict_pins + + +def extract_strict_pins_from_pyproject_diff(diff_content: str) -> List[str]: + """ + Extract strict version pins from a pyproject.toml diff. + Only considers additions in [project] dependencies section. + """ + strict_pins = [] + in_project_dependencies = False + in_other_section = False + + for line in diff_content.split('\n'): + # Skip the +++ and --- file markers + if line.startswith('+++') or line.startswith('---'): + continue + + # Process all lines to track context + is_added = line.startswith('+') + is_removed = line.startswith('-') + is_context = line.startswith(' ') + + if is_added or is_removed or is_context: + actual_line = line[1:].strip() + else: + actual_line = line.strip() + + # Detect [project] section markers in any line (context or changes) + if actual_line.startswith('['): + if actual_line.startswith('[project]'): + in_other_section = False + else: + in_other_section = True + in_project_dependencies = False + + # Detect start of dependencies in [project] section + if not in_other_section and 'dependencies' in actual_line and '=' in actual_line: + if not ('optional-dependencies' in actual_line or + 'dev-dependencies' in actual_line): + in_project_dependencies = True + continue + + # Detect end of dependencies array + if in_project_dependencies and ']' in actual_line and '==' not in actual_line: + in_project_dependencies = False + continue + + # Look for strict pins in added lines within [project] dependencies + if in_project_dependencies and is_added: + if '==' in actual_line and not actual_line.startswith('#'): + # Match package==version pattern + match = re.search(r'["\']([^"\']+==[\d.]+[^"\']*)["\']', actual_line) + if match: + strict_pins.append(match.group(1)) + + return strict_pins + + +def check_file_for_strict_pins(filepath: str, base_ref: str, head_ref: str) -> List[str]: + """Check a specific file for new strict version pins.""" + # Get the diff for this file + diff_output = run_git_command([ + "git", "diff", base_ref, head_ref, "--", filepath + ]) + + if not diff_output: + return [] + + if filepath.endswith('setup.py'): + return extract_strict_pins_from_setup_py_diff(diff_output) + elif filepath.endswith('pyproject.toml'): + return extract_strict_pins_from_pyproject_diff(diff_output) + + return [] + + +def check_architect_approval(pr_number: str, repo: str, github_token: str) -> bool: + """ + Check if any of the architects have approved the PR. + Architects: kashifkhan, annatisch, johanste + """ + architects = {'kashifkhan', 'annatisch', 'johanste'} + + # GitHub API to get PR reviews + url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}/reviews" + + headers = { + 'Authorization': f'token {github_token}', + 'Accept': 'application/vnd.github.v3+json' + } + + try: + req = urllib.request.Request(url, headers=headers) + with urllib.request.urlopen(req) as response: + reviews = json.loads(response.read()) + + for review in reviews: + if review['state'] == 'APPROVED': + reviewer = review['user']['login'] + if reviewer in architects: + print(f"✅ Architect {reviewer} has approved this PR") + return True + + print("❌ No architect approval found") + return False + + except Exception as e: + print(f"Error checking PR reviews: {e}") + # In case of error, we should fail open to not block legitimate PRs + # if the API is down + return False + + +def set_output(name: str, value: str) -> None: + """Set GitHub Actions output.""" + github_output = os.getenv('GITHUB_OUTPUT') + if github_output: + with open(github_output, 'a') as f: + # Escape newlines and special characters + escaped_value = value.replace('%', '%25').replace('\n', '%0A').replace('\r', '%0D') + f.write(f"{name}={escaped_value}\n") + else: + print(f"::set-output name={name}::{value}") + + +def main() -> int: + """Main function to check for strict version pins.""" + base_ref = os.getenv('BASE_REF', 'origin/main') + head_ref = os.getenv('HEAD_REF', 'HEAD') + pr_number = os.getenv('PR_NUMBER') + repo = os.getenv('REPO') + github_token = os.getenv('GITHUB_TOKEN') + + print("Checking for strict version pins...") + print(f"Base: {base_ref}, Head: {head_ref}") + + # Get changed files + changed_files = get_changed_files(base_ref, head_ref) + + if not changed_files: + print("No relevant files changed") + set_output('strict_pins_found', 'false') + set_output('architect_approved', 'false') + set_output('strict_pins_details', '') + return 0 + + print(f"Checking {len(changed_files)} file(s):") + for file_path in changed_files: + print(f" - {file_path}") + + # Check each file for strict pins + all_strict_pins: Dict[str, List[str]] = {} + for filepath in changed_files: + strict_pins = check_file_for_strict_pins(filepath, base_ref, head_ref) + if strict_pins: + all_strict_pins[filepath] = strict_pins + + if not all_strict_pins: + print("✅ No new strict version pins found") + set_output('strict_pins_found', 'false') + set_output('architect_approved', 'false') + set_output('strict_pins_details', '') + return 0 + + # Format the findings + details = [] + for filepath, pins in all_strict_pins.items(): + details.append(f"{filepath}:") + for pin in pins: + details.append(f" - {pin}") + + details_str = '\n'.join(details) + print(f"\n⚠️ Strict version pins found:\n{details_str}") + + # Check for architect approval + architect_approved = False + if pr_number and repo and github_token: + architect_approved = check_architect_approval(pr_number, repo, github_token) + + set_output('strict_pins_found', 'true') + set_output('architect_approved', 'true' if architect_approved else 'false') + set_output('strict_pins_details', details_str) + + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/eng/pipelines/templates/steps/analyze.yml b/eng/pipelines/templates/steps/analyze.yml index c9296287987b..adcee66d22ad 100644 --- a/eng/pipelines/templates/steps/analyze.yml +++ b/eng/pipelines/templates/steps/analyze.yml @@ -75,6 +75,10 @@ steps: SourceDirectory: $(Build.SourcesDirectory) Condition: succeededOrFailed() + - template: ../steps/check_strict_pins.yml + parameters: + ServiceDirectory: ${{ parameters.ServiceDirectory }} + - template: ../steps/verify-autorest.yml parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} diff --git a/eng/pipelines/templates/steps/check_strict_pins.yml b/eng/pipelines/templates/steps/check_strict_pins.yml new file mode 100644 index 000000000000..ca2a80eeed24 --- /dev/null +++ b/eng/pipelines/templates/steps/check_strict_pins.yml @@ -0,0 +1,10 @@ +parameters: + ServiceDirectory: '' + +steps: + - task: PythonScript@0 + displayName: 'Check for Strict Dependency Version Pins' + condition: and(succeededOrFailed(), eq(variables['Build.Reason'], 'PullRequest'), ne(variables['Skip.StrictPinCheck'], 'true')) + inputs: + scriptPath: 'scripts/devops_tasks/check_strict_pins.py' + arguments: '--base origin/$(System.PullRequest.TargetBranch) --head $(Build.SourceVersion) --pr-number $(System.PullRequest.PullRequestNumber)' diff --git a/scripts/devops_tasks/check_strict_pins.py b/scripts/devops_tasks/check_strict_pins.py new file mode 100644 index 000000000000..d9dc83dbcab3 --- /dev/null +++ b/scripts/devops_tasks/check_strict_pins.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python + +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- +# cspell:ignore pyproject kashifkhan annatisch johanste +""" +Detects strict dependency version pins (==) in Python package dependencies. + +Checks setup.py (install_requires) and pyproject.toml ([project] dependencies) +for new strict dependency version pins. Requires architect approval to proceed. +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from typing import Dict, List + +from ci_tools.parsing import ParsedSetup + + +ARCHITECTS = {'kashifkhan', 'annatisch', 'johanste'} +STRICT_PIN_PATTERN = re.compile(r'^([a-zA-Z0-9\-_.]+)==[\d.]+') + + +def run_git_command(cmd: List[str]) -> str: + """Run a git command and return its output.""" + result = subprocess.run(cmd, capture_output=True, text=True, check=False) + return result.stdout if result.returncode == 0 else "" + + +def get_changed_files(base_ref: str, head_ref: str) -> List[str]: + """Get list of changed setup.py and pyproject.toml files in sdk directory.""" + diff_output = run_git_command([ + "git", "diff", "--name-only", "--diff-filter=AM", base_ref, head_ref + ]) + + return [ + line for line in diff_output.strip().split('\n') + if line.startswith('sdk/') and (line.endswith('/setup.py') or line.endswith('/pyproject.toml')) + ] + + +def check_for_new_strict_pins(filepath: str, diff_output: str) -> List[str]: + """Check if strict pins from ParsedSetup appear in the git diff (were added).""" + if not diff_output: + return [] + + # Parse current file with ParsedSetup to get all requirements + package_dir = os.path.dirname(filepath) + try: + parsed = ParsedSetup.from_path(package_dir) + except Exception as e: + print(f"Warning: Could not parse {package_dir}: {e}") + return [] + + # Find strict pins that appear in added lines of the diff + added_lines = [line for line in diff_output.split('\n') if line.startswith('+')] + strict_pins = [] + + for requirement in parsed.requires: + if STRICT_PIN_PATTERN.match(requirement): + package_name = requirement.split('==')[0] + # Check if this package appears in added lines with == + if any(package_name in line and '==' in line for line in added_lines): + strict_pins.append(requirement) + + return strict_pins + + +def check_architect_approval_via_cli(pr_number: str) -> bool: + """Check if any architects have approved the PR using Azure CLI.""" + result = subprocess.run( + ['az', 'repos', 'pr', 'reviewer', 'list', + '--id', pr_number, + '--query', "[?vote==`10`].displayName", + '--output', 'json'], + capture_output=True, + text=True, + check=False + ) + + if result.returncode != 0: + return False + + approvers = json.loads(result.stdout) if result.stdout else [] + return any( + any(architect.lower() in approver.lower() for architect in ARCHITECTS) + for approver in approvers + ) + +def main() -> int: + parser = argparse.ArgumentParser(description='Check for strict version pins in dependencies') + parser.add_argument('--base', default='origin/main', help='Base ref for comparison') + parser.add_argument('--head', default='HEAD', help='Head ref for comparison') + parser.add_argument('--pr-number', help='Pull request number') + args = parser.parse_args() + + # Get changed files + changed_files = get_changed_files(args.base, args.head) + if not changed_files: + return 0 + + # Check each file for new strict pins + all_strict_pins = [] + for filepath in changed_files: + diff_output = run_git_command([ + "git", "diff", args.base, args.head, "--", filepath + ]) + if pins := check_for_new_strict_pins(filepath, diff_output): + all_strict_pins += pins + + if not all_strict_pins: + return 0 + + # Print detected strict pins + print("\n" + "=" * 80) + print("STRICT VERSION PINS DETECTED") + print("=" * 80) + for pins in all_strict_pins: + print(f" - {pins}") + print("\n" + "-" * 80) + print(f"Required approvers: {', '.join(sorted(ARCHITECTS))}") + print("-" * 80) + + if check_architect_approval_via_cli(args.pr_number): + print("\nArchitect approval found") + return 0 + + print(f"\nNo architect approval found, strict version pins detected. Get approval from one of the following architects before merging: {', '.join(ARCHITECTS)}\n") + return 1 + + +if __name__ == '__main__': + sys.exit(main()) \ No newline at end of file