search

mrlesmithjr / ansible-netplan

Ansible role to manage Netplan
MIT License
132 stars 85 forks source link

fix: Added back in the is defined check and nested falsy check #38

Closed bromeara closed 9 months ago

bromeara commented 1 year ago

Description

Found an issue while testing my previous PR where an undefined value would cause an error.

The alternative to this would be creating a default network map with values defaulted to omit, which may be preferable since there is an example block commented out in the defaults file.

However, the slightly easier solution was to nest the if <object name> check inside of the is defined check so it'll only run when the object is defined but will still catch omit values at the inner if

Related Issue

36

Types of changes

Checklist:

mrlesmithjr commented 1 year ago

I rolled the previous changes back. I don’t understand why this is needed as this hasn’t come up before.

bromeara commented 1 year ago

I did mislabel my original issue as I am using None instead of omit.

I have a role with this role as a dependency. My role has a default variable that is named access_points and is initialized to an empty map {}. This variable would be set on a per host basis in the playbook. Some of our devices are wifi enabled others are not. So some will pass a mapping like 

           'ap':
             password: pass
           'ap2':
             password: pass

And others will not set them, so access_points will be passed to this netplan role as an empty map. When I do this, I get the following error. 

RUNNING HANDLER [mrlesmithjr.netplan : Generating Netplan Configuration] *******
fatal: [molecule]: FAILED! => {"changed": true, "cmd": ["netplan", "generate"], "delta": "0:00:00.139799", "end": "2023-03-15 21:52:54.324567", "msg": "non-zero return code", "rc": 1, "start": "2023-03-15 21:52:54.184768", "stderr": "/etc/netplan/01-netcfg.yaml:18:9: Error in network definition: dwlan0: No access points defined\n        access-points: {}\n        ^", "stderr_lines": ["/etc/netplan/01-netcfg.yaml:18:9: Error in network definition: dwlan0: No access points defined", "        access-points: {}", "        ^"], "stdout": "", "stdout_lines": []}

Which isn't wrong I did pass in an empty map. Instead, I wanted to pass in None to the wifis section since I don't want to configure a wlan interface if it has no access points to connect to. So I have this section of code in my meta file.

   wireless_configuration:
    - key: '{{ wireless_lan_interface }}'
      value:
          dhcp4: yes
          dhcp4-overrides:
            use-dns: yes
            access-points: '{{ access_points }}'

  netplan_configuration:
    network:
      version: 2
      ethernets: '{{ ethernet_configuration | ansible.builtin.items2dict }}'
      wifis: '{{ access_points | ansible.builtin.ternary(wireless_configuration | ansible.builtin.items2dict, None) }}'

Which generates the error:

RUNNING HANDLER [mrlesmithjr.netplan : Generating Netplan Configuration] *******
fatal: [molecule]: FAILED! => {"changed": true, "cmd": ["netplan", "generate"], "delta": "0:00:00.088964", "end": "2023-03-16 17:36:26.462358", "msg": "non-zero return code", "rc": 1, "start": "2023-03-16 17:36:26.373394", "stderr": "/etc/netplan/01-netcfg.yaml:17:5: Error in network definition: expected mapping (check indentation)\n    ''\n    ^", "stderr_lines": ["/etc/netplan/01-netcfg.yaml:17:5: Error in network definition: expected mapping (check indentation)", "    ''", "    ^"], "stdout": "", "stdout_lines": []}

and this netplan config file.

---
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    deth0:
        addresses:
        - 192.168.136.1/24
        dhcp4: false
        dhcp6: false
    deth1:
        dhcp4: true
        dhcp6: false
        optional: true

  wifis:
    ''

While running the tests for this post, I think I came across a solution to my issue, which does not require modification which is instead of passing None pass an empty map. This completes without error and generates the following config.

---
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    deth0:
        addresses:
        - 192.168.136.1/24
        dhcp4: false
        dhcp6: false
    deth1:
        dhcp4: true
        dhcp6: false
        optional: true

  wifis:
    {}

Which should work fine but could lead to a config such as.

---
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    deth0:
        addresses:
        - 192.168.136.1/24
        dhcp4: false
        dhcp6: false
    deth1:
        dhcp4: true
        dhcp6: false
        optional: true

  wifis:
    {}

  bonds:
    {}

  bridges:
    {}

  vlans:
    {}

  tunnels:
    {}

Which are valid but maybe a bit messy. This pull request would output the following for undefined, empty map as well as None variables.

 ---
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    deth0:
        addresses:
        - 192.168.136.1/24
        dhcp4: false
        dhcp6: false
    deth1:
        dhcp4: true
        dhcp6: false
        optional: true

After the mistakes in the first PR, I made sure to review that all of the network subfields work with all 3 of those variable types. Sorry about those, but you're right; this change probably isn't necessary, but it could clean up a generated file in my case and handle people throwing a None type in there. although subfields with None can still error in the handler if netplan was expecting a map.

stale[bot] commented 9 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.