Skip to content

Commit 06b5810

Browse files
committed
Task 42348: Fix forked repo local SQL Server passwords
- Addressed Copilot feedback.
1 parent 4b1d30f commit 06b5810

4 files changed

Lines changed: 15 additions & 23 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@ jobs:
162162
# DevOps Library groups. This includes the $(Password) used to configure local SQL Server
163163
# instances. In such a case, we must generate a random password and clobber $(Password).
164164
#
165-
# From this point forward in this stage, any pipeline runtime expansion of $(Password) will use
166-
# the random value. This includes the SQL Server config templates (configure-sql-server-*.yml),
167-
# and the expansion of connection strings via our configProperties parameter.
165+
# From this point forward in this job, any pipeline runtime expansion of $(Password) will use the
166+
# random value. This includes the SQL Server config templates (configure-sql-server-*.yml), and
167+
# the expansion of connection strings via our configProperties parameter.
168168
#
169169
# Azure Pipelines provides the System.PullRequest.IsFork variable that is available at template
170170
# expansion time. See:

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ steps:
259259
}
260260
}
261261
displayName: 'Start Sql Server Browser [Win]'
262-
condition: ${{parameters.condition }}
263262

264263
- ${{ if parameters.enableLocalDB }}:
265264
- powershell: |

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,10 @@ stages:
442442
TCPConnectionString: $(AZURE_DB_TCP_CONN_STRING)
443443
NPConnectionString: $(AZURE_DB_NP_CONN_STRING)
444444
AADAuthorityURL: $(AADAuthorityURL)
445-
# Note: Using the isFork variable to determine if secrets are available is not ideal since
446-
# it's an indirect association. But everything else (referencing secret variables various
447-
# ways to detect if they were present) won't run consistently across forks and non-forks.
448-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
445+
# Pipeline runs against forks of the repo don't have access to Library secrets, so we
446+
# omit them entirely from the configProperties, which causes the dependent tests to be
447+
# skipped.
448+
${{ if eq(variables['System.PullRequest.IsFork'], 'False') }}:
449449
AADPasswordConnectionString: $(AAD_PASSWORD_CONN_STR)
450450
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
451451
AADServicePrincipalId: $(AADServicePrincipalId)
@@ -474,7 +474,7 @@ stages:
474474
TCPConnectionString: $(AZURE_DB_TCP_CONN_STRING_eastus)
475475
NPConnectionString: $(AZURE_DB_NP_CONN_STRING_eastus)
476476
AADAuthorityURL: $(AADAuthorityURL)
477-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
477+
${{ if eq(variables['System.PullRequest.IsFork'], 'False') }}:
478478
AADPasswordConnectionString: $(AAD_PASSWORD_CONN_STR_eastus)
479479
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
480480
AADServicePrincipalId: $(AADServicePrincipalId)
@@ -529,7 +529,7 @@ stages:
529529
TCPConnectionString: $(AZURE_DB_TCP_CONN_STRING)
530530
NPConnectionString: $(AZURE_DB_NP_CONN_STRING)
531531
AADAuthorityURL: $(AADAuthorityURL)
532-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
532+
${{ if eq(variables['System.PullRequest.IsFork'], 'False') }}:
533533
AADPasswordConnectionString: $(AAD_PASSWORD_CONN_STR)
534534
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
535535
AADServicePrincipalId: $(AADServicePrincipalId)
@@ -565,9 +565,9 @@ stages:
565565

566566
# Enclave tests
567567
#
568-
# Only run the AE tests if enable and if we have access to the necessary
569-
# secrets.
570-
${{ if and(eq(parameters.runAlwaysEncryptedTests, true), eq(variables['system.pullRequest.isFork'], 'False')) }}:
568+
# Only run these tests if explicitly enabled, and if we're not a forked repo (which won't
569+
# have access to the necessary Library secrets).
570+
${{ if and(eq(parameters.runAlwaysEncryptedTests, true), eq(variables['System.PullRequest.IsFork'], 'False')) }}:
571571
windows_enclave_sql:
572572
pool: ADO-CI-AE-1ES-Pool
573573
images:
@@ -588,8 +588,7 @@ stages:
588588
EnclaveEnabled: true
589589
AADAuthorityURL: $(AADAuthorityURL)
590590
AADServicePrincipalId: $(AADServicePrincipalId)
591-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
592-
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
591+
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
593592
AzureKeyVaultUrl: $(AzureKeyVaultUrl)
594593
AzureKeyVaultTenantId: $(AzureKeyVaultTenantId)
595594
SupportsIntegratedSecurity: $(SupportsIntegratedSecurity)
@@ -617,8 +616,7 @@ stages:
617616
TCPConnectionStringAASSGX: $(SQL_TCP_CONN_STRING_AASSGX)
618617
EnclaveEnabled: true
619618
AADServicePrincipalId: $(AADServicePrincipalId)
620-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
621-
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
619+
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
622620
AzureKeyVaultUrl: $(AzureKeyVaultUrl)
623621
AzureKeyVaultTenantId: $(AzureKeyVaultTenantId)
624622
SupportsIntegratedSecurity: false

eng/pipelines/jobs/test-azure-package-ci-job.yml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,7 @@ jobs:
238238
# Avoid exposing secrets to pipeline jobs triggered via forks. This
239239
# prevents external contributors from creating PRs and running
240240
# pipelines that could expose these secrets.
241-
#
242-
# Note that this isn't a perfect restriction since internal
243-
# contributors may want to use forks, but this would prevent them from
244-
# running the full test suite. We don't have a better way to detect
245-
# external parties though.
246-
${{ if eq(variables['system.pullRequest.isFork'], 'False') }}:
241+
${{ if eq(variables['System.PullRequest.IsFork'], 'False') }}:
247242
AADPasswordConnectionString: $(AAD_PASSWORD_CONN_STR)
248243
AADServicePrincipalSecret: $(AADServicePrincipalSecret)
249244

0 commit comments

Comments
 (0)