Skip to content

Use Entra credential for symbol upload, removing dnceng-symbol-server-pat#16688

Open
missymessa wants to merge 4 commits intomainfrom
missymessa-10150
Open

Use Entra credential for symbol upload, removing dnceng-symbol-server-pat#16688
missymessa wants to merge 4 commits intomainfrom
missymessa-10150

Conversation

@missymessa
Copy link
Copy Markdown
Member

Summary

Migrates the symbol upload step in the Arcade publishing pipeline from PAT-based authentication (dnceng-symbol-server-pat) to Entra-based authentication via DefaultIdentityTokenCredential.

Changes

PublishArtifactsInManifestBase.cs

  • In CreatePublishSymbolHelper(), when TempSymbolsAzureDevOpsOrgToken is empty/null, use DefaultIdentityTokenCredential (the same credential already used for symbol promotion) instead of PATCredential
  • When TempSymbolsAzureDevOpsOrgToken is provided, fall back to PATCredential for backward compatibility during rollout
  • Added Azure.Core using for the TokenCredential base type

eng/publishing/v3/publish.yml

  • Removed the DotNet-Symbol-Server-Pats variable group (no longer needed)
  • Removed /p:TempSymbolsAzureDevOpsOrgToken MSBuild property -- the step already runs inside AzureCLI@2 with azureSubscription: maestro-build-promotion, so the DefaultIdentityTokenCredential will use that identity

eng/common/core-templates/steps/publish-logs.yml

  • Removed dnceng-symbol-server-pat from the binlog redaction list (variable no longer in scope)

Context

The SymbolPublisherOptions class already accepts Azure.Core.TokenCredential -- the PATCredential was just a TokenCredential wrapper around the raw PAT string. The symbol promotion code already uses DefaultIdentityTokenCredential (Entra). This change extends the same pattern to symbol upload.

Prerequisite: The maestro-build-promotion service principal must have symbol management permissions in the dnceng org.

Fixes AB#10150

…-pat dependency

When TempSymbolsAzureDevOpsOrgToken is not provided, use DefaultIdentityTokenCredential
(the same credential already used for symbol promotion) instead of PATCredential for
symbol uploads. This enables the pipeline to use the AzureCLI@2 task's federated identity
(maestro-build-promotion) for symbol management, eliminating the need for the
dnceng-symbol-server-pat PAT.

- PublishArtifactsInManifestBase.cs: Fall back to DefaultIdentityTokenCredential when
  TempSymbolsAzureDevOpsOrgToken is empty/null; retain PATCredential for backward compat
- publish.yml: Remove DotNet-Symbol-Server-Pats variable group and TempSymbolsAzureDevOpsOrgToken
- publish-logs.yml: Remove dnceng-symbol-server-pat from redaction list

Fixes: AB#10150
FrozenSet<string> exclusions = LoadExclusions(symbolPublishingExclusionsFile);
PATCredential creds = new(TempSymbolsAzureDevOpsOrgToken);

TokenCredential creds = string.IsNullOrEmpty(TempSymbolsAzureDevOpsOrgToken)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not positive this will work. You'll need to ensure that this identity works in the devdiv version of the pipeline.

Copy link
Copy Markdown
Member Author

@missymessa missymessa Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this against the DevDiv-side setup:

  • eng/publishing/v3/publish.yml runs the publish step under AzureCLI@2 with azureSubscription: maestro-build-promotion.
  • There is a matching maestro-build-promotion azurerm service connection in both dnceng/internal and devdiv/DevDiv.
  • Both point at the same backing app ID: 6e870007-e236-4eb1-8734-8bf8cd54c748 (maestro-build-promotion-mi), and the DevDiv one is isReady=true.

So the DevDiv variant should pick up the same federated identity path as the dnceng pipeline. I also kept the code-side fallback to PATCredential when TempSymbolsAzureDevOpsOrgToken is explicitly provided, so there is still a rollout escape hatch if needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Copilot's comment is lying, I need to do some validation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants