From 2f19d56e4c858c304cb4f0bd3bf74017325bb609 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 17:58:53 +0000 Subject: [PATCH 01/20] Initial plan From 8cc3634c4fcc36caa4bb8047dd18fe7b6ae81212 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 18:06:37 +0000 Subject: [PATCH 02/20] Add GitHub Action workflow to block strict version pins without architect approval Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/CODEOWNERS | 5 + .github/scripts/check_strict_pins.py | 289 ++++++++++++++++++ .../workflows/check-strict-version-pins.yml | 107 +++++++ 3 files changed, 401 insertions(+) create mode 100755 .github/scripts/check_strict_pins.py create mode 100644 .github/workflows/check-strict-version-pins.yml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fef97554f044..50118481b8a9 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -871,3 +871,8 @@ /pylintrc @l0lawrence @scbedd @mccoyp /sdk/**/ci.yml @msyyc @lmazuel @scbedd + +# Require architect approval for dependency changes (setup.py and pyproject.toml) +# to prevent strict version pins without proper review +/sdk/**/setup.py @kashifkhan @annatisch @johanste +/sdk/**/pyproject.toml @kashifkhan @annatisch @johanste diff --git a/.github/scripts/check_strict_pins.py b/.github/scripts/check_strict_pins.py new file mode 100755 index 000000000000..d481e9a439a1 --- /dev/null +++ b/.github/scripts/check_strict_pins.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python3 +""" +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 os +import re +import subprocess +import sys +from typing import Dict, List, Set, Tuple +import json +import urllib.request +import urllib.parse + + +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: + if (line.startswith('sdk/') and + (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 +++ file marker + if line.startswith('+++') or line.startswith('---'): + continue + + # Process all lines to track context, but only extract from added lines + actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else 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 line.startswith('+') and '==' in actual_line and not actual_line.strip().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 line.startswith('+'): + if '==' in actual_line and not actual_line.strip().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 + actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else 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 + elif actual_line.startswith('['): + 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 line.startswith('+'): + if '==' in actual_line and not actual_line.strip().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): + """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(): + 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(f"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 f in changed_files: + print(f" - {f}") + + # Check each file for strict pins + all_strict_pins = {} + 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/.github/workflows/check-strict-version-pins.yml b/.github/workflows/check-strict-version-pins.yml new file mode 100644 index 000000000000..3ca15093eb3f --- /dev/null +++ b/.github/workflows/check-strict-version-pins.yml @@ -0,0 +1,107 @@ +name: Check for Strict Version Pins + +on: + pull_request: + paths: + - 'sdk/**/setup.py' + - 'sdk/**/pyproject.toml' + +permissions: + pull-requests: read + contents: read + +jobs: + check-strict-pins: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Fetch base branch + run: | + git fetch origin ${{ github.base_ref }} + + - name: Check for strict version pins + id: check-pins + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + BASE_REF: origin/${{ github.base_ref }} + HEAD_REF: ${{ github.sha }} + run: | + python .github/scripts/check_strict_pins.py + + - name: Comment on PR if strict pins found + if: steps.check-pins.outputs.strict_pins_found == 'true' + uses: actions/github-script@v7 + with: + script: | + const strictPins = `${{ steps.check-pins.outputs.strict_pins_details }}`; + const architectApproved = `${{ steps.check-pins.outputs.architect_approved }}`; + + let message = '## ⚠️ Strict Version Pin Detected\n\n'; + message += 'This PR introduces one or more strict version pins (`==`) in main runtime dependencies:\n\n'; + message += '```\n' + strictPins + '\n```\n\n'; + + if (architectApproved === 'true') { + message += '✅ An architect has approved this PR.\n'; + } else { + message += '❌ **This PR requires approval from one of the following architects:**\n'; + message += '- @kashifkhan\n'; + message += '- @annatisch\n'; + message += '- @johanste\n\n'; + message += 'Please request a review from one of these architects before merging.\n'; + } + + // Check if comment already exists + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const botComment = comments.find(comment => + comment.user.type === 'Bot' && + comment.body.includes('Strict Version Pin Detected') + ); + + if (botComment) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: botComment.id, + body: message + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body: message + }); + } + + - name: Block merge if architect approval required + if: steps.check-pins.outputs.strict_pins_found == 'true' && steps.check-pins.outputs.architect_approved == 'false' + run: | + echo "::error::Strict version pins detected without architect approval" + echo "This PR introduces strict version pins (==) in main runtime dependencies." + echo "Approval required from: kashifkhan, annatisch, or johanste" + exit 1 + + - name: Success - No strict pins or approved + if: steps.check-pins.outputs.strict_pins_found == 'false' || steps.check-pins.outputs.architect_approved == 'true' + run: | + if [ "${{ steps.check-pins.outputs.strict_pins_found }}" == "true" ]; then + echo "✅ Strict version pins detected but architect has approved" + else + echo "✅ No new strict version pins detected in main runtime dependencies" + fi From d0e5840954f3034f69e98776ac03360db1accc74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 18:08:52 +0000 Subject: [PATCH 03/20] Add comprehensive documentation for strict version pin workflow Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .../workflows/README-strict-version-pins.md | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 .github/workflows/README-strict-version-pins.md diff --git a/.github/workflows/README-strict-version-pins.md b/.github/workflows/README-strict-version-pins.md new file mode 100644 index 000000000000..52f87e7becc9 --- /dev/null +++ b/.github/workflows/README-strict-version-pins.md @@ -0,0 +1,154 @@ +# Strict Version Pin Check Workflow + +## Overview + +This GitHub Actions workflow enforces a policy that requires architect approval for any pull requests that introduce strict version pins (`==`) in main runtime dependencies of Python packages within the `sdk/` directory. + +## Purpose + +Strict version pins can cause dependency conflicts and limit flexibility in package management. This workflow ensures that any such pins in main runtime dependencies are reviewed and approved by designated architects before merging. + +## How It Works + +### Trigger +The workflow runs on pull requests that modify: +- `sdk/**/setup.py` +- `sdk/**/pyproject.toml` + +### Detection Logic + +The workflow analyzes the diff to detect: +- **New strict version pins**: Dependencies newly added with `==` operator +- **Modified pins**: Dependencies changed from flexible constraints (e.g., `>=`, `~=`) to strict `==` pins + +The detection is **scope-aware** and only considers: +- `install_requires` in `setup.py` +- `dependencies` under `[project]` in `pyproject.toml` + +The following are **ignored**: +- Dev dependencies (`extras_require`, `[project.optional-dependencies]`) +- Test dependencies (`tests_require`) +- Comments +- Build dependencies + +### Approval Requirements + +If strict version pins are detected, the workflow: +1. Posts a comment on the PR listing the detected pins +2. Checks if any of the following architects have approved the PR: + - `@kashifkhan` + - `@annatisch` + - `@johanste` +3. **Blocks merging** if no architect approval is found (workflow fails with exit code 1) +4. **Allows merging** if an architect has approved + +### CODEOWNERS Integration + +The `.github/CODEOWNERS` file has been updated to require reviews from the architects for: +- `/sdk/**/setup.py` +- `/sdk/**/pyproject.toml` + +This provides a secondary enforcement mechanism through branch protection rules. + +## Examples + +### ✅ Allowed (No Strict Pins) +```python +# setup.py +install_requires=[ + "azure-core>=1.30.0", + "requests>=2.21.0", +] +``` + +### ⚠️ Requires Architect Approval +```python +# setup.py +install_requires=[ + "azure-core==1.30.0", # Strict pin detected! + "requests>=2.21.0", +] +``` + +### ✅ Allowed (Strict Pin in Dev Dependencies) +```python +# setup.py +install_requires=[ + "azure-core>=1.30.0", +], +extras_require={ + "dev": ["pytest==7.0.0"] # OK - this is a dev dependency +} +``` + +## Testing + +The detection logic has been validated with comprehensive test cases covering: +- Adding new strict pins +- Changing from flexible to strict constraints +- Ignoring dev/test dependencies +- Ignoring optional dependencies in pyproject.toml + +Run tests locally: +```bash +python /tmp/test_strict_pins.py +``` + +## Files Modified + +1. **`.github/workflows/check-strict-version-pins.yml`** + - Main workflow definition + - Triggers on PR events + - Runs detection and enforcement logic + +2. **`.github/scripts/check_strict_pins.py`** + - Python script that analyzes git diffs + - Detects strict version pins in appropriate sections + - Checks for architect approvals via GitHub API + +3. **`.github/CODEOWNERS`** + - Added architect requirements for setup.py and pyproject.toml + +## Branch Protection + +To fully enforce this policy, ensure branch protection rules are configured to: +- Require status checks to pass before merging +- Require the "check-strict-pins" workflow to succeed +- Require review from code owners + +## Troubleshooting + +### Workflow Not Running +- Verify the PR modifies files matching `sdk/**/setup.py` or `sdk/**/pyproject.toml` +- Check workflow runs in the Actions tab + +### False Positives +If the workflow incorrectly flags a dependency: +- Verify the dependency is in the main runtime dependencies section +- Check if comments are interfering with detection +- File an issue with the specific case + +### Override Process +If a strict pin is necessary: +1. Document the justification in the PR description +2. Request review from one of the architects: + - @kashifkhan + - @annatisch + - @johanste +3. Architect provides approval review +4. Workflow will pass and allow merge + +## Maintenance + +### Adding/Removing Architects +To modify the list of architects: +1. Update the `architects` set in `.github/scripts/check_strict_pins.py` +2. Update the CODEOWNERS entries in `.github/CODEOWNERS` +3. Update documentation in this README + +### Extending Detection +To add support for additional dependency formats: +1. Add extraction function in `check_strict_pins.py` +2. Update `check_file_for_strict_pins()` to handle new file types +3. Add corresponding test cases +4. Update workflow triggers if needed From c2684befe9d98967404982f0bae8b0ac0f85d268 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 20:59:27 +0000 Subject: [PATCH 04/20] Fix CI errors: add pull-requests write permission and move README out of workflows directory Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/{workflows => }/README-strict-version-pins.md | 0 .github/workflows/check-strict-version-pins.yml | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename .github/{workflows => }/README-strict-version-pins.md (100%) diff --git a/.github/workflows/README-strict-version-pins.md b/.github/README-strict-version-pins.md similarity index 100% rename from .github/workflows/README-strict-version-pins.md rename to .github/README-strict-version-pins.md diff --git a/.github/workflows/check-strict-version-pins.yml b/.github/workflows/check-strict-version-pins.yml index 3ca15093eb3f..afec2732526b 100644 --- a/.github/workflows/check-strict-version-pins.yml +++ b/.github/workflows/check-strict-version-pins.yml @@ -7,7 +7,7 @@ on: - 'sdk/**/pyproject.toml' permissions: - pull-requests: read + pull-requests: write contents: read jobs: From 45517b796b7e7f5f4845a7da8273d8475cbc5933 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 21:46:42 +0000 Subject: [PATCH 05/20] Remove codeowner names from README documentation Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/README-strict-version-pins.md | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/README-strict-version-pins.md b/.github/README-strict-version-pins.md index 52f87e7becc9..6fcdb50e845b 100644 --- a/.github/README-strict-version-pins.md +++ b/.github/README-strict-version-pins.md @@ -35,10 +35,7 @@ The following are **ignored**: If strict version pins are detected, the workflow: 1. Posts a comment on the PR listing the detected pins -2. Checks if any of the following architects have approved the PR: - - `@kashifkhan` - - `@annatisch` - - `@johanste` +2. Checks if any of the designated architects have approved the PR 3. **Blocks merging** if no architect approval is found (workflow fails with exit code 1) 4. **Allows merging** if an architect has approved @@ -131,10 +128,7 @@ If the workflow incorrectly flags a dependency: ### Override Process If a strict pin is necessary: 1. Document the justification in the PR description -2. Request review from one of the architects: - - @kashifkhan - - @annatisch - - @johanste +2. Request review from one of the designated architects 3. Architect provides approval review 4. Workflow will pass and allow merge From 67cc77265986c90eface658173806944fcb3bc9e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 20 Nov 2025 23:11:27 +0000 Subject: [PATCH 06/20] Add .github/scripts/** to cspell ignorePaths Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .vscode/cspell.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index dc421e3d78e9..c35d53023c78 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -14,6 +14,7 @@ ".devcontainer/**", "eng/**", ".github/CODEOWNERS", + ".github/scripts/**", "scripts/**", "**/tests/recordings/**", ".vscode/cspell.json", From a14b5c9af28836a883244d9a4c3728442513f1f8 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Mon, 24 Nov 2025 09:59:35 -0800 Subject: [PATCH 07/20] testing --- sdk/eventgrid/azure-eventgrid/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/eventgrid/azure-eventgrid/setup.py b/sdk/eventgrid/azure-eventgrid/setup.py index b88a566aff72..d0490f6c7624 100644 --- a/sdk/eventgrid/azure-eventgrid/setup.py +++ b/sdk/eventgrid/azure-eventgrid/setup.py @@ -64,7 +64,7 @@ install_requires=[ "isodate>=0.6.1", "azure-core>=1.30.0", - "typing-extensions>=4.6.0", + "typing-extensions==4.6.0", ], python_requires=">=3.8", ) From ee75f22d1cdfd8bfef6448b5fe3ab11709f22412 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 18:12:00 +0000 Subject: [PATCH 08/20] Improve Python code quality and use file-specific cspell ignore Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/scripts/check_strict_pins.py | 61 +++++++++++++++++----------- .vscode/cspell.json | 1 - 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/.github/scripts/check_strict_pins.py b/.github/scripts/check_strict_pins.py index d481e9a439a1..f0c4cd6389c3 100755 --- a/.github/scripts/check_strict_pins.py +++ b/.github/scripts/check_strict_pins.py @@ -1,4 +1,5 @@ #!/usr/bin/env python3 +# cspell:ignore pyproject """ Script to detect strict version pins (==) in Python package dependencies. @@ -12,14 +13,14 @@ - Changes outside of main dependency sections """ +import json import os import re import subprocess import sys -from typing import Dict, List, Set, Tuple -import json import urllib.request import urllib.parse +from typing import Dict, List, Optional def run_git_command(cmd: List[str]) -> str: @@ -47,9 +48,8 @@ def get_changed_files(base_ref: str, head_ref: str) -> List[str]: files = [] for line in diff_output.strip().split('\n'): - if line: - if (line.startswith('sdk/') and - (line.endswith('/setup.py') or line.endswith('/pyproject.toml'))): + if line and line.startswith('sdk/'): + if line.endswith('/setup.py') or line.endswith('/pyproject.toml'): files.append(line) return files @@ -65,12 +65,19 @@ def extract_strict_pins_from_setup_py_diff(diff_content: str) -> List[str]: bracket_depth = 0 for line in diff_content.split('\n'): - # Skip the +++ file marker + # Skip the +++ and --- file markers if line.startswith('+++') or line.startswith('---'): continue # Process all lines to track context, but only extract from added lines - actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else line.strip() + 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: @@ -92,18 +99,18 @@ def extract_strict_pins_from_setup_py_diff(diff_content: str) -> List[str]: # 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 line.startswith('+') and '==' in actual_line and not actual_line.strip().startswith('#'): - match = re.search(r'["\']([^"\']+==[\d\.]+[^"\']*)["\']', actual_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 line.startswith('+'): - if '==' in actual_line and not actual_line.strip().startswith('#'): + 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) + match = re.search(r'["\']([^"\']+==[\d.]+[^"\']*)["\']', actual_line) if match: strict_pins.append(match.group(1)) @@ -125,13 +132,20 @@ def extract_strict_pins_from_pyproject_diff(diff_content: str) -> List[str]: continue # Process all lines to track context - actual_line = line[1:].strip() if (line.startswith('+') or line.startswith('-') or line.startswith(' ')) else line.strip() + 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 - elif actual_line.startswith('['): + else: in_other_section = True in_project_dependencies = False @@ -148,10 +162,10 @@ def extract_strict_pins_from_pyproject_diff(diff_content: str) -> List[str]: continue # Look for strict pins in added lines within [project] dependencies - if in_project_dependencies and line.startswith('+'): - if '==' in actual_line and not actual_line.strip().startswith('#'): + 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) + match = re.search(r'["\']([^"\']+==[\d.]+[^"\']*)["\']', actual_line) if match: strict_pins.append(match.group(1)) @@ -213,7 +227,7 @@ def check_architect_approval(pr_number: str, repo: str, github_token: str) -> bo return False -def set_output(name: str, value: str): +def set_output(name: str, value: str) -> None: """Set GitHub Actions output.""" github_output = os.getenv('GITHUB_OUTPUT') if github_output: @@ -225,14 +239,15 @@ def set_output(name: str, value: str): print(f"::set-output name={name}::{value}") -def main(): +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(f"Checking for strict version pins...") + print("Checking for strict version pins...") print(f"Base: {base_ref}, Head: {head_ref}") # Get changed files @@ -246,11 +261,11 @@ def main(): return 0 print(f"Checking {len(changed_files)} file(s):") - for f in changed_files: - print(f" - {f}") + for file_path in changed_files: + print(f" - {file_path}") # Check each file for strict pins - all_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: diff --git a/.vscode/cspell.json b/.vscode/cspell.json index c35d53023c78..dc421e3d78e9 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -14,7 +14,6 @@ ".devcontainer/**", "eng/**", ".github/CODEOWNERS", - ".github/scripts/**", "scripts/**", "**/tests/recordings/**", ".vscode/cspell.json", From 92f07da4c2e2670190704e742a2f9a09ed42bd12 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 18:13:29 +0000 Subject: [PATCH 09/20] Remove CODEOWNERS entries for setup.py and pyproject.toml - CI workflow handles enforcement Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/CODEOWNERS | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 50118481b8a9..fef97554f044 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -871,8 +871,3 @@ /pylintrc @l0lawrence @scbedd @mccoyp /sdk/**/ci.yml @msyyc @lmazuel @scbedd - -# Require architect approval for dependency changes (setup.py and pyproject.toml) -# to prevent strict version pins without proper review -/sdk/**/setup.py @kashifkhan @annatisch @johanste -/sdk/**/pyproject.toml @kashifkhan @annatisch @johanste From f93a1ae63ef06b888cc0df394157ae20b2d84955 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 18:29:04 +0000 Subject: [PATCH 10/20] Revert accidental change to eventgrid setup.py Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- sdk/eventgrid/azure-eventgrid/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/eventgrid/azure-eventgrid/setup.py b/sdk/eventgrid/azure-eventgrid/setup.py index d0490f6c7624..b88a566aff72 100644 --- a/sdk/eventgrid/azure-eventgrid/setup.py +++ b/sdk/eventgrid/azure-eventgrid/setup.py @@ -64,7 +64,7 @@ install_requires=[ "isodate>=0.6.1", "azure-core>=1.30.0", - "typing-extensions==4.6.0", + "typing-extensions>=4.6.0", ], python_requires=">=3.8", ) From 044c899352bc866fd7197f79a87042244999654d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 23:22:34 +0000 Subject: [PATCH 11/20] Add architect names to cspell ignore in check_strict_pins.py Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/scripts/check_strict_pins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/scripts/check_strict_pins.py b/.github/scripts/check_strict_pins.py index f0c4cd6389c3..bb2c72d63997 100755 --- a/.github/scripts/check_strict_pins.py +++ b/.github/scripts/check_strict_pins.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# cspell:ignore pyproject +# cspell:ignore pyproject kashifkhan annatisch johanste """ Script to detect strict version pins (==) in Python package dependencies. From 79fd49b425155f6578053392940b0636ce57c411 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 23:58:41 +0000 Subject: [PATCH 12/20] Add cspell ignore comments to README and workflow files Co-authored-by: l0lawrence <100643745+l0lawrence@users.noreply.github.com> --- .github/README-strict-version-pins.md | 1 + .github/workflows/check-strict-version-pins.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/README-strict-version-pins.md b/.github/README-strict-version-pins.md index 6fcdb50e845b..75ee2ff87f0c 100644 --- a/.github/README-strict-version-pins.md +++ b/.github/README-strict-version-pins.md @@ -1,3 +1,4 @@ + # Strict Version Pin Check Workflow ## Overview diff --git a/.github/workflows/check-strict-version-pins.yml b/.github/workflows/check-strict-version-pins.yml index afec2732526b..f2e87db4f912 100644 --- a/.github/workflows/check-strict-version-pins.yml +++ b/.github/workflows/check-strict-version-pins.yml @@ -1,3 +1,4 @@ +# cspell:ignore pyproject kashifkhan annatisch johanste name: Check for Strict Version Pins on: From 4addcdd4cfdae605c5db1f3e0ed6e879e4fcadc9 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 12:26:08 -0800 Subject: [PATCH 13/20] remove GH action, add to analyze stage for now --- .github/README-strict-version-pins.md | 149 ----------------- .../workflows/check-strict-version-pins.yml | 108 ------------ eng/pipelines/templates/steps/analyze.yml | 4 + .../templates/steps/check_strict_pins.yml | 10 ++ scripts/devops_tasks/check_strict_pins.py | 154 ++++++++++++++++++ 5 files changed, 168 insertions(+), 257 deletions(-) delete mode 100644 .github/README-strict-version-pins.md delete mode 100644 .github/workflows/check-strict-version-pins.yml create mode 100644 eng/pipelines/templates/steps/check_strict_pins.yml create mode 100644 scripts/devops_tasks/check_strict_pins.py diff --git a/.github/README-strict-version-pins.md b/.github/README-strict-version-pins.md deleted file mode 100644 index 75ee2ff87f0c..000000000000 --- a/.github/README-strict-version-pins.md +++ /dev/null @@ -1,149 +0,0 @@ - -# Strict Version Pin Check Workflow - -## Overview - -This GitHub Actions workflow enforces a policy that requires architect approval for any pull requests that introduce strict version pins (`==`) in main runtime dependencies of Python packages within the `sdk/` directory. - -## Purpose - -Strict version pins can cause dependency conflicts and limit flexibility in package management. This workflow ensures that any such pins in main runtime dependencies are reviewed and approved by designated architects before merging. - -## How It Works - -### Trigger -The workflow runs on pull requests that modify: -- `sdk/**/setup.py` -- `sdk/**/pyproject.toml` - -### Detection Logic - -The workflow analyzes the diff to detect: -- **New strict version pins**: Dependencies newly added with `==` operator -- **Modified pins**: Dependencies changed from flexible constraints (e.g., `>=`, `~=`) to strict `==` pins - -The detection is **scope-aware** and only considers: -- `install_requires` in `setup.py` -- `dependencies` under `[project]` in `pyproject.toml` - -The following are **ignored**: -- Dev dependencies (`extras_require`, `[project.optional-dependencies]`) -- Test dependencies (`tests_require`) -- Comments -- Build dependencies - -### Approval Requirements - -If strict version pins are detected, the workflow: -1. Posts a comment on the PR listing the detected pins -2. Checks if any of the designated architects have approved the PR -3. **Blocks merging** if no architect approval is found (workflow fails with exit code 1) -4. **Allows merging** if an architect has approved - -### CODEOWNERS Integration - -The `.github/CODEOWNERS` file has been updated to require reviews from the architects for: -- `/sdk/**/setup.py` -- `/sdk/**/pyproject.toml` - -This provides a secondary enforcement mechanism through branch protection rules. - -## Examples - -### ✅ Allowed (No Strict Pins) -```python -# setup.py -install_requires=[ - "azure-core>=1.30.0", - "requests>=2.21.0", -] -``` - -### ⚠️ Requires Architect Approval -```python -# setup.py -install_requires=[ - "azure-core==1.30.0", # Strict pin detected! - "requests>=2.21.0", -] -``` - -### ✅ Allowed (Strict Pin in Dev Dependencies) -```python -# setup.py -install_requires=[ - "azure-core>=1.30.0", -], -extras_require={ - "dev": ["pytest==7.0.0"] # OK - this is a dev dependency -} -``` - -## Testing - -The detection logic has been validated with comprehensive test cases covering: -- Adding new strict pins -- Changing from flexible to strict constraints -- Ignoring dev/test dependencies -- Ignoring optional dependencies in pyproject.toml - -Run tests locally: -```bash -python /tmp/test_strict_pins.py -``` - -## Files Modified - -1. **`.github/workflows/check-strict-version-pins.yml`** - - Main workflow definition - - Triggers on PR events - - Runs detection and enforcement logic - -2. **`.github/scripts/check_strict_pins.py`** - - Python script that analyzes git diffs - - Detects strict version pins in appropriate sections - - Checks for architect approvals via GitHub API - -3. **`.github/CODEOWNERS`** - - Added architect requirements for setup.py and pyproject.toml - -## Branch Protection - -To fully enforce this policy, ensure branch protection rules are configured to: -- Require status checks to pass before merging -- Require the "check-strict-pins" workflow to succeed -- Require review from code owners - -## Troubleshooting - -### Workflow Not Running -- Verify the PR modifies files matching `sdk/**/setup.py` or `sdk/**/pyproject.toml` -- Check workflow runs in the Actions tab - -### False Positives -If the workflow incorrectly flags a dependency: -- Verify the dependency is in the main runtime dependencies section -- Check if comments are interfering with detection -- File an issue with the specific case - -### Override Process -If a strict pin is necessary: -1. Document the justification in the PR description -2. Request review from one of the designated architects -3. Architect provides approval review -4. Workflow will pass and allow merge - -## Maintenance - -### Adding/Removing Architects -To modify the list of architects: -1. Update the `architects` set in `.github/scripts/check_strict_pins.py` -2. Update the CODEOWNERS entries in `.github/CODEOWNERS` -3. Update documentation in this README - -### Extending Detection -To add support for additional dependency formats: -1. Add extraction function in `check_strict_pins.py` -2. Update `check_file_for_strict_pins()` to handle new file types -3. Add corresponding test cases -4. Update workflow triggers if needed diff --git a/.github/workflows/check-strict-version-pins.yml b/.github/workflows/check-strict-version-pins.yml deleted file mode 100644 index f2e87db4f912..000000000000 --- a/.github/workflows/check-strict-version-pins.yml +++ /dev/null @@ -1,108 +0,0 @@ -# cspell:ignore pyproject kashifkhan annatisch johanste -name: Check for Strict Version Pins - -on: - pull_request: - paths: - - 'sdk/**/setup.py' - - 'sdk/**/pyproject.toml' - -permissions: - pull-requests: write - contents: read - -jobs: - check-strict-pins: - runs-on: ubuntu-latest - steps: - - name: Checkout repository - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.11' - - - name: Fetch base branch - run: | - git fetch origin ${{ github.base_ref }} - - - name: Check for strict version pins - id: check-pins - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.pull_request.number }} - REPO: ${{ github.repository }} - BASE_REF: origin/${{ github.base_ref }} - HEAD_REF: ${{ github.sha }} - run: | - python .github/scripts/check_strict_pins.py - - - name: Comment on PR if strict pins found - if: steps.check-pins.outputs.strict_pins_found == 'true' - uses: actions/github-script@v7 - with: - script: | - const strictPins = `${{ steps.check-pins.outputs.strict_pins_details }}`; - const architectApproved = `${{ steps.check-pins.outputs.architect_approved }}`; - - let message = '## ⚠️ Strict Version Pin Detected\n\n'; - message += 'This PR introduces one or more strict version pins (`==`) in main runtime dependencies:\n\n'; - message += '```\n' + strictPins + '\n```\n\n'; - - if (architectApproved === 'true') { - message += '✅ An architect has approved this PR.\n'; - } else { - message += '❌ **This PR requires approval from one of the following architects:**\n'; - message += '- @kashifkhan\n'; - message += '- @annatisch\n'; - message += '- @johanste\n\n'; - message += 'Please request a review from one of these architects before merging.\n'; - } - - // Check if comment already exists - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - }); - - const botComment = comments.find(comment => - comment.user.type === 'Bot' && - comment.body.includes('Strict Version Pin Detected') - ); - - if (botComment) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: botComment.id, - body: message - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - body: message - }); - } - - - name: Block merge if architect approval required - if: steps.check-pins.outputs.strict_pins_found == 'true' && steps.check-pins.outputs.architect_approved == 'false' - run: | - echo "::error::Strict version pins detected without architect approval" - echo "This PR introduces strict version pins (==) in main runtime dependencies." - echo "Approval required from: kashifkhan, annatisch, or johanste" - exit 1 - - - name: Success - No strict pins or approved - if: steps.check-pins.outputs.strict_pins_found == 'false' || steps.check-pins.outputs.architect_approved == 'true' - run: | - if [ "${{ steps.check-pins.outputs.strict_pins_found }}" == "true" ]; then - echo "✅ Strict version pins detected but architect has approved" - else - echo "✅ No new strict version pins detected in main runtime dependencies" - fi 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..8990bb714b18 --- /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 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..c212c94770a3 --- /dev/null +++ b/scripts/devops_tasks/check_strict_pins.py @@ -0,0 +1,154 @@ +#!/usr/bin/env python3 +# cspell:ignore pyproject kashifkhan annatisch johanste +""" +Detects strict version pins (==) in Python package dependencies. + +Checks setup.py (install_requires) and pyproject.toml ([project] dependencies) +for new strict version pins. Requires architect approval to proceed. +""" + +import argparse +import json +import os +import re +import subprocess +import sys +from pathlib import Path +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 get_strict_pins(package_path: str) -> List[str]: + """Get list of strict pins from a package using ParsedSetup.""" + try: + parsed = ParsedSetup.from_path(package_path) + strict_pins = [] + + for requirement in parsed.requires: + match = STRICT_PIN_PATTERN.match(requirement) + if match: + strict_pins.append(requirement) + + return strict_pins + except Exception as e: + print(f"Warning: Could not parse {package_path}: {e}") + return [] + + +def check_for_new_strict_pins(filepath: str, base_ref: str, head_ref: str) -> List[str]: + """Check if new strict pins were introduced in this file.""" + package_dir = os.path.dirname(filepath) + + # Get current strict pins from HEAD + current_pins = set(get_strict_pins(package_dir)) + + # Checkout base version temporarily to check old pins + run_git_command(["git", "checkout", base_ref, "--", filepath]) + old_pins = set(get_strict_pins(package_dir)) + + # Restore current version + run_git_command(["git", "checkout", head_ref, "--", filepath]) + + # Return only newly added strict pins + new_pins = current_pins - old_pins + return sorted(new_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 [] + + for approver in approvers: + for architect in ARCHITECTS: + if architect.lower() in approver.lower(): + return True + + return False + +def print_strict_pins(all_strict_pins: Dict[str, List[str]]): + """Print detected strict pins in a formatted way.""" + print("STRICT VERSION PINS DETECTED") + for filepath, pins in all_strict_pins.items(): + print(f"\n{filepath}:") + for pin in pins: + print(f" - {pin}") + + print("\n" + "-" * 80) + print("POLICY: Strict version pins (==) require architect approval") + print(f"Required approvers: {', '.join(sorted(ARCHITECTS))}") + print("-" * 80) + + +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 = { + filepath: pins + for filepath in changed_files + if (pins := check_for_new_strict_pins(filepath, args.base, args.head)) + } + + if not all_strict_pins: + return 0 + + # Found strict pins - check for approval + print_strict_pins(all_strict_pins) + + if check_architect_approval_via_cli(args.pr_number): + return 0 + + # No approval - fail the check + print("\nThis PR introduces strict version pins without architect approval.") + print(f"Please request review and approval from: {', '.join(sorted(ARCHITECTS))}") + print("=" * 80) + return 1 + + +if __name__ == '__main__': + sys.exit(main()) From 53b46afd73371ff26f448acf4d8fd62d899136b2 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:22:38 -0800 Subject: [PATCH 14/20] mini changes --- .../templates/steps/check_strict_pins.yml | 2 +- scripts/devops_tasks/check_strict_pins.py | 110 +++++++----------- 2 files changed, 43 insertions(+), 69 deletions(-) diff --git a/eng/pipelines/templates/steps/check_strict_pins.yml b/eng/pipelines/templates/steps/check_strict_pins.yml index 8990bb714b18..ca2a80eeed24 100644 --- a/eng/pipelines/templates/steps/check_strict_pins.yml +++ b/eng/pipelines/templates/steps/check_strict_pins.yml @@ -3,7 +3,7 @@ parameters: steps: - task: PythonScript@0 - displayName: 'Check for Strict Version Pins' + 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' diff --git a/scripts/devops_tasks/check_strict_pins.py b/scripts/devops_tasks/check_strict_pins.py index c212c94770a3..2587fc9efc8c 100644 --- a/scripts/devops_tasks/check_strict_pins.py +++ b/scripts/devops_tasks/check_strict_pins.py @@ -1,10 +1,15 @@ -#!/usr/bin/env python3 +#!/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 version pins (==) in Python package dependencies. +Detects strict dependency version pins (==) in Python package dependencies. Checks setup.py (install_requires) and pyproject.toml ([project] dependencies) -for new strict version pins. Requires architect approval to proceed. +for new strict dependency version pins. Requires architect approval to proceed. """ import argparse @@ -13,7 +18,6 @@ import re import subprocess import sys -from pathlib import Path from typing import Dict, List from ci_tools.parsing import ParsedSetup @@ -41,40 +45,31 @@ def get_changed_files(base_ref: str, head_ref: str) -> List[str]: ] -def get_strict_pins(package_path: str) -> List[str]: - """Get list of strict pins from a package using ParsedSetup.""" +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_path) - strict_pins = [] - - for requirement in parsed.requires: - match = STRICT_PIN_PATTERN.match(requirement) - if match: - strict_pins.append(requirement) - - return strict_pins + parsed = ParsedSetup.from_path(package_dir) except Exception as e: - print(f"Warning: Could not parse {package_path}: {e}") + print(f"Warning: Could not parse {package_dir}: {e}") return [] - - -def check_for_new_strict_pins(filepath: str, base_ref: str, head_ref: str) -> List[str]: - """Check if new strict pins were introduced in this file.""" - package_dir = os.path.dirname(filepath) - - # Get current strict pins from HEAD - current_pins = set(get_strict_pins(package_dir)) - # Checkout base version temporarily to check old pins - run_git_command(["git", "checkout", base_ref, "--", filepath]) - old_pins = set(get_strict_pins(package_dir)) + # 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 = [] - # Restore current version - run_git_command(["git", "checkout", head_ref, "--", filepath]) + 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 only newly added strict pins - new_pins = current_pins - old_pins - return sorted(new_pins) + return strict_pins def check_architect_approval_via_cli(pr_number: str) -> bool: @@ -93,27 +88,10 @@ def check_architect_approval_via_cli(pr_number: str) -> bool: return False approvers = json.loads(result.stdout) if result.stdout else [] - - for approver in approvers: - for architect in ARCHITECTS: - if architect.lower() in approver.lower(): - return True - - return False - -def print_strict_pins(all_strict_pins: Dict[str, List[str]]): - """Print detected strict pins in a formatted way.""" - print("STRICT VERSION PINS DETECTED") - for filepath, pins in all_strict_pins.items(): - print(f"\n{filepath}:") - for pin in pins: - print(f" - {pin}") - - print("\n" + "-" * 80) - print("POLICY: Strict version pins (==) require architect approval") - print(f"Required approvers: {', '.join(sorted(ARCHITECTS))}") - print("-" * 80) - + 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') @@ -128,27 +106,23 @@ def main() -> int: return 0 # Check each file for new strict pins - all_strict_pins = { - filepath: pins - for filepath in changed_files - if (pins := check_for_new_strict_pins(filepath, args.base, args.head)) - } - - if not all_strict_pins: + all_strict_pins = [] + for filepath in changed_files: + diff_output = run_git_command([ + "git", "diff", args.base, args.head, "--", filepath + ]) + all_strict_pins = check_for_new_strict_pins(filepath, diff_output) + + if not all_strict_pins or len(all_strict_pins) == 0: return 0 - # Found strict pins - check for approval - print_strict_pins(all_strict_pins) - if check_architect_approval_via_cli(args.pr_number): + print("\nArchitect approval found") return 0 - # No approval - fail the check - print("\nThis PR introduces strict version pins without architect approval.") - print(f"Please request review and approval from: {', '.join(sorted(ARCHITECTS))}") - print("=" * 80) + 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()) + sys.exit(main()) \ No newline at end of file From 8d799b964535f770db5c0503f966994add6feb5b Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:25:40 -0800 Subject: [PATCH 15/20] Test: Add strict pin to requests --- sdk/core/azure-core/setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 3f65bdf791d5..8080a39f44f4 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -69,7 +69,7 @@ }, python_requires=">=3.9", install_requires=[ - "requests>=2.21.0", + "requests==2.31.0", "typing-extensions>=4.6.0", ], extras_require={ @@ -81,3 +81,4 @@ ], }, ) + From 2a41637a29f806df8cab759484afa66ff52dd7e6 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:27:04 -0800 Subject: [PATCH 16/20] Test: Add strict pin to requests --- sdk/core/azure-core/setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 8080a39f44f4..256bdb8a2f13 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -70,7 +70,7 @@ python_requires=">=3.9", install_requires=[ "requests==2.31.0", - "typing-extensions>=4.6.0", + "typing-extensions==4.6.0", ], extras_require={ "aio": [ From d98f9ff4c66a3b11cd8e91fa108934f0724d53e4 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:27:44 -0800 Subject: [PATCH 17/20] Test: add strict pin to requests==2.31.0 --- sdk/core/azure-core/setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 256bdb8a2f13..9fc1dfba83a8 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -69,7 +69,7 @@ }, python_requires=">=3.9", install_requires=[ - "requests==2.31.0", + "requests>=2.21.0", "typing-extensions==4.6.0", ], extras_require={ @@ -81,4 +81,3 @@ ], }, ) - From 02a98bbc26b7f5d71e966806e1f047a2b021ca5f Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:34:34 -0800 Subject: [PATCH 18/20] Test: Add strict pin to requests --- scripts/devops_tasks/check_strict_pins.py | 3 ++- sdk/core/azure-core/setup.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/devops_tasks/check_strict_pins.py b/scripts/devops_tasks/check_strict_pins.py index 2587fc9efc8c..a64e7b5220ba 100644 --- a/scripts/devops_tasks/check_strict_pins.py +++ b/scripts/devops_tasks/check_strict_pins.py @@ -111,7 +111,8 @@ def main() -> int: diff_output = run_git_command([ "git", "diff", args.base, args.head, "--", filepath ]) - all_strict_pins = check_for_new_strict_pins(filepath, diff_output) + if pins := check_for_new_strict_pins(filepath, diff_output): + all_strict_pins.extend(pins) if not all_strict_pins or len(all_strict_pins) == 0: return 0 diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 9fc1dfba83a8..256bdb8a2f13 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -69,7 +69,7 @@ }, python_requires=">=3.9", install_requires=[ - "requests>=2.21.0", + "requests==2.31.0", "typing-extensions==4.6.0", ], extras_require={ @@ -81,3 +81,4 @@ ], }, ) + From e08dca50b1efcb5624c0063d047a83a1cbb2c158 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:41:13 -0800 Subject: [PATCH 19/20] no pr test --- scripts/devops_tasks/check_strict_pins.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/scripts/devops_tasks/check_strict_pins.py b/scripts/devops_tasks/check_strict_pins.py index a64e7b5220ba..d9dc83dbcab3 100644 --- a/scripts/devops_tasks/check_strict_pins.py +++ b/scripts/devops_tasks/check_strict_pins.py @@ -112,11 +112,21 @@ def main() -> int: "git", "diff", args.base, args.head, "--", filepath ]) if pins := check_for_new_strict_pins(filepath, diff_output): - all_strict_pins.extend(pins) + all_strict_pins += pins - if not all_strict_pins or len(all_strict_pins) == 0: + 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 From 73a59207b5795e21ba1b0a918de6f0a369623487 Mon Sep 17 00:00:00 2001 From: Libba Lawrence Date: Tue, 2 Dec 2025 13:45:18 -0800 Subject: [PATCH 20/20] oops --- sdk/core/azure-core/setup.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 256bdb8a2f13..3f65bdf791d5 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -69,8 +69,8 @@ }, python_requires=">=3.9", install_requires=[ - "requests==2.31.0", - "typing-extensions==4.6.0", + "requests>=2.21.0", + "typing-extensions>=4.6.0", ], extras_require={ "aio": [ @@ -81,4 +81,3 @@ ], }, ) -