Fix PIM role management policy update bug

[EronWright]

Fixes: https://github.com/pulumi/pulumi-azure-native/actions/runs/19138381455/job/54700360112

Problem

The TestAccPIMRoleManagementPolicies test was failing in CI because PIM policy updates were not working correctly. When a user modified a rule property (e.g., changing maximumDuration from P365D to P90D), the update appeared to succeed but the value in Azure remained unchanged.

Root Cause

The bug was in the restoreDefaultsForDeletedRules function in custom_pim.go. PIM policies in Azure typically have 15-20 rules, but users only specify a few rules they want to customize in their Pulumi programs. The provider is supposed to:

1. Read the full policy from Azure (with all rules) during CREATE
2. Save that as "originalState"
3. During UPDATE, send ALL rules to Azure (user-specified + unspecified)

The bug: restoreDefaultsForDeletedRules was iterating over oldRules (rules from the previous Pulumi state) instead of origRules (rules from Azure's original state). This meant rules that were never specified by the user were NOT included in update requests, resulting in an incomplete policy update that Azure would silently ignore.

Solution

Changed the iteration in restoreDefaultsForDeletedRules from:

for id := range oldRules {  // ❌ Only previously managed rules

to:

for id, origRule := range origRules {  // ✅ All rules from Azure

This ensures ALL rules from Azure's original state are included in every update, not just the ones previously specified by the user.

Testing

• ✅ Added new test case "restores rules never specified by user" that specifically tests this scenario
• ✅ All existing PIM unit tests pass
• ✅ All role management policy tests pass

Example

Before this fix, the following scenario would fail:

const policy = new pim.RoleManagementPolicy("policy", {
    roleManagementPolicyName: policyId,
    scope: `subscriptions/${subscriptionId}`,
    rules: [{
        id: "Expiration_Admin_Eligibility",
        maximumDuration: "P365D",  // Step 1: Set to 365 days
        // ... other properties
    }]
});

Then updating to:

rules: [{
    id: "Expiration_Admin_Eligibility", 
    maximumDuration: "P90D",  // Step 2: Change to 90 days - FAILED to apply
    // ... other properties
}]

The update would complete without error, but Azure would still have P365D because the other ~15 rules weren't included in the request.

After this fix, all rules are properly preserved and the update succeeds.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[github-actions]

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.37%. Comparing base (3524e7a) to head (001863e).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...ovider/pkg/resources/customresources/custom_pim.go	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4398   +/-   ##
=======================================
  Coverage   59.37%   59.37%           
=======================================
  Files          91       91           
  Lines       11448    11444    -4     
=======================================
- Hits         6797     6795    -2     
+ Misses       4015     4014    -1     
+ Partials      636      635    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blampe]

I assume this was a failing test that reproduced the problem?

[EronWright]

Yes, a test failed and this was the result of the analysis. Unfortunately, the problem is intermittent; a re-run was successful.

[Copilot]

Pull Request Overview

This PR refactors the restoreDefaultsForDeletedRules function to change its behavior from restoring rules that were in olds (user-specified) to restoring rules from the original Azure state, regardless of whether they were ever specified by the user. This ensures that unspecified rules from Azure are preserved during updates.

• Simplified logic by removing the need to check oldRules and instead directly iterating over origRules
• Changed comment to reflect the new behavior
• Added comprehensive test case covering the scenario where user never specified certain rules

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
provider/pkg/resources/customresources/custom_pim.go	Refactored restoreDefaultsForDeletedRules to iterate over original state rules instead of old user-specified rules, removing unnecessary conditionals
provider/pkg/resources/customresources/custom_pim_test.go	Added test case verifying that rules never specified by the user are correctly restored from original Azure state

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pulumi-bot]

This PR has been shipped in release v3.11.0.