Add backup and restore Report Server encryption key#2413
Merged
johlju merged 8 commits intodsccommunity:mainfrom Jan 17, 2026
Merged
Add backup and restore Report Server encryption key#2413johlju merged 8 commits intodsccommunity:mainfrom
johlju merged 8 commits intodsccommunity:mainfrom
Conversation
WalkthroughAdds public Reporting Services cmdlets to backup/restore encryption keys, localization strings, unit and integration tests, CI pipeline test-group additions, integration README updates, and new pre/mid/post integration scripts supporting a secure RS service-account change workflow. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant BackupCmd as Backup-SqlDscRSEncryptionKey
participant CIM as MSReportServer_ConfigurationSetting(CIM)
participant FS as Filesystem
User->>BackupCmd: Invoke(Configuration, Path, Password[, Credential])
BackupCmd->>BackupCmd: Convert SecureString Password to plaintext
alt UNC path + Credential
BackupCmd->>BackupCmd: Create temporary PSDrive with Credential
BackupCmd->>CIM: Invoke BackupEncryptionKey(Password)
CIM-->>BackupCmd: Return KeyFile bytes
BackupCmd->>FS: Write KeyFile bytes via PSDrive
BackupCmd->>BackupCmd: Remove temporary PSDrive
else Local/accessible path
BackupCmd->>CIM: Invoke BackupEncryptionKey(Password)
CIM-->>BackupCmd: Return KeyFile bytes
BackupCmd->>FS: Write KeyFile bytes to Path
end
BackupCmd->>BackupCmd: Zero plaintext password in memory
BackupCmd-->>User: Return Configuration (if PassThru) / Success
sequenceDiagram
actor User
participant RestoreCmd as Restore-SqlDscRSEncryptionKey
participant FS as Filesystem
participant CIM as MSReportServer_ConfigurationSetting(CIM)
User->>RestoreCmd: Invoke(Configuration, Path, Password[, Credential])
alt UNC path + Credential
RestoreCmd->>RestoreCmd: Create temporary PSDrive with Credential
RestoreCmd->>FS: Read KeyFile bytes via PSDrive
RestoreCmd->>RestoreCmd: Remove temporary PSDrive
else Local/accessible path
RestoreCmd->>FS: Read KeyFile bytes from Path
end
RestoreCmd->>RestoreCmd: Convert SecureString Password to plaintext
RestoreCmd->>CIM: Invoke RestoreEncryptionKey(KeyFile, Length, Password)
CIM-->>RestoreCmd: Return result
RestoreCmd->>RestoreCmd: Zero plaintext password in memory
RestoreCmd-->>User: Return Configuration (if PassThru) / Success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2413 +/- ##
=====================================
- Coverage 94% 94% -1%
=====================================
Files 223 225 +2
Lines 11071 11144 +73
=====================================
+ Hits 10448 10507 +59
- Misses 623 637 +14
🚀 New features to boost your workflow:
|
…eter and improve path handling
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 227-230: Update the CHANGELOG entry to use the verb phrase “back
up” instead of the noun “backup” and include the issue reference if available;
specifically edit the sentence that mentions the public commands
`Backup-SqlDscRSEncryptionKey` and `Restore-SqlDscRSEncryptionKey` to read
something like “Added public commands `Backup-SqlDscRSEncryptionKey` and
`Restore-SqlDscRSEncryptionKey` to back up and restore Reporting Services
encryption keys” and append the issue number or link (e.g., “(Fixes `#1234`)”) for
tracking.
- Around line 10-12: Update the changelog entry for the added public commands
(Start-SqlDscRSWindowsService, Stop-SqlDscRSWindowsService,
Start-SqlDscRSWebService, Stop-SqlDscRSWebService) to include the associated
issue reference (e.g., “(`#1234`)” or a full issue URL) per changelog guidelines;
locate the line describing these commands and append the issue identifier or
link to the end of the sentence so the entry follows the repository’s example
format for issue references.
In `@source/en-US/SqlServerDsc.strings.psd1`:
- Around line 899-905: The error ID in the localized string
Backup_SqlDscRSEncryptionKey_FailedToBackup currently ends with (BRSEK0001) but
the Backup-SqlDscRSEncryptionKey cmdlet emits ErrorId BSRSEK0001; update the
string suffix to (BSRSEK0001) so the message matches the cmdlet's ErrorId and
tests; locate the Backup_SqlDscRSEncryptionKey_* entries (e.g.,
Backup_SqlDscRSEncryptionKey_FailedToBackup) and replace the mismatched token
accordingly.
In `@source/Public/Restore-SqlDscRSEncryptionKey.ps1`:
- Around line 48-52: Update the insecure examples that construct a plaintext
SecureString: in Restore-SqlDscRSEncryptionKey's second example (and mirror the
same change in Backup-SqlDscRSEncryptionKey's second example) replace the use of
ConvertTo-SecureString -AsPlainText -Force with using Read-Host -AsSecureString
to capture the password securely (keep the rest of the example parameters like
-Configuration, -Path, -Credential, and -Force unchanged).
In `@tests/Integration/Commands/README.md`:
- Around line 178-205: The run-order table entries for ReportingServices are out
of sync: update the listed entries for Get-SqlDscRSSslCertificateBinding,
Add-SqlDscRSSslCertificateBinding, Remove-SqlDscRSSslCertificateBinding,
Set-SqlDscRSSslCertificateBinding, Set-SqlDscRSServiceAccount,
Get-SqlDscRSServiceAccount and Post.ServiceAccountChange.RS to match the actual
pipeline (azure-pipelines.yml) — i.e., set service-account change to Group 6
with the version-specific post-test names and move SSL/TLS certificate-binding
tests into the BI Report Server SSL/TLS stage (or alternatively add the missing
SSL tests into the ReportingServices stage if that was intended); ensure
run-order numbers and test names in the table exactly match the pipeline group
numbers and test job names.
- Around line 238-241: Update the BI Report Server service-account change rows
in the table so the run order and job names match the pipeline: change the rows
referencing Post.ServiceAccountChange.RS to use
Post.ServiceAccountChange.PowerBI.RS and ensure the sequence for
Set-SqlDscRSServiceAccount, Get-SqlDscRSServiceAccount,
Post.ServiceAccountChange.PowerBI.RS, and Post.UrlReservationRecreate.RS
reflects run orders 3–5 in Group 6 (i.e., adjust the run order numbers and the
name in the table to match the pipeline grouping).
In `@tests/Unit/Public/Restore-SqlDscRSEncryptionKey.Tests.ps1`:
- Around line 48-63: The parameter-set expectation for the
Restore-SqlDscRSEncryptionKey command is missing the Credential parameter;
update the ExpectedParameters string in the test (inside the It block that sets
ExpectedParameterSetName and ExpectedParameters) to include '[-Credential]
<PSCredential>' between '[-Configuration] <Object>' and '[-Password]
<securestring>' so the ExpectedParameters value matches the actual Get-Command
output for Restore-SqlDscRSEncryptionKey and the assertions on
$result.ParameterListAsString succeed.
♻️ Duplicate comments (3)
tests/Integration/Commands/README.md (2)
247-247: SMTP configuration run order mismatchLine 247 shows
Set-SqlDscRSSmtpConfigurationat run order 3, but the pipeline places SMTP configuration in Group 7. Please align the run order.
257-260: Get-SqlDscRSLogPath run order differs from pipelineLine 259 lists
Get-SqlDscRSLogPathat run order 5, while the pipeline executes it in Group 3 before Initialize-SqlDscRS. Please adjust the table (or the test order if it changed).source/Public/Backup-SqlDscRSEncryptionKey.ps1 (1)
49-52: Plaintext SecureString example (duplicate)Line 50 uses
ConvertTo-SecureString -AsPlainText -Force. Same issue as noted in Restore-SqlDscRSEncryptionKey; please update this example too. Based on learnings, avoidConvertTo-SecureString -AsPlainTextin production guidance.
🧹 Nitpick comments (9)
source/Public/Restore-SqlDscRSEncryptionKey.ps1 (1)
149-155: Use a unique PSDrive name to prevent collisionsLines 149-155 hardcode
RSKeyRestore; if a drive with that name already exists or parallel runs occur,New-PSDrivewill fail. Consider generating a unique name per invocation.♻️ Suggested tweak
- $driveName = 'RSKeyRestore' + $driveName = "RSKeyRestore_$([System.Guid]::NewGuid().ToString('N'))"source/Public/Backup-SqlDscRSEncryptionKey.ps1 (1)
164-170: Use a unique PSDrive name to prevent collisionsLines 164-170 hardcode
RSKeyBackup; if that drive name already exists,New-PSDrivewill throw. Consider generating a unique name per invocation.♻️ Suggested tweak
- $driveName = 'RSKeyBackup' + $driveName = "RSKeyBackup_$([System.Guid]::NewGuid().ToString('N'))"tests/Integration/Commands/Backup-SqlDscRSEncryptionKey.Integration.Tests.ps1 (1)
71-72: Consider invoking the command directly instead of usingShould -Not -Throw.Per coding guidelines, avoid
Should -Not -Throwand invoke commands directly. Since line 72 already asserts the expected outcome, you can simplify by removing theShould -Not -Throwwrapper.♻️ Suggested refactor
It 'Should backup the encryption key to a persistent location for the service account change workflow' { # Remove any existing backup file from previous test runs if (Test-Path -Path $script:backupPath) { Remove-Item -Path $script:backupPath -Force -ErrorAction 'SilentlyContinue' } - { $script:configuration | Backup-SqlDscRSEncryptionKey -Password $script:securePassword -Path $script:backupPath -Force -ErrorAction 'Stop' } | Should -Not -Throw + $null = $script:configuration | Backup-SqlDscRSEncryptionKey -Password $script:securePassword -Path $script:backupPath -Force -ErrorAction 'Stop' + Test-Path -Path $script:backupPath | Should -BeTrue -Because 'the encryption key backup file should be created at the persistent location' }tests/Integration/Commands/Restore-SqlDscRSEncryptionKey.Integration.Tests.ps1 (1)
67-69: Consider invoking the command directly and adding result verification.Per guidelines, avoid
Should -Not -Throw. Additionally, consider adding verification that the restore was successful (similar to how the backup test verifies the file exists).♻️ Suggested refactor
It 'Should restore the encryption key from the persistent backup location' { - { $script:configuration | Restore-SqlDscRSEncryptionKey -Password $script:securePassword -Path $script:backupPath -Force -ErrorAction 'Stop' } | Should -Not -Throw + $null = $script:configuration | Restore-SqlDscRSEncryptionKey -Password $script:securePassword -Path $script:backupPath -Force -ErrorAction 'Stop' }tests/Unit/Public/Restore-SqlDscRSEncryptionKey.Tests.ps1 (1)
81-92: Consider invoking the command directly instead of usingShould -Not -Throw.Per coding guidelines, avoid
Should -Not -Throwand invoke commands directly. The mock assertions that follow already verify the command executed correctly.♻️ Suggested refactor
It 'Should restore encryption key without errors' { - { $mockCimInstance | Restore-SqlDscRSEncryptionKey -Password $mockPassword -Path 'C:\Backup\RSKey.snk' -Confirm:$false } | Should -Not -Throw + $null = $mockCimInstance | Restore-SqlDscRSEncryptionKey -Password $mockPassword -Path 'C:\Backup\RSKey.snk' -Confirm:$false Should -Invoke -CommandName Get-Content -ParameterFilter { $Path -eq 'C:\Backup\RSKey.snk' -and $Encoding -eq 'Byte' } -Exactly -Times 1 Should -Invoke -CommandName Invoke-RsCimMethod -ParameterFilter { $MethodName -eq 'RestoreEncryptionKey' } -Exactly -Times 1 }Apply the same pattern to lines 140 and 203.
tests/Unit/Public/Backup-SqlDscRSEncryptionKey.Tests.ps1 (4)
81-91: AvoidShould -Not -Throw; invoke command directly.Per coding guidelines,
Should -Not -Throwshould not be used. Since subsequent assertions verify the mock invocation and file existence, invoke the command directly and let any exception naturally fail the test.♻️ Suggested refactor
It 'Should backup encryption key without errors' { $testPath = Join-Path -Path $TestDrive -ChildPath 'RSKey.snk' - { $mockCimInstance | Backup-SqlDscRSEncryptionKey -Password $mockPassword -Path $testPath -Confirm:$false } | Should -Not -Throw + $null = $mockCimInstance | Backup-SqlDscRSEncryptionKey -Password $mockPassword -Path $testPath -Confirm:$false Should -Invoke -CommandName Invoke-RsCimMethod -ParameterFilter { $MethodName -eq 'BackupEncryptionKey' } -Exactly -Times 1 $testPath | Should -Exist }
142-148: AvoidShould -Not -Throw; invoke command directly.Same issue as the previous Context block. Invoke the command directly since the mock invocation assertion will verify the expected behavior.
♻️ Suggested refactor
It 'Should backup encryption key without confirmation' { $testPath = Join-Path -Path $TestDrive -ChildPath 'RSKey.snk' - { $mockCimInstance | Backup-SqlDscRSEncryptionKey -Password $mockPassword -Path $testPath -Force } | Should -Not -Throw + $null = $mockCimInstance | Backup-SqlDscRSEncryptionKey -Password $mockPassword -Path $testPath -Force Should -Invoke -CommandName Invoke-RsCimMethod -Exactly -Times 1 }
208-214: AvoidShould -Not -Throw; invoke command directly.Same pattern as earlier tests. Invoke the command directly.
♻️ Suggested refactor
It 'Should backup encryption key' { $testPath = Join-Path -Path $TestDrive -ChildPath 'RSKey.snk' - { Backup-SqlDscRSEncryptionKey -Configuration $mockCimInstance -Password $mockPassword -Path $testPath -Confirm:$false } | Should -Not -Throw + $null = Backup-SqlDscRSEncryptionKey -Configuration $mockCimInstance -Password $mockPassword -Path $testPath -Confirm:$false Should -Invoke -CommandName Invoke-RsCimMethod -Exactly -Times 1 }
46-216: Missing test coverage for Credential parameter with UNC paths.The parameter set includes
Credential(line 51), and the PR commit message mentions adding Credential parameter support, but there's no test covering the UNC path scenario where aCredentialis provided to create a temporary PSDrive. Consider adding a Context block to test this code path.Example test structure:
Context 'When backing up to UNC path with Credential' { BeforeAll { $mockCimInstance = [PSCustomObject] @{ InstanceName = 'SSRS' } $mockPassword = ConvertTo-SecureString -String 'P@ssw0rd' -AsPlainText -Force $mockCredential = [System.Management.Automation.PSCredential]::new( 'TestUser', (ConvertTo-SecureString -String 'TestPassword' -AsPlainText -Force) ) Mock -CommandName Invoke-RsCimMethod -MockWith { return @{ KeyFile = [System.Text.Encoding]::UTF8.GetBytes('MockKeyFileContent') } } Mock -CommandName New-PSDrive Mock -CommandName Remove-PSDrive } It 'Should create and remove PSDrive when using UNC path with Credential' { $uncPath = '\\server\share\RSKey.snk' $null = Backup-SqlDscRSEncryptionKey -Configuration $mockCimInstance -Password $mockPassword -Path $uncPath -Credential $mockCredential -Confirm:$false Should -Invoke -CommandName New-PSDrive -Exactly -Times 1 Should -Invoke -CommandName Remove-PSDrive -Exactly -Times 1 } }
- Added DriveName parameter to Backup-SqlDscRSEncryptionKey and Restore-SqlDscRSEncryptionKey functions to specify temporary PSDrive names when using UNC paths with credentials. - Updated parameter sets to include DriveName, ensuring it can only be used with the Credential parameter. - Modified the logic in both functions to utilize the DriveName parameter for creating and removing PSDrives. - Enhanced integration tests for Backup-SqlDscRSEncryptionKey and Restore-SqlDscRSEncryptionKey to validate the new DriveName functionality and ensure proper PSDrive management. - Updated unit tests for both functions to reflect changes in parameter sets and to test UNC path scenarios with credentials.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@source/Public/Restore-SqlDscRSEncryptionKey.ps1`:
- Around line 153-171: The code sets $sourcePath to a PSDrive path (via
New-PSDrive when $PSBoundParameters.ContainsKey('Credential')) but then calls
[System.IO.File]::ReadAllBytes($sourcePath) which will fail for PSDrive URIs;
before calling System.IO.File::ReadAllBytes, detect PSDrive-style paths (the
$sourcePath created from $DriveName and file leaf) and resolve them to the
underlying filesystem path using Resolve-Path (e.g. (Resolve-Path -LiteralPath
$sourcePath).ProviderPath) and pass that resolved path into
[System.IO.File]::ReadAllBytes; keep existing $psDriveCreated cleanup logic
intact so the temporary PSDrive created by New-PSDrive is removed after use.
In `@tests/Unit/Public/Backup-SqlDscRSEncryptionKey.Tests.ps1`:
- Around line 46-64: Add unit tests that verify parameter attribute metadata for
the public cmdlet Backup-SqlDscRSEncryptionKey by using Get-Command -Name
'Backup-SqlDscRSEncryptionKey' and inspecting its .Parameters collection for the
Configuration, Path, and Password parameters; specifically assert that each
parameter's .Attributes contains a ParameterAttribute with Mandatory = $true
(and any expected ParameterSetName values if relevant). Create new It blocks
(e.g., "Should have mandatory attributes for Configuration/Path/Password") that
retrieve (Get-Command ...).Parameters['Configuration'|'Path'|'Password'] and
assert the Mandatory flag on the ParameterAttribute is true, following the
existing test style and naming.
♻️ Duplicate comments (1)
source/Public/Restore-SqlDscRSEncryptionKey.ps1 (1)
53-56: Avoid plaintext SecureString in the exampleThe second example uses
ConvertTo-SecureString -AsPlainText -Force, which embeds a password in plaintext. PreferRead-Host -AsSecureStringfor the example instead. As per coding guidelines, avoidConvertTo-SecureString -AsPlainTextin examples.🔐 Suggested example update
- $password = ConvertTo-SecureString -String 'MyP@ssw0rd' -AsPlainText -Force + $password = Read-Host -AsSecureString -Prompt 'Enter encryption key password'
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@source/Public/Restore-SqlDscRSEncryptionKey.ps1`:
- Around line 91-93: Update the OutputType attribute on
Restore-SqlDscRSEncryptionKey to reflect the documented return types: change the
generic [System.Object] to include
Microsoft.Management.Infrastructure.CimInstance (and/or None by default) so that
when PassThru is used the function correctly advertises it returns a CIM
instance; locate the OutputType line in the function declaration and replace or
extend it to reference Microsoft.Management.Infrastructure.CimInstance to match
the actual returned value ($Configuration) when PassThru is specified.
♻️ Duplicate comments (1)
source/Public/Restore-SqlDscRSEncryptionKey.ps1 (1)
53-56: Avoid plaintext SecureString in the example.The second example still constructs a SecureString from plaintext, which conflicts with the module’s security guidance. Use
Read-Host -AsSecureStringhere (and mirror the same change in the backup cmdlet example). As per coding guidelines, avoid plaintext SecureString creation in examples.✅ Suggested update
- $password = ConvertTo-SecureString -String 'MyP@ssw0rd' -AsPlainText -Force + $password = Read-Host -AsSecureString -Prompt 'Enter backup password'
🧹 Nitpick comments (1)
tests/Unit/Public/Restore-SqlDscRSEncryptionKey.Tests.ps1 (1)
96-98: Assign unused command output to$nullin It blocks.Several It blocks invoke
Restore-SqlDscRSEncryptionKeywithout using the result; capture to$nullto keep test output clean (apply similarly to other It blocks that don’t assert on return values). As per test guidelines, assign unused outputs to$null.♻️ Example update
- $mockCimInstance | Restore-SqlDscRSEncryptionKey -Password $mockPassword -Path $script:testKeyFilePath -Confirm:$false + $null = $mockCimInstance | Restore-SqlDscRSEncryptionKey -Password $mockPassword -Path $script:testKeyFilePath -Confirm:$false
…ng Credential parameter with UNC path
…for better debugging
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tests/Unit/Public/Backup-SqlDscRSEncryptionKey.Tests.ps1`:
- Around line 46-79: Add Pester unit tests for Backup-SqlDscRSEncryptionKey that
exercise the ByCredential path handling: mock New-PSDrive, Remove-PSDrive and
Resolve-Path to verify a temporary PSDrive is created when a UNC path is
supplied with Credential and that Resolve-Path returns the mapped path; assert
New-PSDrive is called with the default DriveName when none provided and with the
custom DriveName when supplied; add tests that simulate New-PSDrive throwing and
verify Remove-PSDrive is still invoked (cleanup on error) and the error bubbles
up; include a successful flow test that verifies Remove-PSDrive is called during
normal completion.
In `@tests/Unit/Public/Restore-SqlDscRSEncryptionKey.Tests.ps1`:
- Around line 46-79: Add unit tests to Restore-SqlDscRSEncryptionKey.Tests.ps1
that exercise the Credential/DriveName/UNC code paths: add Context blocks that
(1) call the Restore-SqlDscRSEncryptionKey cmdlet with a UNC Path and a
Credential to assert New-PSDrive is invoked and the cmdlet succeeds, (2) call
the cmdlet with Credential but a non-UNC Path to assert the New-PSDrive branch
is NOT entered, (3) simulate New-PSDrive failure to assert the cmdlet surfaces
the error, and (4) simulate successful New-PSDrive and verify Remove-PSDrive is
invoked for cleanup; target mocks/stubs around New-PSDrive and Remove-PSDrive
and check behavior for the Credential, DriveName, Path, and Password parameters
on the Restore-SqlDscRSEncryptionKey cmdlet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request (PR) description
Backup-SqlDscRSEncryptionKeyandRestore-SqlDscRSEncryptionKeyto backup and restore Reporting Servicesencryption keys. These commands wrap the
BackupEncryptionKeyandRestoreEncryptionKeyCIM methods and require a password to secure the key.This Pull Request (PR) fixes the following issues
None.
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is