patternshellMinor

Powershell Windows Service Deployment

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

powershellservicedeploymentwindows

Problem

Based on this with a couple of changes.

Any issues you can point out would be great.

param([string]$targetServer, [string]$user, [string]$pass)

function Get-Service(
    [string]$serviceName = $(throw "serviceName is required"), 
    [string]$targetServer = $(throw "targetServer is required"))
{
    $service = Get-WmiObject  -Class Win32_Service `
        -ComputerName $targetServer -Filter "Name='$serviceName'" -Impersonation 3    
    return $service
}

Function Copy-Files {
param(
        [Parameter(Mandatory=$true,ValueFromPipeline=$True)]
        [string]$source,
        [Parameter(Mandatory=$true,ValueFromPipeline=$True)]
        [string]$targetServer
        )

$destination = "\\$targetServer\C$\APP\"

"Creating network share on $targetServer"        
$share = Get-WmiObject Win32_Share -List -ComputerName $targetServer
$share.create("C:\share","autoShare", 0)        

#create backup        
Move-Item -path $destination  -destination "$destination\backup"

#copy new files
Copy-Item -path $source  -destination $destination -recurse

"Removing network share on $targetServer"    
if ($s = Get-WmiObject -Class Win32_Share -ComputerName $targetServer -Filter "Name='autoShare'") `
  { $s.delete() }    

}

function Start-Service(
    [string]$serviceName = $(throw "serviceName is required"), 
    [string]$targetServer = $(throw "targetServer is required"))
{
    "Getting service $serviceName on server $targetServer"
    $service = Get-Service $serviceName $targetServer
    if (!($service.Started))
    {
        "Starting service $serviceName on server $targetServer"
        $result = $service.StartService()
        Test-ServiceResult -operation "Starting service $serviceName on $targetServer" -result $result   
    }
}

```
function Uninstall-Service(
[string]$serviceName = $(throw "serviceName is required"),
[string]$targetServer = $(throw "targetServer is required"))
{
$service = Get-Service $serviceName $targetSe

Solution

Your code and overall logic look good. In general the areas of improvement I see are

There are some things you are inconsistent about, e.g., function parameters

You are sending notification information down the pipeline.

Some of the parameter declaration could be improved.

I see that your are trying to make the code fit within a certain amount of characters on each line. While that is perfectly fine the are some features of PowerShell I could show that would improve code functionality and still give that same visual effect.

Param Block

Mandatory Parameters

In your first function you have the param $serviceName throw an error if it is not represent.

[string]$serviceName = $(throw "serviceName is required")

In another function you get the same thing but leveraging advanced parameters.

[Parameter(Mandatory=$true,ValueFromPipeline=$True)]
[string]$source,

So as you can see there is no reason for throw. The Mandatory flag set to true accomplishes the almost exact same thing. Calling the function without passing the parameter will make PowerShell ask for it.

cmdlet Get-Service at command pipeline position 1
Supply values for the following parameters:
serviceName: _ 

Perhaps this has occurred to you and you prefer the messages from

throw

. Setting the parameter will mitigate errors. If nothing else just take it as an FYI.

Watch out for scope

The function

Install-Service has a parameter defined $password with a default of $pass. While that code will still function is can be misleading as $pass is a variable defined in a parent scope. Consider making that one mandatory as well and just use $pass

 when you are calling the function.

Error Checking

You make several calls to WMI but there is not guarantee they are going to work. Consider using a

try/catch block or -SilentlyContinue

 and validating the result. Examples of both are below. These are not production examples but samples to show functionality.

# Try/Catch Example
try{
    $result = Get-WmiObject -Class Win32_Volume -ComputerName doesnotexist
} catch {
    "There was and error: $($_.Exception)"
}

# SilentlyContinue Example
$result = Get-WmiObject -Class Win32_Volume -ComputerName doesnotexist -ErrorAction SilentlyContinue
if(!$result){"Something happened"}


Code Clarity

I refereed to this in a bullet above about mak[ing] the code fit within a certain amount of characters on each line. While playing with the backticks works it can be annoying when you have to make changes and forget to add them. Once place in

Install-Service

 where you use them and it is not even required. PowerShell is forgiving for this:

$params = 
    $serviceName, 
    $displayName, 
    $physicalPath, 
    $serviceType, 
    $serviceErrorControl, 
    $startMode, 
    $interactWithDesktop, 
    $userName, 
    $password,
    $loadOrderGroup,
    $loadOrderGroupDepend,
    $dependencies


In other places you cannot get away with that. When you call

Install-Service

 you use backticks again to get each parameter on its own line. That in itself is fine but I wanted to show you splatting. You pass a hashtable of parameter and value pairs and splat them to the cmdlet. Each one is on its own line and can be edited easily in place. 

$params = @{
    ServiceName = $serviceName
    TargetServer = $targetServer
    DisplayName = "APP Test Syncronization Service"
    PhysicalPath = "C:\APP\APP.SyncService.exe"
    Username = $user
    Description = "Description"  
}

Install-Service @params


Function names

Get-Service and Start-Service

 are already cmdlet names. What you are doing is changing the command order precedence. Yours will get called first but you are using them for the same functionality the builtin cmdlets. 

# Using the builtin
Get-Service -ComputerName $target -Name Spooler


I would opt for just using the built in ones. They return

[System.ServiceProcess.ServiceController]

 objects that you can reference to check command success.

Be careful with your function output

Again, this is really wrong but something you need to be aware of. You are sending lots of status information down the pipeline. Basic example being

"Pausing to avoid potential temporary access denied"


That line is sent down the output stream. If you are going capture output from function, or use the pipeline like you configured on some of your parameters

ValueFromPipeline=$True, you might get unexpected behavior. If this is truly information only then consider Write-host

 or a separate logging function.

Test-ServiceResult

The way you convert the error code to its friendly text could be shortened. If you have at least PowerShell v5 you can use the

enum

 keyword. If not you could always add a type definition as well. To keep it simple I am just going to improve your error code array.

You can also use the

-is

 operator to test the type of a variable.

``
if ($result -is [uint32]) {$retVal = $result}

Code Snippets

[string]$serviceName = $(throw "serviceName is required")

[Parameter(Mandatory=$true,ValueFromPipeline=$True)]
[string]$source,

# Try/Catch Example
try{
    $result = Get-WmiObject -Class Win32_Volume -ComputerName doesnotexist
} catch {
    "There was and error: $($_.Exception)"
}

# SilentlyContinue Example
$result = Get-WmiObject -Class Win32_Volume -ComputerName doesnotexist -ErrorAction SilentlyContinue
if(!$result){"Something happened"}

$params = 
    $serviceName, 
    $displayName, 
    $physicalPath, 
    $serviceType, 
    $serviceErrorControl, 
    $startMode, 
    $interactWithDesktop, 
    $userName, 
    $password,
    $loadOrderGroup,
    $loadOrderGroupDepend,
    $dependencies

$params = @{
    ServiceName = $serviceName
    TargetServer = $targetServer
    DisplayName = "APP Test Syncronization Service"
    PhysicalPath = "C:\APP\APP.SyncService.exe"
    Username = $user
    Description = "Description"  
}

Install-Service @params

Context

StackExchange Code Review Q#63164, answer score: 9

Revisions (0)

No revisions yet.