Fix: Unordered IIS Site Binding Comparison to Prevent Unnecessary Resource Updates

[DarkArc-Github]

Fix IIS Site Binding Comparison to Prevent Unnecessary Resource Updates

Summary

This pull request modifies the insync? method in the bindings property of the iis_site resource type to use a set-based comparison instead of an order-dependent array comparison. This change prevents Puppet from detecting false differences and unnecessarily reapplying bindings when the only difference is the order in which IIS returns the bindings.

The specific change:

• Modified the insync? method in Puppet/iis/lib/puppet/type/iis_site.rb to convert binding arrays to sets before comparison, making the comparison order-independent

Additional Context

• Root cause and the steps to reproduce:

  • Root cause: The current implementation compares bindings as ordered arrays, but the order of bindings returned by IIS can vary between runs, especially for bindings with the same binding information but different protocols (such as net.msmq and msmq.formatname).
  • To reproduce: Create an IIS site with multiple binding types, including at least one pair of non-HTTP bindings with the same binding information (e.g., net.msmq and msmq.formatname both using the same hostname). Run Puppet multiple times and observe that it detects changes and reapplies the bindings on every run.

• Thought process behind the implementation:

  • I considered several approaches to fix this issue:
    1. Modifying the sort_bindings method to ensure a stable sort
    2. Creating a custom provider that overrides the binding comparison
    3. Using a set-based comparison in the insync? method
  • I chose the set-based approach because:
    • It's the simplest and most direct solution
    • It aligns with how IIS actually works (binding order doesn't affect functionality)
    • It's the least invasive change with the lowest risk of unintended side effects
    • It's more robust against variations in how IIS or PowerShell returns bindings

Related Issues

This PR addresses an issue where IIS sites with multiple binding types would be unnecessarily updated on every Puppet run, causing potential service disruptions and increased Puppet run times.

Checklist

• 🟢 Spec tests: Tested locally unit test log
• 🟢 Acceptance tests: NA as it already tests for idempotency.
• 🟢 Manually verified: Tested on a Windows Server with IIS and confirmed that after this change, Puppet correctly recognizes when bindings are in sync, even if their order varies

[CLAassistant]

All committers have signed the CLA.

[DarkArc-Github]

@span786 Can you please review this?

do you want me to add changelog and metadata updates as well to this PR ?

[span786]

@DarkArc-Github

Could you please add the tests?
Also, I noticed some rubocop violations—please address those as well.

[DarkArc-Github]

@DarkArc-Github

Could you please add the tests? Also, I noticed some rubocop violations—please address those as well.

Thanks for the reply, I don't see the spec jobs running when I push, so I can't find those issues when I make changes, I saw that job only when you commented so can you point me in the right direction please how can I get those jobs running spec tests for my push ?

[span786]

@DarkArc-Github
Could you please add the tests? Also, I noticed some rubocop violations—please address those as well.

Thanks for the reply, I don't see the spec jobs running when I push, so I can't find those issues when I make changes, I saw that job only when you commented so can you point me in the right direction please how can I get those jobs running spec tests for my push ?

You can check the test steps, and follow the same while running tests in your local

[DarkArc-Github]

@span786 Added unit test and removed the unused and unreferenced bindings class and tested locally (attached the spec run log here for your reference).
Added details to reference markdown as well for easy lookup.

• Unit Spec test passes locally. spec_test.log
• Tested locally by applying on test node.