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

`Add-DbaDbRoleMember` - Unable to Add Role #9302

Closed mattcargile closed 2 months ago

mattcargile commented 3 months ago

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

WARNING: [10:29:51][Add-DbaDbRoleMember] User MyRole does not exist in [database] on [sqlinstance]

Steps to Reproduce

New-DbaDbRole sqlinstance -Database database -Role MyRole
add-dbadbrolemember sqlinstance -Database database -Role db_datareader,db_denydatawriter -User MyRole

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

Major  Minor  Build  Revision
-----  -----  -----  --------
2      1      11     -1

Other details or mentions

One could update this section of code to check for $db.Roles.Name. https://github.com/dataplat/dbatools/blob/7af0e3aec6b9d621fa93af0e2ca5f1c621e7140c/public/Add-DbaDbRoleMember.ps1#L143

Maybe there needs to be another parameter called -DatabaseRole much like Add-DbaServerRoleMember? I guess that precedent overrides the simpler solution of accepting both $db.Roles and $db.Users for -User. Adding the new parameter -DatabaseRole might cause a breaking change or at least a confusing API. -DatabaseRole should be the roles that are to be altered. Then -Role and -User would be the principals to be added. If we add -DatabaseRole and it is not the role actually being altered then that is the reverse syntax to Add-DbaServerRoleMember. I think I would prefer the breaking change in the semantics of the parameters.

I could also see -Principal being a parameter instead though none of the cmdlets use that language, I think.

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

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

SQL Server Edition and Build number

Microsoft SQL Server 2019 (RTM-CU18) (KB5017593) - 15.0.4261.1 (X64)
        Sep 12 2022 15:07:06
        Copyright (C) 2019 Microsoft Corporation
        Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2019 Datacenter 10.0 <X64> (Build 17763: ) (Hypervisor)

.NET Framework Version

.NET 7.0.11

Workaround

Used T-SQL.

invoke-dbaquery -sqlinstance sqlinstance -Database database -Query 'ALTER ROLE [db_denydatawriter] ADD MEMBER [MyRole]'
andreasjordan commented 3 months ago

Let's have a look at other commands:

Add-LocalGroupMember -Group xyz -Member abc

So we can change the parameter name zu "Member" with an alias for compatibility.

mattcargile commented 3 months ago

I like that reference. Add-ADGroupMember has a similar syntax. Should Add-DbaServerRoleMember be changed? It would be nice to have -Members added that accepted both roles and logins?

andreasjordan commented 3 months ago

Just opened a pull request for that.

andreasjordan commented 3 months ago

The pull request is now ready for review.