Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3fd41b9
added ability for user_role to take a list for roles
harCamConsulting Aug 5, 2025
8386b2a
updated changelog
harCamConsulting Aug 5, 2025
40e634e
updated documentation for new option
harCamConsulting Aug 5, 2025
4c41f7b
change output of module so that unit testing can test on more things …
harCamConsulting Aug 6, 2025
c8f0821
typo in unit tests
harCamConsulting Aug 6, 2025
d9ffdad
Merge branch 'main' into add-list-to-dbroles
lowlydba Aug 16, 2025
b3693bd
Merge branch 'main' into add-list-to-dbroles
lowlydba Aug 17, 2025
62db4fa
refactor to implement add/remove/set functionality
harCamConsulting Aug 26, 2025
3028fe7
Merge branch 'add-list-to-dbroles' of github.com:harCamConsulting/low…
harCamConsulting Aug 26, 2025
3a47ebc
Merge branch 'lowlydba:main' into add-list-to-dbroles
harCamConsulting Aug 26, 2025
97b68c7
removing random symlink
harCamConsulting Aug 26, 2025
d66a924
remove unused param, plus linting
harCamConsulting Aug 26, 2025
5c2ab58
fix typo in docs
harCamConsulting Aug 26, 2025
90783b1
fix typo in tests
harCamConsulting Aug 26, 2025
ceb87e6
updating for more linting / style
harCamConsulting Aug 26, 2025
fca53e7
roleMembership typo
harCamConsulting Aug 26, 2025
dd8946d
typos
harCamConsulting Aug 26, 2025
832774c
defaults in doc to match code
harCamConsulting Aug 26, 2025
d85b0a4
change to other type of foreach
harCamConsulting Aug 26, 2025
16ae9fa
alter results so it returns empty arrays instead of null
harCamConsulting Aug 26, 2025
88f5578
Merge branch 'main' into add-list-to-dbroles
lowlydba Sep 5, 2025
e33a412
use proper array casts
harCamConsulting Sep 9, 2025
4b0dea4
Merge branch 'add-list-to-dbroles' of github.com:harCamConsulting/low…
harCamConsulting Sep 9, 2025
0fcedae
catch null and convert to empty array on output rolemembership
harCamConsulting Sep 9, 2025
504b80e
Merge branch 'main' into add-list-to-dbroles
lowlydba Jan 8, 2026
0fc0192
Apply suggestions from code review
lowlydba Jan 8, 2026
d9f3485
Apply suggestions from code review
lowlydba Jan 8, 2026
8eee960
Apply suggestions from code review
lowlydba Jan 8, 2026
720455d
Apply suggestions from code review
lowlydba Jan 8, 2026
29bccba
Apply suggestions from code review
lowlydba Jan 8, 2026
28a3a2c
Apply suggestions from code review
lowlydba Jan 8, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelogs/changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -573,3 +573,7 @@ releases:
fragments:
- 314-ansible-2-19-compatibility.yml
release_date: '2025-05-03'
2.6.2:
changes:
minor_changes:
- Added ability for user_role module to take a list of roles. Optionally can also remove unlisted roles.
Comment thread
lowlydba marked this conversation as resolved.
2 changes: 1 addition & 1 deletion galaxy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace: lowlydba
name: sqlserver
version: 2.6.1
version: 2.6.2
readme: README.md
authors:
- John McCall (github.com/lowlydba)
Expand Down
156 changes: 103 additions & 53 deletions plugins/modules/user_role.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@ $spec = @{
options = @{
database = @{type = 'str'; required = $true }
username = @{type = 'str'; required = $true }
role = @{type = 'str'; required = $true }
roles = @{type = 'list'; elements = 'str'; required = $true; aliases = 'role' }
state = @{type = 'str'; required = $false; default = 'present'; choices = @('present', 'absent') }
remove_unlisted = @{type = 'bool'; required = $false; default = 'false' }
}
}

$module = [Ansible.Basic.AnsibleModule]::Create($args, $spec, @(Get-LowlyDbaSqlServerAuthSpec))
$sqlInstance, $sqlCredential = Get-SqlCredential -Module $module
$username = $module.Params.username
$database = $module.Params.database
$role = $module.Params.role
$roles = $module.Params.roles
$state = $module.Params.state
$remove_unlisted = $module.Params.remove_unlisted
$checkMode = $module.CheckMode

$module.Result.changed = $false
Expand All @@ -37,78 +39,126 @@ $getUserSplat = @{
User = $username
EnableException = $true
}
$getRoleSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
Database = $database
Role = $role
EnableException = $true

$outputProps = @{}

# Verify user and role(s) exist, DBATools currently fails silently
$existingUser = Get-DbaDbUser @getUserSplat
if ($null -eq $existingUser) {
$module.FailJson("User [$username] does not exist in database [$database].")
}

$roles | ForEach-Object {
$thisRole = $_
$getRoleSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
Database = $database
Role = $thisrole
Comment thread
harCamConsulting marked this conversation as resolved.
Outdated
EnableException = $true
}
$existingRole = Get-DbaDbRole @getRoleSplat
if ($null -eq $existingRole) {
$module.FailJson("Role [$thisRole] does not exist in database [$database].")
}
}

# Get role members of all roles we care about to compare against later
$getRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
Database = $database
Role = $role
IncludeSystemUser = $true
EnableException = $true
}

# Verify user and role exist, DBATools currently fails silently
$existingUser = Get-DbaDbUser @getUserSplat
if ($null -eq $existingUser) {
$module.FailJson("User [$username] does not exist in database [$database].")
}
$existingRole = Get-DbaDbRole @getRoleSplat
if ($null -eq $existingRole) {
$module.FailJson("Role [$role] does not exist in database [$database].")
}

# Get role members
$existingRoleMembers = Get-DbaDbRoleMember @getRoleMemberSplat
$existingRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object

if ($state -eq "absent") {
if ($existingRoleMembers.username -contains $username) {
try {
$removeRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
User = $username
Database = $database
Role = $role
EnableException = $true
WhatIf = $checkMode
Confirm = $false
$roles | ForEach-Object {
$thisRole = $_
if ( $existingRoleMembership -contains $thisRole ) {
try {
$removeRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
User = $username
Database = $database
Role = $thisRole
EnableException = $true
WhatIf = $checkMode
Confirm = $false
}
Remove-DbaDbRoleMember @removeRoleMemberSplat
$module.Result.changed = $true
}
catch {
$module.FailJson("Removing user [$username] from database role [$thisRole] failed: $($_.Exception.Message)", $_)
}
$output = Remove-DbaDbRoleMember @removeRoleMemberSplat
$module.Result.changed = $true
}
catch {
$module.FailJson("Removing user [$username] from database role [$role] failed: $($_.Exception.Message)", $_)
}
}
}
Comment on lines +123 to +150

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module allows specifying roles.set simultaneously with roles.add or roles.remove, but the logic at line 123 gives precedence to roles.set, effectively ignoring any add or remove specifications. This could lead to confusing behavior where users expect add/remove to be applied in addition to set.

Consider either:

  1. Making set mutually exclusive with add/remove in the parameter spec
  2. Documenting that set takes precedence and add/remove are ignored when set is provided
  3. Applying add/remove operations after the set operation

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harCamConsulting This makes sense to me, any of the options given seem good

elseif ($state -eq "present") {
# Add user to role
if ($existingRoleMembers.username -notcontains $username) {
try {
$addRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
User = $username
Database = $database
Role = $role
EnableException = $true
WhatIf = $checkMode
Confirm = $false
$roles | ForEach-Object {
$thisRole = $_
if ($existingRoleMembership -notcontains $thisRole) {
try {
$addRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
User = $username
Database = $database
Role = $thisRole
EnableException = $true
WhatIf = $checkMode
Confirm = $false
}
Add-DbaDbRoleMember @addRoleMemberSplat
$module.Result.changed = $true
}
catch {
$module.FailJson("Adding user [$username] to database role [$thisRole] failed: $($_.Exception.Message)", $_)
}
$output = Add-DbaDbRoleMember @addRoleMemberSplat
$module.Result.changed = $true
}
catch {
$module.FailJson("Adding user [$username] to database role [$role] failed: $($_.Exception.Message)", $_)
}
}
if ($state -eq "present" -and $remove_unlisted -eq $true) {
#remove users from roles that weren't listed (if we got the remove_unlisted option set to true)
$existingRoleMembership | ForEach-Object {
$thisRole = $_
if ($roles -notcontains $thisRole) {
try {
$removeRoleMemberSplat = @{
SqlInstance = $sqlInstance
SqlCredential = $sqlCredential
User = $username
Database = $database
Role = $thisRole
EnableException = $true
WhatIf = $checkMode
Confirm = $false
}
Remove-DbaDbRoleMember @removeRoleMemberSplat
$module.Result.changed = $true
}
catch {
$module.FailJson("Removing user [$username] from extra unlisted database role [$thisRole] failed: $($_.Exception.Message)", $_)
}
}
}
}

try {
#after changing any roles above, see what our new membership is and report it back
$newRoleMembership = Get-DbaDbRoleMember @getRoleMemberSplat | Where-Object {$_.UserName -eq $username} | Select -ExpandProperty role | Sort-Object
}
catch {
$module.FailJson("Failure getting new role membership: $($_.Exception.Message)", $_)
}
$outputProps.newRoleMembership = [array]$newRoleMembership
$outputProps.oldRoleMembership = [array]$existingRoleMembership
$output = New-Object -TypeName PSCustomObject -Property $outputProps

try {

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In non-compatibility mode when no changes occur, the code creates an output with just 'roleMembership' but no 'diff' (lines 198-204). However, when changes do occur, it includes both 'roleMembership' and 'diff'. This is good idempotent behavior. However, in the query-only mode (no role or roles specified), the code should also return the current role membership. The current logic doesn't handle the query-only case where no roles are specified to add/remove/set - it would skip to line 207 with $output potentially undefined if not in compatibility mode.

Suggested change
try {
try {
# In non-compatibility (roles dict) mode, if no output was created earlier (e.g. query-only),
# return the current role membership without a diff.
if (-not $compatibilityMode -and $null -eq $output) {
try {
$membershipObjects = Get-DbaDbRoleMember @commonParamSplat -IncludeSystemUser $true | Where-Object { $_.UserName -eq $username }
$currentRoleMembership = [array]($membershipObjects.role | Sort-Object)
if ($null -eq $currentRoleMembership) { $currentRoleMembership = @() }
}
catch {
$module.FailJson("Failure getting current role membership: $($_.Exception.Message)", $_)
}
if ($null -eq $outputProps) { $outputProps = @{} }
$outputProps.roleMembership = $currentRoleMembership
$output = New-Object -TypeName PSCustomObject -Property $outputProps
}

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harCamConsulting I think this comment makes sense, can you add this query-mode logic in?

if ($null -ne $output) {
$resultData = ConvertTo-SerializableObject -InputObject $output
Expand Down
32 changes: 30 additions & 2 deletions plugins/modules/user_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,18 @@
- Name of the user.
type: str
required: true
role:
roles:
description:
- The database role for the user to be modified.
type: str
type: list
required: true
remove_unlisted:
description:
- When set to true, will remove any other roles that aren't listed in roles.
type: boolean
required: false
default: false
version_added: 2.6.2
author: "John McCall (@lowlydba)"
requirements:
- L(dbatools,https://www.powershellgallery.com/packages/dbatools/) PowerShell module
Expand All @@ -45,6 +52,15 @@
database: InternProject1
role: db_owner

- name: Add a user to a list of db roles
lowlydba.sqlserver.user_role:
sql_instance: sql-01.myco.io
username: TheIntern
database: InternProject1
role:
Comment thread
harCamConsulting marked this conversation as resolved.
Outdated
- db_datareader
- db_datawriter

- name: Remove a user from a fixed db role
lowlydba.sqlserver.login:
sql_instance: sql-01.myco.io
Expand All @@ -60,6 +76,18 @@
database: InternProject1
role: db_intern
state: absent

- name: Specify a list of roles that user should be in and remove all others
lowlydba.sqlserver.login:
sql_instance: sql-01.myco.io
username: TheIntern
database: InternProject1
role:
Comment thread
harCamConsulting marked this conversation as resolved.
Outdated
- db_datareader
- db_datawriter
state: present
Comment thread
lowlydba marked this conversation as resolved.
remove_unlisted: true

'''

RETURN = r'''
Expand Down
1 change: 1 addition & 0 deletions sqlserver
36 changes: 36 additions & 0 deletions tests/integration/targets/user_role/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
- assert:
that:
- result is changed
- result.data.newRoleMembership == ["db_owner"]

- name: Add user to non-existent database role
lowlydba.sqlserver.user_role:
Expand Down Expand Up @@ -102,13 +103,48 @@
that:
- result is not changed
Comment thread
lowlydba marked this conversation as resolved.

- name: Add user to list of database roles
lowlydba.sqlserver.user_role:
role:
- db_datareader
- db_datawriter
register: result
- assert:
that:
- result is changed
- result.data.newRoleMembership == ["db_datareader", "db_datawriter", "db_owner"]

- name: Add user to list of database roles again
lowlydba.sqlserver.user_role:
role:
- db_datareader
- db_datawriter
register: result
- assert:
that:
- result is not changed

- name: Add user to list of database roles, and remove unlisted roles
lowlydba.sqlserver.user_role:
role:
- db_datareader
- db_datawriter
remove_unlisted: true
register: result
- assert:
that:
- result is changed
- result.data.newRoleMembership == ["db_datareader", "db_datawriter"]

- name: Remove user from database role
lowlydba.sqlserver.user_role:
state: "absent"
role: db_datawriter
register: result
- assert:
that:
- result is changed
- result.data.newRoleMembership == ["db_datareader"]

always:
- name: Drop user
Expand Down
Loading