Repository: ansible/ansible
Pull request: ec2_vpc_dhcp_options_facts: if tags don't exist set them to default list instead of crashing - fixes #23825
##### SUMMARY
Set tags to an default list rather than crashing if they don't exist. If there are no tags boto doesn't return the default Name tag and its empty value. Fixes #23825

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py

##### ANSIBLE VERSION
```
2.4.0
```


Problem statement:
ec2_vpc_dhcp_options_facts 
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bug Report

##### COMPONENT NAME
<!--- Name of the module/plugin/task/feature -->
ec2_vpc_dhcp_options_facts

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
ansible 2.4.0 (devel 6aab341248) last updated 2017/04/20 13:32:36 (GMT -400)
  config file = /Users/username/.ansible.cfg
  configured module search path = [u'/Users/username/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/username/devel/ansible/lib/ansible
  executable location = /Users/username/devel/ansible/bin/ansible
  python version = 2.7.13 (default, Dec 18 2016, 07:03:39) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.42.1)]
```
Also occurs with older versions.

##### CONFIGURATION
<!---
Mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).
-->

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say “N/A” for anything that is not platform-specific.
-->
macOS 10.12.4

##### SUMMARY
<!--- Explain the problem briefly -->
When gathering facts about DHCP option sets in AWS EC2 VPC the module will fail if an option set has had no tags applied to it. i.e. The default option set will cause this module to bomb out with a traceback.

Note: A default DHCP Option set appears to have tags applied to it when viewing it in the AWS console because it lists:
Key = "Name"
Value = ""
But in reality it has no tags applied to it.

##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->

<!--- Paste example playbooks or commands between quotes below -->
```yaml
  tasks:
    - name: Testing
      ec2_vpc_dhcp_options_facts:
        region: us-east-1
      register: dhcp_facts
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
```
ok: [localhost] => {
    "changed": false, 
    "dhcp_options": [
        {
            "dhcp_configurations": [
                {
                    "key": "domain-name", 
                    "values": [
                        {
                            "value": "example.com"
                        }
                    ]
                }, 
                {
                    "key": "domain-name-servers", 
                    "values": [
                        {
                            "value": "AmazonProvidedDNS"
                        }
                    ]
                }
            ], 
            "dhcp_options_id": "dopt-a7n47fx5", 
            "tags": {
                "Name": ""
            }
        }
    ]
}
```

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes below -->
```
TASK [Testing] *************************************************************************************************************************************************************************************************************
task path: /Users/username/devel/scratch/task-test.yml:15
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/basic.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/ec2.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/_text.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/pycompat24.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/six/__init__.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/six/_six.py
Using module_utils file /Users/username/devel/ansible/lib/ansible/module_utils/cloud.py
Using module file /Users/username/devel/ansible/lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: daniels
<localhost> EXEC /bin/sh -c 'echo ~ && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006 `" && echo ansible-tmp-1492710436.63-94105070270006="` echo /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006 `" ) && sleep 0'
<localhost> PUT /var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/tmpIMSEEi TO /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006/ec2_vpc_dhcp_options_facts.py
<localhost> EXEC /bin/sh -c 'chmod u+x /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006/ /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006/ec2_vpc_dhcp_options_facts.py && sleep 0'
<localhost> EXEC /bin/sh -c 'python /Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006/ec2_vpc_dhcp_options_facts.py; rm -rf "/Users/username/.ansible/tmp/ansible-tmp-1492710436.63-94105070270006/" > /dev/null 2>&1 && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py", line 158, in <module>
    main()
  File "/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py", line 152, in main
    results = list_dhcp_options(connection, module)
  File "/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py", line 123, in list_dhcp_options
    for option in all_dhcp_options['DhcpOptions']]
  File "/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py", line 103, in get_dhcp_options_info
    'Tags': boto3_tag_list_to_ansible_dict(dhcp_option['Tags'])}
KeyError: 'Tags'

fatal: [localhost]: FAILED! => {
    "changed": false, 
    "failed": true, 
    "module_stderr": "Traceback (most recent call last):\n  File \"/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py\", line 158, in <module>\n    main()\n  File \"/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py\", line 152, in main\n    results = list_dhcp_options(connection, module)\n  File \"/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py\", line 123, in list_dhcp_options\n    for option in all_dhcp_options['DhcpOptions']]\n  File \"/var/folders/sk/k6tqwkbj59x4glzs2hzt2vzrbm012h/T/ansible_Xp5U7h/ansible_module_ec2_vpc_dhcp_options_facts.py\", line 103, in get_dhcp_options_info\n    'Tags': boto3_tag_list_to_ansible_dict(dhcp_option['Tags'])}\nKeyError: 'Tags'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE", 
    "rc": 0
}

```

Hints from issue discussion:
cc @naslanidis
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: dhcp_option will always have a name tag

Patch to review:
diff --git a/lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py b/lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py
index 143c6f45076d58..3b79c2e933d59a 100644
--- a/lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py
@@ -98,6 +98,8 @@
 
 
 def get_dhcp_options_info(dhcp_option):
+    if 'Tags' not in dhcp_option:
+        dhcp_option['Tags'] = [{'Value': '', 'Key': 'Name'}]
     dhcp_option_info = {'DhcpOptionsId': dhcp_option['DhcpOptionsId'],
                         'DhcpConfigurations': dhcp_option['DhcpConfigurations'],
                         'Tags': boto3_tag_list_to_ansible_dict(dhcp_option['Tags'])}


Code review comments:
diff_hunk: @@ -98,6 +98,8 @@
 
 
 def get_dhcp_options_info(dhcp_option):
+    if 'Tags' not in dhcp_option:
+        dhcp_option['Tags'] = [{'Value': '', 'Key': 'Name'}]
     dhcp_option_info = {'DhcpOptionsId': dhcp_option['DhcpOptionsId'],
                         'DhcpConfigurations': dhcp_option['DhcpConfigurations'],
                         'Tags': boto3_tag_list_to_ansible_dict(dhcp_option['Tags'])}
path: lib/ansible/modules/cloud/amazon/ec2_vpc_dhcp_options_facts.py
text: @user1:
A more appropriate fix would just be
```
'Tags': boto3_tag_list_to_ansible_dict(dhcp_option.get('Tags', {}))
```
which doesn't create a fake empty tag.

@author:
I'm not sure. That's definitely less of a hack, but the tag Name: '' is inconsistently returned and this is to fix that. Name: '' will always exist by default. If there is another tag then Name: '' is returned as well (whereas if there is only the Name: '' no tags are returned even though it exists in the console). It just seems inconsistent that Name: '' is *sometimes* returned even though it will always exist.

@user1:
ok, 

```
'Tags': boto3_tag_list_to_ansible_dict(dhcp_option.get('Tags', {'Value': '', 'Key': 'Name'}))
```
would be a simpler change.

@user1:
I dislike the hack but accept your reasons for it :)