user_role: Add roles list input for user_role#352
Merged
Conversation
Introduce a new 'roles' dictionary parameter to lowlydba.sqlserver.user_role to manage multiple roles using add/remove/set semantics. Update PowerShell implementation to handle compatibility mode for the legacy single 'role' parameter (now deprecated and optional), perform validation, and return detailed membership changes. Update Python documentation, examples and return spec, add integration tests covering multi-role add/remove/set and edge cases, and include a changelog fragment noting deprecation of 'role' and the new 'roles' behavior.
Add explicit validation in plugins/modules/user_role.ps1 to reject using the 'state' parameter when 'roles' is supplied; the module now fails with a clear message instructing to use roles.add, roles.remove, or roles.set for membership changes. Remove the ('roles','state') entry from mutually_exclusive and update the integration test to assert the new error message.
Docs Build 📝Thank you for contribution!✨ This PR has been merged and the docs are now incorporated into |
lowlydba
changed the title
PowerShell module: remove default at param level for state and enforce compatibility behavior so state is only applied in legacy `role` mode (defaulted to 'present' when `role` is used). Reject `state` when using the new `roles` dictionary. Clean up spec array formatting (mutually_exclusive / required_one_of) and add explicit checks in both legacy and new code paths. Python docs: update option docs to clarify `roles` keys (add/remove/set) and mark `role` as deprecated with guidance; clarify that `state` applies only to the legacy `role` parameter and remove its module-level default. Tests: minor task name quoting updates to match linting/formatting. These changes prevent ambiguous parameter combinations and improve UX for the new roles API.
PowerShell: Clean up user_role.ps1 by constructing the state error message in a variable, removing unused compatibilityMode and queryMode assignments, and reflowing long Compare-Object pipelines for readability; no behavioral changes intended. Python: update copyright year in user_role.py (2022 -> 2026) and ensure trailing newline. Tests: add missing trailing newline to tests/integration/targets/user_role/tasks/main.yml.
Use double quotes around "state" in the expected error message within the integration test to avoid YAML/quote parsing issues and match the actual error output.
Append PR reference (#352) to the user_role changelog entry and make the integration test assertion more robust: replace a direct substring check with an Ansible search() expression to match the expected error message pattern in tests/integration/targets/user_role/tasks/main.yml.
Extract the Role.Name when building currentRoleMembership so the array contains role names (strings) instead of role objects. Updated three occurrences in plugins/modules/user_role.ps1 to cast and sort the Name property, preventing object comparison issues and ensuring accurate membership checks while preserving existing null handling.
Replace the Compare-Object + SideIndicator pipeline with a direct membership filter when building $toRemove in plugins/modules/user_role.ps1. The new line uses:
$toRemove = @($roles['remove'] | Where-Object { $_ -in $currentRoleMembership })
This is simpler, more readable, avoids relying on Compare-Object SideIndicator semantics, and ensures $toRemove is an array so .Count behaves consistently.
Add validation in the PowerShell module to prevent combining roles.set with roles.add or roles.remove, and remove the previous immediate rejection of the legacy 'state' parameter in the roles branch (handled by overall validation). Update the Python module docs for the 'state' option to state that it "Cannot be used with roles" instead of being "Ignored when using roles", clarifying intended usage and preventing invalid option combinations.
Treat Ansible 'devel' matrix runs as non-blocking by setting continue-on-error for sanity and integration jobs in ansible-test.yml and ansible-test-windows.yml. Add ci-test-rollup and win-ci-test-rollup jobs that run lowlydba/are-we-good to aggregate and report the needs results (use needs + if: always()).
7 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the lowlydba.sqlserver.user_role module to support managing multiple database role memberships in one task via a new roles parameter (with add/remove/set operations), while keeping the legacy single-role + state behavior (now marked deprecated). It also expands integration coverage and updates release/CI metadata consistent with a new collection release.
Changes:
- Add
roles: { add/remove/set }support to theuser_rolemodule (and deprecaterole). - Expand
tests/integration/targets/user_roleto cover multi-role add/remove/set/idempotency and validation errors. - Update changelog/release notes and CI workflow behavior (e.g., devel matrix
continue-on-error+ rollup job).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
plugins/modules/user_role.ps1 |
Implements the new multi-role management logic (roles.add/remove/set) and legacy-mode constraints. |
plugins/modules/user_role.py |
Documents the new parameter, updated examples, and enriched return structure. |
tests/integration/targets/user_role/tasks/main.yml |
Adds integration scenarios for multi-role operations, idempotency, and error cases; keeps legacy tests. |
changelogs/changelog.yaml |
Adds the 2.8.0 release entry describing the feature. |
CHANGELOG.rst |
Publishes rendered release notes for v2.8.0. |
changelogs/fragments/remove-six-usage.yml |
Removes a fragment file as part of release/changelog maintenance. |
README.md |
Adds an “immutable tags” badge/link. |
.github/workflows/ansible-test.yml |
Allows devel matrix jobs to be non-blocking and adds a rollup job. |
.github/workflows/ansible-test-windows.yml |
Same devel non-blocking + rollup job for Windows CI. |
Treat presence of roles keys as intent and allow empty lists: plugins/modules/user_role.ps1 now checks key presence (so roles.set: [] is a valid explicit operation that removes all roles), prevents combining roles.set with add/remove by key presence, and avoids Compare-Object on two empty lists. plugins/modules/user_role.py docs were updated to document that add/remove empty lists are no-ops, set: [] removes all memberships, and that set cannot be combined with add/remove; RETURN docs were clarified. tests/integration/targets/user_role/tasks/main.yml updated: removed an obsolete empty-list query test and added tests to verify set: [] removes all memberships, is idempotent, and errors when combined with add.
Remove the GHWS environment variable from the ansible-test-windows workflow since it is dynamically computed earlier and not needed in the job env. In the release workflow, replace the GHP_BASE_URL env reference with an explicit GitHub Pages URL constructed from the repository owner and repository name (https://<owner>.github.io/<repo>), simplifying environment resolution for releases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A secondary attempt to add
rolesthat accepts lists foruser_role.✨ Use only immutable releases/tags going forward.
How Has This Been Tested?
We'll validate with CI tests
Types of changes
Checklist:
version_addedproperty.