Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Select an option

  • Save soulemike/2bd7afae2bb9a354f257d928c80e9672 to your computer and use it in GitHub Desktop.

Select an option

Save soulemike/2bd7afae2bb9a354f257d928c80e9672 to your computer and use it in GitHub Desktop.
PR 67 Recommendation: Use `Wait-EntraOpsServiceEMCondition` to Fix PIM Policy Race Condition

PR 67 Recommendation: Use Wait-EntraOpsServiceEMCondition to Fix PIM Policy Race Condition

Date: 2026-10-10 Issue: New-EntraOpsServicePIMPolicy.ps1 fails intermittently on newly created groups due to Graph API eventual consistency Existing Solution: Wait-EntraOpsServiceEMCondition (already used throughout ServiceEM cmdlets)


1. Yes — The Existing Method Solves This Problem

Wait-EntraOpsServiceEMCondition is a private helper (EntraOps/Private/Wait-EntraOpsServiceEMCondition.ps1) purpose-built for exactly this scenario. It polls a scriptblock with capped exponential backoff (0, 1, 3, 7, 15 ... up to MaxIntervalSeconds, default 30s) until the condition returns $true or MaxWaitSeconds (default 300s) elapses.

Where It Is Already Used Successfully

Cmdlet What It Waits For
New-EntraOpsServiceEMCatalogResource.ps1 Group to be indexed by Entitlement Management after creation (ResourceNotFoundInOriginSystem retries)
New-EntraOpsServiceEMCatalogResourceRole.ps1 Catalog role assignments to be queryable after POST
New-EntraOpsServiceEMCatalog.ps1 Catalog object to be queryable after creation
New-EntraOpsServiceEMAssignmentPolicy.ps1 Assignment policies to be queryable after POST
New-EntraOpsServiceAZContainer.ps1 Azure resource group to be queryable after creation
New-EntraOpsServiceEMAssignment.ps1 Assignment request state to reach delivered

The PIM policy cmdlet is the only ServiceEM cmdlet that does not use this helper, and it is the only one that fails intermittently on newly created objects.


2. Root Cause Recap

In New-EntraOpsServicePIMPolicy.ps1 (lines 197-199), there is an incorrect assumption:

# Update-MgPolicyRoleManagementPolicy updates policy rules synchronously — there is no
# eventual consistency to wait for. The PolicyId (assignment) never changes across the
# update, so a retry loop comparing PolicyIds is both unnecessary and fragile under
# transient Graph timeouts.

This is wrong. The roleManagementPolicy object itself is not provisioned synchronously. When a new role-assignable group is created:

  1. ✅ Group object is created immediately
  2. ✅ roleManagementPolicyAssignment objects appear and are queryable
  3. ❌ The underlying roleManagementPolicies/{policyId} object is NOT immediately available for PATCH

The race condition manifests as:

  • Slow Graph → GET returns empty → code skips PATCH with a warning → success
  • Fast Graph → GET finds the assignment → PATCH hits NotFound on the policy object → fatal throw

This is exactly the same pattern that New-EntraOpsServiceEMCatalogResource.ps1 handles for ResourceNotFoundInOriginSystem.


3. Specific Recommendations for PR 67

Recommendation A: Replace the comment and add Wait-EntraOpsServiceEMCondition for the PATCH

File: EntraOps/Public/ServiceEM/New-EntraOpsServicePIMPolicy.ps1

Current code (lines 164-185):

try {
    Write-Verbose "$logPrefix Looking up PIM Policies with Assignments"
    $groupPolicyAssignment = Invoke-EntraOpsMsGraphQuery -Method GET -Uri "/v1.0/policies/roleManagementPolicyAssignments?`$filter=scopeId eq '$($group.Id)' and scopeType eq 'Group'" -OutputType PSObject
    $groupPolicyAssignments += $groupPolicyAssignment
} catch {
    Write-Warning "$logPrefix Failed to find PIM Policies with Assignments for group $($group.Id). Ensure the caller has RoleManagementPolicy.ReadWrite.Directory or RoleManagement.Read.Directory permission. Error: $_"
    continue
}

$memberPolicy = $groupPolicyAssignment | Where-Object { $_.id -like "*member" }
if (-not $memberPolicy) {
    Write-Warning "$logPrefix No member policy assignment found for group $($group.Id). Skipping policy update."
    continue
}

try {
    Write-Verbose "$logPrefix Updating PIM Policy ID: $($memberPolicy.policyId)"
    Invoke-EntraOpsMsGraphQuery -Method PATCH -Uri "/v1.0/policies/roleManagementPolicies/$($memberPolicy.policyId)" -Body ($currentGroupPolicyParams | ConvertTo-Json -Depth 10) -ThrowOnFailure | Out-Null
} catch {
    Write-Warning "$logPrefix Failed to update PIM Policy for group $($group.Id). Error: $_"
    $policyUpdateFailures.Add("$($group.Id): $($_.Exception.Message)")
}

Recommended replacement:

# Wait for the roleManagementPolicyAssignment to exist (with -DisableCache to avoid stale empty results)
$assignmentState = @{ Assignment = $null }
$assignmentReady = Wait-EntraOpsServiceEMCondition -Activity "PIM policy assignment for $($group.DisplayName)" -MaxWaitSeconds 60 -logPrefix $logPrefix -Condition {
    try {
        $assignmentState.Assignment = Invoke-EntraOpsMsGraphQuery -Method GET -Uri "/v1.0/policies/roleManagementPolicyAssignments?`$filter=scopeId eq '$($group.Id)' and scopeType eq 'Group'" -OutputType PSObject -DisableCache
        $memberPol = $assignmentState.Assignment | Where-Object { $_.id -like "*member" }
        $null -ne $memberPol
    } catch {
        $false
    }
}

if (-not $assignmentReady) {
    Write-Warning "$logPrefix PIM policy assignment for group $($group.Id) did not appear within 60 seconds. Skipping policy update."
    continue
}

$groupPolicyAssignments += $assignmentState.Assignment
$memberPolicy = $assignmentState.Assignment | Where-Object { $_.id -like "*member" }

# Wait for the roleManagementPolicy object to be PATCHable (retry on NotFound)
$patchState = @{ Succeeded = $false }
$patchReady = Wait-EntraOpsServiceEMCondition -Activity "PIM policy update for $($group.DisplayName)" -MaxWaitSeconds 60 -logPrefix $logPrefix -Condition {
    try {
        Invoke-EntraOpsMsGraphQuery -Method PATCH -Uri "/v1.0/policies/roleManagementPolicies/$($memberPolicy.policyId)" -Body ($currentGroupPolicyParams | ConvertTo-Json -Depth 10) -ThrowOnFailure | Out-Null
        $patchState.Succeeded = $true
        $true
    } catch {
        if ($_.Exception.Message -like "*NotFound*") {
            Write-Verbose "$logPrefix PIM policy object not yet provisioned, will retry"
            $false
        } else {
            throw
        }
    }
}

if (-not $patchReady) {
    Write-Warning "$logPrefix Failed to update PIM Policy for group $($group.Id) after retries."
    $policyUpdateFailures.Add("$($group.Id): Policy object not provisioned or PATCHable within timeout")
}

Why this works:

  • The GET now uses -DisableCache, preventing the stale-empty-result race that caused pr67.16 to "succeed" by accident
  • The PATCH is wrapped in Wait-EntraOpsServiceEMCondition, which retries on NotFound with exponential backoff (0, 1, 3, 7, 15s)
  • Both waits are capped at 60 seconds, which is more than sufficient for Graph replication
  • Non-NotFound errors still throw immediately (e.g., permission errors)

Recommendation B: Remove the incorrect comment in the end block

Current (lines 197-199):

# Update-MgPolicyRoleManagementPolicy updates policy rules synchronously — there is no
# eventual consistency to wait for. The PolicyId (assignment) never changes across the
# update, so a retry loop comparing PolicyIds is both unnecessary and fragile under
# transient Graph timeouts.

Replace with:

# Policy assignments are queryable before the underlying policy object is provisioned.
# Wait-EntraOpsServiceEMCondition handles this eventual consistency during the process block.

Recommendation C: Add runtime license validation (optional but recommended)

In New-EntraOpsServiceBootstrap.ps1, before processing PIM policies, verify the tenant has the required license:

After line 306 (where $pimForGroupsSwitches is computed):

if ($pimForGroupsSwitches.Count -gt 0) {
    $hasPimLicense = $false
    try {
        $skus = Invoke-EntraOpsMsGraphQuery -Method GET -Uri "/v1.0/subscribedSkus" -OutputType PSObject -DisableCache
        $hasPimLicense = $skus.servicePlans | Where-Object {
            $_.servicePlanName -eq "Entra_Identity_Governance" -and $_.provisioningStatus -eq "Success"
        }
    } catch {
        Write-Verbose "$logPrefix Could not verify tenant licenses"
    }
    if (-not $hasPimLicense) {
        Write-Warning "$logPrefix PIM for Groups requires Microsoft Entra ID Governance or Microsoft Entra Suite license. Microsoft Entra ID P2 alone is not sufficient."
    }
}

This makes the warning conditional and accurate, rather than a static string that appears even when the license exists.

Recommendation D: Improve error message in New-EntraOpsServiceEMAccessPackageResourceAssignment.ps1

Current (line 170):

Write-Warning "$logPrefix Eligible Member role not found for '$($group.DisplayName)' in the catalog (group not yet managed by PIM for Groups, or no Microsoft Entra ID Governance license). No resource role is added to the access package, run the deployment again later."

Recommended:

$reason = if (-not $hasPimLicense) { "no Microsoft Entra ID Governance license" } else { "group not yet managed by PIM for Groups (eventual consistency)" }
Write-Warning "$logPrefix Eligible Member role not found for '$($group.DisplayName)' in the catalog ($reason). No resource role is added to the access package; run the deployment again later."

4. Prompts for PR Author

Use these prompts when submitting review feedback or opening follow-up issues:

Prompt 1 — Core Fix

Issue: New-EntraOpsServicePIMPolicy.ps1 fails intermittently with ResourceNotFound when PATCHing roleManagementPolicies/{policyId} on newly created groups.

Root cause: The cmdlet assumes policy objects are provisioned synchronously, but Graph creates the roleManagementPolicyAssignment before the underlying roleManagementPolicy is PATCHable.

Fix: Use the existing Wait-EntraOpsServiceEMCondition helper (already used in every other ServiceEM cmdlet) to retry the PATCH on NotFound with exponential backoff. Also add -DisableCache to the GET query for assignments.

Reference: See New-EntraOpsServiceEMCatalogResource.ps1 for the exact retry pattern used for ResourceNotFoundInOriginSystem.

Prompt 2 — Comment Correction

Issue: The comment in New-EntraOpsServicePIMPolicy.ps1 (lines 197-199) incorrectly states that "there is no eventual consistency to wait for."

Fix: Remove or correct this comment. The policy object IS subject to eventual consistency, which is why the proposed retry mechanism is necessary.

Prompt 3 — License Warning Accuracy

Issue: The warning emitted when -EnablePimForGroups or -EnableWorkloadPlanePimForGroups is used is a static string that appears regardless of actual tenant license status.

Fix: Query /v1.0/subscribedSkus at runtime to verify the presence of Entra_Identity_Governance before emitting the warning. This prevents confusion when the license exists but the operation fails for other reasons (e.g., the race condition above).

Prompt 4 — Error Message Clarity

Issue: The error message in New-EntraOpsServiceEMAccessPackageResourceAssignment.ps1 (line 170) conflates three distinct failure modes: eventual consistency, missing license, and missing catalog resource role.

Fix: Disambiguate the error message so operators know whether to wait, purchase a license, or re-run the deployment.


5. Verification Steps After Fix

  1. Run pr67.17 (-EnableWorkloadPlanePimForGroups) on a freshly created group 5+ times
  2. Confirm zero ResourceNotFound failures
  3. Confirm the cmdlet completes within the 60-second wait window (typical Graph replication is <15s)
  4. Run pr67.16 (-EnablePimForGroups) to ensure the fix does not regress the admin-tier path
  5. Verify that the GET query for assignments no longer relies on cache (use Fiddler or Graph Explorer to confirm no If-None-Match headers)

Recommendations generated from direct tenant inspection, code review, and end-to-end testing on 2026-10-10.

@Cloud-Architekt

Copy link
Copy Markdown

Implemented (in New-EntraOpsServicePIMPolicy)

  • Recommendation A, in a corrected form:
    • The member policy assignment is looked up with -DisableCache and retried with Wait-EntraOpsServiceEMCondition.
    • The policy update (PATCH) is retried only when Graph returns 404. The check uses Exception.Data['StatusCode'], because matching *NotFound* against the message text isn't reliable.
    • Any other error fails straight away.
  • Recommendation B, partly: the misleading comment in the end block was removed, along with the fallback lookup it described. That lookup could return a policy assignment whose policy was never updated.
  • Additions not in the gist:
    • A failed assignment lookup, or an assignment that never appears, is now counted as a failure. Before, the group was skipped with only a warning and left without a policy.
    • All groups are processed first, then the cmdlet throws one error listing every failed group.
    • Intermediate 404 warnings are suppressed, but the last Graph error is included in the final message.
    • Tests cover recovery after a 404, persistent 404, non-404 errors, assignment timeout, lookup failure, and processing that continues after one group fails.

Not implemented

  • Recommendation C (license check via /subscribedSkus): it needs an extra Graph permission. It also relies on a guessed service-plan name and adds a new way for the deployment to fail. The licensing claim needs separating as well: PIM for Groups itself versus eligible membership through access packages.
  • Recommendation D (more specific error message): it depends on the license check from C.
  • The 60-second limit is not a strict timeout: the helper counts only sleep time (up to about 87 seconds) and excludes API and retry time. Messages and docs therefore avoid promising a fixed limit.

Evidence
The gist doesn't include request traces or failure rates, so the specific race in Graph is still unconfirmed. The change is defensive: it is safe either way. Running pr67.17 with -Verbose on fresh groups would show whether the 404 retry is ever needed.

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