Build PowerShell code to be re-usable. Build tools that can be re-used without modification. Build bricks!

As I was lurking on the Interwebz today I came across a script on the Technet Script Repository simply called RemoteRegistry.ps1. �It was a pretty handy powershell script that, as you might guess, manipulates the Windows remote registry service on all computers in an Active Directory domain. �It had some logic in there to account for offline computers but overall it was a very simple script that just worked.

I'm typically not one to make an example from someone's actual script. Especially since I commend the author for sharing it. However, this script is a great example of a mistake that I see some IT pros making. Can you guess what it is? It's not a syntax problem. It's not a performance problem. It's not even a Powershell problem at all! It's a design problem.

We should all strive to write reusable and extensible scripts. We need to write scripts that can be built upon, accept a standardized object input and output (preferably) and can be used in multiple situations. This script had some problems but the most noticeable problem was one where I didn't even have to look at the code.

The script's name was Start Remote Registry for all Computers in a Domain. At first glance you may not think this is a problem. I immediately cringed. The problem is that the script's author specifically wrote this script just for the remote registry service. He hard-coded the service name in his script. This script was designed for a single purpose; to manipulate a single Windows service.

How many Windows services are out there anyway? A bunch!

What if I'd like to use this script's logic but not use the remote registry service? You may say "just change the service name". If so, you're not following best practices. This script is getting all computers from a domain. What if I just want to use a CSV file for input? I'd have to do further modification. Why not build this so that would require no modification at all to accommodate for various scenarios?

Here's the script's code. There are a few different issues with this code but I'm just going to concentrate on the point at hand.

cls
$computers = get-adcomputer -Filter *
foreach ($computer in $computers) {
    if (Test-Connection -count 1 -computer $computer.Name -quiet){
        Write-Host "Updating system" $computer.Name "....." -ForegroundColor Green
        Set-Service -Name remoteregistry -Computer $computer.Name -StartupType Automatic
        Get-Service remoteregistry -ComputerName $computer.Name | start-service
    } else {
        Write-Host "System Offline " $computer.Name "....." -ForegroundColor Red
        echo $computer.Name >> C:\scripts\Inventory\offlineRemoteRegStartup.txt}
}

Let's break this script apart into its functional pieces. This script exists to do 4 things:

  1. Get a list of computer names.
  2. Filter out any offline computers and output them to a text file.
  3. Ensure a service's startup type is set to automatic.
  4. Ensure a service is running.

In its current iteration, it can only do all AD computers. �What if I only want to do some? What if I want to grab computers from some another source completely? �The script immediately tries to set the service without any kind of reporting. �What if I want to only attempt to modify the service if it's actually in a state I want to change? �It immediately tries to start the service. �Again, maybe it's already started. Why try to start it if it's already started? Finally, as I stated earlier, what if I want to change the service name all together?

I would have to modify the script in all of these scenarios. What if I could write this in a way that automatically allowed all of these scenarios without modifying the actual script at all; only the way it is called?

Here is my proposed revision.

[CmdletBinding()]
param (
    [string[]]$ComputerName = 'localhost',
    [Parameter(Mandatory)]
    [string]$ServiceName,
    [Parameter(Mandatory)]
    [ValidateSet('Auto', 'Manual')]
    [string]$StartupType,
    [Parameter(Mandatory)]
    [string]$LogFilePath
)
process {
    try {
        foreach ($Computer in $Computername) {
            if (Test-Connection -Computername $Computer -Quiet -Count 1) {
                Write-Verbose "The computer [$Computer] is online"
                Write-Verbose "Ensuring the service [$ServiceName] on computer [$Computer] is set to [$StartupType]..."
                $Service = Get-WmiObject -Computername $Computer -Class Win32_Service -Filter "Name = '$ServiceName'"
                if (-not $Service) {
                    Write-Warning "The service [$ServiceName] does not exist on computer [$Computer]"
                } elseif ($Service.StartMode -eq $StartupType) {
                    Write-Verbose "The service [$ServiceName] startup type is already set to [$StartupType] on computer [$Computer]"
                } else {
                    Write-Verbose "The service [$ServiceName] startup type is set to [$($Service.StartMode)] on computer [$Computer]. Changing to [$StartupType]..."
                    $Service.StartMode = $StartupType
                    if ($Service.StartMode -eq $StartupType) {
                        Write-Verbose "Successfully changed service [$ServiceName] startup type to [$StartupType] on [$Computer]"
                    } else {
                        Write-Warning "Failed to change service [$ServiceName] startup type to [$StartupType] on [$Computer]"
                    }
                    if (-not $Service.Started) {
                        $Service.StartService()
                        if ($Service.Started) {
                            Write-Verbose "Successfully started the service [$ServiceName] on computer [$Computer]"
                        } else {
                            Write-Warning "Failed to start the service [$ServiceName] on computer [$Computer]"
                        }
                    }
                }
            } else {
                Write-Warning "The computer [$Computer] is offline. Adding to log..."
                Add-Content -Path $LogFilePath -Value $Computer
            }
        }
    } catch {
        Write-Error "$($_.Exception.Message) - Line Number: $($_.InvocationInfo.ScriptLineNumber)"
    }
}

Obviously, this took longer to code but notice how much more robust it is not only from an error control perspective but also how reusable it can be? Want to ensure the remote registry service is started and set to auto now? no problem!

.\Set-MyService.ps1 -Computername ((Get-AdComputer -Filter *).Name) -ServiceName 'remoteregistry' -StartupType 'Auto' -LogFilePath 'C:\scripts\Inventory\offlineRemoteRegStartup.txt'

What about getting computers in a CSV file? No modification necessary; just change the way you call it.

.\Set-MyService.ps1 -Computername ((Import-Csv -Path 'C:\mycsv.csv').ComputerName) -ServiceName 'remoteregistry' -StartupType 'Auto' -LogFilePath 'C:\scripts\Inventory\offlineRemoteRegStartup.txt'

What about getting computers in a CSV file and changing the WinRM service?

.\Set-MyService.ps1 -Computername ((Import-Csv -Path 'C:\mycsv.csv').ComputerName) -ServiceName 'WinRm' -StartupType 'Auto' -LogFilePath 'C:\scripts\Inventory\offlineRemoteRegStartup.txt'

See where I'm going with this? Don't write scripts so statically. Separate the business logic from the actual script. Separate that out from the script itself and use parameters to fill in the variable stuff. If you do, you'll be on your way to building scripts once instead of consistently changing or creating new scripts every time.

Join the Jar Tippers on Patreon

It takes a lot of time to write detailed blog posts like this one. In a single-income family, this blog is one way I depend on to keep the lights on. I'd be eternally grateful if you could become a Patreon patron today!

Become a Patron!