New module ec2_vpc_peering #245

Closed
wants to merge 6 commits into
from

Projects

None yet
@manicai
manicai commented Feb 11, 2015

This module allows the creation of EC2 VPC peering connections.

@bcoca bcoca added this to the next milestone Feb 26, 2015
@bcoca
Member
bcoca commented Feb 27, 2015

the proper path for the module should be cloud/amazon/

@manicai
manicai commented Mar 2, 2015

Have fixed path.

@jimi-c jimi-c added P2 and removed P3 labels Mar 6, 2015
@bcoca bcoca removed this from the next milestone Mar 19, 2015
@bcoca bcoca removed the P2 label Mar 19, 2015
@viper233
Contributor
viper233 commented Jun 4, 2015

This doesn't take into account that it could be a cross account VPC peering connection. I'm not exactly sure how(lack of python skills), but you could set a flag as to whether the AWS accounts are the same for the peering VPC's and then force AWS credentials for the accepting peer.

@bennojoy bennojoy was assigned by bcoca Jun 5, 2015
@bcoca
Member
bcoca commented Jun 5, 2015

@viper233 that might be a good feature to add, but the module should already be useful 'as is', this ticket will closed once it is merged so I would advise opening a feature request ticket once that happens.

@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

@copumpkin

I'd love to see this merged

@michaeljs1990
Contributor

👍 Just needed this module. I will hopefully get sometime this week to review and test this.

@sgzijl
Contributor
sgzijl commented Sep 11, 2015

For a current project I need to establish cross-account peering connections. I will test this module coming week('s) and let you know the findings and results.

@Zeelot Zeelot commented on the diff Oct 6, 2015
cloud/amazon/ec2_vpc_peering.py
@@ -0,0 +1,238 @@
+#!/usr/bin/python
+#
+DOCUMENTATION = """
+---
+module: ec2_vpc_peer
+short_description: create or remove a peering connection between to ec2 VPCs.
@Zeelot
Zeelot Oct 6, 2015

between two ec2 VPCs

@dverbeek84

+1 i use peer connection alot in the same account, so this module will be verry usefull

@mmochan
Contributor
mmochan commented Nov 11, 2015

+1 I need to use this.

@klj613
klj613 commented Nov 24, 2015

+1 need this

@dohoangkhiem
Contributor

Tried using it, the great module.

Just one small bug I found if update_routes=yes & I re-run the module (no variable changes), the output shows 'ok' but actually the route from peer vpc to local vpc in peer vpc subnet's route table is lost (it was created properly at first run).

Anyway, +++1 for this great addition.

@klj613
klj613 commented Nov 25, 2015

I'm going to start using this soon (pull into my own set of modules until it is merged upstream).

I'm going to try and get peering between two VPCs in separate AWS accounts working. Would this be better as 'one task' (which we will need to provide additional AWS credentials as separate varaibles) or two tasks?

Two tasks I think would look like:

- ec2_vpc_peering:
    aws_access_key: '{{ vpc_from_access_key }}'
    aws_secret_key: '{{ vpc_from_secret_key }}'
    vpc_id: 'xxxx'
    vpc_peer_id: 'yyyy'
    state: requested

# requested would return 'OK' if its requested or present

- ec2_vpc_peering:
    aws_access_key: '{{ vpc_to_access_key }}'
    aws_secret_key: '{{ vpc_to_secret_key }}'
    vpc_id: 'yyyy'
    vpc_peer_id: 'xxxx'
    state: present

One task would look like:

- ec2_vpc_peering:
    aws_access_key: '{{ vpc_from_access_key }}'
    aws_secret_key: '{{ vpc_from_secret_key }}'
    aws_peer_access_key: '{{ vpc_peer_access_key }}'
    aws_peer_secret_key: '{{ vpc_peer_secret_key }}'
    vpc_id: 'xxxx'
    vpc_peer_id: 'yyyy'
    state: present

From a playbook level I think one task seems better but not sure about the internals for AWS credentials. Haven't made EC2 modules before.

Any comments on which one would most likely be accepted upstream?

@sgzijl
Contributor
sgzijl commented Dec 17, 2015

@klj613 I think that using one task will break implementations using the sts_assume_role module.

@sgzijl
Contributor
sgzijl commented Dec 17, 2015

Cross account VPC peering will not work, because the module has no peered account id parameter.

@sgzijl sgzijl and 1 other commented on an outdated diff Dec 18, 2015
cloud/amazon/ec2_vpc_peering.py
+ default: None
+ aliases: ['ec2_access_key', 'access_key']
+
+requirements: [ "boto" ]
+"""
+
+import sys
+import time
+
+try:
+ import boto.ec2
+ import boto.vpc
+ import boto.exception
+except ImportError:
+ print "failed=True msg='boto required for this module'"
+ sys.exit(1)
@sgzijl
sgzijl Dec 18, 2015 Contributor

Why not use module.fail_json(msg='boto is required for this module') here, as in the rest of the module?

@manicai
manicai Dec 18, 2015

Because back in February this was the standard pattern used by the core modules. :-)

Have updated it to use the same pattern that they now use.

@mmochan
Contributor
mmochan commented Dec 29, 2015

AWS cross account VPC peering module
#1434

@gregdek
Contributor
gregdek commented Jun 22, 2016

@manicai could you please rebase to strip out the merge commits! Thanks!

@bennojoy bennojoy was unassigned by gregdek Jun 22, 2016
@gregdek
Contributor
gregdek commented Jul 8, 2016

@manicai A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

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

@gregdek
Contributor
gregdek commented Jul 23, 2016

@manicai Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request.

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

@gregdek
Contributor
gregdek commented Aug 8, 2016

@manicai A friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open pending your changes. When you do address the issues, please respond with ready_for_review in your comment, so that we can notify the maintainer.

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

@gregdek
Contributor
gregdek commented Aug 23, 2016

@manicai Another friendly reminder: this pull request has been marked as needing your action. If you still believe that this PR applies, and you intend to address the issues with this PR, just let us know in the PR itself and we will keep it open. If you have addressed the issues and believe it's ready for review, please comment with the text "ready_for_review". If we don't hear from you within another 14 days, we will close this pull request.

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

@manicai
manicai commented Sep 5, 2016

Think this has been rendered redundant by #1434 so closing it.

@manicai manicai closed this Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment