-
Notifications
You must be signed in to change notification settings - Fork 397
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
Add transit-gateway-id parameter to ec2_vpc_vpn module #1877
Add transit-gateway-id parameter to ec2_vpc_vpn module #1877
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 17s (non-voting) |
Build failed.
|
recheck |
Build failed.
|
recheck |
- name: create vpn connection, with customer gateway, vpn_gateway_id and transit_gateway | ||
ec2_vpc_vpn: | ||
customer_gateway_id: '{{ cgw.gateway.customer_gateway.customer_gateway_id }}' | ||
vpn_gateway_id: '{{ vgw.vgw.id }}' | ||
transit_gateway_id: '{{ tgw.transit_gateway.transit_gateway_id }}' | ||
state: present | ||
register: result | ||
ignore_errors: true | ||
|
||
- name: assert creation of vpn failed | ||
assert: | ||
that: | ||
- result is failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What it moving this as a unit test https://github.com/ansible-collections/community.aws/blob/main/tests/unit/plugins/modules/test_ec2_vpc_vpn.py? This is only a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @alinabuzachis I would like to merge this PR, so that I can start the work on Validated Content. I need to understand how placebo recording works before I start with the unit test. I will take it up in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that this just checks for failed. This could throw any exception and pass the test, we should at least be looking for part of an error message that indicates the issue was the VPN not being in a happy state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This task throws an error as transit_gateway_id and vpn_gw_id are mutually exclusive. I have included the assertion to check for the error msg.
Build failed (gate pipeline). For information on how to proceed, see https://ansible.softwarefactory-project.io/zuul/buildset/b04229b621224ade8f526e6ff82eae36
|
recheck |
Build failed.
|
recheck |
Build failed.
|
recheck |
Build failed.
|
plugins/modules/ec2_vpc_vpn.py
Outdated
"vpn_gateway_id and customer_gateway_id or " | ||
"transit_gateway_id and customer_gateway_id. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"vpn_gateway_id and customer_gateway_id or " | |
"transit_gateway_id and customer_gateway_id. " | |
"customer_gateway_id and one of either transit_gateway_id or vpn_gateway_id." |
- name: create vpn connection, with customer gateway, vpn_gateway_id and transit_gateway | ||
ec2_vpc_vpn: | ||
customer_gateway_id: '{{ cgw.gateway.customer_gateway.customer_gateway_id }}' | ||
vpn_gateway_id: '{{ vgw.vgw.id }}' | ||
transit_gateway_id: '{{ tgw.transit_gateway.transit_gateway_id }}' | ||
state: present | ||
register: result | ||
ignore_errors: true | ||
|
||
- name: assert creation of vpn failed | ||
assert: | ||
that: | ||
- result is failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like that this just checks for failed. This could throw any exception and pass the test, we should at least be looking for part of an error message that indicates the issue was the VPN not being in a happy state
Build failed.
|
recheck |
Build failed.
|
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 3m 55s (non-voting) |
Backport to stable-6: 💚 backport PR created✅ Backport PR branch: Backported as #1887 🤖 @patchback |
Add transit-gateway-id parameter to ec2_vpc_vpn module SUMMARY This PR adds transit_gateway_id parameter to ec2_vpc_vpn module. It is needed for the validated content role that manages the creation of transit gateway and attaches VPN to the created transit gateway. ISSUE TYPE Feature Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Bikouo Aubin Reviewed-by: Alina Buzachis Reviewed-by: GomathiselviS Reviewed-by: Mark Chappell (cherry picked from commit d74c698)
[PR #1877/d74c6985 backport][stable-6] Add transit-gateway-id parameter to ec2_vpc_vpn module This is a backport of PR #1877 as merged into main (d74c698). SUMMARY This PR adds transit_gateway_id parameter to ec2_vpc_vpn module. It is needed for the validated content role that manages the creation of transit gateway and attaches VPN to the created transit gateway. ISSUE TYPE Feature Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis
SUMMARY
This PR adds transit_gateway_id parameter to ec2_vpc_vpn module. It is needed for the validated content role that manages the creation of transit gateway and attaches VPN to the created transit gateway.
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION