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.

Sign up for GitHub

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

Jump to bottom

ec2_group add rule description fix #48023

Closed
wants to merge 1 commit into from
Closed

ec2_group add rule description fix #48023

wants to merge 1 commit into from

Conversation

willthames
Copy link
Contributor

SUMMARY

ec2_group should not remove security group rules that
have their rule descriptions updated.

Update descriptions first, then do the comparison of whether
or not groups should be removed.

Fixes #47904

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_group

ANSIBLE VERSION
ansible 2.8.0.dev0 (devel 96a20e0780) last updated 2018/11/03 10:51:34 (GMT +1000)
  config file = None
  configured module search path = [u'/Users/will/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/will/src/ansible/lib/ansible
  executable location = /Users/will/src/ansible/bin/ansible
  python version = 2.7.15 (default, Oct  2 2018, 11:47:18) [GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.2)]
ADDITIONAL INFORMATION

Should be backported to 2.6 and 2.7

@willthames
ec2_group add rule description fix
3085ecb 
ec2_group should not remove security group rules that
have their rule descriptions updated.

Update descriptions first, then do the comparison of whether
or not groups should be removed.
@willthames
Copy link
Contributor Author

This might not be the most elegant fix but it's the simplest fix I could think of to update the descriptions and then reset the group information to know what rules need adding or purging.

@ansibot
Copy link
Contributor

ansibot commented Nov 3, 2018

Hi @willthames, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 3, 2018

cc @adq
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 aws bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Nov 3, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Nov 5, 2018
@Daemoen
Copy link
Contributor

Daemoen commented Nov 6, 2018

Have just applied the patch to our instance, we should know how it operates in the next few executions. Will follow up as we have more feedback.

@s-hertel s-hertel mentioned this pull request Nov 9, 2018
Merged
@Daemoen
Copy link
Contributor

Daemoen commented Nov 13, 2018

Have been running this in our environment for several days now, and it seems to be working ok.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Nov 13, 2018
@Daemoen
Copy link
Contributor

Daemoen commented Nov 20, 2018 •
edited

I moved to the other fix about a week and a half ago.

@jborean93
Copy link
Contributor

Looks like #48443 is meant to fix this issue but with a smaller changeset.

@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 20, 2018
@jborean93
Copy link
Contributor

@willthames I believe this is no longer needed now that 48443 has been merged

@s-hertel
Copy link
Contributor

Closing as per above.

@s-hertel s-hertel closed this Apr 10, 2019
@ansible ansible locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@s-hertel s-hertel Awaiting requested review from s-hertel

@ryansb ryansb Awaiting requested review from ryansb

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 aws bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ec2_group ruleset applies rules improperly
6 participants
@willthames @ansibot @Daemoen @jborean93 @s-hertel @bcoca