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

Add-DbaDbRoleMember - Unable to Add Role #9302

Closed
mattcargile opened this issue Mar 28, 2024 · 4 comments · Fixed by #9319
Closed

Add-DbaDbRoleMember - Unable to Add Role #9302

mattcargile opened this issue Mar 28, 2024 · 4 comments · Fixed by #9319
Assignees
Labels
bugs life triage required New issue that has not been reviewed by maintainers

Comments

@mattcargile
Copy link
Contributor

mattcargile commented Mar 28, 2024

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.

if ($db.Users.Name -contains $username) {

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]'
@mattcargile mattcargile added bugs life triage required New issue that has not been reviewed by maintainers labels Mar 28, 2024
@andreasjordan
Copy link
Contributor

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.

@andreasjordan andreasjordan self-assigned this Mar 31, 2024
@mattcargile
Copy link
Contributor Author

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

Just opened a pull request for that.

@andreasjordan
Copy link
Contributor

The pull request is now ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugs life triage required New issue that has not been reviewed by maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants