Skip to content
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

Integrate Armstrong Validation into the spec PR check. #28829

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
91781e3
add code
ms-zhenhua Apr 23, 2024
da9dda0
fix yml
ms-zhenhua Apr 26, 2024
0755ad3
fix yml
ms-zhenhua Apr 26, 2024
62347fa
update
ms-zhenhua Apr 28, 2024
bae3d8d
update
ms-zhenhua Apr 28, 2024
440c631
debug
ms-zhenhua Apr 28, 2024
7ae49e2
update
ms-zhenhua Apr 28, 2024
ca8ee98
update
ms-zhenhua Apr 29, 2024
63a6f5e
update
ms-zhenhua Apr 29, 2024
0a5623c
Merge branch 'main' into ms-zhenhua/armstrong-validation
mikeharder May 7, 2024
dd48b66
Use shared component get-suppressions
mikeharder May 7, 2024
0e38c65
Merge branch 'main' into ms-zhenhua/armstrong-validation
mikeharder May 7, 2024
66fed05
Merge branch 'main' into ms-zhenhua/armstrong-validation
mikeharder May 7, 2024
722f3f4
Install Node and deps before calling get-suppressions
mikeharder May 7, 2024
b7baad1
Check for errors after "npx get-suppressions"
mikeharder May 7, 2024
babe855
Merge branch 'main' of https://github.com/Azure/azure-rest-api-specs …
ms-zhenhua Nov 7, 2024
c270305
Convert "Armstrong Validation" to GitHub Action
mikeharder Nov 7, 2024
c1a948c
Update GITHUB_PATH
mikeharder Nov 7, 2024
68326bf
Update path to logging helpers
mikeharder Nov 8, 2024
dbf48ca
Merge branch 'main' into ms-zhenhua/armstrong-validation
ms-zhenhua Nov 8, 2024
eaa05ea
Merge branch 'main' into ms-zhenhua/armstrong-validation
ms-zhenhua Nov 11, 2024
3acdffe
remove pipeline task
ms-zhenhua Nov 11, 2024
5ffc3e4
fix armstrong cred scan error
ms-zhenhua Nov 12, 2024
f026987
update armstrong pipeline task
ms-zhenhua Nov 16, 2024
2ae09ae
update Armstrong-Validation.ps1
ms-zhenhua Nov 17, 2024
3775e7e
Merge branch 'main' into ms-zhenhua/armstrong-validation
ms-zhenhua Nov 29, 2024
26726e3
bug fix
ms-zhenhua Nov 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions eng/pipelines/armstrong-validation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
trigger: none

jobs:
- job:
pool:
name: azsdk-pool-mms-ubuntu-2204-general
vmImage: ubuntu-22.04

steps:
- task: GoTool@0
inputs:
version: '1.22.2'

- script: |
go version
go install github.com/azure/armstrong@67fe406e78e3b94075932add869f8b11fb4dd0a6
echo '##vso[task.prependpath]$(HOME)/go/bin'
displayName: 'Setup dependencies'

- pwsh: |
$(Build.SourcesDirectory)/eng/scripts/Armstrong-Validation.ps1 -Verbose
displayName: Armstrong Validation
ignoreLASTEXITCODE: true
151 changes: 151 additions & 0 deletions eng/scripts/Armstrong-Validation.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
[CmdletBinding()]
param (
[Parameter(Position = 0)]
[string] $BaseCommitish = "HEAD^",
[Parameter(Position = 1)]
[string] $TargetCommitish = "HEAD"
)
Set-StrictMode -Version 3

. $PSScriptRoot/ChangedFiles-Functions.ps1
. $PSScriptRoot/Logging-Functions.ps1

function Get-Suppression {
param (
[string]$fileInSpecFolder
)

# -NoEnumerate to prevent single-element arrays from being collapsed to a single object
# -AsHashtable is closer to raw JSON than PSCustomObject
$suppressions = npx get-suppressions ArmstrongValidation $fileInSpecFolder | ConvertFrom-Json -NoEnumerate -AsHashtable
Copy link
Member

@mikeharder mikeharder May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ms-zhenhua: I updated your PR to use the new get-suppressions shared code added in #28927, #29001, and #29008.

You may also want to add parameter $CheckAllUnder to your script for easier testing, like this:

https://github.com/Azure/azure-rest-api-specs/pull/28927/files#diff-3c37fefe028ad9bd154b76ea3371f8db2d7a7609dafa323e92d8461880e808b2R9


return $suppressions ? $suppressions[0] : $null
}

function Get-ChangedTerraformFiles($changedFiles = (Get-ChangedFiles)) {
$changedFiles = Get-ChangedFilesUnderSpecification $changedFiles

$changedSwaggerFiles = $changedFiles.Where({
# since `git diff` returns paths with `/`, use the following code to match the `main.tf`
$_.EndsWith("/main.tf")
})

return $changedSwaggerFiles
}

$script:armstrongInstalled = $false
function Ensure-Armstrong-Installed {
if ($script:armstrongInstalled) {
# If already checked once in this script, don't log anything further
return;
}

$script:armstrongInstalled = $true

# install golang
if (!(Get-Command "go" -ErrorAction SilentlyContinue)) {
LogError "Golang is not installed"
exit 1
}

# install armstrong
if (!(Get-Command "armstrong" -ErrorAction SilentlyContinue)) {
LogError "Armstrong is not installed"
exit 1
}
}

function Validate-Terraform-Error($repoPath, $filePath) {
$fileDirectory = (Split-Path -Parent $filePath)
$outputDirectory = [System.IO.Path]::Combine([System.IO.Path]::GetTempPath(), [System.IO.Path]::GetRandomFileName())
$result = @()

try {
if (!(Test-Path -Path $outputDirectory)) {
New-Item -Path $outputDirectory -ItemType Directory *> $null
Comment on lines +51 to +56
Copy link
Member

@mikeharder mikeharder May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$outputDirectory = [System.IO.Path]::Combine([System.IO.Path]::GetTempPath(), [System.IO.Path]::GetRandomFileName())
$result = @()
try {
if (!(Test-Path -Path $outputDirectory)) {
New-Item -Path $outputDirectory -ItemType Directory *> $null
$outputDirectory = [System.IO.Directory]::CreateTempSubdirectory()
$result = @()
try {

.NET 7 added an easier way to do this.

# run armstrong credscan
$specPath = Join-Path -Path $repoPath -ChildPath "specification"
LogInfo "armstrong credscan -working-dir $fileDirectory -swagger-repo $specPath -output-dir $outputDirectory"
armstrong credscan -working-dir $fileDirectory -swagger-repo $specPath -output-dir $outputDirectory
}

# error reports are stored in a directory named armstrong_credscan_<timestamp>
Get-ChildItem -Path $outputDirectory -Directory -Filter "armstrong_credscan_*" | ForEach-Object {
$errorJsonPath = Join-Path -Path $_.FullName -ChildPath "errors.json"
if (Test-Path -Path $errorJsonPath) {
Get-Content -Path $errorJsonPath -Raw | ConvertFrom-Json | ForEach-Object {
$properties = $_.PSObject.Properties
$item = "Credential Error:"
foreach ($property in $properties) {
$item += "`n $($property.Name): $($property.Value)"
}

$result += $item
}
}
}
}
finally {
Remove-Item -Path $outputDirectory -Recurse -Force
}

return $result
}

# Check if the repository and target branch are the ones that need to do API Testing
$repositoryName = [Environment]::GetEnvironmentVariable("BUILD_REPOSITORY_NAME", [EnvironmentVariableTarget]::Process)
$targetBranchName = [Environment]::GetEnvironmentVariable("SYSTEM_PULLREQUEST_TARGETBRANCH", [EnvironmentVariableTarget]::Process)
LogInfo "Repository: $repositoryName"
LogInfo "Target branch: $targetBranchName"
if ($repositoryName -eq "Azure/azure-rest-api-specs" -and $targetBranchName -eq "ms-zhenhua/armstrong-validation") {
$apiTestingError = "API Testing Warning:"
$apiTestingError += "`n The Pull Request against main branch may need to provide API Testing results. Please follow https://github.com/Azure/armstrong/blob/main/docs/guidance-for-api-test.md to complete the API Testing"
# Though it is a warning, we still log it as error because warning log won't be shown in GitHub
LogError $apiTestingError
}

$repoPath = Resolve-Path "$PSScriptRoot/../.."

$terraformErrors = @()

$filesToCheck = (Get-ChangedTerraformFiles (Get-ChangedFiles $BaseCommitish $TargetCommitish))

if (!$filesToCheck) {
LogInfo "No Terraform files found to check"
}
else {
foreach ($file in $filesToCheck) {
LogInfo "Checking $file"

$fullPath = (Join-Path $repoPath $file)

$suppression = Get-Suppression $fullPath
if ($suppression) {
$reason = $suppression["reason"] ?? "<no reason specified>"

LogInfo " Suppressed: $reason"
# Skip further checks, to avoid potential errors on files already suppressed
continue
}

try {
Ensure-Armstrong-Installed
LogInfo " Validating errors from Terraform file: $fullPath"
$terraformErrors += (Validate-Terraform-Error $repoPath $fullPath)
}
catch {
$terraformErrors += " failed to validate errors from Terraform file: $file`n $_"
}
}
}

if ($terraformErrors.Count -gt 0) {
$errorString = "Armstrong Validation failed for some files. To fix, address the following errors. For false positive errors, please follow https://eng.ms/docs/products/azure-developer-experience/design/specs-pr-guides/pr-suppressions to suppress 'ArmstrongValidation'`n"
$errorString += $terraformErrors -join "`n"
LogError $errorString

LogJobFailure
exit 1
}

exit 0