Repository: ansible/ansible
Pull request: ec2_vpc_nacl fails when the VPC is configured with IPv6
##### SUMMARY
The ec2_vpc_nacl module always fails when an AWS VPC is configured with an IPv6 subnet. When a user executes an ec2_vpc_nacl task, an error message occurs and the following error message is displayed: 

"msg": "An error occurred (InvalidParameterValue) when calling the DeleteNetworkAclEntry operation: Value (32768) for parameter ruleNumber is invalid. Rulenumber must be in range 1..32766"

Fixes #41079 

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
ec2_vpc_nacl

##### ADDITIONAL INFORMATION
During execution, the ec2_vpc_nacl the module builds a list of NACL rules that need to be deleted, then attempts to delete the rules. Rules which exist in the AWS VPC but not in the Ansible YAML are candidates for deletion. However, the module should not attempt to delete the "default" DENY rules because these rules must always be present and cannot be deleted.

The default rule for IPv4 has Rule Number 32767. The ec2_vpc_nacl module has built-in knowledge of this rule. The default rule for IPv6 has Rule Number 32768. The ec2_vpc_nacl module does NOT have built-in knowledge of this rule.  The module calls delete_network_acl_entry() with the default rule for IPv6. The module has a test to skip the default IPv4 rule ( number 32767), but it does not skip the IPv6 default rule, which has rule number 32768.

```
* | ALL Traffic | ALL | ALL | 0.0.0.0/0 | DENY |  
* | ALL Traffic | ALL | ALL | ::/0          | DENY |   <== Rule with Number 32768.
```

DEFAULT_RULE_FIELDS = {
    'RuleNumber': 32767,
    'RuleAction': 'deny',
    'CidrBlock': '0.0.0.0/0',
    'Protocol': '-1'
}

<!--- Paste verbatim command output below, e.g. before and after your change -->
```
"msg": "An error occurred (InvalidParameterValue) when calling the DeleteNetworkAclEntry operation: Value (32768) for parameter ruleNumber is invalid. Rulenumber must be in range 1..32766"
```



Problem statement:
ec2_vpc_nacl failes on update
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### SUMMARY
I have a very basic ACL and the first creation and the deletion this (state: 'absent' ) work fine. If i however change something (and it can be only the value of a tag.) The "update" failes with the following error:

`"msg": "An error occurred (InvalidParameterValue) when calling the DeleteNetworkAclEntry operation: Value (32768) for parameter ruleNumber is invalid. Rulenumber must be in range 1..32766"`

##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
ec2_vpc_nacl

##### ANSIBLE VERSION

```
ansible 2.5.3
  config file = None
  configured module search path = [u'/home/ec2-user/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.14 (default, May  2 2018, 18:31:34) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]
```

##### CONFIGURATION
I am using the plain ansible package from the yum repository with no extra configuration. I do not use ec2 credentials directly they are attached via a ec2 role

##### OS / ENVIRONMENT
Amazon Linux 
amzn-ami-hvm-2017.03.0.20170417-x86_64-gp2 (ami-b968bad6)

##### STEPS TO REPRODUCE

Run this playbook once and then change a detail for example a tag-value.
```yaml
---
- hosts: localhost
  connection: local
  gather_facts: False

  vars:
    region: "eu-central-1"
    subnets: ['subnet-123','subnet-456','subnet-789']
    vpc_id: "vpc-123"
    projectID: 123


  tasks:
  - name: "Create and associate production DMZ network ACL with DMZ subnets"
    ec2_vpc_nacl:
      vpc_id: "{{ vpc_id }}"
      name: ansible-nat-nacl
      region: "{{ region }}"
      subnets: "{{ subnets }}"
      tags:
        Name: ansible-nat-nacl
        ProjectID: "{{ projectID }}"
        Description: Test Ansible  ACL
        Owner: "cloudops"
        Provisioner: "ansible-test"
      ingress: [
          # rule no, protocol, allow/deny, cidr, icmp_code, icmp_type, port from, port to
          [100, 'tcp', 'allow', '0.0.0.0/0', null, null, 80, 80],
          [110, 'tcp', 'allow', '0.0.0.0/0', null, null, 443, 443],
          [140, 'tcp', 'allow', '0.0.0.0/0', null, null, 1024, 65535],
          [150, 'icmp', 'allow', '0.0.0.0/0', -1, -1],
      ]
      egress: [
          # rule no, protocol, allow/deny, cidr, icmp_code, icmp_type, port from, port to
          [100, 'tcp', 'allow', '0.0.0.0/0', null, null, 80, 80],
          [110, 'tcp', 'allow', '0.0.0.0/0', null, null, 443, 443],
          [140, 'tcp', 'allow', '0.0.0.0/0', null, null, 1024, 65535],
          [150, 'icmp', 'allow', '0.0.0.0/0', -1, -1],
      ]
      state: 'present'

```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
Updated ACL

##### ACTUAL RESULTS


<!--- Paste verbatim command output between quotes below -->
```
PLAY [localhost] ***************************************************************************************************************************************************************************************
META: ran handlers

TASK [Create and associate production DMZ network ACL with DMZ subnets] ********************************************************************************************************************************
task path: /home/ec2-user/ansible_trunk/zeb-default-acls.yml:14
Using module file /usr/local/lib/python2.7/site-packages/ansible/modules/cloud/amazon/ec2_vpc_nacl.py
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: ec2-user
<127.0.0.1> EXEC /bin/sh -c 'echo ~ec2-user && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224 `" && echo ansible-tmp-1528101266.94-82377466427224="` echo /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224 `" ) && sleep 0'
<127.0.0.1> PUT /home/ec2-user/.ansible/tmp/ansible-local-5654k0c1CT/tmpN1H_1T TO /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224/ec2_vpc_nacl.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224/ /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224/ec2_vpc_nacl.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python2.7 /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224/ec2_vpc_nacl.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/ec2-user/.ansible/tmp/ansible-tmp-1528101266.94-82377466427224/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
  File "/tmp/ansible_ItNCoU/ansible_module_ec2_vpc_nacl.py", line 417, in delete_network_acl_entry
    client.delete_network_acl_entry(**params)
  File "/usr/local/lib/python2.7/site-packages/botocore/client.py", line 314, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python2.7/site-packages/botocore/client.py", line 612, in _make_api_call
    raise error_class(parsed_response, operation_name)
fatal: [localhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "aws_access_key": null,
            "aws_secret_key": null,
            "ec2_url": null,
            "egress": [
                [
                    100,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    80,
                    80
                ],
                [
                    110,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    443,
                    443
                ],
                [
                    140,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    1024,
                    65535
                ],
                [
                    150,
                    "icmp",
                    "allow",
                    "0.0.0.0/0",
                    -1,
                    -1
                ]
            ],
            "ingress": [
                [
                    100,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    80,
                    80
                ],
                [
                    110,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    443,
                    443
                ],
                [
                    140,
                    "tcp",
                    "allow",
                    "0.0.0.0/0",
                    null,
                    null,
                    1024,
                    65535
                ],
                [
                    150,
                    "icmp",
                    "allow",
                    "0.0.0.0/0",
                    -1,
                    -1
                ]
            ],
            "nacl_id": null,
            "name": "ansible-nat-nacl",
            "profile": null,
            "region": "eu-central-1",
            "security_token": null,
            "state": "present",
            "subnets": [
                "subnet-123",
                "subnet-456",
                "subnet-789"
            ],
            "tags": {
                "Description": "Test Ansible  ACL",
                "Name": "ansible-nat-nacl",
                "Owner": "cloudops",
                "ProjectID": 123,
                "Provisioner": "ansible-test"
            },
            "validate_certs": true,
            "vpc_id": "vpc-123"
        }
    },
    "msg": "An error occurred (InvalidParameterValue) when calling the DeleteNetworkAclEntry operation: Value (32768) for parameter ruleNumber is invalid. Rulenumber must be in range 1..32766"
}
        to retry, use: --limit @/home/ec2-user/ansible_trunk/zeb-default-acls.retry

PLAY RECAP *********************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1
```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @mmochan
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
The same error also occurs with Ansible 2.7.4

ansible 2.7.4
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Nov  6 2016, 00:28:07) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]

UPDATE:
Digging deeper in the ec2_vpc_nacl module, this happens when a default IPv6 rule has been configured. The module calls delete_network_acl_entry() with the default rule for IPv6. The module has a test to skip the default IPv4 rule ( number 32767), but it does not skip the IPv6 default rule, which has rule number 32768.

```
DEFAULT_RULE_FIELDS = {
    'RuleNumber': 32767,
    'RuleAction': 'deny',
    'CidrBlock': '0.0.0.0/0',
    'Protocol': '-1'
}
```

The title of this bug should be renamed to "ec2_vpc_nacl fails when the vpc is configured with IPv6"

Commit message: Simplify logic to skip default deny-all rules

Patch to review:
diff --git a/lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py b/lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py
index d0473d9f4c0c9b..e71ec934ca8462 100644
--- a/lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py
@@ -154,17 +154,6 @@
 from ansible.module_utils.ec2 import boto3_conn, ec2_argument_spec, get_aws_connection_info
 
 
-# Common fields for the default rule that is contained within every VPC NACL.
-DEFAULT_RULE_FIELDS = {
-    'RuleNumber': 32767,
-    'RuleAction': 'deny',
-    'CidrBlock': '0.0.0.0/0',
-    'Protocol': '-1'
-}
-
-DEFAULT_INGRESS = dict(list(DEFAULT_RULE_FIELDS.items()) + [('Egress', False)])
-DEFAULT_EGRESS = dict(list(DEFAULT_RULE_FIELDS.items()) + [('Egress', True)])
-
 # VPC-supported IANA protocol numbers
 # http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
 PROTOCOL_NUMBERS = {'all': -1, 'icmp': 1, 'tcp': 6, 'udp': 17, }
@@ -236,10 +225,10 @@ def nacls_changed(nacl, client, module):
     nacl_id = nacl['NetworkAcls'][0]['NetworkAclId']
     nacl = describe_network_acl(client, module)
     entries = nacl['NetworkAcls'][0]['Entries']
-    tmp_egress = [entry for entry in entries if entry['Egress'] is True and DEFAULT_EGRESS != entry]
+    tmp_egress = [entry for entry in entries if entry['Egress'] is True and entry['RuleNumber'] < 32767]
     tmp_ingress = [entry for entry in entries if entry['Egress'] is False]
-    egress = [rule for rule in tmp_egress if DEFAULT_EGRESS != rule]
-    ingress = [rule for rule in tmp_ingress if DEFAULT_INGRESS != rule]
+    egress = [rule for rule in tmp_egress if rule['RuleNumber'] < 32767]
+    ingress = [rule for rule in tmp_ingress if rule['RuleNumber'] < 32767]
     if rules_changed(egress, params['egress'], True, nacl_id, client, module):
         changed = True
     if rules_changed(ingress, params['ingress'], False, nacl_id, client, module):


Code review comments:
diff_hunk: @@ -236,10 +225,10 @@ def nacls_changed(nacl, client, module):
     nacl_id = nacl['NetworkAcls'][0]['NetworkAclId']
     nacl = describe_network_acl(client, module)
     entries = nacl['NetworkAcls'][0]['Entries']
-    tmp_egress = [entry for entry in entries if entry['Egress'] is True and DEFAULT_EGRESS != entry]
+    tmp_egress = [entry for entry in entries if entry['Egress'] is True and entry['RuleNumber'] < 32767]
     tmp_ingress = [entry for entry in entries if entry['Egress'] is False]
-    egress = [rule for rule in tmp_egress if DEFAULT_EGRESS != rule]
-    ingress = [rule for rule in tmp_ingress if DEFAULT_INGRESS != rule]
+    egress = [rule for rule in tmp_egress if rule['RuleNumber'] < 32767]
+    ingress = [rule for rule in tmp_ingress if rule['RuleNumber'] < 32767]
path: lib/ansible/modules/cloud/amazon/ec2_vpc_nacl.py
text: @user1:
Not sure why this module originally used `tmp_egress` and `tmp_ingress` lists as well as the `egress` and `ingress` lists. I think this could be simplified further to be just:

```
egress = [rule for rule in entries if rule['Egress'] is True and rule['RuleNumber'] < 32767]
ingress = [rule for rule in entries if rule['Egress'] is False and rule['RuleNumber'] < 32767]
```

Or is there a reason the list needs to be filtered twice?

@author:
Agreed. I was wondering the same thing. I will remove the tmp variables.

@author:
I have made and tested the change to remove the tmp_egress and tmp_ingress variables.