ansible-collections / community.routeros

Ansible modules for managing MikroTik RouterOS instances.
https://galaxy.ansible.com/ui/repo/published/community/routeros/
GNU General Public License v3.0
95 stars 44 forks source link

Some paths are showing as changed #258

Closed radokristof closed 7 months ago

radokristof commented 7 months ago

The path ip dns and tool netwatch shows changed every time I run the playbook. --diff shows the following: ip dns:

--- before
+++ after
@@ -1,7 +1,7 @@
 {
     "allow-remote-requests": true,
     "cache-max-ttl": "3d",
-    "cache-size": 4883,
+    "cache-size": "4883KiB",
     "doh-max-concurrent-queries": 2500,
     "doh-max-server-connections": 20,
     "doh-timeout": "10s",
@@ -10,7 +10,7 @@
     "max-udp-packet-size": 4096,
     "query-server-timeout": "2s",
     "query-total-timeout": "10s",
-    "servers": "1.1.1.1,8.8.8.8",
+    "servers": "1.1.1.1, 8.8.8.8",
     "use-doh-server": "",
     "verify-doh-cert": false
 }

tool netwatch:

--- before
+++ after
@@ -13,7 +13,7 @@
             "start-delay": "30s",
             "thr-avg": "10s",
             "thr-jitter": "20s",
-            "thr-loss-percent": 95,
+            "thr-loss-percent": "95%",
             "thr-max": "20s",
             "thr-stdev": "10s",
             "timeout": "5s",

So I have units added to values in the task, router accepts it and in the response it is showing the unit, but next time, it responds without units...

derdeagle commented 7 months ago

Could you please also show the data you are injecting? I know that there is sometimes as different in what RouterOS displays and handles in the API.

E.g. for ports. "443" is not the same as 443 (str vs. int).

radokristof commented 7 months ago

Sure, this is what I was using which was problematic:

---
  - name: Configure DNS
    community.routeros.api_modify:
      path: ip dns
      handle_entries_content: "{{ handle_entries_content }}"
      data:
        - allow-remote-requests: yes
          cache-max-ttl: 3d
          cache-size: 4883KiB
          doh-max-concurrent-queries: 2500
          doh-max-server-connections: 20
          doh-timeout: 10s
          max-concurrent-queries: 5000
          max-concurrent-tcp-sessions: 200
          max-udp-packet-size: 4096
          query-server-timeout: 2s
          query-total-timeout: 10s
          servers: 1.1.1.1, 8.8.8.8
          use-doh-server: "{{ use_doh_server }}"
          verify-doh-cert: "{{ verify_doh_cert }}"
---
  - name: Configure Netwatch addresses
    community.routeros.api_modify:
      path: tool netwatch
      handle_absent_entries: "{{ handle_absent_entries }}"
      handle_entries_content: "{{ handle_entries_content }}"
      data:
        - disabled: no
          name: "vpn_check"
          comment: "VPN connection check"
          host: 172.16.8.1
          interval: 10s
          packet-count: 5
          packet-interval: 100ms
          start-delay: 30s
          thr-avg: 10s
          thr-jitter: 20s
          thr-loss-percent: 95%
          thr-max: 20s
          thr-stdev: 10s
          timeout: 5s
          type: icmp
          down-script: "/system leds set [find where leds={{ user_led }}] type=off"
          up-script: "/system leds set [find where leds={{ user_led }}] type=on"

Currently correctly working ones:

---
  - name: Configure DNS
    community.routeros.api_modify:
      path: ip dns
      handle_entries_content: "{{ handle_entries_content }}"
      data:
        - allow-remote-requests: yes
          cache-max-ttl: 3d
          cache-size: 4883
          doh-max-concurrent-queries: 2500
          doh-max-server-connections: 20
          doh-timeout: 10s
          max-concurrent-queries: 5000
          max-concurrent-tcp-sessions: 200
          max-udp-packet-size: 4096
          query-server-timeout: 2s
          query-total-timeout: 10s
          servers: 1.1.1.1,8.8.8.8
          use-doh-server: "{{ use_doh_server }}"
          verify-doh-cert: "{{ verify_doh_cert }}"
---
  - name: Configure Netwatch addresses
    community.routeros.api_modify:
      path: tool netwatch
      handle_absent_entries: "{{ handle_absent_entries }}"
      handle_entries_content: "{{ handle_entries_content }}"
      data:
        - disabled: no
          name: "vpn_check"
          comment: "VPN connection check"
          host: 172.16.8.1
          interval: 10s
          packet-count: 5
          packet-interval: 100ms
          start-delay: 30s
          thr-avg: 10s
          thr-jitter: 20s
          thr-loss-percent: 95
          thr-max: 20s
          thr-stdev: 10s
          timeout: 5s
          type: icmp
          down-script: "/system leds set [find where leds={{ user_led }}] type=off"
          up-script: "/system leds set [find where leds={{ user_led }}] type=on"
derdeagle commented 7 months ago

Yes, IMHO this is the way to go here. If the API return an int and you want to set a str the comparator will say it differs. RouterOS seems to also strip whitespaces in comma-separated lists. 1.1.1.1, 8.8.8.8 will be changed to 1.1.1.1,8.8.8.8 (without the space after the comma).

For the cache-size in the ip dns path the documentation even says it is an integer. For thr-loss-percent in the tool netwatch path the documentation says the default was 85.0%. It looks like a string but I think it is a flaw in the documentation, so either int or float I'd say.

From my very own point of view I don't think it would make sense to change anything in the collection code.

radokristof commented 7 months ago

Yes, I was not sure about this, because handling all these different cases seems a little out of scope for this project. However for me it is still a little bit weird that it accepts and casts other types to their correct one instead of throwing an error. Anyway, I will close this then. Thank you for your help!