Skip to content

Commit 2ef9a72

Browse files
azure-sdkdanieljurek
azure-sdk
and
danieljurek
authored
Sync eng/common directory with azure-sdk-tools for PR 15525 (#46768)
* Add opt-in CODEOWNERS validation to PRs * Fix compat --------- Co-authored-by: Daniel Jurek <djurek@microsoft.com>
1 parent fb4ed62 commit 2ef9a72

2 files changed

Lines changed: 204 additions & 28 deletions

File tree

‎eng/common/pipelines/templates/steps/verify-codeowners.yml‎

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,17 @@ parameters:
33
type: string
44
default: $(Build.ArtifactStagingDirectory)/PackageInfo
55
- name: Artifacts
6+
# Compatibility: Removing the Artifacts param breaks eng/common. Keeping it
7+
# here for now to avoid breaking changes. This can be removed once the .NET
8+
# invocation of this template is updated to omit Artifacts
69
type: object
710
default: []
11+
- name: EnablePrValidation
12+
type: boolean
13+
default: false
14+
- name: DiffDirectory
15+
type: string
16+
default: $(Build.ArtifactStagingDirectory)/CodeownersPrDiff
817
- name: Repo
918
type: string
1019
default: $(Build.Repository.Name)
@@ -18,19 +27,48 @@ parameters:
1827
- datamovement
1928

2029
steps:
21-
- template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml
22-
parameters:
23-
Condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
30+
- ${{ if and(eq(variables['Build.Reason'], 'PullRequest'), eq(parameters.EnablePrValidation, true)) }}:
31+
- template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml
32+
33+
- task: PowerShell@2
34+
displayName: Generate PR Diff for Codeowners
35+
condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
36+
inputs:
37+
pwsh: true
38+
filePath: $(Build.SourcesDirectory)/eng/common/scripts/Generate-PR-Diff.ps1
39+
arguments: >-
40+
-ArtifactPath '${{ parameters.DiffDirectory }}'
41+
-TargetPath '$(Build.SourcesDirectory)'
42+
workingDirectory: $(Build.SourcesDirectory)
43+
44+
- task: PowerShell@2
45+
displayName: Verify Codeowners
46+
condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
47+
inputs:
48+
pwsh: true
49+
filePath: $(Build.SourcesDirectory)/eng/common/scripts/Test-CodeownersForArtifacts.ps1
50+
arguments: >-
51+
-AzsdkPath '$(AZSDK)'
52+
-PackageInfoDirectory '${{ parameters.ArtifactPath }}'
53+
-SdkTypes ${{ join(',', parameters.SdkTypes) }}
54+
-Repo '${{ parameters.Repo }}'
55+
-PrDiffFile '${{ parameters.DiffDirectory }}/diff.json'
56+
workingDirectory: $(Build.SourcesDirectory)
57+
58+
- ${{ elseif eq(variables['Build.Reason'], 'Manual') }}:
59+
- template: /eng/common/pipelines/templates/steps/install-azsdk-cli.yml
60+
parameters:
61+
Condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
2462

25-
- task: PowerShell@2
26-
displayName: Verify Codeowners
27-
condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
28-
inputs:
29-
pwsh: true
30-
filePath: $(Build.SourcesDirectory)/eng/common/scripts/Test-CodeownersForArtifacts.ps1
31-
arguments: >-
32-
-AzsdkPath '$(AZSDK)'
33-
-PackageInfoDirectory '${{ parameters.ArtifactPath }}'
34-
-SdkTypes ${{ join(',', parameters.SdkTypes) }}
35-
-ArtifactsJson '${{ convertToJson(parameters.Artifacts) }}'
36-
-Repo '${{ parameters.Repo }}'
63+
- task: PowerShell@2
64+
displayName: Verify Codeowners
65+
condition: and(succeeded(), ne(variables['Skip.VerifyCodeowners'], 'true'))
66+
inputs:
67+
pwsh: true
68+
filePath: $(Build.SourcesDirectory)/eng/common/scripts/Test-CodeownersForArtifacts.ps1
69+
arguments: >-
70+
-AzsdkPath '$(AZSDK)'
71+
-PackageInfoDirectory '${{ parameters.ArtifactPath }}'
72+
-SdkTypes ${{ join(',', parameters.SdkTypes) }}
73+
-Repo '${{ parameters.Repo }}'
74+
workingDirectory: $(Build.SourcesDirectory)

‎eng/common/scripts/Test-CodeownersForArtifacts.ps1‎

Lines changed: 151 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,169 @@
1+
<#
2+
.SYNOPSIS
3+
Validates CODEOWNERS coverage for package directories described by package-info JSON files.
4+
5+
.DESCRIPTION
6+
Reads package-info JSON files from a directory and runs `azsdk config codeowners check-package`
7+
for each eligible package directory. Packages can be excluded by package-info artifact details or SDK type.
8+
9+
When a PR diff file is supplied, the script applies PR-aware behavior:
10+
- packages with no direct file changes under their package directory are skipped,
11+
- packages whose directory is brand new on the target branch are skipped,
12+
- packages with direct changes under an existing directory are validated regardless of release status.
13+
14+
Without a PR diff file, the script validates packages that are intended to release.
15+
16+
.PARAMETER AzsdkPath
17+
Path to the `azsdk` CLI executable used to run CODEOWNERS validation.
18+
19+
.PARAMETER PackageInfoDirectory
20+
Directory containing package-info JSON files, typically produced by `Save-Package-Properties.ps1`
21+
or downloaded from a pipeline artifact such as `PackageInfo`.
22+
23+
.PARAMETER SdkTypes
24+
Array of SDK types that should be validated. Package-info entries whose `SdkType` is not in this
25+
list are skipped.
26+
27+
.PARAMETER Repo
28+
Repository name passed through to the AZSDK CLI for CODEOWNERS cache lookup.
29+
30+
.PARAMETER PrDiffFile
31+
Optional path to a `diff.json` file produced by `Generate-PR-Diff.ps1`. When supplied, the script
32+
switches to PR-aware validation behavior and only blocks on direct changes to existing package
33+
directories. If omitted, PR-aware validation is disabled.
34+
35+
.PARAMETER TargetCommittish
36+
Git committish used to inspect existing files for a package directory when `PrDiffFile` is
37+
supplied. Default: the PR target branch from Azure Pipelines, normalized to `origin/<branch>`.
38+
39+
.EXAMPLE
40+
pwsh -File eng/common/scripts/Test-CodeownersForArtifacts.ps1 `
41+
-AzsdkPath "$(AZSDK)" `
42+
-PackageInfoDirectory "$(Build.ArtifactStagingDirectory)/PackageInfo" `
43+
-SdkTypes @('client', 'compat', 'data', 'functions', 'datamovement') `
44+
-Repo 'Azure/azure-sdk-for-net'
45+
46+
.EXAMPLE
47+
pwsh -File eng/common/scripts/Test-CodeownersForArtifacts.ps1 `
48+
-AzsdkPath "$(AZSDK)" `
49+
-PackageInfoDirectory "$(Build.ArtifactStagingDirectory)/PackageInfo" `
50+
-SdkTypes @('client', 'compat', 'data', 'functions', 'datamovement') `
51+
-Repo 'Azure/azure-sdk-for-net' `
52+
-PrDiffFile "$(Build.ArtifactStagingDirectory)/CodeownersPrDiff/diff.json" `
53+
-TargetCommittish 'origin/main'
54+
#>
155
[CmdletBinding()]
256
param(
357
[string] $AzsdkPath,
458
[string] $PackageInfoDirectory,
559
[array] $SdkTypes,
6-
[string] $ArtifactsJson,
7-
[string] $Repo
60+
[string] $Repo,
61+
[string] $PrDiffFile,
62+
[string] $TargetCommittish = ("origin/${env:SYSTEM_PULLREQUEST_TARGETBRANCH}" -replace "refs/heads/")
863
)
964

1065
. "$PSScriptRoot/common.ps1"
1166

1267
Set-StrictMode -Version 3
1368
$ErrorActionPreference = 'Stop'
1469

70+
function getNormalizedRelativePath([string] $Path) {
71+
if (!$Path) {
72+
return ""
73+
}
74+
75+
$normalized = $Path.Replace("\", "/")
76+
while ($normalized.StartsWith("./")) {
77+
$normalized = $normalized.Substring(2)
78+
}
79+
80+
return $normalized.TrimStart('/').TrimEnd('/')
81+
}
82+
83+
function getChangedFilesForDirectory([PSCustomObject] $PrDiff, [string] $DirectoryPath) {
84+
$normalizedDirectoryPath = getNormalizedRelativePath $DirectoryPath
85+
$changedPaths = @($PrDiff.ChangedFiles) + @($PrDiff.DeletedFiles)
86+
$matchingFiles = @()
87+
88+
foreach ($changedPath in $changedPaths) {
89+
if (!$changedPath) {
90+
continue
91+
}
92+
93+
$normalizedChangedPath = getNormalizedRelativePath $changedPath
94+
if ($normalizedChangedPath -eq $normalizedDirectoryPath -or $normalizedChangedPath.StartsWith("$normalizedDirectoryPath/")) {
95+
$matchingFiles += $normalizedChangedPath
96+
}
97+
}
98+
99+
return ,@($matchingFiles)
100+
}
101+
102+
function getExistingFiles([string] $DirectoryPath, [string] $TargetCommittish) {
103+
if (!$targetCommittish -or $targetCommittish -eq "origin/") {
104+
LogError "TargetCommittish must be set for PR-aware CODEOWNERS verification."
105+
exit 1
106+
}
107+
108+
$normalizedDirectoryPath = getNormalizedRelativePath $DirectoryPath
109+
$targetFiles = Invoke-LoggedCommand "git ls-tree -r --name-only `"$targetCommittish`" -- `"$normalizedDirectoryPath`"" -DoNotExitOnFailedExitCode
110+
if ($LASTEXITCODE) {
111+
LogError "Failed to inspect target branch contents for directory '$normalizedDirectoryPath' at '$targetCommittish'."
112+
exit 1
113+
}
114+
115+
return ,@($targetFiles | Where-Object { $_ })
116+
}
117+
118+
function shouldSkipCodeownersInPrContext([PSCustomObject] $PackageProperties, [PSCustomObject] $PrDiff, [string] $TargetCommittish) {
119+
$directoryPath = getNormalizedRelativePath $PackageProperties.DirectoryPath
120+
if (!$directoryPath) {
121+
LogError "Package '$($PackageProperties.Name)' is missing a DirectoryPath property."
122+
exit 1
123+
}
124+
125+
$changedFiles = getChangedFilesForDirectory -PrDiff $PrDiff -DirectoryPath $directoryPath
126+
if (@($changedFiles).Count -eq 0) {
127+
Write-Host " PR context: skipping CODEOWNERS for '$directoryPath' because the PR does not directly change files under that package directory."
128+
return $true
129+
}
130+
131+
$targetFiles = getExistingFiles -DirectoryPath $directoryPath -TargetCommittish $TargetCommittish
132+
if (@($targetFiles).Count -gt 0) {
133+
Write-Host " PR context: not skipping CODEOWNERS for '$directoryPath' because the target branch already contains files under that directory."
134+
return $false
135+
}
136+
137+
Write-Host " PR context: skipping CODEOWNERS for '$directoryPath' because the PR only introduces files in a brand-new directory."
138+
foreach ($changedFile in $changedFiles) {
139+
Write-Host " $changedFile"
140+
}
141+
return $true
142+
}
143+
15144
$failedPackages = @()
16-
$excludedArtifacts = @()
17-
$artifacts = $ArtifactsJson | ConvertFrom-Json
145+
$prDiff = $null
146+
$isPrCheck = $false
18147

19-
foreach ($artifact in $artifacts) {
20-
if ($artifact.PSObject.Properties.Name -contains "skipCodeownersVerification" -and $artifact.skipCodeownersVerification) {
21-
$excludedArtifacts += $artifact.Name
148+
if ($PrDiffFile) {
149+
if (!(Test-Path $PrDiffFile)) {
150+
LogError "PR diff file '$PrDiffFile' does not exist."
151+
exit 1
22152
}
153+
154+
Write-Host "Loading PR diff from '$PrDiffFile'"
155+
$prDiff = Get-Content -Raw -Path $PrDiffFile | ConvertFrom-Json
156+
$isPrCheck = $true
23157
}
24158

25-
Write-Host "Excluded artifacts: $($excludedArtifacts -join ', ')"
159+
Write-Host "SDK types to validate: $($SdkTypes -join ', ')"
26160

27161
foreach ($pkgPropertiesFile in Get-ChildItem -Path $PackageInfoDirectory -Filter '*.json' -File) {
28162
$pkgProperties = Get-Content -Raw -Path $pkgPropertiesFile | ConvertFrom-Json
163+
$artifactDetails = $pkgProperties.ArtifactDetails
29164

30-
if ($excludedArtifacts -contains $pkgProperties.Name) {
31-
Write-Host "Skipping package: $($pkgProperties.Name) $($pkgProperties.DirectoryPath) because it is in the list of artifacts to exclude from validation."
165+
if ($artifactDetails -and $artifactDetails.PSObject.Properties['skipCodeownersVerification'] -and $artifactDetails.skipCodeownersVerification) {
166+
Write-Host "Skipping package: $($pkgProperties.Name) $($pkgProperties.DirectoryPath) because package info marks it to skip CODEOWNERS verification."
32167
continue
33168
}
34169
if ($SdkTypes -notcontains $pkgProperties.SdkType) {
@@ -38,14 +173,17 @@ foreach ($pkgPropertiesFile in Get-ChildItem -Path $PackageInfoDirectory -Filter
38173

39174
Write-Host "Validating codeowners for package: $($pkgProperties.Name) $($pkgProperties.DirectoryPath)"
40175

41-
if (!$pkgProperties.ReleaseStatus) {
176+
if (!$isPrCheck -and !$pkgProperties.ReleaseStatus) {
42177
LogError "Package $($pkgProperties.Name) at $($pkgProperties.DirectoryPath) is missing a ReleaseStatus property."
43178
$failedPackages += $pkgProperties.DirectoryPath
44179
continue
45180
}
46181

47-
# Validate packages with a release date (intended to release)
48-
if ($pkgProperties.ReleaseStatus -ne "Unreleased") {
182+
if ($prDiff -and (shouldSkipCodeownersInPrContext -PackageProperties $pkgProperties -PrDiff $prDiff -TargetCommittish $TargetCommittish)) {
183+
continue
184+
}
185+
186+
if ($isPrCheck -or $pkgProperties.ReleaseStatus -ne "Unreleased") {
49187
$output = & $AzsdkPath config codeowners check-package `
50188
--directory-path $pkgProperties.DirectoryPath `
51189
--repo $Repo `

0 commit comments

Comments
 (0)