Skip to content

Use the governed SqlClient feed for PowerShell module installs - #4932

Draft
mdaigle wants to merge 19 commits into
mainfrom
dev/mdaigle/governed-powershell-modules
Draft

mdaigle wants to merge 19 commits into
mainfrom
dev/mdaigle/governed-powershell-modules

Conversation

@mdaigle

@mdaigle mdaigle commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Description

Restore PowerShell prerequisites from the public SqlClient Azure Artifacts feed through one explicit build hook shared by pipelines and local setup. Anonymous access is retained; no feed-service configuration, NuGet.config, or authentication dependencies are added.

  • PowerShell.config, beside NuGet.config, holds the single PowerShell feed address. Registration, discovery, and installation use PSResourceGet bundled with the restored PowerShell tool and the verified NuGet v3 endpoint.
  • powershell-modules.json pins SqlServer 22.4.5.1 and Pester 6.2.0. Entries contain only a version; there are no publisher-check exceptions or per-module clobber settings. Installation uses -NoClobber to protect existing commands.
  • The unused OpenSSL PowerShell module is no longer installed. Unix certificate setup invokes the native openssl CLI, while Windows expects built-in PKI cmdlets. The wrapper supplies no executable and exports conflicting command names. The existing native CLI prerequisite remains documented; no OS package installation is added.
  • After dotnet tool restore, developers explicitly run dotnet build build.proj -t:RestorePowerShellModules. Restore validates the manifest before side effects, reuses installed exact versions, and installs missing resources for the current user. Only PSResourceGet's specific missing-resource exception is treated as absence; genuine discovery, registration, and installation errors propagate.
  • Registration is lazy and its returned name immediately used. Matching registrations retain their trust policy; conflicting sources are not overwritten. The URI is passed to the client without custom endpoint restrictions. CredentialProvider None explicitly disables automatic Azure authentication, including on matching registrations created before this fix. No credential-provider bootstrap, repository-name environment variables, or fallback to PSGallery.
  • Pipelines call the same standalone restore target after .NET tool restore and before build/setup. Queued tooling can be restored while retaining an older package-producing commit's driver source and build.proj. Consumer scripts remain install-free and use ordinary imports: pins govern installation, not runtime resolution.
  • BUILDGUIDE and related test/setup guides document the explicit workflow. Restore and consumption must use the same user. Both pipeline-test READMEs retain .NET tool restore, including Linux's PowerShell bootstrap.

No public API, driver/runtime, or product-package version changes. Previously unpinned installations now use the manifest; private-feed authentication remains deferred. Removing the manifest entry does not uninstall existing developer modules.

Issues

Maintainer-requested governed-feed migration and shared local/pipeline prerequisite restoration. Independent of merged SecureString fix #4923; no issue closure.

Testing

Latest change: 410382488 removes the unused OpenSSL wrapper.

dotnet tool run pwsh -- -NoProfile -Command 'Import-Module Pester; Invoke-Pester -Path .\eng\pipelines\scripts\tests\Restore-PowerShellModules.Tests.ps1,.\eng\pipelines\scripts\tests\Register-SqlClientPSRepository.Tests.ps1,.\eng\pipelines\scripts\tests\Configure-NetOnlyAccount.Tests.ps1,.\eng\pipelines\scripts\tests\Pipeline-Arguments.Tests.ps1,.\eng\pipelines\scripts\tests\Pipeline-Platforms.Tests.ps1 -CI -Output Normal'
# Passed: 95 tests, 0 failures, 0 skipped.

dotnet build build.proj -t:RestorePowerShellModules -nologo -v:minimal
# Passed: processed only SqlServer 22.4.5.1 and Pester 6.2.0, reusing installed versions.
# 0 warnings, 0 errors.

git diff --check
# Passed.

The new regression failed before removing OpenSSL and now verifies that default manifest restore neither queries nor installs that wrapper. Repository searches found no wrapper-command dependency. Windows command inspection confirmed PKI provides the certificate cmdlets with the parameters used by our scripts; a fresh-session lookup also demonstrated that installing the wrapper can shadow New-SelfSignedCertificate.

The suite covers manifest validation before side effects, exact resource queries/installations, expected missing-resource errors versus genuine discovery failures, command-clobber protection, single-name registration output, anonymous provider selection, source conflicts/trust preservation, pipeline ordering, install-free consumers, and failure propagation. SQL/account calls are mocked; no account or SQL login was provisioned.

Prior anonymous-access fix, retained unchanged: build 187159's Ubuntu restore logs 513 and 562 failed because PSResourceGet automatically enabled Azure authentication for the public feed and could not locate a credential provider. Commit 0a7c3a713 explicitly selects None for new registrations and repairs matching registrations without changing trust. Three regressions failed before that fix. Live verification confirmed provider None, no CredentialInfo, and exact lookup of both retained manifest versions. A separate fresh registration also supported exact SqlServer lookup and was removed afterward. No credentials or environment-variable workarounds were added.

The earlier live installation smoke used OpenSSL before its removal from the manifest. That historical probe verified client installation behavior, not a reason to retain the module. The latest removal was checked through the real restore target and regression suite; fresh installation of the already-installed retained modules was not repeated.

Build-hook probes at 69205a26b (target/wiring unchanged by these follow-ups):

dotnet tool run pwsh -- -NoProfile -Command 'Import-Module Pester; $r = Invoke-Pester -Path .\eng\pipelines\scripts\tests\Build-Orchestration.Tests.ps1 -FullNameFilter "build.proj PowerShell module restore*" -Output Normal -PassThru; if ($r.FailedCount -ne 0 -or $r.PassedCount -ne 4) { throw "Expected all four explicit restore-hook probes to pass." }'
# Passed: all 4 selected real MSBuild probes; 95 unrelated tests not selected.

Earlier full Azure preview at 3158d1edf expanded successfully with 72 restore steps across 72 consuming jobs, each after .NET tool restore. The current migration preserves that wiring. Current-head CI must still establish cross-platform results after the OpenSSL removal; pipelines have not been treated as passing merely because local checks passed.

Known independent limitations: the maintainer previously reported a feed outage. Earlier CodeQL/package-verification failures returned HTTP 500/TF400898 for BenchmarkDotNet and NuGet.VerifyMicrosoftPackage; CodeQL subsequently passed on 69205a26b. Sources and checks were not bypassed or weakened.

Earlier full Build-Orchestration testing had 90 passing and 9 failures, all also reproduced with baseline test source. An earlier VersionChecks run had 632 passing and 18 failed/not-run cases after its packaging fixture could not start csc.exe (MSB3883, "The parameter is incorrect"). These broader suites were not rerun for this manifest change. SQL/Kerberos integration, native Unix certificate generation, inbox-only Pester replacement, and official release pipelines were not exercised locally.

  • Tests added or updated
  • Public API changes documented (not applicable; none)
  • Verified against customer repro (not applicable; engineering source migration)
  • Ensure no breaking changes introduced (driver/runtime behavior unchanged; explicit script prerequisites documented)

Guidelines

Please review the contribution guidelines before submitting a pull request:

Route SQL, certificate, and documented Pester module installs through the public feed's PowerShell Gallery upstream. Validate repository registrations and preserve standalone test-output provisioning.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 22:02
@mdaigle
mdaigle requested a review from a team as a code owner October 27, 2026 22:02
@github-project-automation github-project-automation Bot moved this to To triage in SqlClient Board Oct 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The sole documented Pester setup command fails against the new feed, blocking VersionChecks on clean machines, while the passing regression run used an existing installation and did not verify this bootstrap path.

1 open issue
What changed in this PR

Routes PowerShell module installation through the public SqlClient Azure Artifacts feed, centralizing repository registration without a PSGallery fallback.

Changes:

  • Adds a shared registration helper that preserves matching registrations and rejects conflicts.
  • Migrates SqlServer and OpenSSL installation sites while preserving existing version pins and reuse behavior.
  • Adds registration and deployment tests, and updates developer setup documentation.
File Description
src/​Microsoft.Data.SqlClient/​tests/​ManualTests/​Microsoft.Data.SqlClient.ManualTests.csproj Copies the registration helper into test output.
src/​Microsoft.Data.SqlClient/​tests/​ManualTests/​makepfxcert.ps1 Uses the governed feed and propagates module provisioning failures.
eng/​scripts/​Register-SqlClientPSRepository.ps1 Adds shared feed registration and conflict validation.
eng/​scripts/​Configure-NetOnlyAccount.ps1 Routes pinned SqlServer installation through the helper.
eng/​pipelines/​scripts/​tests/​Register-SqlClientPSRepository.Tests.ps1 Tests registration, failure propagation, wiring, and deployment layouts.
eng/​pipelines/​scripts/​tests/​README.md Updates Pester setup and test inventory.
eng/​pipelines/​scripts/​tests/​Configure-NetOnlyAccount.Tests.ps1 Verifies governed sourcing and stops provisioning on lookup failure.
eng/​pipelines/​onebranch/​scripts/​tests/​README.md Updates Pester installation to use the governed feed.
eng/​pipelines/​ci/​kerberos/​linux-setup-step.yml Uses the helper for SqlServer installation.
doc/​connection-pooling-impersonation.md Documents the new net-only provisioning module source.
BUILDGUIDE.md Updates Pester setup and explains feed registration behavior.

Review effort: Balanced

Comment thread BUILDGUIDE.md Outdated
Derive the feed endpoint from NuGet.config and pass repository names to module consumers. Register once per test job and remove registration and helper deployment from certificate setup.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 27, 2026 22:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The required feed reportedly returns HTTP 500 for Pester 5.7.1, while Azure pipeline and Unix certificate provisioning remain unvalidated and require maintainer confirmation and affected-agent execution.

1 open issue

Review effort: Balanced

@mdaigle
mdaigle marked this pull request as draft October 28, 2026 16:49
@mdaigle mdaigle added this to the 8.0.0-preview1 milestone Oct 28, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 17:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Supplemental review guidance could not be verified under the review’s trust requirements because only worktree paths were available, without a trusted base SHA or protected copy.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Manually queued Linux Kerberos runs against pre-PR package builds fail because source alignment restores the new pipeline template but not its required helper under eng/scripts.

1 open issue

Review effort: Balanced

Comment thread eng/pipelines/common/templates/steps/register-powershell-repository-step.yml Outdated
Restore the queued helper alongside pipeline definitions and cover old package commits with a Git fixture. Document importing Pester once per test session.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 17:56
Comment thread eng/pipelines/ci/kerberos/linux-setup-step.yml Outdated
Comment thread eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Outdated
Comment thread eng/pipelines/onebranch/scripts/tests/README.md Outdated
Comment thread eng/scripts/Register-SqlClientPSRepository.ps1 Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues
1 resolved since last review

Review effort: Lite

Comment thread eng/pipelines/ci/kerberos/linux-setup-step.yml
Comment thread eng/pipelines/common/templates/steps/configure-netonly-account-step.yml Outdated
Comment thread eng/scripts/Register-SqlClientPSRepository.ps1 Outdated
Comment thread eng/scripts/Register-SqlClientPSRepository.ps1 Outdated
Comment thread src/Microsoft.Data.SqlClient/tests/ManualTests/makepfxcert.ps1 Outdated
Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Managed Instance setup fails when an upstream package commit has an older config without PowerShellRepository, because assigning the missing property throws; the property must be added safely and the older-config case covered by a regression test.

1 open issue
5 resolved since last review

Review effort: Balanced

Comment thread eng/pipelines/common/templates/steps/update-config-file-step.yml Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 28, 2026 18:27
Comment thread eng/pipelines/onebranch/scripts/tests/README.md Outdated
Comment thread eng/pipelines/onebranch/scripts/tests/README.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The tests enforce an unversioned SqlServer import in the Kerberos setup, allowing a newer installed version to override the manifest’s required version.

1 open issue

Review effort: Balanced

Comment thread eng/scripts/Configure-NetOnlyAccount.ps1 Outdated
Remove redundant manifest parsing from both Pester import examples. Keep dotnet tool restore before module restoration and clarify that it installs pwsh on Linux.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 18:19
Comment thread eng/scripts/Register-SqlClientPSRepository.ps1 Outdated
Comment thread eng/scripts/Restore-PowerShellModules.ps1 Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Required policy checks remain unverified without trusted reference copies, and the CI-wide restore gate still needs confirmed latest-head CI and clean-agent module-consumption validation.

1 open issue

Review effort: Balanced

Comment thread eng/scripts/Restore-PowerShellModules.ps1 Outdated
Import SqlServer normally in net-only setup and cleanup, preserving import error propagation before account creation. Align Pester examples and document installation pins separately from normal runtime module resolution.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 18:24
Comment thread BUILDGUIDE.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Repository-policy compliance could not be fully assessed because supplemental policy references were supplied only as checkout paths without a trusted immutable revision.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Comment thread powershell-modules.json Outdated
Simplify resource versions and source registration, protect existing commands, and handle missing installed resources explicitly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 18:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The mandatory PSResourceGet restore remains unverified in current-head cross-platform CI and clean Windows bootstraps, including whether -NoClobber installs all three prerequisites alongside preinstalled modules.

No open issues 🎉

Review effort: Balanced

Comment thread TESTGUIDE.md Outdated
Disable PSResourceGet's automatic Azure credential-provider selection for new and matching registrations without changing trust. Clarify the native OpenSSL prerequisite with its script reference.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human review is needed because an existing untrusted repository can trigger a confirmation prompt that blocks missing-module restoration under noninteractive PowerShell.

No open issues 🎉

Review effort: Balanced

Certificate setup uses the native openssl CLI on Unix and built-in PKI cmdlets on Windows. Guard against restoring the unused command-shadowing wrapper.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 29, 2026 19:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The mandatory shared restore bootstrap affects all consuming jobs, but current-head cross-platform CI and clean Windows installation alongside inbox Pester remain unverified.

No open issues 🎉

Review effort: Balanced

This branch has not been deployed

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

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

2 participants