-
Notifications
You must be signed in to change notification settings - Fork 356
Sync eng/common directory with azure-sdk-tools for PR 14661 #9766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
fe4b1ed
dfbd11a
9986cc1
ec8a0bb
958e9bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| parameters: | ||
| SourceRootPath: '$(System.DefaultWorkingDirectory)' | ||
| TempDirectory: '$(Agent.TempDirectory)/codeowners-check' | ||
| Sections: | ||
| - 'Client Libraries' | ||
| - 'Management Top Level Owners' | ||
| - 'Management Libraries' | ||
|
|
||
| steps: | ||
| - ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: | ||
| # 1. Check if CODEOWNERS changed; skip everything if it hasn't | ||
| - pwsh: | | ||
| $scriptPath = [System.IO.Path]::Combine("${{ parameters.SourceRootPath }}", "eng", "common", "scripts", "get-changedfiles.ps1") | ||
| try { | ||
| $changedFiles = & $scriptPath ` | ||
| -DiffPath ".github/CODEOWNERS" ` | ||
| -DiffFilterType '' | ||
|
|
||
| if ($LASTEXITCODE -gt 1) { | ||
| Write-Host "git diff returned exit code $LASTEXITCODE, which indicates an error. Output: $changedFiles" | ||
| exit 1 | ||
| } | ||
| } catch { | ||
| Write-Host "Error running get-changedfiles.ps1: $_" | ||
| exit 1 | ||
| } | ||
|
|
||
| if (!$changedFiles) { | ||
| Write-Host "CODEOWNERS file has not changed." | ||
| Write-Host "##vso[task.setvariable variable=CodeownersChanged]false" | ||
| } else { | ||
| Write-Host "CODEOWNERS file has changed." | ||
| Write-Host "##vso[task.setvariable variable=CodeownersChanged]true" | ||
| } | ||
| displayName: 'Check if CODEOWNERS changed' | ||
| workingDirectory: ${{ parameters.SourceRootPath }} | ||
| condition: and(succeeded(), ne(variables['Skip.VerifyCodeownersSections'], 'true')) | ||
|
|
||
| # 2. Install the azsdk CLI (only if CODEOWNERS changed) | ||
| - template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml | ||
| parameters: | ||
| SourceRootPath: ${{ parameters.SourceRootPath }} | ||
| Condition: and(succeeded(), eq(variables['CodeownersChanged'], 'true')) | ||
|
|
||
| # 3. Prepare before/after CODEOWNERS files | ||
| - pwsh: | | ||
| $tempDir = "${{ parameters.TempDirectory }}" | ||
| if (Test-Path $tempDir) { Remove-Item -Recurse -Force $tempDir } | ||
| New-Item -ItemType Directory -Path $tempDir -Force | Out-Null | ||
|
|
||
| $afterFile = Join-Path $tempDir "CODEOWNERS.after" | ||
| $beforeFile = Join-Path $tempDir "CODEOWNERS.before" | ||
|
|
||
| # Copy current (PR head) version | ||
| Copy-Item -Path ".github/CODEOWNERS" -Destination $afterFile -Force | ||
| Write-Host "Copied current CODEOWNERS to $afterFile" | ||
|
|
||
| # Extract parent commit version | ||
| git show "HEAD~1:.github/CODEOWNERS" > $beforeFile | ||
| Write-Host "Retrieved parent commit CODEOWNERS to $beforeFile" | ||
|
|
||
| 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 | ||
| } | ||
| Write-Host "Extracted parent CODEOWNERS to $beforeFile" | ||
| displayName: 'Prepare CODEOWNERS before/after files' | ||
| workingDirectory: ${{ parameters.SourceRootPath }} | ||
| condition: and(succeeded(), eq(variables['CodeownersChanged'], 'true')) | ||
|
|
||
| # 4. Run the section comparison script | ||
| - task: Powershell@2 | ||
| displayName: 'Test protected CODEOWNERS sections' | ||
| inputs: | ||
| targetType: 'filePath' | ||
| filePath: '${{ parameters.SourceRootPath }}/eng/common/scripts/Test-CodeownersSections.ps1' | ||
| arguments: >- | ||
| -AzsdkCliPath "$(AZSDK)" | ||
| -BeforeFile "${{ parameters.TempDirectory }}/CODEOWNERS.before" | ||
| -AfterFile "${{ parameters.TempDirectory }}/CODEOWNERS.after" | ||
| -Sections "${{ join('","', parameters.Sections) }}" | ||
| -TempDirectory "${{ parameters.TempDirectory }}" | ||
| pwsh: true | ||
| workingDirectory: ${{ parameters.SourceRootPath }} | ||
| condition: and(succeeded(), eq(variables['CodeownersChanged'], 'true')) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| # cSpell:ignore CODEOWNERS | ||
| <# | ||
| .SYNOPSIS | ||
| Tests that specified CODEOWNERS sections are identical between two file versions. | ||
|
|
||
| .DESCRIPTION | ||
| Uses the azsdk CLI to export named sections from a "before" and "after" copy of | ||
| the CODEOWNERS file. If any of the specified sections differ between the two | ||
| files the script exits with code 1. | ||
|
|
||
| All filesystem and git setup (creating the before/after files, installing the | ||
| CLI, etc.) is expected to be done by the calling pipeline step template. | ||
|
|
||
| .PARAMETER AzsdkCliPath | ||
| Path to the azsdk CLI executable. | ||
|
|
||
| .PARAMETER BeforeFile | ||
| Path to the CODEOWNERS file representing the base state (e.g. parent commit). | ||
|
|
||
| .PARAMETER AfterFile | ||
| Path to the CODEOWNERS file representing the current state (e.g. PR head). | ||
|
|
||
| .PARAMETER Sections | ||
| An array of section names to compare (e.g. "Client Libraries"). | ||
|
|
||
| .PARAMETER TempDirectory | ||
| Scratch directory for intermediate section export files. | ||
| #> | ||
| [CmdletBinding()] | ||
| param ( | ||
| [Parameter(Mandatory)] | ||
| [string] $AzsdkCliPath, | ||
|
|
||
| [Parameter(Mandatory)] | ||
| [string] $BeforeFile, | ||
|
|
||
| [Parameter(Mandatory)] | ||
| [string] $AfterFile, | ||
|
|
||
| [Parameter(Mandatory)] | ||
| [string[]] $Sections, | ||
|
|
||
| [string] $TempDirectory = (Join-Path ([System.IO.Path]::GetTempPath()) "codeowners-check") | ||
| ) | ||
|
|
||
| ."$PSScriptRoot\common.ps1" | ||
|
|
||
| Set-StrictMode -Version 3 | ||
| $ErrorActionPreference = "Stop" | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 1. Validate inputs | ||
| # --------------------------------------------------------------------------- | ||
| if (-not (Test-Path $BeforeFile)) { | ||
| Write-Error "BeforeFile not found: $BeforeFile" | ||
| exit 1 | ||
| } | ||
| if (-not (Test-Path $AfterFile)) { | ||
| Write-Error "AfterFile not found: $AfterFile" | ||
| exit 1 | ||
| } | ||
| if (-not (Test-Path $AzsdkCliPath)) { | ||
| Write-Error "azsdk CLI not found: $AzsdkCliPath" | ||
| exit 1 | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 2. Ensure temp directory exists | ||
| # --------------------------------------------------------------------------- | ||
| if (-not (Test-Path $TempDirectory)) { | ||
| New-Item -ItemType Directory -Path $TempDirectory -Force | Out-Null | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 3. Export and compare each section | ||
| # --------------------------------------------------------------------------- | ||
| $failed = $false | ||
|
|
||
| $beforePath = Resolve-Path $BeforeFile | ||
| Write-Host "Before file: $beforePath" | ||
| $afterPath = Resolve-Path $AfterFile | ||
| Write-Host "After file: $afterPath" | ||
|
|
||
| foreach ($section in $Sections) { | ||
| $safeName = $section -replace ' ', '_' | ||
| $beforeSection = Join-Path $TempDirectory "before.${safeName}.txt" | ||
| $afterSection = Join-Path $TempDirectory "after.${safeName}.txt" | ||
|
Comment on lines
+84
to
+87
|
||
|
|
||
| Write-Host "Exporting section '$section' from before file..." | ||
| & $AzsdkCliPath config codeowners export-section --codeowners-path $beforePath --section $section --output-file $beforeSection | ||
| if ($LASTEXITCODE) { | ||
| LogError "Failed to export section '$section' from before file (exit code $LASTEXITCODE)." | ||
| exit 1 | ||
| } | ||
|
|
||
| Write-Host "Exporting section '$section' from after file..." | ||
| & $AzsdkCliPath config codeowners export-section --codeowners-path $afterPath --section $section --output-file $afterSection | ||
| if ($LASTEXITCODE) { | ||
| LogError "Failed to export section '$section' from after file (exit code $LASTEXITCODE)." | ||
| exit 1 | ||
| } | ||
|
|
||
| $beforeContent = Get-Content -Path $beforeSection -Raw | ||
| $afterContent = Get-Content -Path $afterSection -Raw | ||
|
|
||
| if ($beforeContent -ne $afterContent) { | ||
| LogError "Protected CODEOWNERS section '$section' has been modified. Changes to this section are not allowed through normal PRs. To update CODEOWNERS, follow instructions at https://aka.ms/azsdk/codeowners" | ||
| Write-Host "--- Diff for section '$section' ---" | ||
| Write-Host "" | ||
| git diff --no-index -- $beforeSection $afterSection | ||
| $failed = $true | ||
| } else { | ||
| Write-Host "Section '$section' is unchanged." | ||
| } | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 4. Exit | ||
| # --------------------------------------------------------------------------- | ||
| if ($failed) { | ||
| $sectionList = ($Sections | ForEach-Object { "'$_'" }) -join ", " | ||
|
|
||
| Write-Host "" | ||
| LogError "##vso[task.LogIssue type=error;]One or more protected CODEOWNERS sections have been modified. Please revert changes to the $sectionList sections." | ||
| exit 1 | ||
| } | ||
|
|
||
| Write-Host "All protected CODEOWNERS sections are unchanged. Check passed." | ||
| exit 0 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "before" CODEOWNERS file is pulled from
HEAD~1and on failure the step skips the entire check by settingCodeownersChanged=false. This is unreliable for PR builds where the checked-out ref isn’t guaranteed to haveHEAD~1locally (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 asget-changedfiles.ps1) and avoid treating retrieval failures as a reason to skip enforcement.