ec2_vpc_vpn.py: Facilitate VPN TunnelOptions #35210
Conversation
SUMMARY AWS (and Boto3) supports VPN TunnelOption keys to be specified for VPNs. Specifically (at least at the moment), the options for 'TunnelInsideCidr' (str) and 'PreSharedKey' (str) are particularly noteworthy. One or both may be specified within a 'dict' object (max 2 dicts). AWS defaults shall apply in absence of said parameters. ISSUE TYPE * Feature Pull Request COMPONENT NAME * ec2_vpc_vpn.py ANSIBLE VERSION * devel * 2.4 ADDITIONAL INFORMATION At the time of this writing, the Boto3 documentation on this topic can be referenced at: * http://boto3.readthedocs.io/en/latest/reference/services/ec2.html#EC2.Client.create_vpn_connection USAGE EXAMPLES Note this feature requires so-called anonymous dict objects be referenced as shown. An example playbook excerpt for specifying PreSharedKeys for both VPN tunnels: - name: TunnelOptions include 'PreSharedKey' only ec2_vpc_vpn: state: present filters: vpn: vpn-XXXXXXXX static_only: true tunnel_options: - PreSharedKey: 'abc123xyz789' - PreSharedKey: 'abc123xyz789' Same as above, except for 'TunnelInsideCidr' only: - name: TunnelOptions include 'TunnelInsideCidr' only ec2_vpc_vpn: state: present filters: vpn: vpn-XXXXXXXX static_only: true tunnel_options: - TunnelInsideCidr: '169.254.100.1/30' - TunnelInsideCidr: '169.254.100.5/30' Both available 'TunnelOptions': - name: All currently available TunnelOptions ec2_vpc_vpn: state: present filters: vpn: vpn-XXXXXXXX static_only: true tunnel_options: - TunnelInsideCidr: '169.254.100.1/30' PreSharedKey: 'abc123xyz789' - TunnelInsideCidr: '169.254.100.5/30' PreSharedKey: 'abc123xyz789'
ansibot
added
affects_2.5
|
The test The test The test |
ansibot
added
ci_verified
Fixed apparent indentation/format errors ...
ansibot
removed
the
ci_verified
|
The test |
ansibot
added
community_review
ansibot
added
needs_ci
gundalow
removed
the
needs_triage
|
|
||
| options = {'StaticRoutesOnly': static_only} | ||
|
|
||
| if isinstance(tunnel_options, list) and 0 < len(tunnel_options) <= 2: |
There was a problem hiding this comment.
tunnel_options will never not be a list here since it's defaulting to an empty list and has type='list' in the arg spec so you could simplify this a bit to:if tunnel_options and len(tunnel_options) <=2:
However, would it be better not to silently ignore if you list more than 2? The error that would be returned by boto is probably enough in terms of validation for this option (though I haven't tested it, and that should be checked). If it isn't caught and handled properly, botocore.exceptions.BotoCoreError handling may need to be added to the call in addition to the botocore.exceptions.ClientError (the difference being that BotoCoreError has no .response attribute).
There was a problem hiding this comment.
Regarding your first point on 'simplification', I agree and will massage the code accordingly.
Regarding the list size, my original thinking was that if someone is doing something funky (e.g: sending more dicts than Boto3/AWS are capable of handling), we should just ignore it. I could see throwing a harmless warning, advising the executor that only up to 2 pairs of options are supported. I am, however, open to throwing a fatal exception as you indicated.
There was a problem hiding this comment.
I don't have a strong preference. LGTM!
| if not isinstance(m, dict): | ||
| raise TypeError("non-dict list member") | ||
| t_opt.append(m) | ||
| if len(t_opt) > 0: |
There was a problem hiding this comment.
This can just be if t_opt: since an empty list is false.
There was a problem hiding this comment.
Good point, I'll update this line (452).
Some list-specific cleanup per s-hertel's suggestions.
ansibot
added
the
stale_ci
ryansb
removed
the
stale_ci
ansibot
added
feature
SUMMARY
AWS (and Boto3) supports VPN TunnelOption keys to be specified for VPNs. Specifically (at least at the moment), the options for 'TunnelInsideCidr' (str) and 'PreSharedKey' (str) are particularly noteworthy. One or both may be specified within a 'dict' object (max 2 dicts). AWS defaults shall apply in absence of said parameters.
ISSUE TYPE
COMPONENT NAME
ANSIBLE VERSION
TESTING
This change was tested during a recent Ansible playbook run (unfortunately console output was not preserved). The VPN tunnels came online after a typical amount of time, and all specified TunnelOptions were present as expected.
ADDITIONAL INFORMATION
At the time of this writing, the Boto3 documentation covers this topic: