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.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (3)

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

• The template interface was changed (e.g., removal of downloadPackageStep / packageType parameters), but there are still callers passing downloadPackageStep (e.g., eng/pipelines/dotnet-sqlclient-signing-pipeline.yml). This will fail template expansion. Either restore the parameter(s) for backward compatibility or update all callers in this PR to match the new parameter set.

parameters:

  - name: packageFolderName
    type: string

  - name: isPreview
    type: boolean

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

• $packageType is referenced in the signature verification script, but the variable is no longer set (the previous assignment from a packageType parameter was removed). This will cause the if ($packageType -eq ...) checks to behave incorrectly or throw. Reintroduce the parameter/variable or remove the branching and always verify the expected artifacts.

  - 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
      }

eng/pipelines/common/templates/steps/code-analyze-step.yml:39

• Removing -p:SigningKeyPath=... from the Roslyn analyzers MSBuild command can break builds when CDP_BUILD_TYPE=Official (projects enable strong-name signing and require the key file). Either ensure the key is downloaded and SigningKeyPath is passed for this step as well, or explicitly disable signing for the analyzer build to keep it independent of secure files.

      msBuildCommandline: >-
        msbuild
        ${{parameters.sourceRoot}}\build.proj
        -t:BuildAllConfigurations
        -p:configuration=Release
        -p:GenerateNuget=false
        -p:BuildTools=false

[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