Fix forked repo local SQL Server passwords #3950

paulmedynski · 2026-02-11T13:43:43Z

Description

#3928 broke forked pipeline runs by removing the generated password used for local SQL Server configuration. This PR re-introduces it with documentation explaining why it exists :)

Testing

PR/CI runs will validate that non-forked repos still work.
I will ask a contributor to test forked builds via the existing Expose copy default transient error list #3903.

Copilot

Pull request overview

This PR fixes an issue introduced in #3928 where the removal of a password generation mechanism broke pipeline runs from forked repositories. Forked repos don't have access to Azure DevOps Library variable groups, which contain the $(Password) variable needed for local SQL Server configuration. The fix re-introduces password generation for forked PRs and removes redundant condition parameters from SQL Server configuration templates.

Changes:

Adds password generation logic for forked repos using System.PullRequest.IsFork check in the test job template
Removes redundant condition parameter and its uses from all SQL Server configuration step templates (Windows, Linux, macOS)
Removes condition parameter overrides from the build stage that were calling these templates

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
eng/pipelines/common/templates/jobs/ci-run-tests-job.yml	Adds password generation step for forked repos using GUID; sets Password variable before SQL Server setup
eng/pipelines/stages/build-azure-package-ci-stage.yml	Removes condition parameter overrides when calling SQL Server setup templates
eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml	Removes condition parameter definition and all condition attributes from steps
eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml	Removes condition parameter definition and condition attribute from step
eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml	Removes condition parameter definition and condition attribute from step

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml

paulmedynski · 2026-02-11T14:05:01Z

eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml

 # Test Configuration properties", brought in by common/templates/libraries/ci-build-variables.yml.

-parameters:
-  - name: condition


These condition parameters were not being used. The OS-specific config templates were already being used only for the appropriate OS anyway.

ErikEJ · 2026-02-11T15:59:49Z

@paulmedynski Happy to help verify this

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml

paulmedynski · 2026-02-11T16:04:52Z

@ErikEJ - Sure, have at 'er. Can you make a PR based on the branch of this PR and submit it as draft? That should use these changes and let us see how the PR runs perform.

codecov · 2026-02-11T17:10:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.23%. Comparing base (3ffbba7) to head (06b5810).

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

HEAD has 1 upload less than BASE

Flag BASE (3ffbba7) HEAD (06b5810)

addons 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3950      +/-   ##
==========================================
- Coverage   74.53%   67.23%   -7.31%     
==========================================
  Files         266      260       -6     
  Lines       42915    65704   +22789     
==========================================
+ Hits        31987    44176   +12189     
- Misses      10928    21528   +10600

Flag	Coverage Δ
addons	`?`
netcore	`67.56% <ø> (-7.22%)`	⬇️
netfx	`66.06% <ø> (-7.66%)`	⬇️

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.

paulmedynski · 2026-02-11T17:42:15Z

I'm testing these changes with my own fork via #3952.

- Re-introduce a pipeline-generated random password for forked builds.

- Addressed Copilot feedback.

Copilot

Pull request overview

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

eng/pipelines/stages/stress-tests-ci-stage.yml

eng/pipelines/stages/build-azure-package-ci-stage.yml

eng/pipelines/common/templates/stages/ci-run-tests-stage.yml

eng/pipelines/sqlclient-pr-project-ref-pipeline.yml

eng/pipelines/common/templates/steps/update-config-file-step.yml

eng/pipelines/sqlclient-pr-project-ref-pipeline.yml

- Adding diagnostics to help understand why AAD Password auth is failing.

- Removed forced debug flag. - Addressed Copilot comments.

paulmedynski

Commentary for reviewers, and some things for me to fix.

paulmedynski · 2026-02-12T12:58:53Z

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml

    default: false

+  # The SA password to set when configuring SQL Server.
+  - name: saPassword


I decided to plumb the SA password down to all of the steps that need it via parameters rather than using (global) variables. This is the approach we will be taking with the new pipelines.

paulmedynski · 2026-02-12T13:00:05Z

eng/pipelines/common/templates/jobs/run-tests-package-reference-job.yml

  - template: /eng/pipelines/common/templates/steps/update-config-file-step.yml
    parameters:
+      # We use the Library $(Password) variable as the SA password in this pipeline.
+      saPassword: $(Password)


The ADO.Net pipelines never run against forks, so they always have access to the Library variables. I didn't convert this pipeline to use parameters instead of global variables - it wasn't worth it.

paulmedynski · 2026-02-12T13:02:07Z

eng/pipelines/common/templates/stages/ci-run-tests-stage.yml

+      variables:
+        # Bring the SA password from the secrets_stage into scope here.
+        - name: saPassword
+          value: $[stageDependencies.secrets_stage.secrets_job.outputs['SaPassword.Value']]


This is the magic that fetches the SA password from the secrets_stage and expands it into a variable we can use as a parameter to the downstream jobs and steps.

This $[ ... ] syntax is not evaluated if used directly as a parameter value, so we must use this intermediate variable.

paulmedynski · 2026-02-12T13:05:54Z

eng/pipelines/common/templates/steps/update-config-file-step.yml

-        $p.TCPConnectionString="${{parameters.TCPConnectionString }}"

-        $p.NPConnectionString="${{parameters.NPConnectionString }}"
+  # Some of the connection strings brought in from Azure DevOps Library groups expect a runtime


This sneakiness will disappear in the new pipelines because we won't be storing connection strings in Library variables. They will be checked-in with the pipeline YAML.

paulmedynski · 2026-02-12T13:09:09Z

eng/pipelines/stages/generate-secrets-ci-stage.yml

+#
+#   secrets_stage
+#
+# None of the values produced here are actual secrets - they are just random values that are


This is mainly here to suppress Copilot complaining.

paulmedynski · 2026-02-12T13:09:57Z

eng/pipelines/stages/stress-tests-ci-stage.yml

-# and password of '$(Password)'.  The $(Password) variable is defined in the ADO
-# Library "ADO Test Configuration properties", brought in by
-# common/templates/libraries/ci-build-variables.yml.
+# and the provided password.


@paulmedynski No password is provided here.

paulmedynski · 2026-02-12T13:11:29Z

eng/pipelines/dotnet-sqlclient-ci-core.yml

+            # Pipeline runs against forks of the repo don't have access to Library secrets, so we
+            # omit them entirely from the configProperties, which causes the dependent tests to be
+            # skipped.
+            ${{ if eq(variables['System.PullRequest.IsFork'], 'False') }}:


The docs define this variable with title-case.

paulmedynski · 2026-02-12T13:11:58Z

eng/pipelines/dotnet-sqlclient-ci-core.yml

              EnclaveEnabled: true
              AADAuthorityURL: $(AADAuthorityURL)
              AADServicePrincipalId: $(AADServicePrincipalId)
-              ${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:


No need to check for a fork again.

paulmedynski · 2026-02-12T13:12:34Z

src/Microsoft.Data.SqlClient.Extensions/Azure/test/Config.cs

        DebugEmit = GetEnvFlag("TEST_DEBUG_EMIT");
        SystemAccessToken = GetEnvVar("SYSTEM_ACCESSTOKEN");

+        // Circumvent pipeline masking of things it considers secrets by base64 encoding them.  We


The whole point of debug mode is to see the config values, regardless of whether or not Azure DevOps thinks they are secrets.

Copilot AI review requested due to automatic review settings February 11, 2026 13:43

paulmedynski added the Area\Engineering Use this for issues that are targeted for changes in the 'eng' folder or build systems. label Feb 11, 2026

Copilot started reviewing on behalf of paulmedynski February 11, 2026 13:44 View session

paulmedynski mentioned this pull request Feb 11, 2026

Stabilize macOS agent setup #3928

Merged

Copilot AI reviewed Feb 11, 2026

View reviewed changes

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Outdated Show resolved Hide resolved

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Outdated Show resolved Hide resolved

paulmedynski mentioned this pull request Feb 11, 2026

Expose copy default transient error list #3903

Open

paulmedynski commented Feb 11, 2026

View reviewed changes

paulmedynski added this to the 7.0.0-preview4 milestone Feb 11, 2026

David-Engel reviewed Feb 11, 2026

View reviewed changes

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml Outdated Show resolved Hide resolved

paulmedynski mentioned this pull request Feb 12, 2026

Forked Pipeline SA Password Test #3952

Merged

Copilot AI review requested due to automatic review settings February 12, 2026 10:37

Copilot started reviewing on behalf of paulmedynski February 12, 2026 10:38 View session

paulmedynski added 3 commits February 12, 2026 06:39

Task 42348: Fix forked repo local SQL Server passwords

aa1d590

- Re-introduce a pipeline-generated random password for forked builds.

Task 42348: Fix forked repo local SQL Server passwords

064e9af

- Addressed Copilot feedback.

Forked Pipeline SA Password Test (#3952)

0948c0b

Copilot AI reviewed Feb 12, 2026

View reviewed changes

paulmedynski force-pushed the dev/paul/db-password branch from a567b28 to 0948c0b Compare February 12, 2026 10:46

Task 42348: Fix forked repo local SQL Server passwords

2fa86a0

- Adding diagnostics to help understand why AAD Password auth is failing.

Copilot AI review requested due to automatic review settings February 12, 2026 11:27

Copilot started reviewing on behalf of paulmedynski February 12, 2026 11:28 View session

Task 42348: Fix forked repo local SQL Server passwords

af6cfae

- Removed forced debug flag. - Addressed Copilot comments.

paulmedynski commented Feb 12, 2026

View reviewed changes

paulmedynski marked this pull request as ready for review February 12, 2026 13:14

paulmedynski requested a review from a team as a code owner February 12, 2026 13:14

Fix forked repo local SQL Server passwords #3950

Are you sure you want to change the base?

Fix forked repo local SQL Server passwords #3950

Conversation

paulmedynski commented Feb 11, 2026

Description

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ErikEJ commented Feb 11, 2026

Uh oh!

Uh oh!

paulmedynski commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 11, 2026

Codecov Report

Uh oh!

paulmedynski commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulmedynski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paulmedynski commented Feb 11, 2026 •

edited

Loading