Repository navigation
[release/9.0] Backport refreshable Helix Entra authentication - #17563
missymessa merged 4 commits into
Conversation
Copilot-Session: e890b71a-c1aa-416c-a15c-be8da9fdd9b4 Copilot-Session: 0e1a942f-a44f-4e3a-8d35-af3fe8bee535 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a6ad3a92-2023-4c67-8948-f6d34de1a67c Copilot-Session: 521e657d-a005-4f60-bcff-25a05ebcc390 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4aa6b2e8-2313-40c0-92f6-1d578db0af15 Copilot-Session: a891597e-6dca-4706-8628-004c5837d0c9
There was a problem hiding this comment.
🟡 Changes recommended
The Azure service-connection login is not reliably available to the subsequent MSBuild process or consumable by DefaultAzureCredential.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Backports refreshable Entra authentication for the Helix SDK and Azure Pipelines on release/9.0.
Changes:
- Adds
TokenCredentialauthentication, scopes, and refresh tests. - Propagates Entra authentication through SDK tasks and client APIs.
- Integrates Azure service connections, source-build handling, and documentation.
File summaries
| File | Summary |
|---|---|
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.props |
Defines the authentication default. |
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets |
Propagates Entra settings to multi-queue tasks. |
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets |
Propagates Entra settings to job submission. |
src/Microsoft.DotNet.Helix/Sdk/tools/download-results/DownloadFromResultsContainer.targets |
Propagates authentication to downloads. |
src/Microsoft.DotNet.Helix/Sdk/SendHelixJob.cs |
Updates creator validation. |
src/Microsoft.DotNet.Helix/Sdk/Readme.md |
Documents authentication options. |
src/Microsoft.DotNet.Helix/Sdk/Microsoft.DotNet.Helix.Sdk.csproj |
Adds Azure Identity and source-build handling. |
src/Microsoft.DotNet.Helix/Sdk/HelixTask.cs |
Creates authenticated Helix clients. |
src/Microsoft.DotNet.Helix/Sdk/GetHelixWorkItems.cs |
Preserves PAT file-link behavior. |
src/Microsoft.DotNet.Helix/Sdk/CancelHelixJob.cs |
Supports Entra cancellation. |
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests.csproj |
Adds client test references. |
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/HelixApiAuthenticationTests.cs |
Tests scopes, refresh, and task authentication. |
src/Microsoft.DotNet.Helix/Client/CSharp/HelixApiOptions.cs |
Adds authentication modes and scopes. |
src/Microsoft.DotNet.Helix/Client/CSharp/ApiFactory.cs |
Adds Entra client factories. |
eng/common/core-templates/steps/send-to-helix.yml |
Adds service-connection integration. |
eng/common/core-templates/job/job.yml |
Clarifies token handling. |
Documentation/AzureDevOps/SendingJobsToHelix.md |
Documents Entra usage. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Result downloads do not yet receive the configured Helix base URI, and Creator-validation coverage is incomplete.
Review details
Suppressed comments (2)
src/Microsoft.DotNet.Helix/Sdk/SendHelixJob.cs:170
- The new authentication decision also changes the Creator contract (anonymous requires it; PAT/Entra forbid it), but the added authentication tests do not exercise
GetCreatorValidationErrorat all. Please add a theory covering anonymous, PAT, and Entra with both empty and populated Creator values so this boundary cannot regress silently.
string creatorValidationError = GetCreatorValidationError(
UseEntraAuthentication,
AccessToken,
Creator);
src/Microsoft.DotNet.Helix/Sdk/tools/download-results/DownloadFromResultsContainer.targets:29
- This task still uses HelixTask's default
https://helix.dot.net/becauseBaseUriis not passed here. WhenHelixBaseUritargetshttps://helix.int-dot.net, the job is sent to staging but this follow-up query goes to production and selects the wrong Entra scope, so result downloads fail. Pass the same base URI used by the other Helix tasks.
UseEntraAuthentication="$(HelixUseEntraAuthentication)"
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Backports refreshable Helix Entra authentication to
release/9.0for AB#12269:TokenCredential-based Helix client foundation and production/staging scopes.send-to-helix.ymlAzure service-connection integration.For release/9.0, the Helix SDK uses the branch's existing
Azure.Identitydependency rather than importing the main-onlyMicrosoft.DotNet.ArcadeAzureIntegrationproject. It constructsAzurePipelinesCredentialdirectly when the service-connection/OIDC environment is available, matching the proven Arcade credential-selection pattern, and otherwise falls back toDefaultAzureCredentialfor managed identity and local Azure CLI scenarios. Source-build remains explicitly unsupported for Entra authentication.Release-specific omissions
The release/9.0 branch does not contain the main-era standalone JobMonitor project or template, so this intentionally omits:
src/Microsoft.DotNet.Helix/JobMonitor/*eng/common/core-templates/job/helix-job-monitor.ymlNo unused JobMonitor project or template was added.
Validation
net472andnet9.0): succeeded with 0 warnings and 0 errors.HelixApiAuthenticationTests: 33 passed, 0 failed, 0 skipped.