salt: [BUG] user.present does not leave existing groups alone when `groups` is unset

Description salt.states.user.present documentation states that when the groups parameter is left unset, the groups for a given user will not be changed. The current behavior in saltstack seems to diverge from this behavior and treats groups as an empty list, clearing out all groups except for the user’s primary group.

The documented behavior is actually very useful depending on how a saltstack administrator wants to manage group memberships. An administrator can choose to list of each group a user is a member of, or list each user that is in a given group. In our case, we chose the latter as it seems to be easier to manage across multiple systems.

Setup Salt-Stack 3006.0 installed on a VM (KVM) running on-prem. Salt is installed via onedir packaging and is up-to-date at the creation of this issue. Based on behavior, I don’t believe that installation method will affect the apparent bug.

I tested this against the following operating systems and they all have similar behavior:

Debian 11, amd64 VM
Alma Linux 9, amd64 VM (our primary target platform)
Alma Linux 8, amd64 VM
Ubuntu 22.04, amd64 VM

SLS File:

Manage account - test-user:
  group.present:
  - name: test-user
  - gid: 61001

  user.present:
  - name: test-user
  - uid: 61001
  - gid: 61001
  - fullname: Test User
  - shell: /bin/bash
  - home: /home/test-user
  - password: $6$REDACTED

Manage group - test-group1:
  group.present:
  - name: test-group1
  - members:
    - test-user

Manage group - test-group2:
  group.present:
  - name: test-group2
  - members:
    - test-user

Steps to Reproduce the behavior

Apply the above state to establish a normal account
Apply the above state with test=true
- observe that user.present is attempting to modify groups for test-user
- observe that group.apply are not attempting to modify group membership
Apply the above state; observe
- observe that user.present is modifying the groups for test-user.
- observe that group.present is adding the user back to test-group1 and test-group2.

Expected behavior When the groups parameter of user.present is left unset, it should leave the groups as-is. This is what the documentation states for salt.states.user.present

groups

A list of groups to assign the user to, pass a list object. If a group specified here does not exist on the minion, the state will fail. If set to the empty list, the user will be removed from all groups except the default group. If unset, salt will assume current groups are still wanted, and will make no changes to them.

Screenshots

State run (2nd and 3rd run)

% salt 'test-*' state.apply exp.user_bug2
test-debian11:
----------
          ID: Manage account - test-user
    Function: group.present
        Name: test-user
      Result: True
     Comment: Group test-user is present and up to date
     Started: 16:10:43.019995
    Duration: 3.306 ms
     Changes:   
----------
          ID: Manage account - test-user
    Function: user.present
        Name: test-user
      Result: True
     Comment: Updated user test-user
     Started: 16:10:43.023837
    Duration: 33.082 ms
     Changes:   
              ----------
              groups:
                  - test-user
----------
          ID: Manage group - test-group1
    Function: group.present
        Name: test-group1
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:10:43.057029
    Duration: 6.138 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully
----------
          ID: Manage group - test-group2
    Function: group.present
        Name: test-group2
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:10:43.063261
    Duration: 5.259 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully

Summary for test-debian11
------------
Succeeded: 4 (changed=3)
Failed:    0
------------
Total states run:     4
Total run time:  47.785 ms
test-ubuntu2204:
----------
          ID: Manage account - test-user
    Function: group.present
        Name: test-user
      Result: True
     Comment: Group test-user is present and up to date
     Started: 20:12:35.922017
    Duration: 3.33 ms
     Changes:   
----------
          ID: Manage account - test-user
    Function: user.present
        Name: test-user
      Result: True
     Comment: Updated user test-user
     Started: 20:12:35.925901
    Duration: 40.327 ms
     Changes:   
              ----------
              groups:
                  - test-user
----------
          ID: Manage group - test-group1
    Function: group.present
        Name: test-group1
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 20:12:35.966358
    Duration: 7.435 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully
----------
          ID: Manage group - test-group2
    Function: group.present
        Name: test-group2
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 20:12:35.973914
    Duration: 6.29 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully

Summary for test-ubuntu2204
------------
Succeeded: 4 (changed=3)
Failed:    0
------------
Total states run:     4
Total run time:  57.382 ms
test-alma9:
----------
          ID: Manage account - test-user
    Function: group.present
        Name: test-user
      Result: True
     Comment: Group test-user is present and up to date
     Started: 16:12:35.967843
    Duration: 3.066 ms
     Changes:   
----------
          ID: Manage account - test-user
    Function: user.present
        Name: test-user
      Result: True
     Comment: Updated user test-user
     Started: 16:12:35.971431
    Duration: 53.881 ms
     Changes:   
              ----------
              groups:
                  - test-user
----------
          ID: Manage group - test-group1
    Function: group.present
        Name: test-group1
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:12:36.025473
    Duration: 22.06 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully
----------
          ID: Manage group - test-group2
    Function: group.present
        Name: test-group2
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:12:36.047647
    Duration: 22.128 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully

Summary for test-alma9
------------
Succeeded: 4 (changed=3)
Failed:    0
------------
Total states run:     4
Total run time: 101.135 ms
test-alma8:
----------
          ID: Manage account - test-user
    Function: group.present
        Name: test-user
      Result: True
     Comment: Group test-user is present and up to date
     Started: 16:12:36.015804
    Duration: 3.456 ms
     Changes:   
----------
          ID: Manage account - test-user
    Function: user.present
        Name: test-user
      Result: True
     Comment: Updated user test-user
     Started: 16:12:36.019788
    Duration: 257.84 ms
     Changes:   
              ----------
              groups:
                  - test-user
----------
          ID: Manage group - test-group1
    Function: group.present
        Name: test-group1
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:12:36.277789
    Duration: 210.946 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully
----------
          ID: Manage group - test-group2
    Function: group.present
        Name: test-group2
      Result: True
     Comment: The following group attributes are set to be changed:
              members: ['test-user']
     Started: 16:12:36.488873
    Duration: 216.239 ms
     Changes:   
              ----------
              Final:
                  All changes applied successfully

Summary for test-alma8
------------
Succeeded: 4 (changed=3)
Failed:    0
------------
Total states run:     4
Total run time: 688.481 ms

Versions Report

salt --versions-report (master)

Salt Version:
          Salt: 3006.0
 
Python Version:
        Python: 3.10.11 (main, Apr 14 2023, 05:57:16) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 5.4.1
         PyZMQ: 23.2.0
        relenv: 0.11.2
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: almalinux 9.1 Lime Lynx
        locale: utf-8
       machine: x86_64
       release: 5.14.0-162.18.1.el9_1.x86_64
        system: Linux
       version: AlmaLinux 9.1 Lime Lynx

salt --versions-report (minion)

```yaml Salt Version: Salt: 3006.0

Python Version: Python: 3.10.11 (main, Apr 14 2023, 05:57:16) [GCC 11.2.0]

Dependency Versions: cffi: 1.14.6 cherrypy: unknown dateutil: 2.8.1 docker-py: Not Installed gitdb: Not Installed gitpython: Not Installed Jinja2: 3.1.2 libgit2: Not Installed looseversion: 1.0.2 M2Crypto: Not Installed Mako: Not Installed msgpack: 1.0.2 msgpack-pure: Not Installed mysql-python: Not Installed packaging: 22.0 pycparser: 2.21 pycrypto: Not Installed pycryptodome: 3.9.8 pygit2: Not Installed python-gnupg: 0.4.8 PyYAML: 5.4.1 PyZMQ: 23.2.0 relenv: 0.11.2 smmap: Not Installed timelib: 0.2.4 Tornado: 4.5.3 ZMQ: 4.3.4

System Versions: dist: almalinux 9.1 Lime Lynx locale: utf-8 machine: x86_64 release: 5.14.0-162.18.1.el9_1.x86_64 system: Linux version: AlmaLinux 9.1 Lime Lynx

</details>

**Additional context**
It's not clear to me when this behavior emerged, but 12-18mo ago, I don't remember this behavior being around.

About this issue

Original URL
State: closed
Created a year ago
Comments: 17 (10 by maintainers)

Most upvoted comments

same issue here @Ch3LL @OrangeDog at v3005.1-2 state user.present when groups param is not set, state will not modify group, it works expected as document said:

groups
A list of groups to assign the user to, pass a list object. If a group specified here does not exist on the minion, the state will fail. If set to the empty list, the user
will be removed from all groups except the default group. If unset, salt will assume current groups are still wanted, and will make no changes to them.

and saltstack already made a breaking change, if it is a bug, it should be fixed.

FlowBreeze on May 10, 2023

Maybe I can provide a more atomic example:

Sample SLS file:

user-auser-present:
  user.present:
    - name: auser

Assume the user exists already.

If the state is run on 3005.1:

ID: user-auser-present
Function: user.present
Name: auser
Result: True
Comment: User auser is present and up to date
Started: 20:03:23.571999
Duration: 1.905 ms
Changes:

If the same state is run on 3006.1:

ID: user-auser-present
Function: user.present
Name: auser
Result: None
Comment: The following user attributes are set to be changed:
         groups: []
Started: 20:05:28.658696
Duration: 1.859 ms
Changes:

The behaviour in 3006.1 contradicts the documentation as @FlowBreeze quoted, and the old behaviour was definitely preferable for me.

Adding remove_groups: False to the state is a workaround.

stooj on May 24, 2023

Yes thats what I meant, we should not introduce a breaking change without warning. I have not dove into this issue though so my assessment could change, once I get time to look into this to see if there is a reason behind this change.

Ch3LL on May 17, 2023