Inconsistency in parameters parsing for the ec2_group module when with ICMP rules

[Razique]

Summary

Current amazon.aws.ec2_group module supports a variety of IP protocols. When writing rules for ICMP from_port and to_port parameters have different meaning than when using TCP or UDP rules.

This behavior aligns with current Boto3 implementation for which payload does not distinguish between a port range and ICMP code and type:

FromPort (integer) --
The start of port range for the TCP and UDP protocols, or an ICMP type number. For the ICMP type number, use -1 to specify all types. If you specify all ICMP types, you must specify all codes.
Alternatively, use a set of IP permissions to specify multiple rules and a description for the rule.
GroupName (string) -- [EC2-Classic, default VPC] The name of the security group. You must specify either the security group ID or the security group name in the request.
IpPermissions (list) --

ToPort (integer) --
The end of port range for the TCP and UDP protocols, or an ICMP/ICMPv6 code. A value of -1 indicates all ICMP/ICMPv6 codes. If you specify all ICMP/ICMPv6 types, you must specify all codes.

I suggest to create supplementary parameters to decouple usage of the module from the payload that the method expects.
This would involve creating a set of two new parameters icmp_code and icmp_type that would be mutually exclusive with from_port and to_port parameters.

      - proto: tcp
        from_port: 3306
        to_port: 3306
        group_id: 123412341234/sg-87654321/exact-name-of-sg
      - proto: udp
        from_port: 10050
        to_port: 10050
        cidr_ip: 10.0.0.0/8
      - proto: icmp
        icmp_code: 8      # New parameter
        icmp_type:  -1    # New parameter
        cidr_ip: 10.0.0.0/8

The usage of from_port and to_port would fail if proto reads icmp and vice versa, Ansible would complain if the icmp_port and icmp_type are used without a icmp or icmpv6.

Regarding the new ports key instead, there may be two solutions:

1. Updating the documentation to make it clear that the list defines ranges for TCP and UDP protocols and and ICMP port range and type.
2. Make this parameter exclusive to TCP and UDP protocols.

This change would align the semantics of the module with Ansible's philosophy, that is, using mutually exclusive parameters (for instance instance_name and instance_id) instead of basing the behavior on the module based on the value of a parameter.
The change would not change the payload being sent, only create a "superset" for consistency.

Issue Type

Feature Idea

Component Name

ec2_group

Additional Information

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• [plugins/modules/ec2_group.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/ec2_group.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

[ansibullbot]

cc @adq @jillr @s-hertel @tremble
click here for bot help

[jillr]

Any change that modifies existing behaviour would need to maintain the existing behaviour, at least for a deprecation period, so that we don't break user's playbooks. We'd definitely consider a PR that implements this feature!