Sync eng/common directory with azure-sdk-tools for PR 14661#9766
Sync eng/common directory with azure-sdk-tools for PR 14661#9766
Conversation
azure-sdk
added
EngSys
There was a problem hiding this comment.
Pull request overview
Adds a new eng/common pipeline check to prevent unauthorized modifications to protected sections of the .github/CODEOWNERS file by exporting and comparing named sections via the azsdk CLI.
Changes:
- Add
Test-CodeownersSections.ps1to export/compare specific CODEOWNERS sections and emit diffs on mismatch. - Add
verify-codeowners-sections.ymlpipeline step template to detect CODEOWNERS changes, prepare before/after snapshots, and run the comparison. - Extend
install-azsdk-cli.ymlwith aConditionparameter so the CLI install can be skipped when not needed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| eng/common/scripts/Test-CodeownersSections.ps1 | New script that exports and compares protected CODEOWNERS sections using azsdk CLI. |
| eng/common/pipelines/templates/steps/verify-codeowners-sections.yml | New step template that detects CODEOWNERS edits and runs the section comparison in PR builds. |
| eng/common/pipelines/templates/steps/install-azsdk-cli.yml | Adds a Condition parameter to conditionally run the azsdk CLI install task. |
| $diffOutput = git diff --name-only HEAD~1 HEAD -- ".github/CODEOWNERS" 2>&1 | ||
| if ($LASTEXITCODE) { | ||
| Write-Host "Unable to compute diff (exit code $LASTEXITCODE). Skipping CODEOWNERS section check." | ||
| Write-Host "##vso[task.setvariable variable=CodeownersChanged]false" | ||
| exit 0 |
There was a problem hiding this comment.
The CODEOWNERS change detection is based on git diff HEAD~1 HEAD, and if that diff fails the template sets CodeownersChanged=false and exits 0. In shallow checkouts (or any scenario where HEAD~1 isn't available), this effectively bypasses the protected-section enforcement. Consider using the existing eng/common/scripts/get-changedfiles.ps1 (which diffs PR source vs target) and/or failing the job when the diff can’t be computed so the protection can’t be skipped.
| # Extract parent commit version | ||
| git show "HEAD~1:.github/CODEOWNERS" > $beforeFile | ||
| Write-Host "Retrieved parent commit CODEOWNERS to $beforeFile" | ||
| Get-Content $beforeFile | Select-Object -First 20 | Write-Host | ||
| if ($LASTEXITCODE) { | ||
| Write-Host "Could not retrieve CODEOWNERS from HEAD~1. The file may be newly added. Skipping." | ||
| Write-Host "##vso[task.setvariable variable=CodeownersChanged]false" | ||
| exit 0 |
There was a problem hiding this comment.
The "before" CODEOWNERS file is pulled from HEAD~1 and on failure the step skips the entire check by setting CodeownersChanged=false. This is unreliable for PR builds where the checked-out ref isn’t guaranteed to have HEAD~1 locally (e.g., shallow fetch) and can also compare against the wrong baseline depending on how the PR ref is checked out. Prefer retrieving the base version via the PR target commit/branch (e.g., using the same source/target commitish logic as get-changedfiles.ps1) and avoid treating retrieval failures as a reason to skip enforcement.
| foreach ($section in $Sections) { | ||
| $safeName = $section -replace ' ', '_' | ||
| $beforeSection = Join-Path $TempDirectory "before.${safeName}.txt" | ||
| $afterSection = Join-Path $TempDirectory "after.${safeName}.txt" |
There was a problem hiding this comment.
$TempDirectory is used to build the --output-file paths, but the script never ensures that directory exists. If the caller forgets to create it (or if it was cleaned up), the azsdk export command will fail writing the output files. Consider creating $TempDirectory (e.g., New-Item -ItemType Directory -Force) during input validation.
azure-sdk
force-pushed
the
sync-eng/common-djurek/codeowners-block-edit-14661
branch
2 times, most recently
from
1e808a8 to
2794950
Compare
azure-sdk
force-pushed
the
sync-eng/common-djurek/codeowners-block-edit-14661
branch
from
2794950 to
958e9bd
Compare
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#14661 See eng/common workflow