-
Notifications
You must be signed in to change notification settings - Fork 321
Forked Pipeline SA Password Test #3952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Forked Pipeline SA Password Test #3952
Conversation
Added a note about testing fork PR pipeline runs.
Added a comment to trigger PR pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR is a dummy change intended to validate how CI behaves for pull requests opened from forks.
Changes:
- Adds a placeholder line to
README.mdto create a diff and trigger fork PR pipeline execution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
- Plumbed saPassword down through all of the stages, jobs, and steps that need it. - No more global $(Password) variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml
Outdated
Show resolved
Hide resolved
- Moved secrets_stage dependency into stages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev/paul/db-password #3952 +/- ##
========================================================
- Coverage 67.23% 66.98% -0.26%
========================================================
Files 260 260
Lines 65704 65704
========================================================
- Hits 44176 44010 -166
- Misses 21528 21694 +166
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a567b28
into
dotnet:dev/paul/db-password
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
eng/pipelines/stages/build-azure-package-ci-stage.yml:233
- This call passes
sqlRootPath, butconfigure-sql-server-win-step.ymldefines the parameter asSQLRootPath. As written, the template will fall back to the default (empty) and skip SQLRootPath-dependent setup (e.g., FileStream). Rename the argument key to match the template parameter.
# These variables are from an Azure DevOps Library variable
# group.
fileStreamDirectory: $(FileStreamDirectory)
sqlRootPath: $(SQL22RootPath)
# The ADO-MMS22-SQL22 image includes a local SQL Server that supports
| # Configure the prompt to show the current timestamp so we can see how long each command takes. | ||
| export PS4='+ [$(date "+%Y-%m-%d %H:%M:%S")] ' | ||
| set -x | ||
|
|
||
| # Password for the SA user (required) | ||
| MSSQL_SA_PW="$(Password)" | ||
| # Install Docker and SQLCMD tools. | ||
| brew install colima | ||
| brew install --cask docker | ||
| brew tap microsoft/mssql-release https://github.com/Microsoft/homebrew-mssql-release | ||
| brew update | ||
| HOMEBREW_ACCEPT_EULA=Y brew install mssql-tools18 | ||
| colima start --arch x86_64 | ||
| docker --version | ||
| docker pull mcr.microsoft.com/mssql/server:2022-latest | ||
|
|
||
| docker run -e "ACCEPT_EULA=Y" -e "MSSQL_SA_PASSWORD=$MSSQL_SA_PW" -p 1433:1433 -p 1434:1434 --name sql1 --hostname sql1 -d mcr.microsoft.com/mssql/server:2022-latest | ||
| # Password for the SA user (required) | ||
| MSSQL_SA_PW="${{ parameters.saPassword }}" | ||
|
|
||
| sleep 5 | ||
| docker run -e "ACCEPT_EULA=Y" -e "MSSQL_SA_PASSWORD=$MSSQL_SA_PW" -p 1433:1433 -p 1434:1434 --name sql1 --hostname sql1 -d mcr.microsoft.com/mssql/server:2022-latest | ||
|
|
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script enables set -x before running commands that include the SA password (e.g., docker run ... MSSQL_SA_PASSWORD=... and sqlcmd -P ...), which will echo the expanded password into logs. Disable xtrace around password-bearing commands and/or source the password from a masked secret variable.
| SQL_USER: ${{parameters.user }} | ||
| SQL_PASSWD: ${{ parameters.saPassword }} | ||
|
|
||
| - ${{ if ne(parameters.FileStreamDirectory, '') }}: |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FileStream directory block checks parameters.FileStreamDirectory, but this template defines fileStreamDirectory (lowercase f). This will either fail template expansion (unknown parameter) or skip the step unexpectedly. Update the condition to use the correct parameter name.
| - ${{ if ne(parameters.FileStreamDirectory, '') }}: | |
| - ${{ if ne(parameters.fileStreamDirectory, '') }}: |
| # TODO: Temporaryily forcing debug for forked repo pipeline runs. | ||
| # debug: ${{ parameters.debug }} | ||
| debug: true |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug is hard-coded to true, which ignores the pipeline parameter and will force extra debug steps/logging on all PR runs. If this is only for temporary fork testing, gate it behind a parameter/condition (e.g., only when System.PullRequest.IsFork is true) or revert before merging.
| # TODO: Temporaryily forcing debug for forked repo pipeline runs. | |
| # debug: ${{ parameters.debug }} | |
| debug: true | |
| debug: ${{ parameters.debug }} |
This PR changes how forked repos acquire the SA password used to configure local SQL Server instances. It tests the pipeline runs from a forked repo, and will be merged to a normal dev branch before being properly reviewed and merged to main.