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

rds_instance: Add purge_security_groups #500

Merged
merged 5 commits into from
Apr 7, 2021

Conversation

alinabuzachis
Copy link
Contributor

@alinabuzachis alinabuzachis commented Mar 25, 2021

SUMMARY

rds_instance: Add purge_security_groups parameter for both db_security_groups and vpc_security_groups_ids.
Should fix: #385

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

rds_instance

ADDITIONAL INFORMATION

According to the boto3 documentation https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/rds.html#RDS.Client.describe_db_instances, describe_db_instances() provides this information for the associated security groups

'DBSecurityGroups': [
                {
                    'DBSecurityGroupName': 'string',
                    'Status': 'string'
                },
            ],
 'VpcSecurityGroups': [
                {
                    'VpcSecurityGroupId': 'string',
                    'Status': 'string'
                },
            ],

With this patch the status is reflected accordingly as showed in the following example.

- name: Create a DB instance in the VPC with two security groups
   rds_instance:
     id: "{{ instance_id }}"
     state: present
     engine: mariadb
     username: "{{ username }}"
     password: "{{ password }}"
     db_instance_class: "{{ db_instance_class }}"
     allocated_storage: "{{ allocated_storage }}"
     vpc_security_group_ids:
            - "sg-1"
            - "sg-2"

This gives:

....
"vpc_security_groups": [
        {
            "status": "active",
            "vpc_security_group_id": "sg-1"
        },
        {
            "status": "active",
            "vpc_security_group_id": "sg-2"
        }
    ]

And then if we want to set the a new security group sg-3 (purge_security_groups=True by default).

- name: Add a new security group (purge_security_groups=True by default)
   rds_instance:
     id: "{{ instance_id }}"
     state: present
     vpc_security_group_ids:
            - "sg-3"

The log produced:

"vpc_security_groups": [
        {
            "status": "removing",
            "vpc_security_group_id": "sg-1"
        },
        {
            "status": "removing",
            "vpc_security_group_id": "sg-2"
        },
        {
            "status": "adding",
            "vpc_security_group_id": "sg-3"
        }
    ]

Since some vpc security groups are reported "removing" status, do we want to show them? Others are also with "adding" state. I was wondering if there is a way to only show security groups with an active status or however wait until a security group passes from "adding" to "active".

@alinabuzachis
Copy link
Contributor Author

alinabuzachis commented Mar 25, 2021

@s-hertel Could you please take a look at this?

@ansibullbot
Copy link

@ansibullbot ansibullbot added WIP Work in progress bug This issue/PR relates to a bug integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Mar 25, 2021
	* Sanity fix

Signed-off-by: Alina Buzachis <[email protected]>
@s-hertel
Copy link
Collaborator

@alinabuzachis Yes, will review in a day or two. Thanks for fixing this!

Copy link
Collaborator

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

Looks great!

I was wondering if there is a way to only show security groups with an active status or however wait until a security group passes from "adding" to "active".

You could add a waiter to return only 'active' security groups by using a pathAll matcher - something like

"DBInstanceActiveSecurityGroups": {
    "delay": 20,
    "maxAttempts": 60,
    "operation": "DescribeDBInstances",
    "acceptors": [
        {
            "state": "success",
            "matcher": "pathAll",
            "argument": "DBInstances[].VpcSecurityGroups[].Status",
            "expected": "active"  # check if this is lowercase pre-camel_to_snake_case
        },
        {
            "state": "success",
            "matcher": "pathAll",
            "argument": "DBInstances[].DBSecurityGroups[].Status",
            "expected": "active"
        },
}

But I actually like this as it is, so it's your call/others can weigh in if that would be useful.

plugins/modules/rds_instance.py Outdated Show resolved Hide resolved
	* Add changelog

Signed-off-by: Alina Buzachis <[email protected]>
@alinabuzachis alinabuzachis changed the title [WIP] rds_instance: Add purge_security_groups rds_instance: Add purge_security_groups Mar 30, 2021
@ansibullbot ansibullbot added community_review and removed WIP Work in progress labels Mar 30, 2021
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Generally looks good. A couple of white-space nit-picks.

I'm currently running the tests locally, assuming they pass I'll get this merged.

@tremble
Copy link
Contributor

tremble commented Apr 7, 2021

Tests passed successfully after unrelated fixes.

@tremble tremble merged commit cd32e65 into ansible-collections:main Apr 7, 2021
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* rds_instance: Add purge_security_groups feature for vpc_security_groups_ids.
	* Fixes: ansible-collections#385
alinabuzachis added a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* rds_instance: Add purge_security_groups feature for vpc_security_groups_ids.
	* Fixes: ansible-collections#385
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
* rds_instance: Add purge_security_groups feature for vpc_security_groups_ids.
	* Fixes: ansible-collections#385
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
Add debugging info to ENI tests

SUMMARY
We're seeing some flakes, describe the ENIs before we nuke them to try and see where the issue might be.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_eni
ADDITIONAL INFORMATION
Example failure: waiter timeout:
https://ebf5fd30c6ae7bcc0e77-bb555a5b613b25366f4ba04980cee756.ssl.cf1.rackcdn.com/488/2808477252b5339f2f1af24f1658f756c8b3b931/check/ansible-test-cloud-integration-aws-py36_1/975c9cb/job-output.txt

Reviewed-by: None <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
* rds_instance: Add purge_security_groups feature for vpc_security_groups_ids.
	* Fixes: ansible-collections#385

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@cd32e65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review integration tests/integration module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rds_instance does not remove VPC security groups until you add another one
4 participants