From 5384cb815764af43f287bcb6ed49a9a68d5cd5c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
 <psuarezhernandez@suse.com>
Date: Wed, 10 Jan 2018 11:59:33 +0000
Subject: [PATCH] Return error when gid_from_name and group does not
 exist.

Fixes #45345

Ensure empty string gid is set to None

Make pylint happy

Fix integration tests for 'user.present' state.

Update documentation for 'gid_from_name' parameter

Refactor to prevent logical bug when gid is 0
---
 salt/states/user.py                   |  7 +++++-
 tests/integration/states/test_user.py | 42 +++++++++++++++++++++++------------
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/salt/states/user.py b/salt/states/user.py
index 8a731cc2a1..737c39f4b4 100644
--- a/salt/states/user.py
+++ b/salt/states/user.py
@@ -240,7 +240,8 @@ def present(name,
 
     gid_from_name
         If True, the default group id will be set to the id of the group with
-        the same name as the user, Default is ``False``.
+        the same name as the user. If the group does not exist the state will
+        fail. Default is ``False``.
 
     groups
         A list of groups to assign the user to, pass a list object. If a group
@@ -455,6 +456,10 @@ def present(name,
 
     if gid_from_name:
         gid = __salt__['file.group_to_gid'](name)
+        if gid == '':
+            ret['comment'] = 'Default group with name "{0}" is not present'.format(name)
+            ret['result'] = False
+            return ret
 
     changes = _changes(name,
                        uid,
diff --git a/tests/integration/states/test_user.py b/tests/integration/states/test_user.py
index 1317f12f97..ae9774a241 100644
--- a/tests/integration/states/test_user.py
+++ b/tests/integration/states/test_user.py
@@ -97,15 +97,25 @@ class UserTest(ModuleCase, SaltReturnAssertsMixin):
                              home=HOMEDIR)
         self.assertSaltTrueReturn(ret)
 
-    def test_user_present_nondefault(self):
+    @requires_system_grains
+    def test_user_present_nondefault(self, grains=None):
         '''
         This is a DESTRUCTIVE TEST it creates a new user on the on the minion.
         '''
         ret = self.run_state('user.present', name=self.user_name,
                              home=self.user_home)
         self.assertSaltTrueReturn(ret)
+        ret = self.run_function('user.info', [self.user_name])
+        self.assertReturnNonEmptySaltType(ret)
+        group_name = grp.getgrgid(ret['gid']).gr_name
         if not salt.utils.is_darwin():
             self.assertTrue(os.path.isdir(self.user_home))
+        if grains['os_family'] in ('Suse',):
+            self.assertEqual(group_name, 'users')
+        elif grains['os_family'] == 'MacOS':
+            self.assertEqual(group_name, 'staff')
+        else:
+            self.assertEqual(group_name, self.user_name)
 
     @requires_system_grains
     def test_user_present_gid_from_name_default(self, grains=None):
@@ -120,22 +130,26 @@ class UserTest(ModuleCase, SaltReturnAssertsMixin):
         # user
         gid_from_name = False if grains['os_family'] == 'MacOS' else True
 
-        ret = self.run_state('user.present', name=self.user_name,
+        ret_user_present = self.run_state('user.present', name=self.user_name,
                              gid_from_name=gid_from_name, home=self.user_home)
-        self.assertSaltTrueReturn(ret)
 
-        ret = self.run_function('user.info', [self.user_name])
-        self.assertReturnNonEmptySaltType(ret)
-        group_name = grp.getgrgid(ret['gid']).gr_name
-
-        if not salt.utils.is_darwin():
-            self.assertTrue(os.path.isdir(self.user_home))
-        if grains['os_family'] in ('Suse',):
-            self.assertEqual(group_name, 'users')
-        elif grains['os_family'] == 'MacOS':
-            self.assertEqual(group_name, 'staff')
+        if gid_from_name:
+            self.assertSaltFalseReturn(ret_user_present)
+            ret_user_present = ret_user_present[next(iter(ret_user_present))]
+            self.assertTrue('is not present' in ret_user_present['comment'])
         else:
-            self.assertEqual(group_name, self.user_name)
+            self.assertSaltTrueReturn(ret_user_present)
+            ret_user_info = self.run_function('user.info', [self.user_name])
+            self.assertReturnNonEmptySaltType(ret_user_info)
+            group_name = grp.getgrgid(ret_user_info['gid']).gr_name
+            if not salt.utils.is_darwin():
+                self.assertTrue(os.path.isdir(self.user_home))
+            if grains['os_family'] in ('Suse',):
+                self.assertEqual(group_name, 'users')
+            elif grains['os_family'] == 'MacOS':
+                self.assertEqual(group_name, 'staff')
+            else:
+                self.assertEqual(group_name, self.user_name)
 
     def test_user_present_gid_from_name(self):
         '''
-- 
2.15.1