-
Notifications
You must be signed in to change notification settings - Fork 321
Add Manual OneBranch Release Stage & Publish Support (Internal/Public) #3761
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?
Conversation
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.
Pull Request Overview
This PR adds a manual release stage to the OneBranch signing pipeline, enabling controlled NuGet package publishing with approval gates. The implementation supports both internal and public publishing destinations, includes dry-run capability for testing, and integrates symbol publishing for the MDS product.
Key Changes:
- Adds manual release parameters (destination, dry run, product) to the signing pipeline
- Implements approval workflow with human validation before package publication
- Creates templated release infrastructure supporting multiple products (MDS, MSS, AKV)
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/dotnet-sqlclient-signing-pipeline.yml | Adds release parameters and invokes new release-stage template |
| eng/pipelines/common/templates/stages/release-stage.yml | Defines manual release stage with approval and publish jobs |
| eng/pipelines/common/templates/jobs/approval-job.yml | Implements manual validation job with release checklist |
| eng/pipelines/common/templates/jobs/publish-packages-job.yml | Orchestrates package download and conditional publishing to internal/public feeds |
| eng/pipelines/common/templates/steps/publish-internal-feed-step.yml | Handles internal feed publishing with dry-run support |
| eng/pipelines/common/templates/steps/publish-public-nuget-step.yml | Handles NuGet.org publishing with dry-run support |
| eng/pipelines/common/templates/steps/list-packages-step.yml | Lists packages for verification before publishing |
| eng/pipelines/common/templates/steps/publish-symbols-step.yml | Updates symbol publishing to use boolean type and add AKV product support |
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-public-nuget-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-public-nuget-step.yml
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
eng/pipelines/common/templates/steps/publish-public-nuget-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-public-nuget-step.yml
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
| echo "3 Cancelled; The request was cancelled" | ||
| - powershell: 'Write-Host "##vso[task.setvariable variable=ArtifactServices.Symbol.AccountName;]${{parameters.SymAccount}}"' | ||
| displayName: "Update Symbol.AccountName with ${{parameters.SymAccount}}" | ||
| condition: and(succeeded(), eq(parameters.publishSymbols, true)) |
Copilot
AI
Jan 5, 2026
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 parameter type for publishSymbols has been changed from 'string' to 'boolean', which is the correct type for a boolean flag. However, the condition on line 79 should be updated to use the boolean comparison syntax. Change 'eq(parameters.publishSymbols, true)' instead of comparing to the string 'true'.
| parameters: | ||
| dryRun: ${{ parameters.dryRun }} | ||
| publicNuGetSource: ${{ parameters.publicNuGetSource }} | ||
| packagesGlob: ${{ variables.targetDownloadPath }}/*.nupkg |
Copilot
AI
Jan 5, 2026
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.
Same issue as line 83 - the variable targetDownloadPath should be referenced using runtime syntax
eng/pipelines/common/templates/steps/publish-internal-feed-step.yml
Outdated
Show resolved
Hide resolved
…p.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
| # Parse the glob pattern to extract directory and filename pattern | ||
| $glob = $packagesGlob | ||
| $lastSlashIndex = $glob.LastIndexOf('/') | ||
|
|
||
| if ($lastSlashIndex -ge 0) { | ||
| $dir = $glob.Substring(0, $lastSlashIndex) | ||
| $namePattern = $glob.Substring($lastSlashIndex + 1) | ||
| } else { | ||
| $dir = "." | ||
| $namePattern = $glob | ||
| } | ||
|
|
||
| # Handle ** wildcard for recursive search | ||
| $recurse = $dir -like '*/**' | ||
| if ($recurse) { | ||
| $dir = $dir -replace '/?\*\*/?', '' | ||
| } | ||
|
|
||
| Write-Host "Resolved directory: $dir" -ForegroundColor Yellow | ||
| Write-Host "Filename pattern: $namePattern" -ForegroundColor Yellow | ||
|
|
||
| if (Test-Path $dir -PathType Container) { | ||
| Write-Host "Matched files:" -ForegroundColor Green | ||
|
|
||
| # Find matching .nupkg files | ||
| $packages = Get-ChildItem -Path $dir -Filter "*.nupkg" -Recurse:$recurse -File -ErrorAction SilentlyContinue | ||
|
|
Copilot
AI
Feb 11, 2026
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.
$namePattern is derived from packagesGlob, but the actual file enumeration always uses -Filter "*.nupkg", so the provided glob/pattern is effectively ignored. Either use $namePattern (and possibly handle .snupkg if intended) or remove the unused parsing to avoid misleading configuration.
| variables: | ||
| - name: targetDownloadPath | ||
| value: "$(Pipeline.Workspace)/release/packages" | ||
| dependsOn: AwaitApproval | ||
| condition: succeeded() | ||
| pool: | ||
| vmImage: "ubuntu-latest" | ||
| steps: | ||
| - task: DownloadPipelineArtifact@2 | ||
| displayName: "Download Signed Packages" | ||
| inputs: | ||
| buildType: current | ||
| artifactName: ${{ parameters.packageFolderName }} | ||
| targetPath: ${{ variables.targetDownloadPath }} | ||
| - script: | | ||
| echo "NuGet Package Version: ${{ parameters.nugetPackageVersion }}" | ||
| echo "Downloaded signed packages to: ${{ variables.targetDownloadPath }}" | ||
| displayName: "Echo NuGet Package Version" |
Copilot
AI
Feb 11, 2026
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.
targetDownloadPath is defined as a job variable, but it’s referenced via template expression ${{ variables.targetDownloadPath }}. Template expressions are evaluated before runtime variables exist, so these paths will be empty/incorrect. Use runtime variable syntax ($(targetDownloadPath)) consistently for targetPath, echo output, and packagesGlob.
| - powershell: 'Write-Host "##vso[task.setvariable variable=ArtifactServices.Symbol.AccountName;]${{parameters.SymAccount}}"' | ||
| displayName: "Update Symbol.AccountName with ${{parameters.SymAccount}}" | ||
| condition: and(succeeded(), eq(parameters.publishSymbols, true)) | ||
|
|
Copilot
AI
Feb 11, 2026
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 step conditions use eq(parameters.publishSymbols, true) without ${{ }} expansion. parameters.* isn’t available in runtime conditions, so these steps will be skipped even when the template parameter is true. Either remove the runtime condition (since the surrounding ${{ if ... }} already gates inclusion) or expand the boolean at compile time inside the condition.
| if (-not $dryRun) { | ||
| Write-Host "`nPushing packages to feed..." -ForegroundColor Cyan | ||
| foreach ($package in $packages) { | ||
| Write-Host "`nPushing packages to feed..." -ForegroundColor Cyan | ||
| $anyPushFailed = $false | ||
| foreach ($package in $packages) { | ||
| Write-Host "Pushing package: $($package.FullName)" -ForegroundColor Yellow | ||
| dotnet nuget push $package.FullName --source $SRC --api-key az | ||
|
|
||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Host "Failed to push package: $($package.FullName)" -ForegroundColor Red | ||
| $anyPushFailed = $true | ||
| } else { | ||
| Write-Host "Successfully pushed: $($package.Name)" -ForegroundColor Green | ||
| } | ||
| } |
Copilot
AI
Feb 11, 2026
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 push block has duplicated/nested foreach ($package in $packages) loops and $anyPushFailed is declared inside the wrong scope; as written it looks like braces are unbalanced and the script will either fail to parse or never execute the intended push loop. Refactor to a single loop, initialize $anyPushFailed once before iterating, and remove the redundant inner loop/log line.
| # TODO: Build other products as well based on parameters['product'] | ||
| # This pipeline currently only builds MDS product. | ||
| - ${{ if eq(parameters['product'], 'MDS') }}: | ||
| - stage: buildMDS | ||
| displayName: "Build MDS" | ||
| jobs: | ||
| - template: eng/pipelines/common/templates/jobs/build-signed-package-job.yml@self | ||
| parameters: | ||
| symbolsFolder: $(symbolsFolder) | ||
| softwareFolder: $(softwareFolder) | ||
| publishSymbols: ${{ parameters['publishSymbols'] }} | ||
| isPreview: ${{ parameters['isPreview'] }} | ||
| - stage: mds_package_validation | ||
| displayName: "MDS Package Validation" | ||
| dependsOn: buildMDS |
Copilot
AI
Feb 11, 2026
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.
This pipeline exposes product values MSS/AKV, but the build/validation stages are only instantiated when product == 'MDS' while the Release stage template is always included. If a user queues a manual run with product set to MSS/AKV and runRelease=true, the release job will attempt to download a non-existent artifact and fail. Consider restricting product values until builds exist, or conditionally include the release stage per product as well.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3761 +/- ##
==========================================
- Coverage 74.53% 0 -74.54%
==========================================
Files 266 0 -266
Lines 42915 0 -42915
==========================================
- Hits 31987 0 -31987
+ Misses 10928 0 -10928
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Adds a manual, parameter‑gated release stage to the signing pipeline enabling:
Next Steps:
Investigate: