Skip to content

Conversation

@cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Nov 13, 2025

Description

Adds a manual, parameter‑gated release stage to the signing pipeline enabling:

  • Internal or public NuGet publish (publishDestination)
  • Dry run preview (dryRun)
  • Optional symbol publishing (MDS only)
  • Human approval checklist (destination, preview, dry run, symbols, version, product)

Next Steps:

  • Run Dryrun and Approval workflow to validate changes ensuring configuration is up-to-date.
  • Make an attempt to publish MDS packages to internal feed.
  • Validate workflows for all 3 packages: MDS, MSS, AKV

Investigate:

  • Can symbols be published using the same "compound-publish-symbols-step" for AKV and MSS packages?

@cheenamalhotra cheenamalhotra requested a review from a team as a code owner November 13, 2025 00:29
Copilot AI review requested due to automatic review settings November 13, 2025 00:29
Copy link
Contributor

Copilot AI left a 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

Copilot AI review requested due to automatic review settings November 13, 2025 05:01
Copy link
Contributor

Copilot AI left a 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.

@paulmedynski paulmedynski self-assigned this Nov 13, 2025
Copilot AI review requested due to automatic review settings December 11, 2025 07:24
Copy link
Contributor

Copilot AI left a 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.

Copilot AI review requested due to automatic review settings December 11, 2025 07:39
Copy link
Contributor

Copilot AI left a 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.

Copilot AI review requested due to automatic review settings December 11, 2025 08:00
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings December 11, 2025 08:34
Copy link
Contributor

Copilot AI left a 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.

cheenamalhotra and others added 2 commits December 11, 2025 09:01
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a 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.

paulmedynski
paulmedynski previously approved these changes Dec 23, 2025
Copy link
Contributor

Copilot AI left a 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))
Copy link

Copilot AI Jan 5, 2026

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'.

Copilot uses AI. Check for mistakes.
parameters:
dryRun: ${{ parameters.dryRun }}
publicNuGetSource: ${{ parameters.publicNuGetSource }}
packagesGlob: ${{ variables.targetDownloadPath }}/*.nupkg
Copy link

Copilot AI Jan 5, 2026

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 $(targetDownloadPath) instead of template syntax ${{ variables.targetDownloadPath }}.

Copilot uses AI. Check for mistakes.
@cheenamalhotra cheenamalhotra added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Jan 13, 2026
@cheenamalhotra cheenamalhotra requested a review from a team February 11, 2026 18:30
…p.yml

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 11, 2026 18:32
Copy link
Contributor

Copilot AI left a 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.

Comment on lines +30 to +56
# 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

Copy link

Copilot AI Feb 11, 2026

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.

Copilot uses AI. Check for mistakes.
Comment on lines +70 to +87
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"
Copy link

Copilot AI Feb 11, 2026

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.

Copilot uses AI. Check for mistakes.
Comment on lines +77 to +80
- 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))

Copy link

Copilot AI Feb 11, 2026

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.

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +77
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
}
}
Copy link

Copilot AI Feb 11, 2026

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.

Copilot uses AI. Check for mistakes.
Comment on lines +192 to +206
# 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
Copy link

Copilot AI Feb 11, 2026

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.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (3ffbba7) to head (d337d8f).
⚠️ Report is 4 commits behind head on main.

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     
Flag Coverage Δ
addons ?
netcore ?
netfx ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants