refactor: Ansible 2.19 support - #200
Merged
Merged
Conversation
Reviewer's GuideThis PR refactors the VPN role to comply with Ansible 2.19’s immutable data model by replacing all in-place Jinja mutations with accumulator patterns and filter-based transformations, modularizing the PSK generation into a separate task, converting implicit boolean checks into explicit length tests, and standardizing variable prefixes for clarity. Sequence diagram for PSK generation modularizationsequenceDiagram
participant MainTask as tasks/main.yml
participant PSKTask as tasks/vpn_get_psks_for_tunnel.yml
loop For each tunnel in __vpn_connections_fixed
MainTask->>PSKTask: include_tasks: vpn_get_psks_for_tunnel.yml (with tunnel, tunnel_idx)
PSKTask->>PSKTask: Reset __vpn_host_pairs
PSKTask->>PSKTask: Generate host pairs and PSKs
PSKTask->>PSKTask: Set __vpn_psks[tunnel_idx]
PSKTask-->>MainTask: Update __vpn_psks
end
Class diagram for VPN connection data structure refactorclassDiagram
class VPNConnection {
+auth_method
+opportunistic
+hosts
+policies
+shared_key_content
+cert_name
}
class VPNConnectionsFixed {
+List<VPNConnection>
}
class VPNPSKs {
+List<HostPairPSK>
}
class HostPairPSK {
+host_pairs: Tuple
+pre_shared_key: String
}
VPNConnectionsFixed "1" -- "*" VPNConnection
VPNPSKs "1" -- "*" HostPairPSK
VPNConnection "1" -- "*" Policy
class Policy {
+policy
+cidr
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
Author
|
[citest] |
Contributor
Author
|
hmm - installing collection dependencies from galaxy is timing out, but the script does not report that as an error |
Contributor
Author
|
[citest] |
Ansible 2.19 introduces some big changes https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_core_2.19.html One big change is that data structures are no longer mutable by the use of python methods such as `__setitem__`, `setdefault`, `update`, etc. in Jinja constructs. Instead, items must use filters or other Jinja operations. One common idiom is to mutate each element in a list. Since we cannot do this "in-place" anymore, a common way to do this is: ```yaml - name: Construct a new list from an existing list and mutate each element set_fact: __new_list: "{{ __new_list | d([]) + [mutated_item] }}" loop: "{{ old_list }}" mutated_item: "{{ some value based on item from old list }}" - name: Reset original old list set_fact: old_list: "{{ __new_list }}" ``` Similarly with `dict` items: ```yaml - name: Construct a new dict from an existing dict and mutate each element set_fact: __new_dict: "{{ __new_dict | d({}) | combine(mutated_item) }}" loop: "{{ old_dict | dict2items }}" mutated_item: "{{ {item.key: mutation of item.value} }}" - name: Reset original old dict set_fact: old_dict: "{{ __new_dict }}" ``` Another big change is that a boolean expression in a `when` or similar construct must be converted to a boolean - we cannot rely on the implicit evaluation in a boolean context. For example, if `var` is some iterable, like a `dict`, `list`, or `string`, you used to be able to evaluate an empty value in a boolean context: ```yaml when: var # do this only if var is not empty ``` You now have to explicitly test for empty using `length`: ```yaml when: var | length > 0 # do this only if var is not empty ``` These are the biggest changes. See the porting guide for others. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Contributor
Author
|
[citest] |
Contributor
Author
|
@spetrosi This one is pretty complicated - would appreciate it if you could take a look |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ansible 2.19 introduces some big changes
https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_core_2.19.html
One big change is that data structures are no longer mutable by the use of python
methods such as
__setitem__,setdefault,update, etc. in Jinja constructs.Instead, items must use filters or other Jinja operations.
One common idiom is to mutate each element in a list. Since we cannot do this
"in-place" anymore, a common way to do this is:
Similarly with
dictitems:Another big change is that a boolean expression in a
whenor similar constructmust be converted to a boolean - we cannot rely on the implicit evaluation in
a boolean context. For example, if
varis some iterable, like adict,list,or
string, you used to be able to evaluate an empty value in a boolean context:You now have to explicitly test for empty using
length:These are the biggest changes. See the porting guide for others.
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Refactor the VPN role’s tasks, templates, and tests to comply with Ansible 2.19 by eliminating in-place Jinja mutations and replacing them with filter-based list/dict constructions and explicit boolean checks.
Enhancements:
Tests: