|
| 1 | +# Strict Version Pin Check Workflow |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +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. |
| 6 | + |
| 7 | +## Purpose |
| 8 | + |
| 9 | +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. |
| 10 | + |
| 11 | +## How It Works |
| 12 | + |
| 13 | +### Trigger |
| 14 | +The workflow runs on pull requests that modify: |
| 15 | +- `sdk/**/setup.py` |
| 16 | +- `sdk/**/pyproject.toml` |
| 17 | + |
| 18 | +### Detection Logic |
| 19 | + |
| 20 | +The workflow analyzes the diff to detect: |
| 21 | +- **New strict version pins**: Dependencies newly added with `==` operator |
| 22 | +- **Modified pins**: Dependencies changed from flexible constraints (e.g., `>=`, `~=`) to strict `==` pins |
| 23 | + |
| 24 | +The detection is **scope-aware** and only considers: |
| 25 | +- `install_requires` in `setup.py` |
| 26 | +- `dependencies` under `[project]` in `pyproject.toml` |
| 27 | + |
| 28 | +The following are **ignored**: |
| 29 | +- Dev dependencies (`extras_require`, `[project.optional-dependencies]`) |
| 30 | +- Test dependencies (`tests_require`) |
| 31 | +- Comments |
| 32 | +- Build dependencies |
| 33 | + |
| 34 | +### Approval Requirements |
| 35 | + |
| 36 | +If strict version pins are detected, the workflow: |
| 37 | +1. Posts a comment on the PR listing the detected pins |
| 38 | +2. Checks if any of the following architects have approved the PR: |
| 39 | + - `@kashifkhan` |
| 40 | + - `@annatisch` |
| 41 | + - `@johanste` |
| 42 | +3. **Blocks merging** if no architect approval is found (workflow fails with exit code 1) |
| 43 | +4. **Allows merging** if an architect has approved |
| 44 | + |
| 45 | +### CODEOWNERS Integration |
| 46 | + |
| 47 | +The `.github/CODEOWNERS` file has been updated to require reviews from the architects for: |
| 48 | +- `/sdk/**/setup.py` |
| 49 | +- `/sdk/**/pyproject.toml` |
| 50 | + |
| 51 | +This provides a secondary enforcement mechanism through branch protection rules. |
| 52 | + |
| 53 | +## Examples |
| 54 | + |
| 55 | +### ✅ Allowed (No Strict Pins) |
| 56 | +```python |
| 57 | +# setup.py |
| 58 | +install_requires=[ |
| 59 | + "azure-core>=1.30.0", |
| 60 | + "requests>=2.21.0", |
| 61 | +] |
| 62 | +``` |
| 63 | + |
| 64 | +### ⚠️ Requires Architect Approval |
| 65 | +```python |
| 66 | +# setup.py |
| 67 | +install_requires=[ |
| 68 | + "azure-core==1.30.0", # Strict pin detected! |
| 69 | + "requests>=2.21.0", |
| 70 | +] |
| 71 | +``` |
| 72 | + |
| 73 | +### ✅ Allowed (Strict Pin in Dev Dependencies) |
| 74 | +```python |
| 75 | +# setup.py |
| 76 | +install_requires=[ |
| 77 | + "azure-core>=1.30.0", |
| 78 | +], |
| 79 | +extras_require={ |
| 80 | + "dev": ["pytest==7.0.0"] # OK - this is a dev dependency |
| 81 | +} |
| 82 | +``` |
| 83 | + |
| 84 | +## Testing |
| 85 | + |
| 86 | +The detection logic has been validated with comprehensive test cases covering: |
| 87 | +- Adding new strict pins |
| 88 | +- Changing from flexible to strict constraints |
| 89 | +- Ignoring dev/test dependencies |
| 90 | +- Ignoring optional dependencies in pyproject.toml |
| 91 | + |
| 92 | +Run tests locally: |
| 93 | +```bash |
| 94 | +python /tmp/test_strict_pins.py |
| 95 | +``` |
| 96 | + |
| 97 | +## Files Modified |
| 98 | + |
| 99 | +1. **`.github/workflows/check-strict-version-pins.yml`** |
| 100 | + - Main workflow definition |
| 101 | + - Triggers on PR events |
| 102 | + - Runs detection and enforcement logic |
| 103 | + |
| 104 | +2. **`.github/scripts/check_strict_pins.py`** |
| 105 | + - Python script that analyzes git diffs |
| 106 | + - Detects strict version pins in appropriate sections |
| 107 | + - Checks for architect approvals via GitHub API |
| 108 | + |
| 109 | +3. **`.github/CODEOWNERS`** |
| 110 | + - Added architect requirements for setup.py and pyproject.toml |
| 111 | + |
| 112 | +## Branch Protection |
| 113 | + |
| 114 | +To fully enforce this policy, ensure branch protection rules are configured to: |
| 115 | +- Require status checks to pass before merging |
| 116 | +- Require the "check-strict-pins" workflow to succeed |
| 117 | +- Require review from code owners |
| 118 | + |
| 119 | +## Troubleshooting |
| 120 | + |
| 121 | +### Workflow Not Running |
| 122 | +- Verify the PR modifies files matching `sdk/**/setup.py` or `sdk/**/pyproject.toml` |
| 123 | +- Check workflow runs in the Actions tab |
| 124 | + |
| 125 | +### False Positives |
| 126 | +If the workflow incorrectly flags a dependency: |
| 127 | +- Verify the dependency is in the main runtime dependencies section |
| 128 | +- Check if comments are interfering with detection |
| 129 | +- File an issue with the specific case |
| 130 | + |
| 131 | +### Override Process |
| 132 | +If a strict pin is necessary: |
| 133 | +1. Document the justification in the PR description |
| 134 | +2. Request review from one of the architects: |
| 135 | + - @kashifkhan |
| 136 | + - @annatisch |
| 137 | + - @johanste |
| 138 | +3. Architect provides approval review |
| 139 | +4. Workflow will pass and allow merge |
| 140 | + |
| 141 | +## Maintenance |
| 142 | + |
| 143 | +### Adding/Removing Architects |
| 144 | +To modify the list of architects: |
| 145 | +1. Update the `architects` set in `.github/scripts/check_strict_pins.py` |
| 146 | +2. Update the CODEOWNERS entries in `.github/CODEOWNERS` |
| 147 | +3. Update documentation in this README |
| 148 | + |
| 149 | +### Extending Detection |
| 150 | +To add support for additional dependency formats: |
| 151 | +1. Add extraction function in `check_strict_pins.py` |
| 152 | +2. Update `check_file_for_strict_pins()` to handle new file types |
| 153 | +3. Add corresponding test cases |
| 154 | +4. Update workflow triggers if needed |
0 commit comments