Changes to xSQLServerNetwork

Refactored some of the code, cleaned up the rest and fixed PSSA rules warnings (issue dsccommunity#261).
johlju · May 20, 2017 · 4d52a20 · 4d52a20
1 parent dff1555
commit 4d52a20
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,12 +45,12 @@
 - Changes to xSQLServerNetwork
   - Added optional parameter SQLServer with default value of $env:COMPUTERNAME (issue #528).
   - Added optional parameter RestartTimeout with default value of 120 seconds.
-  - Now the resource supports restarting a sql server in a cluster (issue #527).
+  - Now the resource supports restarting a sql server in a cluster (issue #527 and issue #455).
   - Now the resource allows to set the parameter TcpDynamicPorts to a blank value (partly fixes issue #534). Setting a blank value for parameter TcpDynamicPorts together with a value for parameter TcpPort means that static port will be used.
   - Now the resource will not call Alter() in the Set-TargetResource when there is no change necessary (issue #537).
   - Updated example 1-EnableTcpIpOnCustomStaticPort.
   - Added unit tests (issue #294).
-
+  - Refactored some of the code, cleaned up the rest and fixed PSSA rules warnings (issue #261).
 
 ## 7.0.0.0
 

diff --git a/DSCResources/MSFT_xSQLServerNetwork/MSFT_xSQLServerNetwork.psm1 b/DSCResources/MSFT_xSQLServerNetwork/MSFT_xSQLServerNetwork.psm1
@@ -1,80 +1,124 @@
 Import-Module -Name (Join-Path -Path (Split-Path (Split-Path $PSScriptRoot -Parent) -Parent) `
                                -ChildPath 'xSQLServerHelper.psm1') `
                                -Force
+<#
+    .SYNOPSIS
+    Returns the current state of the SQL Server network properties.
 
+    .PARAMETER InstanceName
+    The name of the SQL instance to be configured.
+
+    .PARAMETER ProtocolName
+    The name of network protocol to be configured. Only tcp is currently supported.
+#>
 Function Get-TargetResource
 {
     [CmdletBinding()]
     [OutputType([System.Collections.Hashtable])]
-    param(
-        [parameter(Mandatory = $true)]
+    param
+    (
+        [Parameter(Mandatory = $true)]
         [System.String]
         $InstanceName,
 
         # For now there is only support for the tcp protocol.
-        [parameter(Mandatory = $true)]
+        [Parameter(Mandatory = $true)]
         [ValidateSet('Tcp')]
         [System.String]
         $ProtocolName
     )
 
-    Try
+    try
     {
-        $dom_get = Register-SqlWmiManagement -SQLInstanceName $InstanceName
+        $applicationDomainObject = Register-SqlWmiManagement -SQLInstanceName $InstanceName
 
-        $wmi = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Wmi.ManagedComputer
+        $managedComputerObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Wmi.ManagedComputer
 
-        Write-Verbose "Getting [$ProtocolName] network protocol for [$InstanceName] SQL instance"
-        $tcp = $wmi.ServerInstances[$InstanceName].ServerProtocols[$ProtocolName]
+        Write-Verbose "Getting [$ProtocolName] network protocol for [$InstanceName] SQL instance."
+        $tcp = $managedComputerObject.ServerInstances[$InstanceName].ServerProtocols[$ProtocolName]
 
-        Write-Verbose "Reading state values:"
+        Write-Verbose "Reading current network properties."
         $returnValue = @{
             InstanceName = $InstanceName
             ProtocolName = $ProtocolName
             IsEnabled = $tcp.IsEnabled
-            TcpDynamicPorts = $tcp.IPAddresses["IPAll"].IPAddressProperties["TcpDynamicPorts"].Value
-            TcpPort = $tcp.IPAddresses["IPAll"].IPAddressProperties["TcpPort"].Value
+            TcpDynamicPorts = $tcp.IPAddresses['IPAll'].IPAddressProperties['TcpDynamicPorts'].Value
+            TcpPort = $tcp.IPAddresses['IPAll'].IPAddressProperties['TcpPort'].Value
         }
 
-        $returnValue.Keys | % { Write-Verbose "$_ = $($returnValue[$_])" }
-
+        $returnValue.Keys | ForEach-Object {
+            Write-Verbose "$_ = $($returnValue[$_])"
+        }
     }
-    Finally
+    finally
     {
-        Unregister-SqlAssemblies -ApplicationDomain $dom_get
+        Unregister-SqlAssemblies -ApplicationDomain $applicationDomainObject
     }
 
     return $returnValue
 }
 
+<#
+    .SYNOPSIS
+    Sets the SQL Server network properties.
+
+    .PARAMETER SQLServer
+    The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME.
+
+    .PARAMETER InstanceName
+    The name of the SQL instance to be configured.
+
+    .PARAMETER ProtocolName
+    The name of network protocol to be configured. Only tcp is currently supported.
+
+    .PARAMETER IsEnabled
+    Enables or disables the network protocol.
+
+    .PARAMETER TcpDynamicPorts
+    Set the value to '0' if dynamic ports should be used. If static port should be used set this to a empty string value.
+
+    .PARAMETER TcpPort
+    The TCP port that SQL Server should be listening on.
+
+    .PARAMETER RestartService
+    If set to $true then SQL Server and dependent services will be restarted if a change to the configuration is made. The default value is $false.
+
+    .PARAMETER RestartTimeout
+    Timeout value for restarting the SQL Server services. The default value is 120 seconds.
+#>
 Function Set-TargetResource
 {
     [CmdletBinding()]
-    param(
+    param
+    (
         [Parameter()]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $SQLServer = $env:COMPUTERNAME,
 
-        [parameter(Mandatory = $true)]
+        [Parameter(Mandatory = $true)]
         [System.String]
         $InstanceName,
 
-        [parameter(Mandatory = $true)]
+        [Parameter(Mandatory = $true)]
         [ValidateSet('Tcp')]
         [System.String]
         $ProtocolName,
 
+        [Parameter()]
         [System.Boolean]
         $IsEnabled,
 
-        [ValidateSet("0","")]
+        [Parameter()]
+        [ValidateSet('0','')]
         [System.String]
         $TcpDynamicPorts,
 
+        [Parameter()]
         [System.String]
         $TcpPort,
 
+        [Parameter()]
         [System.Boolean]
         $RestartService = $false,
 
@@ -83,11 +127,11 @@ Function Set-TargetResource
         $RestartTimeout = 120
     )
 
-    $currentState = Get-TargetResource -InstanceName $InstanceName -ProtocolName $ProtocolName
+    $getTargetResourceResult = Get-TargetResource -InstanceName $InstanceName -ProtocolName $ProtocolName
 
-    $dom_get = Register-SqlWmiManagement -SQLInstanceName $InstanceName
+    $applicationDomainObject = Register-SqlWmiManagement -SQLInstanceName $InstanceName
 
-    Try
+    try
     {
         $desiredState = @{
             InstanceName = $InstanceName
@@ -99,52 +143,86 @@ Function Set-TargetResource
 
         $isRestartNeeded = $false
 
-        $wmi = New-Object Microsoft.SqlServer.Management.Smo.Wmi.ManagedComputer
+        $managedComputerObject = New-Object Microsoft.SqlServer.Management.Smo.Wmi.ManagedComputer
 
         Write-Verbose "Getting [$ProtocolName] network protocol for [$InstanceName] SQL instance"
-        $tcp = $wmi.ServerInstances[$InstanceName].ServerProtocols[$ProtocolName]
+        $tcp = $managedComputerObject.ServerInstances[$InstanceName].ServerProtocols[$ProtocolName]
 
-        Write-Verbose "Checking [IsEnabled] property."
-        if($desiredState["IsEnabled"] -ine $currentState["IsEnabled"])
+        Write-Verbose 'Checking [IsEnabled] property.'
+        if ($desiredState['IsEnabled'] -ine $getTargetResourceResult['IsEnabled'])
         {
-            Write-Verbose "Updating [IsEnabled] from $($currentState["IsEnabled"]) to $($desiredState["IsEnabled"])"
-            $tcp.IsEnabled = $desiredState["IsEnabled"]
+            Write-Verbose "Updating [IsEnabled] from $($getTargetResourceResult['IsEnabled']) to $($desiredState['IsEnabled'])."
+            $tcp.IsEnabled = $desiredState['IsEnabled']
             $tcp.Alter()
 
             $isRestartNeeded = $true
         }
 
-        Write-Verbose "Checking [TcpDynamicPorts] property."
-        if($desiredState["TcpDynamicPorts"] -ine $currentState["TcpDynamicPorts"])
+        Write-Verbose 'Checking [TcpDynamicPorts] property.'
+        if ($desiredState['TcpDynamicPorts'] -ine $getTargetResourceResult['TcpDynamicPorts'])
         {
-            Write-Verbose "Updating [TcpDynamicPorts] from $($currentState["TcpDynamicPorts"]) to $($desiredState["TcpDynamicPorts"])"
-            $tcp.IPAddresses["IPAll"].IPAddressProperties["TcpDynamicPorts"].Value = $desiredState["TcpDynamicPorts"]
+            Write-Verbose "Updating [TcpDynamicPorts] from $($getTargetResourceResult['TcpDynamicPorts']) to $($desiredState['TcpDynamicPorts'])."
+            $tcp.IPAddresses['IPAll'].IPAddressProperties['TcpDynamicPorts'].Value = $desiredState['TcpDynamicPorts']
             $tcp.Alter()
 
             $isRestartNeeded = $true
         }
 
-        Write-Verbose "Checking [TcpPort property]."
-        if($desiredState["TcpPort"] -ine $currentState["TcpPort"])
+        Write-Verbose 'Checking [TcpPort property].'
+        if ($desiredState['TcpPort'] -ine $getTargetResourceResult['TcpPort'])
         {
-            Write-Verbose "Updating [TcpPort] from $($currentState["TcpPort"]) to $($desiredState["TcpPort"])"
-            $tcp.IPAddresses["IPAll"].IPAddressProperties["TcpPort"].Value = $desiredState["TcpPort"]
+            Write-Verbose "Updating [TcpPort] from $($getTargetResourceResult['TcpPort']) to $($desiredState['TcpPort'])."
+            $tcp.IPAddresses['IPAll'].IPAddressProperties['TcpPort'].Value = $desiredState['TcpPort']
             $tcp.Alter()
 
             $isRestartNeeded = $true
         }
 
-        if($RestartService -and $isRestartNeeded)
+        if ($RestartService -and $isRestartNeeded)
         {
             Restart-SqlService -SQLServer $SQLServer -SQLInstanceName $InstanceName -Timeout $RestartTimeout
         }
     }
-    Finally
+    finally
     {
-        Unregister-SqlAssemblies -ApplicationDomain $dom_get
+        Unregister-SqlAssemblies -ApplicationDomain $applicationDomainObject
     }
 }
 
+<#
+    .SYNOPSIS
+    Sets the SQL Server network properties.
+
+    .PARAMETER SQLServer
+    The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME.
+
+    Not used in Test-TargetResource.
+
+    .PARAMETER InstanceName
+    The name of the SQL instance to be configured.
+
+    .PARAMETER ProtocolName
+    The name of network protocol to be configured. Only tcp is currently supported.
+
+    .PARAMETER IsEnabled
+    Enables or disables the network protocol.
+
+    .PARAMETER TcpDynamicPorts
+    Set the value to '0' if dynamic ports should be used. If static port should be used set this to a empty string value.
+
+    .PARAMETER TcpPort
+    The TCP port that SQL Server should be listening on.
+
+    .PARAMETER RestartService
+    If set to $true then SQL Server and dependent services will be restarted if a change to the configuration is made. The default value is $false.
+
+    Not used in Test-TargetResource.
+
+    .PARAMETER RestartTimeout
+    Timeout value for restarting the SQL Server services. The default value is 120 seconds.
+
+    Not used in Test-TargetResource.
+#>
 Function Test-TargetResource
 {
     [CmdletBinding()]
@@ -155,25 +233,29 @@ Function Test-TargetResource
         [System.String]
         $SQLServer = $env:COMPUTERNAME,
 
-        [parameter(Mandatory = $true)]
+        [Parameter(Mandatory = $true)]
         [System.String]
         $InstanceName,
 
-        [parameter(Mandatory = $true)]
+        [Parameter(Mandatory = $true)]
         [ValidateSet('Tcp')]
         [System.String]
         $ProtocolName,
 
+        [Parameter()]
         [System.Boolean]
         $IsEnabled,
 
-        [ValidateSet("0","")]
+        [Parameter()]
+        [ValidateSet('0','')]
         [System.String]
         $TcpDynamicPorts,
 
+        [Parameter()]
         [System.String]
         $TcpPort,
 
+        [Parameter()]
         [System.Boolean]
         $RestartService = $false,
 
@@ -190,20 +272,26 @@ Function Test-TargetResource
         TcpPort = $TcpPort
     }
 
-    $currentState = Get-TargetResource -InstanceName $InstanceName -ProtocolName $ProtocolName
+    $getTargetResourceResult = Get-TargetResource -InstanceName $InstanceName -ProtocolName $ProtocolName
 
-    Write-Verbose "Comparing desiredState with currentSate ..."
-    foreach($key in $desiredState.Keys)
+    $isInDesiredState = $true
+
+    Write-Verbose "Comparing desired state with current state."
+    foreach ($key in $desiredState.Keys)
     {
-        if($desiredState[$key] -ine $currentState[$key] )
+        if( $desiredState[$key] -ine $getTargetResourceResult[$key] )
         {
-            Write-Verbose "$key is different: desired = $($desiredState[$key]); current = $($currentState[$key])"
-            return $false
+            Write-Verbose "$key is different: desired = $($desiredState[$key]); current = $($getTargetResourceResult[$key])"
+            $isInDesiredState = $false
         }
     }
 
-    Write-Verbose "States match"
-    return $true
+    if ($isInDesiredState)
+    {
+        Write-Verbose "In desired state."
+    }
+
+    return $isInDesiredState
 }
 
 Export-ModuleMember -Function *-TargetResource
diff --git a/DSCResources/MSFT_xSQLServerNetwork/MSFT_xSQLServerNetwork.schema.mof b/DSCResources/MSFT_xSQLServerNetwork/MSFT_xSQLServerNetwork.schema.mof
@@ -1,12 +1,12 @@
 [ClassVersion("1.0.0.0"), FriendlyName("xSQLServerNetwork")]
 class MSFT_xSQLServerNetwork : OMI_BaseResource
 {
-    [Key, Description("SQL Server instance name of which network protocol should be configured")] String InstanceName;
-    [Required, Description("Network protocol name that should be configured"), ValueMap{"Tcp"}, Values{"Tcp"}] String ProtocolName;
-    [Write, Description("The host name of the SQL Server to be configured.")] String SQLServer;
-    [Write, Description("Is network protocol should be enabled or disabled")] Boolean IsEnabled;
-    [Write, Description("If dynamic ports are used should be set to 0, otherwise leave empty"), ValueMap{"0",""}, Values{"0",""}] String TcpDynamicPorts;
-    [Write, Description("Sets static port for TCP/IP")] String TcpPort;
-    [Write, Description("Controls if affected SQL Service should be restarted automatically")] Boolean RestartService;
-    [Write, Description("Timeout value for restarting the SQL services. The default value is 120 seconds.")] UInt16 RestartTimeout;
+    [Key, Description("The name of the SQL instance to be configured.")] String InstanceName;
+    [Required, Description("The name of network protocol to be configured. Only tcp is currently supported."), ValueMap{"Tcp"}, Values{"Tcp"}] String ProtocolName;
+    [Write, Description("The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME.")] String SQLServer;
+    [Write, Description("Enables or disables the network protocol.")] Boolean IsEnabled;
+    [Write, Description("Set the value to '0' if dynamic ports should be used. If static port should be used set this to a empty string value."), ValueMap{"0",""}, Values{"0",""}] String TcpDynamicPorts;
+    [Write, Description("The TCP port that SQL Server should be listening on.")] String TcpPort;
+    [Write, Description("If set to $true then SQL Server and dependent services will be restarted if a change to the configuration is made. The default value is $false.")] Boolean RestartService;
+    [Write, Description("Timeout value for restarting the SQL Server services. The default value is 120 seconds.")] UInt16 RestartTimeout;
 };
diff --git a/README.md b/README.md
@@ -766,14 +766,14 @@ This resource is used to change the network settings for the instance.
 
 #### Parameters
 
-* **[String] InstanceName** _(Key)_: Name of SQL Server instance for which network will be configured.
-* **[String] ProtocolName** _(Required)_: Name of network protocol to be configured. Only tcp is currently supported. { tcp }.
+* **[String] InstanceName** _(Key)_: The name of the SQL instance to be configured.
+* **[String] ProtocolName** _(Required)_: The name of network protocol to be configured. Only tcp is currently supported. { tcp }.
 * **[String] SQLServer** _(Write)_: The host name of the SQL Server to be configured. Default value is $env:COMPUTERNAME.
-* **[Boolean] IsEnabled** _(Write)_: Enables/Disables network protocol.
+* **[Boolean] IsEnabled** _(Write)_: Enables or disables the network protocol.
 * **[String] TCPDynamicPorts** _(Write)_: Set the value to '0' if dynamic ports should be used. If static port should be used set this to a empty string value. { '0','' }.
-* **[String] TCPPort** _(Write)_: Custom TCP port.
-* **[Boolean] RestartService** _(Write)_: If true will restart SQL Service instance service after update. The default value is $false.
-* **[Uint16] RestartTimeout** _(Write)_: Timeout value for restarting the SQL services. The default value is 120 seconds.
+* **[String] TCPPort** _(Write)_: The TCP port that SQL Server should be listening on.
+* **[Boolean] RestartService** _(Write)_: If set to $true then SQL Server and dependent services will be restarted if a change to the configuration is made. The default value is $false.
+* **[Uint16] RestartTimeout** _(Write)_: Timeout value for restarting the SQL Server services. The default value is 120 seconds.
 
 #### Examples