Repository: saltstack/salt
Pull request: [master] Prevent possible KeyError exception on calling salt.utils.user.get_group_dict
### What does this PR do?

### What issues does this PR fix or reference?
Fixes: https://github.com/saltstack/salt/issues/62377 (the fix was merged already) with https://github.com/saltstack/salt/pull/62378, but this change is addressing specific corner case when the user has imporper duplicate of the group.

Steps to reproduce the issue:
Add the following entry to `/etc/group`:
```
+:::
```
And set `group:		files compat` in `/etc/nsswitch.conf`
Note: it's not a proper configuration, but a way to reproduce the issue appearing on some of the system having NIS configured.

### Previous Behavior
Possible `KeyError` exception on calling `salt.utils.user.get_group_dict` if user has improper duplicate group assigned.
```
KeyError: "getgrnam(): name not found: '+'"
```

### New Behavior
Ignoring such group entry.

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltproject.io/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [ ] Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
- [ ] Tests written/updated

### Commits signed with GPG?
Yes/No

Please review [Salt's Contributing Guide](https://docs.saltproject.io/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
[BUG] groups with duplicate GIDs are not returned by get_group_list
**Description**

The following error is thrown when attempting to enforce optional group membership for a group which does not have a unique GID.

```
[ERROR   ] These values could not be changed: {'groups': ['adm', 'audio', 'cdrom', 'dialout', 'dip', 'floppy', 'netdev', 'plugdev', 'sudo', 'user', 'users', 'video']}
local:
----------
          ID: groups_manage_user
    Function: user.present
        Name: debian
      Result: False
     Comment: These values could not be changed: {'groups': ['adm', 'audio', 'cdrom', 'dialout', 'dip', 'floppy', 'netdev', 'plugdev', 'sudo', 'user', 'users', 'video']}
     Started: 14:57:44.820074
    Duration: 39.273 ms
     Changes:   

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:  39.273 ms
```

**Setup**

Example state file:

```
# /srv/salt/groups.sls

manage_user:
  user.present:
    - name: debian
    - optional_groups: [user]
```
  
**Steps to Reproduce the behavior**

Add the `user` group with a duplicate GID, such as copying the `users` group line in `/etc/group` and then run the state.

```
salt-call state.apply groups
```

Running in `TRACE` log level points toward the problem:

```
TRACE   ] Trying os.getgrouplist for 'debian'
[TRACE   ] Group list for user 'debian': ['adm', 'audio', 'cdrom', 'debian', 'dialout', 'dip', 'floppy', 'netdev', 'plugdev', 'sudo', 'users', 'video']
```

The group listing is derived from GIDs, so the `user` group is not shown.

**Expected behavior**
While not the best practice, duplicate GIDs are a valid configuration which might be experienced on systems. The function should be able to list unique group names which might have duplicate GIDs.

**Versions Report**
<details><summary>salt --versions-report</summary>
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) 

```yaml
Salt Version:
          Salt: 3005+0na.b18281e
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: Not Installed
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.3
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: 2.6.1
  pycryptodome: 3.14.1
        pygit2: Not Installed
        Python: 3.7.3 (default, Jan 22 2021, 20:04:44)
  python-gnupg: Not Installed
        PyYAML: 6.0
         PyZMQ: 23.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 10 buster
        locale: UTF-8
       machine: x86_64
       release: 4.19.0-21-amd64
        system: Linux
       version: Debian GNU/Linux 10 buster

```
</details>

Hints from issue discussion:

Commit message: Add test of calling salt.utils.user.get_group_dict

for the user having improper duplicate group

Patch to review:
diff --git a/salt/utils/user.py b/salt/utils/user.py
index 97636674430a..2f1ca65cf99a 100644
--- a/salt/utils/user.py
+++ b/salt/utils/user.py
@@ -352,7 +352,11 @@ def get_group_dict(user=None, include_default=True):
     group_dict = {}
     group_names = get_group_list(user, include_default=include_default)
     for group in group_names:
-        group_dict.update({group: grp.getgrnam(group).gr_gid})
+        try:
+            group_dict.update({group: grp.getgrnam(group).gr_gid})
+        except KeyError:
+            # In case if imporer duplicate group was returned by get_group_list
+            pass
     return group_dict
 
 
diff --git a/tests/pytests/functional/utils/user/test_get_group_dict.py b/tests/pytests/functional/utils/user/test_get_group_dict.py
new file mode 100644
index 000000000000..abd569eec2c8
--- /dev/null
+++ b/tests/pytests/functional/utils/user/test_get_group_dict.py
@@ -0,0 +1,24 @@
+import logging
+
+import pytest
+
+import salt.utils.platform
+import salt.utils.user
+from tests.support.mock import patch
+
+log = logging.getLogger(__name__)
+
+pytestmark = [
+    pytest.mark.skip_on_windows,
+]
+
+
+@pytest.mark.skip_on_platforms(
+    darwin=True,
+    freebsd=True,
+    reason="This test should not run on FreeBSD and Mac due to lack of duplicate GID support",
+)
+def test_get_group_dict_with_improper_duplicate_root_group():
+    with patch("salt.utils.user.get_group_list", return_value=["+", "root"]):
+        group_list = salt.utils.user.get_group_dict("root")
+        assert group_list == {"root": 0}


Code review comments:
diff_hunk: @@ -0,0 +1,24 @@
+import logging
+
+import pytest
+
+import salt.utils.platform
+import salt.utils.user
+from tests.support.mock import patch
+
+log = logging.getLogger(__name__)
+
+pytestmark = [
+    pytest.mark.skip_on_windows,
+]
+
+
+@pytest.mark.skip_on_platforms(
+    darwin=True,
+    freebsd=True,
+    reason="This test should not run on FreeBSD and Mac due to lack of duplicate GID support",
+)
path: tests/pytests/functional/utils/user/test_get_group_dict.py
text: @user1:
```suggestion
pytestmark = [
    pytest.mark.skip_unless_on_linux(reason="Should only run in platforms which have duplicate GID support"),
]
```

@author:
Thanks, it makes sense to do it this way :+1: