Merge pull request #407 from DarkArc-Github/fix_binding_reapply

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

Author: span786

REFERENCE.md

@@ -673,6 +673,56 @@ The sslflags parameter accepts integer values from 0 to 3 inclusive.
   the centralized SSL certificate store while requiring Server Name
   Indicator
 
+For non-web protocols, the bindinginformation format varies by protocol:
+
+**net.pipe protocol:**
+- Format: `hostname` (hostname only, no port or colons)
+- Example: `'bindinginformation' => 'hostname'`
+
+**net.tcp protocol:**
+- Format: `port:hostname` (port number required, followed by colon and hostname)
+- Example: `'bindinginformation' => '808:hostname'`
+
+**net.msmq protocol:**
+- Format: `hostname` (hostname only, no port or colons)
+- Example: `'bindinginformation' => 'hostname'`
+
+**msmq.formatname protocol:**
+- Format: `hostname` (hostname only, no port or colons)
+- Example: `'bindinginformation' => 'hostname'`
+
+**Note:** SSL-related parameters (sslflags, certificatehash, certificatestorename) 
+are only valid for HTTPS protocol bindings and will cause validation errors 
+if used with other protocols.
+Example with multiple protocols:
+```puppet
+bindings => [
+  {
+    'bindinginformation' => '*:80:',
+    'protocol'           => 'http',
+  },
+  {
+    'bindinginformation'   => '*:443:hostname',
+    'certificatehash'      => 'ABCDEF1234567890ABCDEF1234567890ABCDEF12',
+    'certificatestorename' => 'MY',
+    'protocol'             => 'https',
+    'sslflags'             => 1,
+  },
+  {
+    'bindinginformation' => 'hostname',
+    'protocol'           => 'net.pipe',
+  },
+  {
+    'bindinginformation' => '808:hostname',
+    'protocol'           => 'net.tcp',
+  },
+  {
+    'bindinginformation' => 'hostname',
+    'protocol'           => 'net.msmq',
+  },
+]
+```
+
 ##### `defaultpage`
 
 Specifies the default page of the site.
@@ -846,4 +896,3 @@ The name of the virtual directory to manage
 
 The specific backend to use for this `iis_virtual_directory` resource. You will seldom need to specify this --- Puppet
 will usually discover the appropriate provider for your platform.
-

lib/puppet/type/iis_site.rb

@@ -5,7 +5,6 @@
 require_relative '../../puppet_x/puppetlabs/iis/property/hash'
 require_relative '../../puppet_x/puppetlabs/iis/property/path'
 require_relative '../../puppet_x/puppetlabs/iis/property/authenticationinfo'
-require_relative '../../puppet_x/puppetlabs/iis/bindings'
 
 Puppet::Type.newtype(:iis_site) do
   @doc = "Allows creation of a new IIS Web Site and configuration of site
@@ -156,17 +155,8 @@ def insync?(is)
       value
     end
 
-    def should
-      PuppetX::PuppetLabs::IIS::Bindings.sort_bindings(super)
-    end
-
-    def should=(values)
-      super
-      @should = PuppetX::PuppetLabs::IIS::Bindings.sort_bindings(@should)
-    end
-
     def insync?(is)
-      PuppetX::PuppetLabs::IIS::Bindings.sort_bindings(is) == should
+      (is || []).to_set == (should || []).to_set
     end
   end
 

lib/puppet_x/puppetlabs/iis/bindings.rb

@@ -137,6 +137,40 @@
         'bindinginformation' => '*:80:'
       }
     end
+
+    context 'order independent comparison' do
+      let(:bindings_property) { resource.property(:bindings) }
+
+      before(:each) do
+        resource[:bindings] = [
+          { 'protocol' => 'http', 'bindinginformation' => '*:80:' },
+          { 'protocol' => 'net.msmq', 'bindinginformation' => 'hostname' },
+        ]
+      end
+
+      it 'considers same bindings in different order as in sync' do
+        current_bindings = [
+          { 'protocol' => 'net.msmq', 'bindinginformation' => 'hostname' },
+          { 'protocol' => 'http', 'bindinginformation' => '*:80:' },
+        ]
+        expect(bindings_property.insync?(current_bindings)).to be true
+      end
+
+      it 'considers different bindings as out of sync' do
+        current_bindings = [
+          { 'protocol' => 'net.msmq', 'bindinginformation' => 'hostname' },
+          { 'protocol' => 'http', 'bindinginformation' => '*:8080:' },
+        ]
+        expect(bindings_property.insync?(current_bindings)).to be false
+      end
+
+      it 'considers different number of bindings as out of sync' do
+        current_bindings = [
+          { 'protocol' => 'http', 'bindinginformation' => '*:80:' },
+        ]
+        expect(bindings_property.insync?(current_bindings)).to be false
+      end
+    end
   end
 
   context 'property :limits' do