Skip to content

Commit 7bb5ecc

Browse files
committed
Address Copilot review comments: Node LTS, cert gate+cleanup, pipefail comment, Build condition, explicit artifact download, prereq docs
1 parent 364eb7d commit 7bb5ecc

File tree

2 files changed

+30
-11
lines changed

2 files changed

+30
-11
lines changed

.Pipelines/ADO-PUBLISH-SETUP.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ The `.Pipelines/` folder follows the same template convention as [MSAL.NET](http
1010
|------|---------|
1111
| [`pipeline-publish.yml`](pipeline-publish.yml) | Thin top-level wrapper — triggers, parameters, calls `template-pipeline-stages.yml` with `runPublish: true` |
1212
| [`template-pipeline-stages.yml`](template-pipeline-stages.yml) | Shared stages template — Validate, CI, Build, Publish stages; reusable by PR-gate and post-merge CI pipelines |
13+
| [`credscan-exclusion.json`](credscan-exclusion.json) | CredScan suppression file — suppresses known false positives for test fixture files (`certificate-with-password.pfx`, `test_mi.py`) |
1314

1415
---
1516

@@ -35,9 +36,11 @@ Every publish requires explicitly entering a version and selecting a destination
3536
|-------------|-------|
3637
| ADO Organization | [Create one](https://learn.microsoft.com/en-us/azure/devops/organizations/accounts/create-organization) if you don't have one |
3738
| ADO Project | Under the org; enable **Pipelines** and **Artifacts** |
39+
| [Secure Development Tools](https://marketplace.visualstudio.com/items?itemName=securedevelopmentteam.vss-secure-development-tools) extension | Must be installed in the ADO organization — required for the PreBuildCheck stage (PoliCheck, CredScan, PostAnalysis tasks) |
3840
| GitHub account with admin rights | Needed to authorize the ADO GitHub App |
3941
| PyPI API token | Scoped to the `msal` project — generate at <https://pypi.org/manage/account/token/> |
4042
| MSAL-Python (test.pypi.org) API token | Scoped to the `msal` project on test.pypi.org |
43+
| `AuthSdkResourceManager` Azure service connection *(optional)* | Required only if `LAB_APP_CLIENT_ID` is set to enable e2e tests. ARM service connection with **Get** access to the `LabAuth` secret in the `msidlabs` Key Vault. When not set, the Key Vault steps are automatically skipped. |
4144

4245
---
4346

.Pipelines/template-pipeline-stages.yml

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,9 @@ stages:
4949
Codeql.SkipTaskAutoInjection: true
5050
steps:
5151
- task: NodeTool@0
52-
displayName: 'Install NPM'
52+
displayName: 'Install Node.js (includes npm)'
5353
inputs:
54-
versionSpec: '16.x'
54+
versionSpec: 'lts/*'
5555

5656
- task: securedevelopmentteam.vss-secure-development-tools.build-task-policheck.PoliCheck@2
5757
displayName: 'Run PoliCheck'
@@ -143,12 +143,11 @@ stages:
143143
python.version: '3.14'
144144
steps:
145145
# Retrieve the MSID Lab certificate from Key Vault (via AuthSdkResourceManager SC).
146-
# The cert is written to disk and LAB_APP_CLIENT_CERT_PFX_PATH is set as a variable.
147-
# Kept for when e2e tests are fully enabled on a lab-capable agent pool.
148-
# Prerequisites: service connection 'AuthSdkResourceManager' with Get/List access
149-
# to the 'msidlabs' Key Vault. The 'LabAuth' secret is a base64-encoded PFX.
146+
# Gated on LAB_APP_CLIENT_ID being non-empty — if e2e tests are not enabled (the default),
147+
# both steps are skipped and the pipeline has no Key Vault dependency.
150148
- task: AzureKeyVault@2
151149
displayName: 'Retrieve lab certificate from Key Vault'
150+
condition: and(succeeded(), ne(variables['LAB_APP_CLIENT_ID'], ''))
152151
inputs:
153152
azureSubscription: 'AuthSdkResourceManager'
154153
KeyVaultName: 'msidlabs'
@@ -157,11 +156,12 @@ stages:
157156

158157
- bash: |
159158
set -euo pipefail
160-
CERT_PATH="$(Build.SourcesDirectory)/lab-auth.pfx"
159+
CERT_PATH="$(Agent.TempDirectory)/lab-auth.pfx"
161160
printf '%s' "$(LabAuth)" | base64 -d > "$CERT_PATH"
162161
echo "##vso[task.setvariable variable=LAB_APP_CLIENT_CERT_PFX_PATH]$CERT_PATH"
163162
echo "Lab cert written to: $CERT_PATH ($(wc -c < "$CERT_PATH") bytes)"
164163
displayName: 'Write lab certificate to disk'
164+
condition: and(succeeded(), ne(variables['LAB_APP_CLIENT_ID'], ''))
165165
166166
- task: UsePythonVersion@0
167167
inputs:
@@ -173,9 +173,9 @@ stages:
173173
pip install -r requirements.txt
174174
displayName: 'Install dependencies'
175175
176-
# Use bash: (not script:) so set -o pipefail works — script: uses /bin/sh on
177-
# Linux which does not support pipefail; without it, tee always exits 0,
178-
# masking test failures.
176+
# Use bash: explicitly; set -o pipefail so that pytest failures aren't hidden by the pipe to tee.
177+
# Without pipefail, tee exits 0 and the step can succeed even when tests fail.
178+
# (set -o pipefail also works in script: steps, but bash: makes the shell choice explicit.)
179179
- bash: |
180180
pip install pytest pytest-azurepipelines
181181
mkdir -p test-results
@@ -201,13 +201,17 @@ stages:
201201
failTaskOnFailedTests: true
202202
testRunTitle: 'Python $(python.version)'
203203

204+
- bash: rm -f "$(Agent.TempDirectory)/lab-auth.pfx"
205+
displayName: 'Clean up lab certificate'
206+
condition: and(always(), ne(variables['LAB_APP_CLIENT_ID'], ''))
207+
204208
# ══════════════════════════════════════════════════════════════════════════════
205209
# Stage 3 · Build — build sdist + wheel (release only)
206210
# ══════════════════════════════════════════════════════════════════════════════
207211
- stage: Build
208212
displayName: 'Build package'
209213
dependsOn: CI
210-
condition: and(eq(dependencies.CI.result, 'Succeeded'), eq('${{ parameters.runPublish }}', 'true'))
214+
condition: and(eq(dependencies.CI.result, 'Succeeded'), eq(${{ parameters.runPublish }}, true))
211215
jobs:
212216
- job: BuildDist
213217
displayName: 'Build sdist + wheel (Python 3.12)'
@@ -260,6 +264,12 @@ stages:
260264
runOnce:
261265
deploy:
262266
steps:
267+
- task: DownloadPipelineArtifact@2
268+
displayName: 'Download python-dist artifact'
269+
inputs:
270+
artifactName: python-dist
271+
targetPath: $(Pipeline.Workspace)/python-dist
272+
263273
- task: UsePythonVersion@0
264274
inputs:
265275
versionSpec: '3.12'
@@ -307,6 +317,12 @@ stages:
307317
runOnce:
308318
deploy:
309319
steps:
320+
- task: DownloadPipelineArtifact@2
321+
displayName: 'Download python-dist artifact'
322+
inputs:
323+
artifactName: python-dist
324+
targetPath: $(Pipeline.Workspace)/python-dist
325+
310326
- task: UsePythonVersion@0
311327
inputs:
312328
versionSpec: '3.12'

0 commit comments

Comments
 (0)