Re-land: Migrate AzDO feed publishing from PAT to Entra-based auth (#17113) - #17140
Conversation
…otnet#17113) Reverts the revert (dotnet#17136). The original change was correct; it failed only because the WIF service connection infrastructure was not present in both orgs the Build Promotion Pipeline runs in. That infra (devdiv SC + FICs + pipeline authorizations) has now been created. Also adds a non-secret diagnostic + targeted error message to the GetFeedToken step to make SC/FIC misconfiguration obvious in build logs.
There was a problem hiding this comment.
Pull request overview
Re-lands the migration of Azure DevOps feed publishing from a long-lived PAT to Entra/WIF-based authentication, enabling either (1) a provided token (PAT or AAD) or (2) automatic token acquisition via DefaultIdentityTokenCredential when no feed key is supplied.
Changes:
- Switch AzDO HTTP auth to use Bearer for AAD/JWT tokens while preserving Basic for PATs; add clearer failure messaging when Entra token acquisition fails.
- Allow AzDO NuGet feeds to be configured without
AzureDevOpsFeedsKeyand rely on Entra fallback instead (with updated tests). - Update the publishing pipeline to acquire an AzDO access token via
AzureCLI@2using thednceng-artifact-feeds-publishWIF service connection and use it for publishing.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.DotNet.Build.Tasks.Feed/src/PublishArtifactsInManifestBase.cs | Adds Bearer-vs-Basic auth selection and Entra token acquisition fallback with improved error messaging. |
| src/Microsoft.DotNet.Build.Tasks.Feed/src/model/SetupTargetFeedConfigV3.cs | Stops treating missing AzDO feed key as a hard error; logs info and keeps feed config. |
| src/Microsoft.DotNet.Build.Tasks.Feed/src/AzureDevOpsNugetFeedAssetPublisher.cs | Implements token fallback via DefaultIdentityTokenCredential and Bearer-vs-Basic selection. |
| src/Microsoft.DotNet.Build.Tasks.Feed.Tests/SetupTargetFeedConfigV3Tests.cs | Adds coverage ensuring missing AzDO feed key still produces feeds and does not log build errors. |
| src/Microsoft.DotNet.Arcade.Sdk/toolset/PublishArtifactsInManifest.proj | Only includes FeedKey item when $(AzureDevOpsFeedsKey) is non-empty to enable fallback behavior. |
| eng/publishing/v3/publish.yml | Replaces PAT-based publishing with WIF-acquired AzDO token + adds diagnostics around token acquisition. |
Comments suppressed due to low confidence (1)
eng/publishing/v3/publish.yml:177
AzDoFeedTokenis passed to MSBuild as/p:AzureDevOpsFeedsKey=..., which will typically end up in binlogs underartifacts/log/.... Thepublish-logsredaction step only redacts secrets explicitly listed (plus a static list) and currently only includes the Maestro token viaCustomSensitiveDataList, so the Entra access token risks being uploaded in PostBuildLogs. Add the output token toCustomSensitiveDataList(or toeng/BinlogSecretsRedactionFile.txt) so the binlog redaction step reliably scrubs it.
/p:AzureDevOpsFeedsKey='$(GetFeedToken.AzDoFeedToken)'
/p:AkaMSClientId=$(akams-app-id)
/p:AkaMSClientCertificate=$(Agent.TempDirectory)/akamsclientcert.pfx
${{ parameters.artifactsPublishingAdditionalParameters }}
/p:PDBArtifactsBasePath='$(Build.ArtifactStagingDirectory)/PDBArtifacts/'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Promotion validation only exercised the dnceng promotion path, so the devdiv SC-missing failure that broke the Build Promotion Pipeline was never caught. Adds a pre-flight (eng/validation/validate-feed-publish-connections.ps1) that asserts the dnceng-artifact-feeds-publish service connection exists and is ready in BOTH dnceng and devdiv before the test promotion, and hardens GetFeedToken to fail fast if the minted token is not a JWT (which would silently fall back to PAT/Basic auth downstream).
Use StringSplitOptions.RemoveEmptyEntries in the JWT-vs-PAT detection (both C# publishers and the publish.yml GetFeedToken assertion) so malformed tokens like '..' are not misclassified as JWTs. URL-encode the endpoint name in the feed-publish SC pre-flight query string.
Both publishers allocate the HttpClient before token acquisition and throw from the catch if it fails, so Dispose() never runs and the handler/sockets leak on repeated failures. Dispose the client in the catch before rethrowing.
…empty, matching V3
|
Kicked off a fresh official build on the current HEAD (
Once it succeeds and registers a BAR build ID, I'll recreate the Maestro Build Promotion validation against it (channel |
|
✅ Promotion validation passed — reproduced mmitche's earlier test end-to-end against the fixed branch.
Result:
The Entra/WIF publish path (null-guard + V4 |
|
Clarification / correction on my previous comment about the "PartiallySucceeded" status: The overall status is a rollup of two non-blocking things, neither introduced by this PR:
These same two errors appear in the earlier promotion run 3029006 (for the build it promoted, 3028944), so they are pre-existing — these arcade builds don't produce separate The material difference vs. 3029006: that run failed at |
|
Looks like that broke arcade publish: https://dev.azure.com/dnceng/internal/_build/results?buildId=3029933&view=results |
Summary
Re-lands #17113 (migrating AzDO feed publishing from the
dn-bot-all-orgs-artifact-feeds-rwPAT to Entra/WIF auth), which was reverted in #17136 — but with a simpler auth flow based on review feedback.Instead of pre-acquiring a token via a dedicated service connection, the Build Promotion publish step now publishes packages using the existing
maestro-build-promotionidentity that the step already runs under. The feed publisher acquires an Azure DevOps token on demand viaDefaultIdentityTokenCredential, so there is no separately pre-acquired token that could expire mid-publish on a large build.Related: dnceng WI 10145 — [Packaging] PAT Migration:
dn-bot-all-orgs-artifact-feeds-rw.Auth flow (what actually happens now)
Publish packages, blobs and symbolstask runs underazureSubscription: maestro-build-promotionwithaddSpnToEnvironment: true.AzureDevOpsNugetFeedAssetPublisher/PublishArtifactsInManifestBaseacquire an AzDO token viaDefaultIdentityTokenCredential(supportsAzurePipelinesCredentialfromAzureCLI@2, managed identity, and workload identity). Tokens are obtained as needed rather than pinned to one ~1h token.NuGetAuthenticate@1is retained for the ambient NuGet credential provider.There is no
GetFeedTokenstep and noAzureDevOpsFeedsKeypass-through anymore.Out-of-repo infrastructure (already done)
The
maestro-build-promotionidentity (appId6e870007-e236-4eb1-8734-8bf8cd54c748) was granted Contributor (push) on the shipping feeds:dotnet-eng,dotnet-tools,dotnet-eng-internal,dotnet-tools-internalgeneral-testing(used by the promotion-validation test that runs in the official build'sValidateSdkstage)The same service principal backs the
maestro-build-promotionservice connection in both dnceng and devdiv, so a single set of feed grants covers both orgs where the promotion pipeline runs.What was removed vs. #17113
GetFeedTokenAzureCLI@2step and theAzureDevOpsFeedsKeyargument ineng/publishing/v3/publish.yml.dnceng-artifact-feeds-publishcross-org service connections and the pre-flight validation step (eng/validation/validate-feed-publish-connections.ps1deleted; the corresponding step removed fromeng/validate-promotion.yml).Validation
general-testingfeed using themaestro-build-promotionidentity.main; the definitive proof is the post-merge promotion build — that should be monitored to confirm the publish step authenticates in both dnceng and devdiv.