Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New-DbaDbUser - Multiple -SqlInstance Without -Database Doesn't Add User To All Dbs #9340

Open
mattcargile opened this issue May 2, 2024 · 3 comments

Comments

@mattcargile
Copy link
Contributor

mattcargile commented May 2, 2024

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

No error message. Login/User isn't added on all non-system databases.

Steps to Reproduce

New-DbaDbUser -SqlInstance db1, db2 -User 'User'

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

Pulled from recent development.

Other details or mentions

Some how $getdbparam retains multiple databases in the Database key after the first full iteration of the $instance loop. I'm not entirely sure how this happens. Maybe something with the handling of the cloning.

I was playing with the below breakpoint.

Set-PSBreakpoint -Script public\New-DbaDbUser.ps1 -Line 166 -Action { if($getdbparam.Database.Count -ne 1) { break } }

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

Name                           Value
----                           -----
PSVersion                      7.4.1
PSEdition                      Core
GitCommitId                    7.4.1
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

SQL Server Edition and Build number

Microsoft SQL Server 2017 (RTM-CU28) (KB5008084) - 14.0.3430.2 (X64) 
	Dec 17 2021 14:30:27 
	Copyright (C) 2017 Microsoft Corporation
	Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2016 Standard 10.0 <X64> (Build 14393: ) (Hypervisor)

.NET Framework Version

.NET 8.0.1
@mattcargile mattcargile added bugs life triage required New issue that has not been reviewed by maintainers labels May 2, 2024
@andreasjordan
Copy link
Contributor

Hi Matt,

here is (most likely) the bug that was introduced in #9078:

            #This block is required so that correct error message can be returned to the user when incorrect database name is given or the database doesn't exists in the server.
            if (-not $Database) {
                $Database = $databases.Name
            }

This only works in the first run, where $Database is not set and thus all the databases on that instance are used.
On the second run, $Database is set (by the first run) and thus only the databases on the first instance are used.

To get this to work, $Database must remain unchanged, So for the loop, we need another variable which holds that databases that should be processed for this instance.

Maybe @sqlarticles can provide a fix for that. Or I can have a look in the next week or two.

@andreasjordan andreasjordan removed the triage required New issue that has not been reviewed by maintainers label May 19, 2024
@sqlarticles
Copy link
Contributor

Thanks @andreasjordan for looking into this.

I will try and reproduce the error and release a fix.

@wsmelton
Copy link
Member

The test in that PR isn't actually validating the expected result. It only looks at the login name existing, it should also include validating it exist in all the databases too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants