Add assembly signing for Microsoft.Data.SqlClient.Extensions.Azure - #4570
Draft
paulmedynski wants to merge 2 commits into
Draft
Add assembly signing for Microsoft.Data.SqlClient.Extensions.Azure#4570paulmedynski wants to merge 2 commits into
paulmedynski wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Enables strong-name signing for the Microsoft.Data.SqlClient.Extensions.Azure assembly in internal CI package-mode builds, including signing the corresponding test assembly to keep InternalsVisibleTo (IVT) working when the product assembly is signed.
Changes:
- Adds conditional test-assembly signing in
Azure.Test.csprojwhenTestSigningKeyPathis provided. - Updates
Azure.csprojIVT to include the test public key only for signed + Package reference builds. - Threads an
isInternalBuildflag through the Azure CI stage/jobs and passes signing key properties todotnet build/pack; also updatesbuild.projto pass signing properties to the Azure test invocation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.Data.SqlClient.Extensions/Azure/test/Azure.Test.csproj | Conditionally strong-name signs the test assembly when a test key path is supplied. |
| src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj | Adjusts IVT to support signed package-mode builds (public-key IVT only when signing is enabled). |
| eng/pipelines/stages/build-azure-package-ci-stage.yml | Adds and propagates isInternalBuild into Azure build/test/pack job templates. |
| eng/pipelines/jobs/test-azure-package-ci-job.yml | Downloads signing keys and passes SigningKeyPath/TestSigningKeyPath for internal package-mode builds; avoids {command}Arguments env-var injection by renaming the variable. |
| eng/pipelines/jobs/pack-azure-package-ci-job.yml | Adds signing key download and passes SigningKeyPath into dotnet pack for internal package-mode builds. |
| eng/pipelines/dotnet-sqlclient-ci-core.yml | Threads isInternalBuild into the Azure package stage invocation. |
| build.proj | Includes signing-related MSBuild properties when running the Azure test project via build orchestration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
paulmedynski
force-pushed
the
dev/paul/assembly-signing-azure
branch
from
August 21, 2026 15:33
5daa1af to
9c13cb9
Compare
Sign the Azure package and its test assembly on internal Package-mode builds. Azure references Abstractions and Logging, both of which are signed earlier in this stack, so the reference closure is complete. - Thread isInternalBuild from the CI core into the Azure stage, pack job and test jobs, downloading the driver and test signing keys as needed. - Add a signed InternalsVisibleTo grant for the Azure test assembly, which needs internal access for the WAM broker tests, and sign that assembly when a test key is supplied. - Thread signing arguments into the build.proj TestAzure target. - Rename the test job's buildArguments variable to dotnetBuildOpts; Azure Pipelines exposes variables as environment variables and the dotnet CLI injects BUILDARGUMENTS into dotnet build.
Pre-emptively align this PR with the review feedback already addressed lower in the stack, so the same comments aren't raised again. - Reference the split download-driver-signing-key-step.yml and download-test-signing-key-step.yml templates instead of the removed download-assembly-signing-key.yml. - Quote SigningKeyPath and TestSigningKeyPath to tolerate whitespace in the secure file paths. - Fold the driver key download into the existing signing conditional in the pack job, removing the duplicated conditional. - Use positive referenceType comparisons (eq 'Package'). - Drop the isInternalBuild parameter defaults; every caller already passes it explicitly. - Normalize the BuildNumber/FileVersion note across all pack branches. - Restore an accidentally dropped blank line in Azure.Test.csproj.
paulmedynski
force-pushed
the
dev/paul/assembly-signing-azure
branch
from
September 8, 2026 17:40
9c13cb9 to
a0ebdcb
Compare
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The signing and IVT changes are consistently gated to internal Package-mode builds and are correctly threaded through the Azure CI stage/jobs and build.proj without introducing API surface changes.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's This All About?
This is PR 5 of 6 in the assembly signing stack that builds on #4569.
The first 5 PRs in this stack are all about adding assembly signing and public-key-protected InternalsVisibleTo support to the legacy CI pipeline. This is infrastructure work necessary to support the Native AOT fix in the final PR.
We have never been including assembly signing in our CI, which IMO was a blind spot. Now, when CI runs in our internal ADO.Net project, all assemblies will be signed, we will be running tests against signed assemblies, and fully testing our nascent inter-assembly IVT just as it would be in a real app. Public project CI and all of our PR pipelines will continue to use unsigned assemblies, and testing that requires inter-assembly IVT will only be done in Project mode (PR, legacy CI Project-mode pipeline) or via internal CI with signed assemblies.
The 6th and final PR in the stack addresses the Native AOT issue #4193 by eliminating inter-assembly reflection and using signed IVT with proper package dependencies.
Description
This PR is the last of the signing-infrastructure PRs, and covers
Microsoft.Data.SqlClient.Extensions.Azure. With it, every package produced by an internal Package-mode CI run is strong named, and every test assembly that needs internal access is signed with the test key.The shape mirrors #4568: Azure has a test project, so both the product assembly and the test assembly need signing, plus a public-key-qualified
InternalsVisibleTogrant to connect them.This PR:
isInternalBuildthrough the CI core into the Azure stage, the Azure pack job, and all five Azure test jobs;SigningKeyPathwhen packingMicrosoft.Data.SqlClient.Extensions.Azure, and bothSigningKeyPathandTestSigningKeyPathwhen building the test project;InternalsVisibleTogrant for the test assembly, used only when the build is both signed and in Package mode. The existing unqualified grant still covers every unsigned configuration, and signed + Project mode is deliberately omitted so that production-signed assemblies never grant internal access to locally built test assemblies;TestSigningKeyPathis supplied; andbuild.projTestAzuretarget.It also renames the test job's
buildArgumentsvariable todotnetBuildOpts. That is not cosmetic: Azure Pipelines exports job variables as environment variables, anddotnet buildpicks up$BUILDARGUMENTSimplicitly, so a variable namedbuildArgumentsgets injected into the build a second time. The same rename was made in the Abstractions test job in #4568.Supplying
SigningKeyPathactivates the existing signing behavior insrc/Directory.Build.props; this PR does not change product source, public APIs, package contents beyond assembly signing, or compatibility behavior.The second commit pre-applies the review feedback already addressed on #4566 and #4567 so the same comments aren't raised again here: the split
download-driver-signing-key-step.yml/download-test-signing-key-step.ymltemplates, quoted secure-file paths, positivereferenceTypecomparisons, no parameter defaults, the duplicated signing conditional collapsed, and a normalized BuildNumber/FileVersion note across the pack branches.Issues
Works towards addressing #4193.
Testing
This is a pipeline and project-file change, so no unit or integration tests were added.
As with #4568, the coverage gain is that the Azure test suite now runs on internal CI with both the product and test assemblies strong named, so a broken public-key-qualified IVT grant surfaces as a compile error rather than passing silently.
The GitHub PR validation pipelines exercise the public Package-reference and Project-reference paths. The internal signing branch requires the ADO.Net secure files and is exercised only by an internal Package-reference pipeline run.
CI pipeline runs
The GitHub PR pipelines never set
isInternalBuild, so they always take the unsigned path. As with #4566 and #4567, the four CI runs below are what actually exercise this change, covering all four quadrants of the signing matrix. Run links and thetools/PackageValidatorartifact verification table will be added here once the runs complete.The expectation is that the
Build Azure Packagestage succeeds in all four runs, that theDownload Driver Signing KeyandDownload Test Signing Keysteps appear only in MDS Main CI-Package, and that only that quadrant'sAzure.Artifactspackage reportsSignedwith the23ec7fc2d6eaa4a5public key token.