Skip to content

Comments

Export-DbaCredential: Open DAC unless ExcludePassword used#10112

Merged
potatoqualitee merged 4 commits intodevelopmentfrom
change_Export-DbaCredential
Jan 30, 2026
Merged

Export-DbaCredential: Open DAC unless ExcludePassword used#10112
potatoqualitee merged 4 commits intodevelopmentfrom
change_Export-DbaCredential

Conversation

@andreasjordan
Copy link
Collaborator

Helps to solve #10111 as the code is now aligned with Export-DbaLinkedServer.

@andreasjordan
Copy link
Collaborator Author

I did a complete refactoring and used the main logic from Export-DbaLinkedServer.

@potatoqualitee
Copy link
Member

than you andreas! but where did inputobject go?

@andreasjordan
Copy link
Collaborator Author

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.

@potatoqualitee
Copy link
Member

@claude can the piping problem be solved with PipelineValueByPropertyName?

@claude
Copy link

claude bot commented Jan 30, 2026

Claude finished @potatoqualitee's task in 1m 2s —— View job


Analysis: ValueFromPipelineByPropertyName Solution

  • Read Export-DbaCredential.ps1 (current refactored version)
  • Read Export-DbaLinkedServer.ps1 (pattern being followed)
  • Read Get-DbaCredential.ps1 (to understand pipeline properties)
  • Analyze ValueFromPipelineByPropertyName feasibility
  • Provide technical answer with code examples

Short Answer

No, ValueFromPipelineByPropertyName won't solve the fundamental problem here. The issue isn't about binding properties—it's about needing a second DAC connection when piping from Get-DbaCredential.

The Core Problem

When you pipe from Get-DbaCredential to Export-DbaCredential, here's what happens:

  1. Get-DbaCredential opens a normal connection and returns SMO credential objects
  2. Export-DbaCredential (without -ExcludePassword) needs to decrypt passwords, which requires a DAC connection
  3. The problem: You can only have one DAC connection per instance at a time

So even if we bind SqlInstance via ValueFromPipelineByPropertyName, we still hit this wall:

  • The piped object came from a connection that's already open (normal or DAC)
  • Export needs to open a DAC to decrypt passwords
  • If Get used normal connection → Export can open DAC ✓
  • But Export would need to connect to the same instance that the piped credential came from
  • We'd need to extract the instance from the credential object's parent server

How Export-DbaLinkedServer Handles This (Line 131)

Looking at Export-DbaLinkedServer.ps1:131, it does something clever:

$InputObject += $server.LinkedServers

It adds to $InputObject rather than accepting pre-fetched objects exclusively. This means:

  • When you provide -SqlInstance, it connects itself and gets the linked servers
  • When you provide -InputObject, it uses those objects (but they must be full SMO objects with parent server references)

Could Export-DbaCredential Do The Same?

Technically yes, but there's a catch. You'd need to:

  1. Accept [Microsoft.SqlServer.Management.Smo.Credential[]]$InputObject parameter
  2. Extract the parent server from $InputObject.Parent
  3. Check if that server is a DAC connection
  4. If not DAC, open a new DAC connection to that instance
  5. Re-fetch the credentials with decryption

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-off

Andreas's point in the comments is valid: This adds significant complexity:

  • Need to handle multiple instances if piping mixed credentials
  • Need to ensure DAC isn't already in use
  • Need proper error handling if DAC fails
  • Need to match piped credential names with decrypted ones
  • Code becomes harder to maintain

Compare this to the current simple approach:

  • You specify the instance explicitly → one connection path
  • Clear error messages if something fails
  • Maintainable, predictable behavior

Recommendation

For typical use cases:

# Simple and clear
Export-DbaCredential -SqlInstance sql2017 -Identity "MyApp_Credential"

# Instead of piping
Get-DbaCredential -SqlInstance sql2017 -Name "MyApp_Credential" | Export-DbaCredential

If piping is truly desired, the best approach would be:

  1. Document that -InputObject requires you to handle the instance grouping
  2. Use -SqlInstance as the primary interface
  3. Keep the code simple and maintainable

Bottom line: ValueFromPipelineByPropertyName alone doesn't solve it—the real issue is the DAC connection requirement conflicting with pipeline scenarios. The complexity of supporting both patterns may not justify the convenience benefit.


@andreasjordan
Copy link
Collaborator Author

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.

@potatoqualitee
Copy link
Member

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
Copy link

claude bot commented Jan 30, 2026

Claude finished @potatoqualitee's task in 1m 6s —— View job


Analysis: Collecting Objects Then Reopening DAC

  • Read Export-DbaCredential.ps1 (current refactored version)
  • Read Get-DbaCredential.ps1 (to understand piped objects)
  • Analyze feasibility of collect → close → reopen DAC approach
  • Provide technical assessment with code examples

Short Answer

Yes, this is technically feasible and would be much simpler than the approach discussed earlier. The key insight is that Get-DbaCredential returns SMO credential objects that contain the credential names and their parent server reference, which gives us everything we need.

How It Would Work

param (
    [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

  1. SMO objects contain instance info: Get-DbaCredential returns Microsoft.SqlServer.Management.Smo.Credential objects that have a .Parent property referencing the server
  2. Connection independence: By the time we reach the end {} block, the pipeline has completed and any connections used by Get-DbaCredential are independent
  3. Simple filtering: We just need to match credential names from the piped objects
  4. No connection conflicts: We open our own fresh DAC connection in the end {} block

Key Benefits

  • Preserves pipeline support: Natural PowerShell pattern works
  • Simple implementation: Just collect → group → reconnect → export
  • No complexity: Avoid the multi-instance coordination issues from the earlier discussion
  • Predictable behavior: Each instance gets its own DAC connection as needed

Potential Issues

  1. Connection overhead: Opens new connection even though Get-DbaCredential already connected

    • Mitigation: This is normal PowerShell behavior; piping is for convenience, not performance
  2. DAC availability: If Get-DbaCredential used DAC for some reason, we'd conflict

    • Reality check: Get-DbaCredential uses normal connections (line 115), never DAC
  3. Credentials might be deleted between Get and Export

    • Mitigation: Same risk exists today with any time gap; not a pipeline-specific issue

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:\temp

Implementation Complexity

This is significantly simpler than the approach discussed in my earlier comment because:

  • No need to handle connections from piped objects
  • No need to check if connections are DAC
  • Just extract instance names and credential names, then reconnect fresh
  • The end {} block pattern is standard PowerShell practice

Recommendation

This 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:

  • Pipeline scenarios typically aren't performance-critical
  • The code remains simple and maintainable
  • Users get the natural piping experience they expect from PowerShell

If you want pipeline support back, this would be a clean way to implement it.


@potatoqualitee
Copy link
Member

Alllllllllright

thank you andreas 😁

@potatoqualitee potatoqualitee merged commit 6449a95 into development Jan 30, 2026
14 checks passed
@potatoqualitee potatoqualitee deleted the change_Export-DbaCredential branch January 30, 2026 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants