Azure Split - Step 4 - Cleanup

[paulmedynski]

Description

This PR includes any remaining cleanup:

• Addresses some suggestions/comments from the previous 3 PRs.
• Fixes any issues with the downstream pipelines (CI, MDS Main, Official, AKV).
• Anything else I've noticed along the way.

PR series:

• Azure Split - Step 1 - Prep Work #3902:
  • Merge changes that aren't directly related to the Azure split work, but that setup the stage for that work.
• Azure Split - Step 2 - New Files #3904:
  • Merge new files and related changes that do not affect the existing MDS projects or artifacts.
• Azure Split - Step 3 - Tie Everything Together #3908:
  • Merge the changes to MDS and pipelines that tie everything together.
• Azure Split - Step 4 - Cleanup #3917:
  • Any remaining tasks or cleanup.

Testing

• Normal PR/CI pipeline runs will validate most changes.
• Manual runs of the ADO.Net pipelines (MDS Main, Official, AKV).
• Manual inspection of pipeline logs and artifacts will confirm some of the fiddly parts.

[Copilot]

Pull request overview

Final cleanup pass for the Azure split work, focusing on small code tidy-ups and pipeline/template adjustments for Azure/AKV/signing/PR validation.

Changes:

• Minor test/source cleanup (namespace alignment, formatting, retry-after millisecond conversion).
• Pipeline/template refactors to consolidate .NET SDK installation and simplify signing/package validation steps.
• Updates to PR validation and CodeQL trigger configuration.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/SqlAuthenticationProviderManagerTests.cs	Aligns unit test namespace/formatting with the rest of the UnitTests tree.
src/Microsoft.Data.SqlClient.Extensions/Azure/test/StringExtensions.cs	Adds XML docs for Empty() (currently attached to the wrong symbol).
src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs	Fixes retry-after delta conversion to use total milliseconds; whitespace cleanup.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProviderException.cs	Formatting-only change for ternary expression.
eng/pipelines/steps/compound-build-akv-step.yml	Removes redundant SDK install step from the AKV compound step.
eng/pipelines/sqlclient-pr-project-ref-pipeline.yml	Adjusts PR branch include pattern for dev/*.
eng/pipelines/sqlclient-pr-package-ref-pipeline.yml	Adjusts PR branch include pattern for dev/*.
eng/pipelines/jobs/test-azure-package-ci-job.yml	YAML formatting cleanup (folded block style) for debug output step.
eng/pipelines/jobs/build-akv-official-job.yml	Adds explicit .NET SDK install before AKV build/analyzers.
eng/pipelines/dotnet-sqlclient-signing-pipeline.yml	Updates artifact folder naming and simplifies validation job invocation.
eng/pipelines/common/templates/steps/code-analyze-step.yml	Changes analyzer MSBuild invocation to explicitly target BuildAllConfigurations.
eng/pipelines/common/templates/steps/build-all-configurations-signed-dlls-step.yml	Removes redundant SDK install from this step template.
eng/pipelines/common/templates/jobs/validate-signed-package-job.yml	Simplifies parameters and inlines the artifact download step.
eng/pipelines/common/templates/jobs/build-signed-package-job.yml	Switches to pwsh, installs SDK, passes Abstractions version property, and sets build-type output variable (currently broken).
.github/workflows/codeql.yml	Updates CodeQL triggers/comments (push trigger now unscoped).

[codecov]

Codecov Report

❌ Patch coverage is 0% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.90%. Comparing base (3ffbba7) to head (203bef0).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
...Data/SqlClient/Connection/SqlConnectionInternal.cs	0.00%	21 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (3ffbba7) and HEAD (203bef0). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (3ffbba7)	HEAD (203bef0)
addons	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3917      +/-   ##
==========================================
- Coverage   74.53%   67.90%   -6.64%     
==========================================
  Files         266      260       -6     
  Lines       42915    65724   +22809     
==========================================
+ Hits        31987    44627   +12640     
- Misses      10928    21097   +10169     

Flag	Coverage Δ
addons	?
netcore	68.06% <0.00%> (-6.72%)	⬇️
netfx	66.55% <0.00%> (-7.16%)	⬇️

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.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[ErikEJ]

@paulmedynski any updates on the status of preview 4?

[paulmedynski]

Preview 4 will be pushed back, and likely combined with Preview 5. There is still significant infrastructure/engineering work to do for the new Abstractions and Azure packages. Meeting this week to decide on new dates/milestones/etc.

[ErikEJ]

Thanks for the update

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

eng/pipelines/common/templates/jobs/validate-signed-package-job.yml:84

• The packageType parameter was removed from the template, but PowerShell scripts at lines 76 and 80 still reference the undefined $packageType variable. Since the parameter's default was 'both', the scripts should either:

1. Remove the conditionals and always verify both dll and pdb (nuget verify for .nupkg and .snupkg), or
2. Define a hardcoded variable at the start of the script like $packageType = 'both'

Without this fix, the nuget verify commands will not execute because $packageType will be null/empty.

  - powershell: |
      Write-Host "--------------------------------------------------"
      Write-Host "This will verify the artifact signature" -ForegroundColor Green
      Write-Host "--------------------------------------------------"

      if ($packageType -eq 'dll' -or $packageType -eq 'both')
      {
        nuget verify -All $(pathToDownloadedNuget)\*.nupkg
      }
      if ($packageType -eq 'pdb' -or $packageType -eq 'both')
      {
        nuget verify -All $(pathToDownloadedNuget)\*.snupkg
      }
    displayName: 'Verify nuget signature'

[Copilot]

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

[PauloHMattos]

@paulmedynski Great work with the Azure split 💯.

I noticed that SqlClient still carries dependencies on Microsoft.IdentityModel.JsonWebTokens and Microsoft.IdentityModel.Protocols.OpenIdConnect due to the internal AzureAttestationEnclaveProvider class.

As part of the ongoing Azure Split initiative, is there a plan to move this provider to the Azure extensions package? This would significantly reduce the dependency footprint for users not utilizing Always Encrypted with Azure Attestation

[paulmedynski]

@PauloHMattos - Yes, that is a later phase of work that we will be exploring.

[ErikEJ]

@paulmedynski Congrats! Any chance to test this from a local pipeline package?

[paulmedynski]

@ErikEJ - Here's the CI run that produced the packages related to this PR:

https://sqlclientdrivers.visualstudio.com/public/_build/results?buildId=139595&view=artifacts&pathAsName=false&type=publishedArtifacts

You will need to download the following packages:

• Microsoft.Data.SqlClient.Extensions.Abstractions.1.0.0.4407-ci.nupkg
• Microsoft.Data.SqlClient.7.0.0.4407-ci.nupkg
• Microsoft.Data.SqlClient.Extensions.Azure.1.0.0.4407-ci.nupkg

Note that I haven't had a chance to vet any of these or try them out in a sample app myself yet. However, the package-reference-based tests for the Azure package do use the Abstractions and MDS NuGet packages - so there's hope!

For example, here is the sibling package-based CI run:

https://sqlclientdrivers.visualstudio.com/public/_build/results?buildId=139596&view=results

I can see the Azure tests pulling in the Abstractions and MDS packages locally:

    Installed Microsoft.Data.SqlClient.Extensions.Abstractions 1.0.0.4409-ci from /home/vsts/work/1/s/packages/ to /home/vsts/.nuget/packages/microsoft.data.sqlclient.extensions.abstractions/1.0.0.4409-ci with content hash Nuq0zAhaOw2SwUzXQ0K6XNVRyFk5Nw2vCS8SvBKiGgc2RlCDhrhRt4WCIZ6YCUaXxEsZgxroVeKhYL2EnwDrNQ==.

    Installed Microsoft.Data.SqlClient 7.0.0.4409-ci from /home/vsts/work/1/s/packages/ to /home/vsts/.nuget/packages/microsoft.data.sqlclient/7.0.0.4409-ci with content hash jS83vC3DaveLJ7Z8/iQQg1wlsLsbEXuAD+V4Fnxd00vRtbqzAxX+oq1Rlm6nng27qzBY+MBUdmHJQ0Dk2Yio6w==.

And then tests like this one are calling MDS APIs which use the Azure package implementation to acquire Entra ID tokens:

https://sqlclientdrivers.visualstudio.com/public/_build/results?buildId=139596&view=logs&j=a5d83443-ece9-5acd-e844-d86ac35620b4&t=ad30a4a0-3284-5f07-9583-6aff027a4f98

Please use the above artifacts and let me know how it goes! You can start a new Discussion or open an Issue if you run into any problems.

[ErikEJ]

@paulmedynski Thanks, I have created a couple of issues