-
Notifications
You must be signed in to change notification settings - Fork 342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for icmp_type and icmp_code parameters (close #628) #783
Add support for icmp_type and icmp_code parameters (close #628) #783
Conversation
The job mentions the following: ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'purge_rules' in argument_spec defines default as (True) but documentation defines default as (False)
ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'purge_rules_egress' in argument_spec defines default as (True) but documentation defines default as (False)
ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'purge_tags' in argument_spec defines default as (True) but documentation defines default as (False)
ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'state' in argument_spec defines default as ('present') but documentation defines default as (None)
ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'validate_certs' in argument_spec defines default as (True) but documentation defines default as (False)
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'aws_access_key' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'aws_secret_key' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'description' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'ec2_url' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'group_id' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'name' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'profile' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'region' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'security_token' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: doc-missing-type: Argument 'vpc_id' in argument_spec uses default type ('str') but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'aws_ca_bundle' in argument_spec defines type as 'path' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'aws_config' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'debug_botocore_endpoint_logs' in argument_spec defines type as 'bool' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'purge_rules' in argument_spec defines type as 'bool' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'purge_rules_egress' in argument_spec defines type as 'bool' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'purge_tags' in argument_spec defines type as 'bool' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'rules' in argument_spec defines type as 'list' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'rules_egress' in argument_spec defines type as 'list' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'state' in argument_spec defines type as 'str' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'tags' in argument_spec defines type as 'dict' but documentation doesn't define type
ERROR: plugins/modules/ec2_group.py:0:0: parameter-type-not-in-doc: Argument 'validate_certs' in argument_spec defines type as 'bool' but documentation doesn't define type Taking the first error: ERROR: plugins/modules/ec2_group.py:0:0: doc-default-does-not-match-spec: Argument 'purge_rules' in argument_spec defines default as (True) but documentation defines default as (False) However, I do see that purge_rules:
description:
- Purge existing rules on security group that are not found in rules.
required: false
default: 'true'
aliases: []
type: bool |
A slew of errors like that usually means it's failed to parse the docs properly, which in turn results in lots of "mismatched" types |
plugins/modules/ec2_group.py
Outdated
icmp_type: | ||
type: int | ||
description: | ||
- When using C(proto: icmp) or C(proto: icmpv6), allows you to specify | ||
- the ICMP type to use. The option is mutually exclusive with from_port. | ||
- A value of C(-1) indicates all ICMP types. | ||
icmp_code: | ||
type: int | ||
description: | ||
- When using C(proto: icmp) or C(proto: icmpv6), allows you to specify | ||
- the ICMP code to use. The option is mutually exclusive with C(to_port). | ||
- A value of C(-1) indicates all ICMP codes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these are redundant since you have already specified these parameters in the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure whether the documentation section in the code was used elsewhere, such as via the ansible-docs
command. Should I should remove these two entries?
Co-authored-by: Alina Buzachis <[email protected]>
Co-authored-by: Alina Buzachis <[email protected]>
Co-authored-by: Alina Buzachis <[email protected]>
Co-authored-by: Alina Buzachis <[email protected]>
Co-authored-by: Alina Buzachis <[email protected]>
Co-authored-by: Alina Buzachis <[email protected]>
plugins/modules/ec2_group.py
Outdated
version_added: 3.3.0 | ||
type: int | ||
description: | ||
- When using C(proto: icmp) or C(proto: icmpv6), allows you to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2022-04-22 22:56:43.230449 | controller | ERROR: Found 2 yamllint issue(s) which need to be resolved:
2022-04-22 22:56:43.230573 | controller | ERROR: plugins/modules/ec2_group.py:117:49: error: DOCUMENTATION: syntax error: mapping values are not allowed here (syntax)
2022-04-22 22:56:43.230844 | controller | ERROR: plugins/modules/ec2_group.py:117:49: unparsable-with-libyaml: None - mapping values are not allowed in this context
This is likely what triggered the big slew of errors, the :
is interpreted by YAML as the divider between the key and value. (this catches me out too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good catch. It seems that treating colons as dividers between keys and values regardless of where they are is entirely up the parser. It never occurred to me that this would happen. I'll use the equal sign.
recheck |
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections/community.aws@4c44372
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None>
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None>
Rename rds_snapshot to rds_instance_snapshot SUMMARY Rename rds_snapshot to rds_instance_snapshot since rds_snapshot only handles snapshotting of DB instances. A new module for snapshotting RDS clusters will be added in a future PR. ISSUE TYPE New Module Pull Request COMPONENT NAME rds_snapshot Reviewed-by: Mark Chappell <None> Reviewed-by: None <None>
SUMMARY
This pull request add support for new
icmp_type
andicmp_code
parameters withproto: icmp
orproto: icmpv6
. This initial suggestion was aimed at closing the inconsistency when the meaning offrom_port
andto_port
would change based on the protocol type.Fixes #628
ISSUE TYPE
COMPONENT NAME
ec2_group