Trigger Kerberos CI from package pipeline - #4499
Conversation
paulmedynski
added
the
Area\Engineering
There was a problem hiding this comment.
Pull request overview
Updates the Kerberos CI Azure Pipelines YAML to use the package-triggered pipeline model (introduced in #4482), aligning source to the upstream package commit while keeping eng/pipelines pinned to the queued commit so runtime scripts match compiled YAML.
Changes:
- Switched Kerberos CI from a daily schedule to a pipeline-completion trigger on
sqlclient-ci-package. - Enhanced the shared source-alignment step to persist checkout credentials and explicitly fetch the upstream SHA before checkout.
- Added a validated
dotnetVerbosityparameter and threaded it through Kerberos build/test steps.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| eng/pipelines/common/steps/align-source-with-upstream-step.yml | Persists credentials and fetches the upstream commit SHA to make source alignment resilient. |
| eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-pipeline.yml | Replaces schedule with pipeline completion trigger and adopts the shared source-alignment template; adds dotnetVerbosity pipeline parameter. |
| eng/pipelines/ci/kerberos/build-and-test-steps.yml | Adds a dotnetVerbosity template parameter and applies it to all dotnet build invocations. |
765ca3d to
9e92e24
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eng/pipelines/common/steps/align-source-with-upstream-step.yml:51
- The upstream commit SHA comes from a pipeline resource variable and is passed directly into
git fetch/git checkout. Even though it should be a 40-hex SHA, validating it defensively avoids accidental malformed values (or argument injection if the variable were ever unexpected) causing the fetch/checkout to behave incorrectly.
$sha = "$(resources.pipeline.${{ parameters.upstreamPipeline }}.sourceCommit)"
$pipelineSourceSha = git rev-parse HEAD
if ($LASTEXITCODE -ne 0) { throw "Failed to resolve the queued pipeline commit." }
Write-Host "Fetching ${{ parameters.upstreamPipeline }} commit $sha"
git fetch --no-tags origin $sha
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Suppressed comments (6)
eng/pipelines/ci/managed-instance/sqlclient-ci-managed-instance-stages.yml:70
- This stage pins the Windows vmImage to
ADO-Win25. If that value isn’t a validimageOverridecapability in Managed-Instance-pool, the job will be stuck waiting for an agent. Consider using the standard Windows SQL 2022 image used across other pipelines.
runtime: ${{ runtime }}
useManagedSNI: false
vmImage: ADO-Win25
eng/pipelines/ci/managed-instance/sqlclient-ci-managed-instance-stages.yml:100
- The Linux vmImage is set to
ADO-UB24, which isn’t referenced anywhere else in the repo’s pipeline image lists (commonlyADO-UB22-SQL22). If Managed-Instance-pool doesn’t have an agent withimageOverride=ADO-UB24, these jobs will not start.
operatingSystem: Linux
runtime: ${{ runtime }}
vmImage: ADO-UB24
eng/pipelines/ci/managed-instance/sqlclient-ci-managed-instance-stages.yml:82
- This stage pins the Windows vmImage to
ADO-Win25, which is inconsistent with the repo’s typical Windows image naming (e.g.,ADO-MMS22-SQL22). If Managed-Instance-pool doesn’t provide this imageOverride value, the managed-SNI leg will not run.
runtime: ${{ runtime }}
useManagedSNI: true
vmImage: ADO-Win25
eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-stages.yml:73
- This Windows Kerberos job also demands
ImageOverride=ADO-Win25. If that capability isn’t available in the ADO-Trusted-Domain-Win-WestUS2 pool, this leg won’t get an agent. Consider using the same established image name as other Windows runs.
poolName: ADO-Trusted-Domain-Win-WestUS2
runtime: ${{ runtime }}
useManagedSNI: false
vmImage: ADO-Win25
eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-stages.yml:86
- This managed-SNI Windows Kerberos job also demands
ImageOverride=ADO-Win25. If that imageOverride value isn’t present in the pool capabilities, the managed-SNI legs will never run.
poolName: ADO-Trusted-Domain-Win-WestUS2
runtime: ${{ runtime }}
useManagedSNI: true
vmImage: ADO-Win25
eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-stages.yml:107
- The Linux Kerberos jobs demand
ImageOverride=ADO-UB24. The repo’s other Linux jobs typically useADO-UB20-SQL22/ADO-UB22-SQL22; if this pool doesn’t provideADO-UB24, these jobs will stay queued.
poolName: ADO-Trusted-Linux-WestUS2
runtime: ${{ runtime }}
vmImage: ADO-UB24
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4499 +/- ##
==========================================
- Coverage 64.71% 62.83% -1.88%
==========================================
Files 288 283 -5
Lines 44088 67041 +22953
==========================================
+ Hits 28532 42127 +13595
- Misses 15556 24914 +9358
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
This should appear as a rename+edit from linux-init-step.yml, but alas appears as a new file.
| -t:TestSqlClientManual | ||
| -p:TestFramework=${{ parameters.runtime }} | ||
| -p:TestSet=3 | ||
| -p:TestFilters="category!=failing&category!=flaky&category!=interactive&(FullyQualifiedName~KerberosTests|FullyQualifiedName~IntegratedAuthenticationTest|FullyQualifiedName~InstanceNameTest)" |
There was a problem hiding this comment.
This is an aggressive filter. Kerberos is an authentication mechanism, so the theory is that post-login, there isn't much worth testing. Other pipelines will run the larger integration suite against a wide variety of SQL Server types and versions. @David-Engel - Looking for your thoughts here.
| # branch for manual and scheduled builds" definition. Because both pipelines use the same | ||
| # repository, an eligible run executes this YAML from the triggering package run's branch and | ||
| # commit, preserving branch-specific pipeline definitions. | ||
| resources: |
There was a problem hiding this comment.
We're now triggering on completion of the upstream sqlclient-ci-package pipeine - no more scheduled runs.
| - template: /eng/pipelines/common/templates/jobs/ci-code-coverage-job.yml@self | ||
| parameters: | ||
| upload: false | ||
| - template: /eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-stages.yml@self |
There was a problem hiding this comment.
The remaining changes here are re-modelling the pipeline to have the same layout as Managed Instance. We will endeavour to keep these new CI test pipelines looking and feeling similar.
There was a problem hiding this comment.
This is another rename+edit that isn't being treated properly.
| $sha = "$(resources.pipeline.${{ parameters.upstreamPipeline }}.sourceCommit)" | ||
| $pipelineSourceSha = git rev-parse HEAD | ||
| if ($LASTEXITCODE -ne 0) { throw "Failed to resolve the queued pipeline commit." } | ||
| Write-Host "Fetching ${{ parameters.upstreamPipeline }} commit $sha" |
There was a problem hiding this comment.
This ensures that $sha is available to be checked-out below.
afbf9a7 to
2d952e2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eng/pipelines/ci/kerberos/linux-setup-step.yml:50
- The Kerberos password is injected directly into the bash script via a double-quoted assignment. If the secret contains characters like
$, bash will treat them as variable expansions and corrupt the password (e.g.,$FOObecomes empty). Use a quoted heredoc (or an env var) to capture the secret literally before piping it torealm join/kinit.
DOMAIN="${{ parameters.kerberosDomain }}"
DOMAIN_OU="${{ parameters.kerberosDomainOU }}"
DOMAIN_USER="${{ parameters.kerberosDomainUser }}"
DOMAIN_PASSWORD="${{ parameters.kerberosDomainPassword }}"
DOMAIN_UPPER=$(echo "$DOMAIN" | tr '[:lower:]' '[:upper:]')
|
Regarding the suppressed SHA-validation comment in Copilot review 4856214908: fixed in |
|
Regarding the six suppressed image-capability comments in Copilot review 4858050227: no image change is needed. |
|
Regarding the suppressed password-interpolation comment in Copilot review 4874085573: fixed in |
|
🔴 The linked validation runs predate every fix in The Testing section links So everything added in
Could we re-run the |
| runtime: ${{ runtime }} | ||
| vmImage: ADO-UB24 | ||
|
|
||
| - stage: merge |
There was a problem hiding this comment.
🔴 This merge stage produced no coverage in the linked validation run.
From build 164554:
| Signal | Value |
|---|---|
| Merge stage warning | No coverage data generated to publish. |
Cobertura Merge Results artifact |
5,227 bytes |
| Per-job test artifacts | 63–76 KB (TRX-sized, no .coverage files) |
build.proj:299 defaults TestCodeCoverage=true, so --collect "Code coverage" should be on the dotnet test line. But the coverage job's itemPattern: '**/net*/**/*.coverage' finds nothing to download, so the merge is a no-op.
One hypothesis, given this pipeline just moved to ReferenceType=Package: the tests now bind to Microsoft.Data.SqlClient.dll restored from the NuGet cache rather than a local build output. CodeCoverage.runsettings still matches it by ModulePath, but the collector needs PDBs to instrument, and symbols ship separately in the .snupkg. Worth checking whether Managed Instance has the same gap, since it made the same Package-mode move.
Either way — could we confirm coverage works in Package mode, or drop the merge stage until it does? A stage that always warns trains people to ignore warnings.
There was a problem hiding this comment.
Code coverage support for this entire new CI pipeline chain is a future effort - see AB#46784. I will remove the stage.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eng/pipelines/ci/kerberos/sqlclient-ci-kerberos-stages.yml:40
- PR description says the Kerberos pipeline is split into Windows/Linux stages “followed by merged coverage”, but this stages template defines only
windowsandlinuxstages and the per-job template no longer publishes any coverage artifacts for a merge job to consume. Either add a merge/coverage stage (and restore per-job coverage artifact publishing) or update the PR description/testing notes to reflect that coverage is no longer produced/merged for this pipeline.
stages:
- stage: windows
displayName: Windows
dependsOn: []
mdaigle
left a comment
mdaigle
left a comment
There was a problem hiding this comment.
Thanks for the updates. The env: change in linux-setup-step.yml and the sniSuffix naming both look right. On the fallback schedule, your reasoning makes sense to me, and the branch filters cover what I was actually worried about.
One leftover: the env: fix didn't make it into linux-teardown-step.yml. Line 35 still does DOMAIN_PASSWORD="${{ parameters.kerberosDomainPassword }}", so the expanded password ends up in the agent's generated _temp/*.sh on a shared domain-joined box.
Suggestion: mirror the setup change.
DOMAIN_USER="${{ parameters.kerberosDomainUser }}"
DOMAIN_UPPER=$(echo "$DOMAIN" | tr '[:lower:]' '[:upper:]')
# Leave the domain
echo "${DOMAIN_PASSWORD:-}" | sudo realm leave "$DOMAIN_UPPER" --verbose \
-U "$DOMAIN_USER@$DOMAIN_UPPER" || true
# Destroy the TGT and credential cache
kdestroy || true
displayName: Clean up Kerberos (domain leave + kdestroy)
condition: always()
env:
DOMAIN_PASSWORD: ${{ parameters.kerberosDomainPassword }}${DOMAIN_PASSWORD:-} rather than $DOMAIN_PASSWORD because set -u is on and cleanup should still run if the variable is missing. Worth applying the same treatment to the empty-password guard at linux-setup-step.yml:56, which is now unreachable for the same reason.
Description
Updates the Kerberos integration pipeline to consume and test the exact packages produced by
sqlclient-ci-package, and restructures the Kerberos and Managed Instance pipelines into explicit OS stages.Kerberos pipeline
sqlclient-ci-packagepipeline. Because both pipelines use the same repository, eligible runs use the triggering package run's branch and commit.eng/pipelinesfrom the commit that queued the downstream run. The shared alignment step now persists checkout credentials and fetches the upstream SHA so it also works when that commit is ahead of the queued commit.ReferenceType=Packageand the exact package versions instead of rebuilding SqlClient.KerberosTests,IntegratedAuthenticationTest, andInstanceNameTest, which exercise the configured Kerberos environment.net462native SNI plusnet8.0,net9.0, andnet10.0with native and managed SNI) and three Linux jobs (net8.0,net9.0, andnet10.0).dotnetVerbosityparameter through package-mode test commands.Related pipelines
sqlclient-ci-package.Testing