Add pipeline to detect breaking change in swagger #1099
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||||
| trigger: none | ||||||
|
|
||||||
| resources: | ||||||
| repositories: | ||||||
| - repository: azure-rest-api-specs | ||||||
| type: github | ||||||
| name: Azure/azure-rest-api-specs | ||||||
| endpoint: azure | ||||||
| - repository: azure-sdk-tools | ||||||
| type: github | ||||||
| name: Azure/azure-sdk-tools | ||||||
| endpoint: azure | ||||||
|
|
||||||
| stages: | ||||||
| - stage: Build_and_Validate | ||||||
| jobs: | ||||||
| - job: Validate_generated_swagger | ||||||
| pool: | ||||||
| name: azsdk-pool-mms-ubuntu-2004-general | ||||||
| vmImage: ubuntu-20.04 | ||||||
| steps: | ||||||
| # Step 1: Build TypeSpec | ||||||
| - checkout: self | ||||||
| submodules: true | ||||||
| - template: /eng/pipelines/templates/install.yml | ||||||
| parameters: | ||||||
| nodeVersion: "18.x" | ||||||
|
Member
There was a problem hiding this comment.
Suggested change
don't respecify here, this is otherwise an extgra place we'll need to upgrade, use the default |
||||||
| workingDirectory: $(Build.SourcesDirectory)/typespec-azure | ||||||
| - script: pnpm run build | ||||||
| displayName: Build | ||||||
| workingDirectory: $(Build.SourcesDirectory)/typespec-azure | ||||||
|
|
||||||
| #Step 2: Update swagger by TypeSpec in Step 1 | ||||||
| - checkout: azure-rest-api-specs | ||||||
| fetchDepth: 1 | ||||||
| - pwsh: | | ||||||
|
Member
There was a problem hiding this comment. We already have this logic for the cadl-ranch e2e test, could we reuse that instead of having to maintain an extra place and deal with powershell |
||||||
| $packageJson = Get-Content -Raw "package.json" | ConvertFrom-Json -AsHashtable | ||||||
|
|
||||||
| $packageJson.devDependencies["@azure-tools/typespec-autorest"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-autorest" | ||||||
| $packageJson.devDependencies["@azure-tools/typespec-azure-core"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-azure-core" | ||||||
| $packageJson.devDependencies["@azure-tools/typespec-azure-portal-core"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-azure-portal-core" | ||||||
| $packageJson.devDependencies["@azure-tools/typespec-azure-resource-manager"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-azure-resource-manager" | ||||||
| $packageJson.devDependencies["@azure-tools/typespec-client-generator-core"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-client-generator-core" | ||||||
| $packageJson.devDependencies["@azure-tools/typespec-azure-rulesets"] = "file:$(Build.SourcesDirectory)/typespec-azure/packages/typespec-azure-rulesets" | ||||||
| $packageJson.devDependencies["@typespec/compiler"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/compiler" | ||||||
| $packageJson.devDependencies["@typespec/http"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/http" | ||||||
| $packageJson.devDependencies["@typespec/openapi"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/openapi" | ||||||
| $packageJson.devDependencies["@typespec/openapi3"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/openapi3" | ||||||
| $packageJson.devDependencies["@typespec/rest"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/rest" | ||||||
| $packageJson.devDependencies["@typespec/versioning"] = "file:$(Build.SourcesDirectory)/typespec-azure/core/packages/versioning" | ||||||
|
|
||||||
| $packageJson | ConvertTo-Json -Depth 100 | Out-File -Path "package.json" -Encoding utf8 -NoNewline -Force | ||||||
| displayName: "Update package.json in azure-rest-api-specs" | ||||||
| workingDirectory: $(Build.SourcesDirectory)/azure-rest-api-specs | ||||||
| - script: npm install | ||||||
|
Member
There was a problem hiding this comment.
Suggested change
|
||||||
| displayName: Install updated TypeSpec dependencies | ||||||
| workingDirectory: $(Build.SourcesDirectory)/azure-rest-api-specs | ||||||
| - pwsh: ./eng/scripts/TypeSpec-Generate-Swagger.ps1 -CheckAll | ||||||
|
Member
There was a problem hiding this comment. Where is this script comming from I don't see it in azure-rest-api-specs.
Member
Author
There was a problem hiding this comment. Yeah I put details in the description that it comes from this PR.
|
||||||
| displayName: "Generate all swagger files" | ||||||
| workingDirectory: $(Build.SourcesDirectory)/azure-rest-api-specs | ||||||
| - pwsh: | | ||||||
| git restore package.json | ||||||
| git restore package-lock.json | ||||||
| displayName: Discard versions change | ||||||
| workingDirectory: $(Build.SourcesDirectory)/azure-rest-api-specs | ||||||
|
|
||||||
| # Step 3: Create PR | ||||||
| - checkout: azure-sdk-tools | ||||||
| fetchDepth: 1 | ||||||
| - template: /eng/common/pipelines/templates/steps/git-push-changes.yml@azure-sdk-tools | ||||||
| parameters: | ||||||
| BaseRepoBranch: auto-update-swagger-$(Build.BuildNumber) | ||||||
| BaseRepoOwner: azure-sdk | ||||||
| CommitMsg: Update swagger with TypeSpec built from $(Build.BuildNumber) | ||||||
| TargetRepoOwner: Azure | ||||||
| TargetRepoName: azure-rest-api-specs | ||||||
| WorkingDirectory: $(Build.SourcesDirectory)/azure-rest-api-specs | ||||||
| ScriptDirectory: $(Build.SourcesDirectory)/azure-sdk-tools/eng/common/scripts | ||||||
| - task: PowerShell@2 | ||||||
|
Member
There was a problem hiding this comment. How is creating a PR going to help if everything was reverted here? What is the purpose?
Member
Author
There was a problem hiding this comment. What you mean by "everything was reverted here"? I only revert package.json and package-lock.json, which is not what we care about. We care about swagger change. An example is here.
Member
There was a problem hiding this comment. I see, so from experience with the spec repo most of the change we need to apply are typespec changes in the typespec-next branch so wondering if this just does a small portion of the solution
Member
There was a problem hiding this comment. also because of the typespec-next branch I feel like this is just going to fail anyway if you run on main
Member
Author
There was a problem hiding this comment. I didn't find the typespec-next branch you are saying. Could you point me to it? I want to clarify the user story here:
Therefore, our target is WIP change, instead of the code already merged into main.
Member
Author
There was a problem hiding this comment. I see. I understand it is very possible (even maybe 100%) we expect to change tsp before each release. But our intention is to uncover all the unexpected change in swagger, which might come from bug or lack of consideration. Every time you are not confident in your change, you could run the pipeline before merge. The granularity is for each RP. We might expect tsp change for each release, but we won't expect tsp change for each PR, right?
Member
There was a problem hiding this comment. Ok but if you test against main you will always have diff from previous PRs that might have had expected change as well
Member
There was a problem hiding this comment. I feel like we get this protection from the azure-rest-api-specs typespec-next ci, which automatically takes up the developer versions and validates them against typespec-next. It would be nice to have a more automated way to check a particular PR against typespec-next directly
Member
Author
There was a problem hiding this comment. Could you point me to typespec-next ci? Are you saying "running typespec-validation-all.yml against typespec-next branch"?
Member
There was a problem hiding this comment. It is just the |
||||||
| displayName: Create pull request | ||||||
| inputs: | ||||||
| pwsh: true | ||||||
| filePath: $(Build.SourcesDirectory)/azure-sdk-tools/eng/common/scripts/Submit-PullRequest.ps1 | ||||||
|
Member
There was a problem hiding this comment. Why are we creating these PR's against the spec repo? We don't wish to start creating a lot of PRs. Are there other ways we can get the validation we are after? |
||||||
| arguments: > | ||||||
| -RepoOwner "Azure" | ||||||
| -RepoName "azure-rest-api-specs" | ||||||
| -BaseBranch "main" | ||||||
| -PROwner "azure-sdk" | ||||||
| -PRBranch "auto-update-swagger-$(Build.BuildNumber)" | ||||||
|
Member
There was a problem hiding this comment. This naming pattern for a branch will create a branch/PR per build in the specs repo which is something we should definitely avoid, that will likely cause way too much noise.
Member
There was a problem hiding this comment. In the typespec preview pipelines, we set the branch name early in the build via inline powershell. We attempt to reuse the same PR branch name for the same source PR, scheduled runs use a single branch name and all other runs will use a unique dynamic name: azure-sdk-tools / archetype-autorest-preview.yml - pwsh: |
$sourceBranch = '$(Build.SourceBranch)'
$buildReason = '$(Build.Reason)'
$buildNumber = '$(Build.BuildNumber)'
if ($buildReason -eq 'Schedule') {
$branchName = 'auto-update-autorest-scheduled'
} elseif ($sourceBranch -match "^refs/pull/(\d+)/(head|merge)$") {
$branchName = "auto-update-autorest-pr-$($Matches[1])"
} else {
$branchName = "auto-update-autorest-$buildNumber"
}
Write-Host "Setting variable 'branchName' to '$branchName'"
Write-Host "##vso[task.setvariable variable=branchName;isOutput=true]$branchName"
displayName: Set branch name
name: set_branch_name
Member
There was a problem hiding this comment. That would definitely be better if we need a PR. However, I'd still like to understand if a PR into the specs repo is needed to accomplish the testing we are looking for. If it is needed who will be responsible for monitoring it and closing it.
Member
Author
There was a problem hiding this comment. We want to have a visualized way to see the diff. We could reduce the number by the way Patrick put above. Or maybe we could even put the PRs in azure-rest-api-specs-pr if your concern is you don't want to confuse customers.
@hallipr do you know how it is monitored in azure-sdk-for-net?
Member
There was a problem hiding this comment. You can have a visual change by pushing to a branch without needing to create a PR. Creating a PR causes a lot of other things to kick off that we should avoid if they aren't needed. Noise inside of the private repo is still a concern.
Member
Author
There was a problem hiding this comment. How about we just create a closed PR? It sounds really wired to me that pushing to a branch which is almost 100% going to create a PR.
Member
There was a problem hiding this comment. @pshao25 we can compose the URL to view the difference between two branches like this
Member
There was a problem hiding this comment. If the purpose of the branch (or pr) is to use github's UI to visualize a diff, I like Ray's suggestion to just use a ref comparison instead of a PR. We could even add that compare URL to the pipeline output so it's clickable from the ADO UI |
||||||
| -AuthToken "$(azuresdk-github-pat)" | ||||||
| -PRTitle "Swagger Regen Preview from TypeSpec built from $(Build.BuildNumber)" | ||||||
| -PRBody "Swagger Regen Preview from TypeSpec built from $(Build.BuildNumber)" | ||||||
| -OpenAsDraft $true | ||||||
| -PRLabels "Do Not Merge" | ||||||
Uh oh!
There was an error while loading. Please reload this page.