Skip to content
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

ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names #214

Merged

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Aug 29, 2020

SUMMARY

Fixes: #213

While working on #212 we spent a lot of time trying to debug the code in discover_security_groups. The bulk of this code is duplicated by module_utils.ec2.get_ec2_security_group_ids_from_names(). Let's use the shared code so we'll only need to test and debug one location in future.

Side effect: Where you have a mismatch between the security groups and the VPC of the selected subnets, this will now error with a warning like "Security group sg-0d67d5f78de12f7ce and subnet subnet-3f9eb911 belong to different networks." instead of silently dropping the security group on the floor.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_instance

ADDITIONAL INFORMATION

@ansibullbot
Copy link

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review module module needs_triage owner_pr PR created by owner/maintainer plugins plugin (any type) labels Sep 22, 2020
Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tremble The change LGTM but could you please add a changelog noting the minor behaviour change (warning instead of silently dropping the group)?

@tremble tremble force-pushed the find_security_group/ec2_instance branch from 3e07b8f to aff0d39 Compare October 23, 2020 09:00
@tremble tremble requested a review from jillr October 23, 2020 10:44
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Nov 16, 2020
@tremble tremble force-pushed the find_security_group/ec2_instance branch from aff0d39 to 5bf9bc7 Compare February 10, 2021 11:05
@tremble tremble force-pushed the find_security_group/ec2_instance branch from 5bf9bc7 to 2d1cefe Compare February 10, 2021 11:37
@tremble tremble removed the stale_ci CI is older than 7 days, rerun before merging label Feb 10, 2021
@jillr jillr merged commit 2b97236 into ansible-collections:main Feb 10, 2021
@tremble tremble deleted the find_security_group/ec2_instance branch February 11, 2021 09:05
ethemcemozkan pushed a commit to ethemcemozkan/community.aws that referenced this pull request Feb 18, 2021
…roup_ids_from_names (ansible-collections#214)

* ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names

* changelog
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…roup_ids_from_names (ansible-collections#214)

* ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names

* changelog
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…roup_ids_from_names (ansible-collections#214)

* ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names

* changelog
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
…roup_ids_from_names (ansible-collections#214)

* ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names

* changelog
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…fault

Sanity test / doc-default-does-not-match-spec fixups
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…roup_ids_from_names (ansible-collections#214)

* ec2_instance - Use shared module implementation of get_ec2_security_group_ids_from_names

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 bug This issue/PR relates to a bug community_review module module needs_triage owner_pr PR created by owner/maintainer plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ec2_instance fails with strange error message when subnet and security_group don't match.
3 participants