saltstack/salt — pull request #2721 by thekuffs (2012-11-27 21:08:17+00:00)
Title: Fix ambiguous conditional for missing group name

I just ran into an issue using a `file.managed` declaration that was working on previous releases.

```
State: - file
Name: /etc/selinux/config
Function: managed
Result: False
Comment: Failed to change group to root
Changes:
```

The file is already owned by root:
```
[root@MINION ~]# ls -la /etc/selinux/config
-rw-r--r--. 1 root root 454 Nov 14 22:56 /etc/selinux/config
[root@MINION ~]# getfacl /etc/selinux/config
getfacl: Removing leading '/' from absolute path names
# file: etc/selinux/config
# owner: root
# group: root
user::rw-
group::r--
other::r--
```

So I started diving into the `file` state and module:
```
[root@MINION ~]# /salt/salt-call -c /etc/salt file.get_group /etc/selinux/config
[INFO ] Loaded configuration file: /etc/salt/minion
{'local': ''}
[root@MINION ~]# /salt/salt-call -c /etc/salt file.get_gid /etc/selinux/config
[INFO ] Loaded configuration file: /etc/salt/minion
{'local': 0}
[root@MINION ~]# /salt/salt-call -c /etc/salt file.group_to_gid root
[INFO ] Loaded configuration file: /etc/salt/minion
{'local': 0}
[root@MINION ~]# /salt/salt-call -c /etc/salt file.gid_to_group 0
[INFO ] Loaded configuration file: /etc/salt/minion
{'local': ''}
```

The group name of gid 0 is certainly not blank - which lead me to the ambiguous conditional here:

```python
if not gid:
return ''
```

Which clearly returns an empty string for the root group.

**I would recommend a re-evaluation of the way invalid gids are handled between the mapping functions in the file module. TypeError and ValueError exceptions are a good way to handle this without the type ambiguity in calling code.**