dataplat / dbatools

🚀 SQL Server automation and instance migrations have never been safer, faster or freer
https://dbatools.io
MIT License
2.39k stars 787 forks source link

Set-DbaLogin - processing all logins even if you only pass in one #8980

Open wsmelton opened 1 year ago

wsmelton commented 1 year ago

Verified issue does not already exist?

No, I did not search

What error did you receive?

No error, look at the verbose output of this command. Even if you provide a list of Logins to -Login it is still pulling every login on the instance.

Because of this section of code, not sure why it was done this way, but we should not be pulling every possible login, and then filtering it.

https://github.com/dataplat/dbatools/blob/0433f3260cc448b8ca932b8da4a4b777a656768a/public/Set-DbaLogin.ps1#L240-L250

Steps to Reproduce

Set-DbaLogin -SqlInstance SomeServer -Login SomeLogin1 -Verbose

image

Please confirm that you are running the most recent version of dbatools

Development branch

Other details or mentions

No response

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

N/A

SQL Server Edition and Build number

N/A

.NET Framework Version

N/A

wsmelton commented 1 year ago

I can see here that all the logins get referenced for the rename process but there is lighter methods for checking this; even if we just create internal test function like Test-LoginExist or something.

https://github.com/dataplat/dbatools/blob/0433f3260cc448b8ca932b8da4a4b777a656768a/public/Set-DbaLogin.ps1#L298-L311

wsmelton commented 1 year ago

I don't think there is a need to actually check for a login because the rename operation should fail with a clean error indicating it already exist.

wsmelton commented 1 year ago

Removed pulling all logins and the test alone dropped more than half the time to run

image

Also adding more granular WhatIf so folks can see what actually gets changed.

image

Adjust the Alter method to only get hit if something beside the password parameters were provided.