Merge pull request #2 from ansible-collections/main

updating

changelogs/fragments/878-ec2_group.yml

@@ -0,0 +1,5 @@
+minor_changes:
+- ec2_group - add ``purge_egress_rules`` as an alias for ``purge_rules_egress`` (https://github.com/ansible-collections/amazon.aws/pull/878).
+- ec2_group - add ``egress_rules`` as an alias for ``rules_egress`` (https://github.com/ansible-collections/amazon.aws/pull/878).
+bugfixes:
+- ec2_group - fix uncaught exception when running with ``--diff`` and ``--check`` to create a new security group (https://github.com/ansible-collections/amazon.aws/issues/440).

docs/docsite/rst/dev_guidelines.rst

@@ -86,21 +86,22 @@ Breaking changes include:
 Adding new features
 -------------------
 
-Try to keep backward compatibility with relatively recent versions of boto3. That means that if you
-want to implement some functionality that uses a new feature of boto3, it should only fail if that
-feature actually needs to be run, with a message stating the missing feature and minimum required
-version of boto3.
-
-Use feature testing (for example, ``hasattr('boto3.module', 'shiny_new_method')``) to check whether boto3
-supports a feature rather than version checking. For example, from the ``ec2`` module:
+Try to keep backward compatibility with versions of boto3/botocore that are at least a year old.
+This means that if you want to implement functionality that uses a new feature of boto3/botocore,
+it should only fail if that feature is explicitly used, with a message stating the missing feature
+and minimum required version of botocore. (Feature support is usually defined in botocore and then
+used by boto3)
 
 .. code-block:: python
 
-   if boto_supports_profile_name_arg(ec2):
-       params['instance_profile_name'] = instance_profile_name
-   else:
-       if instance_profile_name is not None:
-           module.fail_json(msg="instance_profile_name parameter requires boto version 2.5.0 or higher")
+    module = AnsibleAWSModule(
+        argument_spec=argument_spec,
+        ...
+    )
+
+    if module.params.get('scope') == 'managed':
+        module.require_botocore_at_least('1.23.23', reason='to list managed rules')
+
 
 .. _ansible_collections.amazon.aws.docsite.dev_module_create:
 
@@ -169,8 +170,10 @@ authentication parameters.  To do the same for your new module, add an entry for
 
 .. code-block:: yaml
 
-  aws_module_name:
-  - aws
+  action_groups:
+    aws:
+       ...
+       aws_example_module
 
 Module behavior
 ---------------
@@ -199,7 +202,7 @@ These handle some of the more esoteric connection options, such as security toke
 If using the basic AnsibleModule then you should use ``get_aws_connection_info`` and then ``boto3_conn``
 to connect to AWS as these handle the same range of connection options.
 
-These helpers also for missing profiles or a region not set when it needs to be, so you don't have to.
+These helpers also check for missing profiles or a region not set when it needs to be, so you don't have to.
 
 An example of connecting to ec2 is shown below. Note that unlike boto there is no ``NoAuthHandlerFound``
 exception handling like in boto. Instead, an ``AuthFailure`` exception will be thrown when you use the
@@ -237,8 +240,9 @@ Common Documentation Fragments for Connection Parameters
 There are two :ref:`common documentation fragments <module_docs_fragments>`
 that should be included into almost all AWS modules:
 
-* ``aws`` - contains the common boto connection parameters
+* ``aws`` - contains the common boto3 connection parameters
 * ``ec2`` - contains the common region parameter required for many AWS modules
+* ``tags`` - contains the common tagging parameters used by many AWS modules
 
 These fragments should be used rather than re-documenting these properties to ensure consistency
 and that the more esoteric connection options are documented. For example:
@@ -275,7 +279,7 @@ Using is_boto3_error_code
 
 To use ``ansible_collections.amazon.aws.plugins.module_utils.core.is_boto3_error_code`` to catch a single
 AWS error code, call it in place of ``ClientError`` in your except clauses. In
-this case, *only* the ``InvalidGroup.NotFound`` error code will be caught here,
+this example, *only* the ``InvalidGroup.NotFound`` error code will be caught here,
 and any other error will be raised for handling elsewhere in the program.
 
 .. code-block:: python
@@ -293,9 +297,7 @@ In the AnsibleAWSModule there is a special method, ``module.fail_json_aws()`` fo
 exceptions.  Call this on your exception and it will report the error together with a traceback for
 use in Ansible verbose mode.
 
-You should use the AnsibleAWSModule for all new modules, unless not possible. If adding significant
-amounts of exception handling to existing modules, we recommend migrating the module to use AnsibleAWSModule
-(there are very few changes required to do this)
+You should use the AnsibleAWSModule for all new modules, unless not possible.
 
 .. code-block:: python
 

plugins/modules/ec2_group.py

@@ -137,6 +137,7 @@
     required: false
     type: list
     elements: dict
+    aliases: ['egress_rules']
     suboptions:
         cidr_ip:
             type: str
@@ -232,7 +233,7 @@
       - Purge existing rules_egress on security group that are not found in rules_egress.
     required: false
     default: 'true'
-    aliases: []
+    aliases: ['purge_egress_rules']
     type: bool
 
 extends_documentation_fragment:
@@ -1163,16 +1164,28 @@ def get_final_rules(client, module, security_group_rules, specified_rules, purge
                             rule[source_type] = [rule[source_type]]
                         format_rule[rule_key] = [{source_type: target} for target in rule[source_type]]
             if rule.get('group_id') or rule.get('group_name'):
-                rule_sg = camel_dict_to_snake_dict(group_exists(client, module, module.params['vpc_id'], rule.get('group_id'), rule.get('group_name'))[0])
-                format_rule['user_id_group_pairs'] = [{
-                    'description': rule_sg.get('description', rule_sg.get('group_desc')),
-                    'group_id': rule_sg.get('group_id', rule.get('group_id')),
-                    'group_name': rule_sg.get('group_name', rule.get('group_name')),
-                    'peering_status': rule_sg.get('peering_status'),
-                    'user_id': rule_sg.get('user_id', get_account_id(security_group, module)),
-                    'vpc_id': rule_sg.get('vpc_id', module.params['vpc_id']),
-                    'vpc_peering_connection_id': rule_sg.get('vpc_peering_connection_id')
-                }]
+                rule_sg = group_exists(client, module, module.params['vpc_id'], rule.get('group_id'), rule.get('group_name'))[0]
+                if rule_sg is None:
+                    # --diff during --check
+                    format_rule['user_id_group_pairs'] = [{
+                        'group_id': rule.get('group_id'),
+                        'group_name': rule.get('group_name'),
+                        'peering_status': None,
+                        'user_id': get_account_id(security_group, module),
+                        'vpc_id': module.params['vpc_id'],
+                        'vpc_peering_connection_id': None
+                    }]
+                else:
+                    rule_sg = camel_dict_to_snake_dict(rule_sg)
+                    format_rule['user_id_group_pairs'] = [{
+                        'description': rule_sg.get('description', rule_sg.get('group_desc')),
+                        'group_id': rule_sg.get('group_id', rule.get('group_id')),
+                        'group_name': rule_sg.get('group_name', rule.get('group_name')),
+                        'peering_status': rule_sg.get('peering_status'),
+                        'user_id': rule_sg.get('user_id', get_account_id(security_group, module)),
+                        'vpc_id': rule_sg.get('vpc_id', module.params['vpc_id']),
+                        'vpc_peering_connection_id': rule_sg.get('vpc_peering_connection_id')
+                    }]
                 for k, v in list(format_rule['user_id_group_pairs'][0].items()):
                     if v is None:
                         format_rule['user_id_group_pairs'][0].pop(k)
@@ -1243,7 +1256,7 @@ def get_ip_permissions_sort_key(rule):
         return rule.get('prefix_list_ids')[0]['prefix_list_id']
     elif rule.get('user_id_group_pairs'):
         rule.get('user_id_group_pairs').sort(key=get_rule_sort_key)
-        return rule.get('user_id_group_pairs')[0]['group_id']
+        return rule.get('user_id_group_pairs')[0].get('group_id', '')
     return None
 
 
@@ -1254,10 +1267,10 @@ def main():
         description=dict(),
         vpc_id=dict(),
         rules=dict(type='list', elements='dict'),
-        rules_egress=dict(type='list', elements='dict'),
+        rules_egress=dict(type='list', elements='dict', aliases=['egress_rules']),
         state=dict(default='present', type='str', choices=['present', 'absent']),
         purge_rules=dict(default=True, required=False, type='bool'),
-        purge_rules_egress=dict(default=True, required=False, type='bool'),
+        purge_rules_egress=dict(default=True, required=False, type='bool', aliases=['purge_egress_rules']),
         tags=dict(required=False, type='dict', aliases=['resource_tags']),
         purge_tags=dict(default=True, required=False, type='bool')
     )

tests/integration/targets/ec2_group/aliases

@@ -1,9 +1,6 @@
-# reason: broken
-# Tests frequently failing
-# https://github.com/ansible-collections/amazon.aws/issues/440
-disabled
-
+# duration: 15
 slow
 
 cloud/aws
+
 ec2_group_info

tests/integration/targets/ec2_group/tasks/icmp_verbs.yml

@@ -178,9 +178,39 @@
           - result is failed
 
   always:
+    - name: tidy up egress rule test security group rules
+      ec2_group:
+        name: '{{ec2_group_name}}-auto-create-2'
+        description: 'sg-group-referencing'
+        vpc_id: '{{ vpc_result.vpc.id }}'
+        rules: []
+        rules_egress: []
+      ignore_errors: yes
+
+    - name: tidy up egress rule test security group rules
+      ec2_group:
+        name: '{{ec2_group_name}}-icmp-{{ item }}'
+        description: '{{ec2_group_description}}'
+        vpc_id: '{{ vpc_result.vpc.id }}'
+        rules: []
+        rules_egress: []
+      ignore_errors: yes
+      with_items:
+        - 1
+        - 2
+        - 3
+        - 4
+
+    - name: tidy up egress rule test security group rules
+      ec2_group:
+        name: '{{ec2_group_name}}-auto-create-2'
+        state: absent
+        vpc_id: '{{ vpc_result.vpc.id }}'
+      ignore_errors: yes
+
     - name: tidy up egress rule test security group
       ec2_group:
-        name: '{{ec2_group_name}}-auto-create-{{ item }}'
+        name: '{{ec2_group_name}}-icmp-{{ item }}'
         state: absent
         vpc_id: '{{ vpc_result.vpc.id }}'
       ignore_errors: yes