Speed up AAD setup by reconciling transient Graph 404s - #5888
Merged
Mikael Weaver (mikaelweave) merged 5 commits intoOct 2, 2026
Merged
Conversation
When a delegated-permission POST fails ambiguously (Directory_ObjectNotFound or an already-exists conflict), the read-only reconciliation GET can also return HTTP 404 while the new client service principal replicates. That read error escaped immediately, surfaced the original POST error, failed the AAD setup task, and triggered a full task retry (build 51871: +3m18s replay). Retry only HTTP 404 reconciliation reads whose Graph code is absent, Request_ResourceNotFound, or Directory_ObjectNotFound, with bounded backoff (5, 10, 20, 40 seconds). Other read errors and exhausted retries still surface the original POST error, and a grant is re-POSTed only after a successful read confirms it is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add "AAD setup timing:" lines for the setup phases and for each of the test client applications (lookup, credential, delegated grant, Key Vault write, role assignment, and scratch SecretManagement round-trip time), so pipeline runs can show where the setup task spends its time. The lines contain only durations, positions, and test-configuration keys; no secrets, Graph identifiers, headers, or response bodies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mikael Weaver (mikaelweave)
added
Build
No-PaaS-breaking-change
No-ADR
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
The test introduced after the xUnit v3 migration still used SkippableTheory and Skip.IfNot, which fail to compile under the current test framework. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Each client staged its secret and app ID through the default SecretManagement vault (the test Key Vault) with Set-Secret/Get-Secret before writing the real app--<id>--* entries. That cost four Key Vault round trips per client (43-52s across 15 clients in builds 51877/51880) and left secretSecure/appIdSecure copies that e2e-set-variables exports with every other test Key Vault secret. Wrap the in-memory values with ConvertTo-SecureString instead. The values written to Key Vault are unchanged; Set-FhirServerApiUsers still uses the registered vault, so the registration stays. Drop the scratchVaultMs timing field, which no longer measures anything. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5888 +/- ##
=======================================
Coverage ? 79.05%
=======================================
Files ? 1020
Lines ? 37428
Branches ? 5728
=======================================
Hits ? 29589
Misses ? 6393
Partials ? 1446 🚀 New features to boost your workflow:
|
New client applications now request their password credential in the same New-MgApplication (POST /applications) call, using the documented passwordCredentials create body, and read secretText once from the 201 response. This removes the separate Add-MgApplicationPassword call and its propagation retry loop (18 waits, roughly 136s, across 15 clients in build 51884), which existed only because the new application object was not yet addressable for addPassword. If Graph does not return a secret, fail explicitly without echoing any credential data. The existing-application rotation path, service principal creation/retry, and Key Vault secret writes are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Mikael Weaver (mikaelweave)
added
the
Open source
Fernando Henrique Inocêncio Borba Ferreira (fhibf)
approved these changes
Oct 2, 2026
Mikael Weaver (mikaelweave)
deleted the
personal/mikaelw/speed-up-aad-setup
branch
October 2, 2026 14:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Evidence
In PR Build & Deploy 51871, Setup AAD took 13m12s. A
User.Readconsent POST returnedDirectory_ObjectNotFound, then its reconciliation GET returned NotFound, causing the entire task to retry and replay earlier client provisioning. The retry path is reproduced locally against the real PowerShell function and a fake Graph endpoint; it fails before this fix and passes afterward. Other read errors still fail fast.This follows merged #5887 on a fresh branch from
main. No package or SQL fixture updates are included. A live PR run will establish the new phase timings; it may not hit the intermittent Graph race on every execution.AB#208533