Skip to content

Commit a4cc440

Browse files
manuelgr0manuelgr0
authored
SqlScript: Fix graceful error handling (#2449)
1 parent 875f868 commit a4cc440

5 files changed

Lines changed: 148 additions & 12 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
55

66
## [Unreleased]
77

8+
### Fixed
9+
10+
- SqlScript
11+
- Fixed logic in `Get-TargetResource` and `Set-TargetResource` to throw an error
12+
when the SQL script file is missing, instead of incorrectly reporting success
13+
or using a null variable.
14+
- Fixed `Test-TargetResource` to return `$false` (instead of throwing) when
15+
the SQL script file is missing, enabling `DependsOn` scenarios where the file
16+
is created at runtime.
17+
818
### Changed
919

1020
- SqlScript

‎source/DSCResources/DSC_SqlScript/DSC_SqlScript.psm1‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ function Get-TargetResource
121121

122122
$serverInstance = ConvertTo-ServerInstanceName -InstanceName $InstanceName -ServerName $ServerName
123123

124+
if (-not (Test-Path -Path $GetFilePath -PathType Leaf))
125+
{
126+
$errorMessage = $script:localizedData.GetFilePath_FileNotFound -f $GetFilePath
127+
128+
New-ObjectNotFoundException -Message $errorMessage
129+
}
130+
124131
$invokeParameters = @{
125132
ServerInstance = $serverInstance
126133
InputFile = $GetFilePath
@@ -278,6 +285,13 @@ function Set-TargetResource
278285

279286
$serverInstance = ConvertTo-ServerInstanceName -InstanceName $InstanceName -ServerName $ServerName
280287

288+
if (-not (Test-Path -Path $SetFilePath -PathType Leaf))
289+
{
290+
$errorMessage = $script:localizedData.SetFilePath_FileNotFound -f $SetFilePath
291+
292+
New-ObjectNotFoundException -Message $errorMessage
293+
}
294+
281295
Write-Verbose -Message (
282296
$script:localizedData.ExecutingSetScript -f $SetFilePath, $InstanceName, $ServerName
283297
)
@@ -419,6 +433,14 @@ function Test-TargetResource
419433
$script:localizedData.TestingConfiguration
420434
)
421435

436+
if (-not (Test-Path -Path $TestFilePath -PathType Leaf))
437+
{
438+
$errorMessage = $script:localizedData.TestFilePath_FileNotFound -f $TestFilePath
439+
Write-Verbose -Message $errorMessage
440+
441+
return $false
442+
}
443+
422444
$serverInstance = ConvertTo-ServerInstanceName -InstanceName $InstanceName -ServerName $ServerName
423445

424446
$invokeParameters = @{

‎source/DSCResources/DSC_SqlScript/en-US/DSC_SqlScript.strings.psd1‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,7 @@ ConvertFrom-StringData @'
55
TestingConfiguration = Determines if the configuration in the Set script is in desired state.
66
InDesiredState = The configuration is in desired state.
77
NotInDesiredState = The configuration is not in desired state.
8+
GetFilePath_FileNotFound = The file specified in GetFilePath ('{0}') does not exist or is not accessible. Cannot determine resource state.
9+
SetFilePath_FileNotFound = The file specified in SetFilePath ('{0}') does not exist or is not accessible. Cannot apply desired state.
10+
TestFilePath_FileNotFound = Test script file '{0}' not found. Assuming resource is not in desired state.
811
'@

‎tests/Integration/Resources/DSC_SqlScript.config.ps1‎

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,12 @@ Configuration DSC_SqlScript_CreateDependencies_Config
9191
#>
9292
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
9393

94-
return $getScriptResult.Result -eq $Using:Node.GetSqlScript
94+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
95+
{
96+
return $false
97+
}
98+
99+
return $true
95100
}
96101

97102
GetScript = {
@@ -102,9 +107,11 @@ Configuration DSC_SqlScript_CreateDependencies_Config
102107
$fileContent = Get-Content -Path $Using:Node.GetSqlScriptPath -Raw
103108
}
104109

105-
return @{
110+
$returnValue = @{
106111
Result = $fileContent
107112
}
113+
114+
return $returnValue
108115
}
109116
}
110117

@@ -117,7 +124,12 @@ Configuration DSC_SqlScript_CreateDependencies_Config
117124
TestScript = {
118125
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
119126

120-
return $getScriptResult.Result -eq $Using:Node.TestSqlScript
127+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
128+
{
129+
return $false
130+
}
131+
132+
return $true
121133
}
122134

123135
GetScript = {
@@ -128,9 +140,11 @@ Configuration DSC_SqlScript_CreateDependencies_Config
128140
$fileContent = Get-Content -Path $Using:Node.TestSqlScriptPath -Raw
129141
}
130142

131-
return @{
143+
$returnValue = @{
132144
Result = $fileContent
133145
}
146+
147+
return $returnValue
134148
}
135149
}
136150

@@ -143,7 +157,12 @@ Configuration DSC_SqlScript_CreateDependencies_Config
143157
TestScript = {
144158
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
145159

146-
return $getScriptResult.Result -eq $Using:Node.SetSqlScript
160+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
161+
{
162+
return $false
163+
}
164+
165+
return $true
147166
}
148167

149168
GetScript = {
@@ -154,9 +173,11 @@ Configuration DSC_SqlScript_CreateDependencies_Config
154173
$fileContent = Get-Content -Path $Using:Node.SetSqlScriptPath -Raw
155174
}
156175

157-
return @{
176+
$returnValue = @{
158177
Result = $fileContent
159178
}
179+
180+
return $returnValue
160181
}
161182
}
162183

@@ -320,7 +341,12 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
320341
TestScript = {
321342
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
322343

323-
return $getScriptResult.Result -eq $Using:Node.GetSqlScript
344+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
345+
{
346+
return $false
347+
}
348+
349+
return $true
324350
}
325351

326352
GetScript = {
@@ -331,9 +357,11 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
331357
$fileContent = Get-Content -Path $Using:Node.GetSqlScriptPath2 -Raw
332358
}
333359

334-
return @{
360+
$returnValue = @{
335361
Result = $fileContent
336362
}
363+
364+
return $returnValue
337365
}
338366
}
339367

@@ -346,7 +374,12 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
346374
TestScript = {
347375
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
348376

349-
return $getScriptResult.Result -eq $Using:Node.TestSqlScript
377+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
378+
{
379+
return $false
380+
}
381+
382+
return $true
350383
}
351384

352385
GetScript = {
@@ -357,9 +390,11 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
357390
$fileContent = Get-Content -Path $Using:Node.TestSqlScriptPath2 -Raw
358391
}
359392

360-
return @{
393+
$returnValue = @{
361394
Result = $fileContent
362395
}
396+
397+
return $returnValue
363398
}
364399
}
365400

@@ -372,7 +407,12 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
372407
TestScript = {
373408
$getScriptResult = & ([ScriptBlock]::Create($GetScript))
374409

375-
return $getScriptResult.Result -eq $Using:Node.SetSqlScript
410+
if ([System.String]::IsNullOrEmpty($getScriptResult.Result))
411+
{
412+
return $false
413+
}
414+
415+
return $true
376416
}
377417

378418
GetScript = {
@@ -383,9 +423,11 @@ Configuration DSC_SqlScript_RunSqlScriptAsWindowsUserWithDependencies_Config
383423
$fileContent = Get-Content -Path $Using:Node.SetSqlScriptPath2 -Raw
384424
}
385425

386-
return @{
426+
$returnValue = @{
387427
Result = $fileContent
388428
}
429+
430+
return $returnValue
389431
}
390432
}
391433

‎tests/Unit/DSC_SqlScript.Tests.ps1‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ Describe 'SqlScript\Get-TargetResource' -Tag 'Get' {
9797

9898
Context 'When Get-TargetResource returns script results successfully' {
9999
BeforeAll {
100+
Mock -CommandName Test-Path -MockWith { return $true }
100101
Mock -CommandName Invoke-SqlScript -MockWith {
101102
return ''
102103
}
@@ -120,6 +121,7 @@ Describe 'SqlScript\Get-TargetResource' -Tag 'Get' {
120121

121122
Context 'When Get-TargetResource returns script results successfully with query timeout' {
122123
BeforeAll {
124+
Mock -CommandName Test-Path -MockWith { return $true }
123125
Mock -CommandName Invoke-SqlScript -MockWith {
124126
return ''
125127
}
@@ -145,6 +147,7 @@ Describe 'SqlScript\Get-TargetResource' -Tag 'Get' {
145147

146148
Context 'When Get-TargetResource throws an error when running the script in the GetFilePath parameter' {
147149
BeforeAll {
150+
Mock -CommandName Test-Path -MockWith { return $true }
148151
Mock -CommandName Invoke-SqlScript -MockWith {
149152
throw 'Failed to run SQL Script'
150153
}
@@ -160,6 +163,22 @@ Describe 'SqlScript\Get-TargetResource' -Tag 'Get' {
160163
}
161164
}
162165
}
166+
167+
Context 'When the GetFilePath file is missing' {
168+
BeforeAll {
169+
Mock -CommandName Test-Path -MockWith { return $false }
170+
}
171+
172+
It 'Should throw the localized file not found exception' {
173+
InModuleScope -ScriptBlock {
174+
Set-StrictMode -Version 1.0
175+
176+
$expectedError = $script:localizedData.GetScript_FileNotFound -f $script:mockGetTargetResourceParameters.GetFilePath
177+
178+
{ Get-TargetResource @mockGetTargetResourceParameters } | Should -Throw -ExpectedMessage $expectedError
179+
}
180+
}
181+
}
163182
}
164183

165184
Describe 'SqlScript\Set-TargetResource' -Tag 'Set' {
@@ -186,6 +205,7 @@ Describe 'SqlScript\Set-TargetResource' -Tag 'Set' {
186205

187206
Context 'When Set-TargetResource runs script without issue' {
188207
BeforeAll {
208+
Mock -CommandName Test-Path -MockWith { return $true }
189209
Mock -CommandName Invoke-SqlScript -MockWith {
190210
return ''
191211
}
@@ -202,6 +222,7 @@ Describe 'SqlScript\Set-TargetResource' -Tag 'Set' {
202222

203223
Context 'When Set-TargetResource runs script without issue using timeout' {
204224
BeforeAll {
225+
Mock -CommandName Test-Path -MockWith { return $true }
205226
Mock -CommandName Invoke-SqlScript -MockWith {
206227
return ''
207228
}
@@ -220,6 +241,7 @@ Describe 'SqlScript\Set-TargetResource' -Tag 'Set' {
220241

221242
Context 'When Set-TargetResource throws an error when running the script in the SetFilePath parameter' {
222243
BeforeAll {
244+
Mock -CommandName Test-Path -MockWith { return $true }
223245
Mock -CommandName Invoke-SqlScript -MockWith {
224246
throw 'Failed to run SQL Script'
225247
}
@@ -236,6 +258,22 @@ Describe 'SqlScript\Set-TargetResource' -Tag 'Set' {
236258
}
237259
}
238260
}
261+
262+
Context 'When the SetFilePath file is missing' {
263+
BeforeAll {
264+
Mock -CommandName Test-Path -MockWith { return $false }
265+
}
266+
267+
It 'Should throw the localized file not found exception' {
268+
InModuleScope -ScriptBlock {
269+
Set-StrictMode -Version 1.0
270+
271+
$expectedError = $script:localizedData.SetScript_FileNotFound -f $script:mockSetTargetResourceParameters.SetFilePath
272+
273+
{ Set-TargetResource @mockSetTargetResourceParameters } | Should -Throw -ExpectedMessage $expectedError
274+
}
275+
}
276+
}
239277
}
240278

241279
Describe 'SqlScript\Test-TargetResource' {
@@ -263,6 +301,7 @@ Describe 'SqlScript\Test-TargetResource' {
263301
Context 'When the system is in the desired state' {
264302
Context 'When Test-TargetResource runs script without issue' {
265303
BeforeAll {
304+
Mock -CommandName Test-Path -MockWith { return $true }
266305
Mock -CommandName Invoke-SqlScript
267306
}
268307

@@ -279,6 +318,7 @@ Describe 'SqlScript\Test-TargetResource' {
279318

280319
Context 'When Test-TargetResource runs script without issue with timeout' {
281320
BeforeAll {
321+
Mock -CommandName Test-Path -MockWith { return $true }
282322
Mock -CommandName Invoke-SqlScript
283323
}
284324

@@ -299,6 +339,7 @@ Describe 'SqlScript\Test-TargetResource' {
299339
Context 'When the system is not in the desired state' {
300340
Context 'When Invoke-SqlScript returns an SQL error code from the script that was ran' {
301341
BeforeAll {
342+
Mock -CommandName Test-Path -MockWith { return $true }
302343
Mock -CommandName Invoke-SqlScript -MockWith {
303344
return 1
304345
}
@@ -319,6 +360,7 @@ Describe 'SqlScript\Test-TargetResource' {
319360

320361
Context 'When Test-TargetResource throws the exception SqlPowerShellSqlExecutionException when running the script in the TestFilePath parameter' {
321362
BeforeAll {
363+
Mock -CommandName Test-Path -MockWith { return $true }
322364
Mock -CommandName Invoke-SqlScript -MockWith {
323365
throw New-Object -TypeName Microsoft.SqlServer.Management.PowerShell.SqlPowerShellSqlExecutionException
324366
}
@@ -337,6 +379,7 @@ Describe 'SqlScript\Test-TargetResource' {
337379

338380
Context 'When Test-TargetResource throws an unexpected error when running the script in the TestFilePath parameter' {
339381
BeforeAll {
382+
Mock -CommandName Test-Path -MockWith { return $true }
340383
Mock -CommandName Invoke-SqlScript -MockWith {
341384
throw 'Failed to run SQL Script'
342385
}
@@ -353,4 +396,20 @@ Describe 'SqlScript\Test-TargetResource' {
353396
}
354397
}
355398
}
399+
400+
Context 'When the TestFilePath file is missing' {
401+
BeforeAll {
402+
Mock -CommandName Test-Path -MockWith { return $false }
403+
}
404+
405+
It 'Should return false' {
406+
InModuleScope -ScriptBlock {
407+
Set-StrictMode -Version 1.0
408+
409+
$result = Test-TargetResource @mockTestTargetResourceParameters
410+
411+
$result | Should -BeFalse
412+
}
413+
}
414+
}
356415
}

0 commit comments

Comments
 (0)