aws Redshift Security Group module #293

Closed
wants to merge 1 commit into
from

Projects

None yet

7 participants

@j-carl
Contributor
j-carl commented Mar 2, 2015

I created a new module to manage the aws Redshift Security Group(s).

@gregdek
Contributor
gregdek commented Jun 17, 2015

Thanks for submitting this module to Ansible Extras. Apologies that it’s taken a while to get your module reviewed.

To help facilitate reviews, we’ve broadened the number of people who can approve modules for inclusion into the Extras repository. The list of official reviewers can be found here:

https://github.com/ansible/ansible-modules-extras/blob/devel/REVIEWERS.md

Our new policy is that if a new module is reviewed and approved by at least two official module reviewers, the module will be approved for inclusion. We will be asking the community of reviewers to take a look at these modules on a regular basis.

To ensure that your module has the best chance of being approved, please double-check that you adhere to the Ansible module guidelines: http://docs.ansible.com/developing_modules.html#module-checklist

@gregdek
Contributor
gregdek commented Jul 4, 2015

@j-carl can you add the "version_added: 2.0" field to the docs?

@robynbergeron
Contributor

Hi @j-carl --

Sorry this sat for so long. :( (The good news is, we're working to clean things up and improve turnaround time so it doesn't happen in the future!)

Moving out of needs_revision and putting back into community_review.

@robynbergeron
Contributor

Adding new community review process for this module -- which can hopefully help move this along towards inclusion in extras. :)

Thanks for submitting this new module to Ansible Extras! This module is now in community review, a process that is open to all Ansible users. In order for this module to be approved, it must gain the following votes, which can be contributed by anyone other than the author of the new module:

“works_for_me”: If you have tested the module thoroughly, including testing of all of the module’s options, and if the module works for you, please add “works_for_me” in the comments.

“passes_guidelines”: If you have gone through the module guidelines and the module meets all of the requirements, please add “passes_guidelines” in the comments. Guidelines are available here: http://docs.ansible.com/developing_modules.html#module-checklist

“needs_revision”: If the module fails to work for you, or if it doesn’t meet guidelines, please add “needs_revision” in the comments with details about what needs to be fixed.

When a module has both “works_for_me” and “passes_guidelines” tags, we will promote the module for inclusion in Ansible Extras. At this point, you will be expected to maintain the module by fixing bugs and evaluating pull requests in a timely manner.

More information about the review process can be seen here:
https://github.com/ansible/ansible-modules-extras/blob/devel/REVIEWERS.md

Thanks again for submitting your Ansible module!

@robynbergeron
Contributor

Also pinging the AWS crowd in case any of them specifically can help review this:

@jsmartin @scicoin-project @tombamford @garethr @jarv @jsdalton @silviud @adq @zbal @zeekin @willthames @lwade @carsongee @defionscode
@tastychutney @bpennypacker @loia

Thanks for your patience, @j-carl -- hoping we can get this through soon. Cheers!

@robynbergeron robynbergeron removed the P3 label Dec 22, 2015
@gregdek
Contributor
gregdek commented Mar 29, 2016

Thanks @j-carl for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@j-carl
Contributor
j-carl commented Apr 5, 2016

Fixed the problems and it's ready_for_review.

@willthames
Contributor

The commit log contains a merge commit, which suggests it needs a rebase onto latest master, rather than a merge.

@willthames willthames commented on an outdated diff Apr 6, 2016
cloud/amazon/redshift_security_group.py
+
+ try:
+ for rule in rules:
+ group_kwargs = {}
+ if 'cidr' in rule:
+ group_kwargs['cidrip'] = rule['cidr']
+ elif 'sg_name' in rule:
+ group_kwargs['ec2_security_group_name'] = rule['sg_name']
+ if 'sg_owner' in rule:
+ group_kwargs['ec2_security_group_owner_id'] = rule['sg_owner']
+
+ redshift.authorize_cluster_security_group_ingress(group_name, **group_kwargs)
+
+ except boto.exception.JSONResponseError, e:
+ # FIXME: Change this to just the error message when
+ # https://github.com/boto/boto/issues/2776 is fixed.
@willthames
willthames Apr 6, 2016 Contributor

I'm going to suggest that this is never going to happen (all the development effort has gone to boto3). Might as well just delete the FIXME comments across the board.

@willthames willthames commented on an outdated diff Apr 6, 2016
cloud/amazon/redshift_security_group.py
+ redshift_security_group:
+ state: present
+ group_name: test_with_ec2_sec_group
+ description: Security group with EC2 security group and CIDR
+ rules:
+ - sg_name: ec2_sg_test
+ sg_owner: <AWS account id>
+ - cidr: 0.0.0.0/0
+
+# Delete a security group.
+ redshift_security_group:
+ state: absent
+ group_name: test_with_ec2_sec_group
+'''
+
+RETURN = '''# '''
@willthames
willthames Apr 6, 2016 Contributor

Adding a RETURN section with no actual content is pointless.

@gregdek
Contributor
gregdek commented Apr 6, 2016

Thanks @j-carl for this new module. When this module receives 'shipit' comments from two community members and any 'needs_revision' comments have been resolved, we will mark for inclusion.

[This message brought to you by your friendly Ansibull-bot.]

@j-carl
Contributor
j-carl commented Apr 6, 2016

Did the last mentioned fixes and it's ready for ready_for_review.

@gregdek
Contributor
gregdek commented May 5, 2016

Thanks @j-carl for this PR. This PR requires revisions, either because it fails to build or by reviewer request. Please make the suggested revisions. When you are done, please comment with text 'ready_for_review' and we will put this PR back into review.

[This message brought to you by your friendly Ansibull-bot.]

@j-carl
Contributor
j-carl commented May 5, 2016

I'll still give it 'ready_for_review' regardless it failed, because I unfortunately don't have the time to rewrite the module with boto3.

@ansibot
ansibot commented Dec 7, 2016

This repository has been locked. All new issues and pull requests should be filed in https://github.com/ansible/ansible

Please read through the repomerge page in the dev guide. The guide contains links to tools which automatically move your issue or pull request to the ansible/ansible repo.

@j-carl j-carl referenced this pull request in ansible/ansible Jan 6, 2017
Open

aws Redshift Security Group module #19963

@ansibot ansibot closed this Jan 6, 2017
@sivel sivel removed the needs_revision label Jan 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment