[release/10.0] Backport refreshable Helix Entra authentication - #17564
missymessa merged 7 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
Critical compatibility findings show the SDK cannot compile for its net472/net481 targets because DefaultIdentityTokenCredential is unavailable there; add the required guards or framework stub.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Backports refreshable Entra authentication across Helix SDK tasks, API clients, Job Monitor, and Azure Pipelines while retaining existing authentication modes.
Changes:
- Adds scoped, expiry-aware Entra authentication and refresh.
- Propagates settings through SDK targets, Job Monitor, and pipeline templates.
- Adds tests, documentation, and .NET 8 integration support.
File summaries
| File | Change |
|---|---|
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.props |
Defines authentication defaults. |
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets |
Propagates authentication settings. |
src/Microsoft.DotNet.Helix/Sdk/tools/Microsoft.DotNet.Helix.Sdk.MonoQueue.targets |
Propagates authentication settings. |
src/Microsoft.DotNet.Helix/Sdk/tools/download-results/DownloadFromResultsContainer.targets |
Passes authentication to result downloads. |
src/Microsoft.DotNet.Helix/Sdk/SendHelixJob.cs |
Updates authentication validation. |
src/Microsoft.DotNet.Helix/Sdk/Readme.md |
Documents Entra configuration. |
src/Microsoft.DotNet.Helix/Sdk/Microsoft.DotNet.Helix.Sdk.csproj |
Adds Azure integration reference. |
src/Microsoft.DotNet.Helix/Sdk/HelixTask.cs |
Selects PAT, anonymous, or Entra clients. |
src/Microsoft.DotNet.Helix/Sdk/GetHelixWorkItems.cs |
Preserves PAT file-link behavior. |
src/Microsoft.DotNet.Helix/Sdk/CancelHelixJob.cs |
Supports authenticated cancellation. |
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/QueueStatsLoggingTests.cs |
Tests creator validation. |
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests.csproj |
Adds client project references. |
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/HelixApiAuthenticationTests.cs |
Tests authentication and refresh behavior. |
src/Microsoft.DotNet.Helix/JobMonitor/Microsoft.DotNet.Helix.JobMonitor.csproj |
Adds Azure integration reference. |
src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorRunner.cs |
Selects and configures Entra authentication. |
src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorOptions.cs |
Adds CLI and environment configuration. |
src/Microsoft.DotNet.Helix/Client/CSharp/HelixApiOptions.cs |
Adds authentication modes, scopes, and bearer policy. |
src/Microsoft.DotNet.Helix/Client/CSharp/ApiFactory.cs |
Adds Entra client factories. |
src/Microsoft.DotNet.ArcadeAzureIntegration/Microsoft.DotNet.ArcadeAzureIntegration.csproj |
Adds the .NET 8 target. |
eng/common/core-templates/steps/send-to-helix.yml |
Configures pipeline Entra authentication. |
eng/common/core-templates/job/job.yml |
Clarifies token-variable behavior. |
eng/common/core-templates/job/helix-job-monitor.yml |
Adds monitor Entra configuration. |
Documentation/AzureDevOps/SendingJobsToHelix.md |
Documents pipeline authentication options. |
Review details
- Files reviewed: 23/23 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.
🟡 Changes recommended
Two unresolved critical issues affect job submission and bearer-token transport security.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Custom HTTPS Helix base URI handling remains unresolved for Entra authentication in JobMonitor and HelixTask.
Review details
Suppressed comments (2)
src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorRunner.cs:825
- The standalone monitor exposes
helixBaseUri, and PAT authentication accepts arbitrary base URIs, but the Entra path always selects default scope discovery here. For a custom HTTPS Helix host,HelixApiOptionsthrows because no default scope is known; expose an explicit Entra scope throughJobMonitorOptions/the template or document and enforce the supported hosts.
return ApiFactory.GetAuthenticatedWithEntra(
baseUri,
new DefaultIdentityTokenCredential());
src/Microsoft.DotNet.Helix/Sdk/HelixTask.cs:103
- With Entra enabled, a custom HTTPS
BaseUrireaches this overload, which only has default scopes forhelix.dot.netandhelix.int-dot.netand otherwise throws before making a request.ApiFactoryexposes an explicit-scope overload for custom Helix instances, butHelixTaskhas no scope input, so this new auth mode breaks the existingBaseUricustomization; thread the scope through the task/template or explicitly constrain the supported hosts.
return ApiFactory.GetAuthenticatedWithEntra(
baseUri,
new DefaultIdentityTokenCredential());
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed the latest Copilot review's suppressed custom-host findings in ee37b23. Both submission tasks and JobMonitor now accept an optional explicit Entra scope ( Validation:
|
There was a problem hiding this comment.
🟡 Changes recommended
The default-host download regression and public authentication-path compatibility issue remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/Microsoft.DotNet.Helix/Client/CSharp/HelixApiOptions.cs:70
- This changes the existing public
HelixApiOptions(Uri, TokenCredential)path for every credential that is notHelixApiTokenCredential: it used to go throughHelixApiTokenAuthenticationPolicywith empty scopes, but now every such credential is treated as Entra and gets a host-derived scope. Consumers that supplied their ownTokenCredentialfor the legacy policy can therefore fail on custom/HTTP hosts or receive a different authorization scheme; preserve the old overload semantics and expose Entra through a distinct constructor/factory, or document this as a breaking API change.
else
{
AuthenticationMode = HelixApiAuthenticationMode.EntraId;
if (TokenScopes.Count == 0)
{
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4aa6b2e8-2313-40c0-92f6-1d578db0af15
|
Addressed the suppressed public-constructor compatibility finding in aa6ddf6. Existing |
Summary
Backports refreshable Helix Entra authentication to
release/10.0for AB#12269:The backport also targets
Microsoft.DotNet.ArcadeAzureIntegrationfor$(NetMinimum)so the release/10.0net8.0JobMonitor can consume the shared credential implementation without raising its runtime requirement.Validation
dotnet test src\Microsoft.DotNet.Helix\Sdk.Tests\Microsoft.DotNet.Helix.Sdk.Tests\Microsoft.DotNet.Helix.Sdk.Tests.csproj --configuration Release --nologo --no-restoreMicrosoft.DotNet.Helix.JobMonitor(net8.0).