Export-DbaCredential: Open DAC unless ExcludePassword used#10112
Export-DbaCredential: Open DAC unless ExcludePassword used#10112potatoqualitee merged 4 commits intodevelopmentfrom
Conversation
|
I did a complete refactoring and used the main logic from Export-DbaLinkedServer. |
|
than you andreas! but where did inputobject go? |
|
InputObject is gone. We have a general problem with piping: We can use pipeline input in every command that only needs information from the piped input. But as soon as we need additional information we run into problems. In case of Export-DbaCredential, the command does not only export information that we get from Get-DbaCredential, but also decryptes the passwords. And then we need a new DAC to the server. Maybe that is possible but there would be massive code to check all cases. Having only a limited amount of time, we should generally review some of the edge case functionallity of the commands and remove them to enhance code quality and maintanability. |
|
@claude can the piping problem be solved with PipelineValueByPropertyName? |
|
Claude finished @potatoqualitee's task in 1m 2s —— View job Analysis: ValueFromPipelineByPropertyName Solution
Short AnswerNo, The Core ProblemWhen you pipe from
So even if we bind
How Export-DbaLinkedServer Handles This (Line 131)Looking at $InputObject += $server.LinkedServersIt adds to
Could Export-DbaCredential Do The Same?Technically yes, but there's a catch. You'd need to:
Here's the pseudocode: param (
[Parameter(ValueFromPipeline)]
[DbaInstanceParameter[]]$SqlInstance,
[Microsoft.SqlServer.Management.Smo.Credential[]]$InputObject,
# ... other params
)
process {
if ($InputObject) {
# Group credentials by their parent instance
$groupedCreds = $InputObject | Group-Object { $_.Parent.ConnectionContext.ServerInstance }
foreach ($group in $groupedCreds) {
$instance = $group.Name
$credsToExport = $group.Group
if (-not $ExcludePassword) {
# Open new DAC to this instance for password decryption
$dacServer = Connect-DbaInstance -SqlInstance $instance -DedicatedAdminConnection
# Re-fetch credentials with decryption
$decrypted = Get-DecryptedObject -SqlInstance $dacServer -Type Credential
# Match piped credentials with decrypted ones
# ... export logic ...
}
}
}
# Handle SqlInstance parameter as it does now
}The Complexity Trade-offAndreas's point in the comments is valid: This adds significant complexity:
Compare this to the current simple approach:
RecommendationFor typical use cases: # Simple and clear
Export-DbaCredential -SqlInstance sql2017 -Identity "MyApp_Credential"
# Instead of piping
Get-DbaCredential -SqlInstance sql2017 -Name "MyApp_Credential" | Export-DbaCredentialIf piping is truly desired, the best approach would be:
Bottom line: |
|
I totally agree with claude. This is where I like claude: It can write english much better than me. So yes, it's not so much about "is it possible", but more about "who is willing to spend the time for implementation and maintanance(!!!)". And I am currently not willing to spend that extra time just for the pipeline functionality. |
|
Claude writes better English than everyone 😅 And is good at organizing for skimming. @claude im having a hard time letting go of the piping.. could we simply collect the objects then in the end {} block, close their connections then reopen for dac? |
|
Claude finished @potatoqualitee's task in 1m 6s —— View job Analysis: Collecting Objects Then Reopening DAC
Short AnswerYes, this is technically feasible and would be much simpler than the approach discussed earlier. The key insight is that How It Would Workparam (
[DbaInstanceParameter[]]$SqlInstance,
[Parameter(ValueFromPipeline)]
[Microsoft.SqlServer.Management.Smo.Credential[]]$InputObject,
[PSCredential]$SqlCredential,
# ... other params
)
begin {
$credentialCollection = @()
}
process {
if ($InputObject) {
# Collect all piped credential objects
$credentialCollection += $InputObject
}
if ($SqlInstance) {
# Handle SqlInstance parameter immediately (current behavior)
foreach ($instance in $SqlInstance) {
# ... existing export logic ...
}
}
}
end {
if ($credentialCollection) {
# Group credentials by their parent instance
$groupedCreds = $credentialCollection | Group-Object {
$_.Parent.ConnectionContext.ServerInstance
}
foreach ($group in $groupedCreds) {
$instance = $group.Name
$credNames = $group.Group.Name
# At this point, the piped connection is no longer in use
# We can safely open our own DAC connection
if (-not $ExcludePassword) {
$server = Connect-DbaInstance -SqlInstance $instance -SqlCredential $SqlCredential -MinimumVersion 9 -DedicatedAdminConnection
$credentials = Get-DecryptedObject -SqlInstance $server -Type Credential
} else {
$server = Connect-DbaInstance -SqlInstance $instance -SqlCredential $SqlCredential -MinimumVersion 9
$credentials = foreach ($cred in $server.Credentials) {
# ... build credential objects without password ...
}
}
# Filter to only the credentials that were piped
$credentials = $credentials | Where-Object Name -in $credNames
# ... existing export logic ...
if (-not $ExcludePassword) {
$null = $server | Disconnect-DbaInstance -WhatIf:$false
}
}
}
}Why This Works
Key Benefits
Potential Issues
Comparison with Current Approach# Current approach (no piping)
Export-DbaCredential -SqlInstance sql2017 -Identity "MyApp_Cred"
# With this solution (piping works)
Get-DbaCredential -SqlInstance sql2017 -Credential "MyApp_Cred" | Export-DbaCredential
# Also supports cross-instance scenarios
Get-DbaCredential -SqlInstance sql2017, sql2019 | Export-DbaCredential -Path C:\tempImplementation ComplexityThis is significantly simpler than the approach discussed in my earlier comment because:
RecommendationThis approach is viable and maintains the expected PowerShell pipeline behavior without significant complexity. The trade-off is a new connection per instance, but that's acceptable given:
If you want pipeline support back, this would be a clean way to implement it. |
|
Alllllllllright thank you andreas 😁 |
Helps to solve #10111 as the code is now aligned with Export-DbaLinkedServer.