Powershell Script for setting DHCP reservation description & host value

Question

I'm new to Powershell scripting and trying to write a script that will go through printer queues on a print server, get the print queue's port address, then compare the print queues port address to the DHCP reservation associated with the print queue port address on the server. If the port address of the print queue compares, it should then set the description of the DHCP reservation to reflect the print queue name and location. It should then also set the hostname option to be the print queue's name.

#Set DHCP reservation description and host value on server

#DHCP Server for site
$DHCPSvr = "Server1"

#DHCP Server Scope ID
$DHCPSvrScope = "10.224.148.0"

#Get Printer Management list of shared available printers
$PrinterObjects = Get-Printer -ComputerName Server1 * | Select-Object Name, PortName, Shared, Location | 
                  Where-Object { ($_.Name -match "^.{6}$") -and ($_.PortName -match "IP*" -or $_.PortName -match '^10*') -and ($_.Shared -EQ $True)} |
                  Sort-Object -Property PortName -Descending

#Get DHCP Reservation list from server
$DHCPSvrReservations = Get-DhcpServerv4Reservation -ComputerName $DHCPSvr -ScopeId $DHCPSvrScope

foreach ($DHCPSvrReservation in $DHCPSvrReservations){
    foreach ($PrinterObject in $PrinterObjects)
       {
        if ($DHCPSvrReservation.IPAddress -eq $PrinterObjects.PortName)
            {        
                {Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$PrinterObjects.Name & $PrinterObjects.Location"}
                {Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name}
            }
    }
}

I'm a bit unsure of the following code within the IF statement, is this the right way to have the loop process the two commands?

{Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$PrinterObjects.Name & $PrinterObjects.Location"} 
{Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name}

Anyway, it would be good if someone could go over what I have at the moment and point out any issues with formatting and so on.

Answer 1

Very impressed with your formatting and spacing. Top notch to see that in practice. I see a couple areas that could be improved and I see at least 2 problems.

Variable Expansion in Strings

Variables will expand in double quotes easy. However you need to be using subexpressions in order to get the property portion to work correctly. You can also use the format operator -f but that is better used for more complicated string.

... -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"

If this code is in use already check existing descriptions for dependencies.

Efficient and Reliable conditions

Where-Object { ($_.Name -match "^.{6}$") -and ($_.PortName -match "IP*" -or $_.PortName -match '^10*') -and ($_.Shared -EQ $True)}

The first clause looks like you are just trying to find Name's that are exactly 6 characters long? Just use the length property all strings have instead. No need for regex: $_.Name.Length -eq 6).

In the second clause you are using regex as well and while I doubt it will cause conflicts you need to be aware you would match IP as well as IPPPPPPPPPPP. -like "IP*" or the string method .StartsWith("IP") would work just as well. Note that StartsWith() is case sensitive.

Similarily with $_.PortName -match '^10*' if you are just trying to find ones where the first octet is 10 you will also match ones that are 100 or 1000000 since the greedy quantifier is in use here. Solution is the same as the last one. -like "10.*" or the string method .StartsWith("10."). Notice I added periods. That may or not be an issue for you in production.

Possible trade off of readabilty but for the last clause here (($_.Shared -EQ $True)) you don't need to use -eq. PowerShell will evaluate $_.Shared as its own expression. If it is a non-zero length string or non-zero integer it will evaluate to true.

Curly Braces

You are slightly inconsistent in your foreach blocks. Some have the opening brace after the block and some have it on the following line. I would pick on standard and stick with it. I personally favour the former.

foreach ($DHCPSvrReservation in $DHCPSvrReservations){
    # Do stuff
}

Same goes for your if and other similar blocks and constructs.

Your Set-DhcpServerv4Reservation and Set-DhcpServerv4OptionValue dont need to be surrounded by curly braces. They should function the same without them.

Using if when Where would do

You act on all $printerobject's that have a matching reservation. You loop though is still running through the $printerobject's that don't match. True, you don't do anything with them but you could move that condition into your loop then you can drop the if

foreach ($DHCPSvrReservation in $DHCPSvrReservations){
    foreach ($PrinterObject in ($PrinterObjects | Where-Object{$DHCPSvrReservation.IPAddress -eq $_.PortName})){     
            Set-DhcpServerv4Reservation -ComputerName $DHCPSvr -IPAddress $PrinterObjects.PortName -Description "$($PrinterObjects.Name) & $($PrinterObjects.Location)"
            Set-DhcpServerv4OptionValue -ScopeId $DHCPSvrScope -OptionId 12 $PrinterObjects.Name
    }
}

That should work but you should test first.