Remove OneLoc PAT dependencies - #17455
Conversation
Import the legacy variable group only when the Ceapex or GitHub PAT fallback can be selected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc4e1c74-f3b4-4965-932e-5e63cff7e877
There was a problem hiding this comment.
🟢 Approval recommended
Pull request overview
This PR updates the OneLocBuild job template to avoid attaching the OneLocBuildVariables variable group (and thereby hydrating legacy PAT secrets) when the pipeline configuration will use the newer Ceapex WIF/Entra token and/or GitHub App authentication paths instead of PAT fallback.
Changes:
- Conditioned inclusion of the
OneLocBuildVariablesvariable group so it’s only imported when Ceapex PAT fallback and/or GitHub PAT fallback may be used. - Preserved existing fallback behavior for scenarios that still require legacy PAT-based auth.
File summaries
| File | Description |
|---|---|
| eng/common/core-templates/job/onelocbuild.yml | Conditionally imports the OneLoc variable group to avoid downloading unused legacy PAT secrets when modern auth paths are selected. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Simplified the condition in commit 6fac367f6. The variable group is needed in exactly three template-selected cases: (1) projects outside internal/DevDiv, where Ceapex still selects CeapexPat; (2) an explicit empty CeapexServiceConnection; or (3) a GitHub job with UseGitHubAppAuthentication disabled. The prior external-project GitHub clause was redundant because case 1 already imports the group. I also replaced the paired project comparisons with notIn(...). The simplified condition is truth-table equivalent to the previous expression across all 48 combinations. I'll hold the release backports until main is resolved. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc4e1c74-f3b4-4965-932e-5e63cff7e877
|
Push completed after rebasing onto the latest PR branch; the simplification is now commit e50fb29. |
There was a problem hiding this comment.
🟡 Changes recommended
The new condition uses notIn() in a ${{ if }} template expression, which is not supported and is likely to break template expansion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🟢 Approval recommended
The change is a small, syntactically-correct YAML template adjustment that aligns with existing conditional auth selection logic and reduces unnecessary secret hydration without changing the selected auth paths.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
@mmitche please take a look! |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The implementation removes opt-out/legacy fallback behaviors that the PR description still claims are preserved, and the docs should explicitly call out the newly enforced supported-project restriction.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
missymessa
changed the title
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently remove PAT fallback paths, add clear enforcement for supported projects and required WIF inputs, and update documentation to match the new behavior.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
OneLocBuildVariablesvariable group and its Ceapex and GitHub PAT dependenciesdnceng/internalandDevDiv/DevDivprojectsTracks AB#10151 and AB#12331.