make api_modify to ignore builtin items

[therfert]

SUMMARY

api_modify to ignore built-in items

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

api_modify

ADDITIONAL INFORMATION

this allows to use handle_absent_entries: remove with paths where builtin entries exist - e.g. interface list without the need of explicitly defining those entries in the data module parametr as a workaround.

This might be potentially breaking change (for those who used the workaround above) as far as any of the supported paths in the released version contains builtin entries.
I don't know if it is the case or not. (the interface list was added recently and not released yet). I could potentially loop through all the supported paths to see if there are any builtin entries to confirm this if needed.

If this is going to be approved, let me know if you would like to implement the same in the api_info module (if so - do you want to make it configurable via separate parameter or just cover it by the indlude_dynamic parameter?) .

[codecov]

Codecov Report

Merging #130 (6859fd2) into main (4194ae9) will decrease coverage by 0.12%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##             main     #130      +/-   ##
==========================================
- Coverage   87.56%   87.44%   -0.13%     
==========================================
  Files          28       28              
  Lines        3499     3505       +6     
  Branches      619      622       +3     
==========================================
+ Hits         3064     3065       +1     
- Misses        308      310       +2     
- Partials      127      130       +3     

Flag	Coverage Δ
integration	66.86% <ø> (ø)
sanity	22.38% <0.00%> (-0.08%)	⬇️
units	87.48% <28.57%> (-0.13%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
plugins/modules/api_info.py	83.09% <16.66%> (-6.14%)	⬇️
plugins/modules/api_modify.py	79.90% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[github-actions]

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

[felixfontein]

If it affects any path supported yet, this would be a breaking change and we cannot merge it.

For api_info it should definitely be configurable, so you can also list builtin items with it. The option here should probably be called include_builtin, similar to the existing include_dynamic option.

If any existing path supports it for api_modify, the behavior must be made configurable (maybe an option ignore_builtin with default false?). The default to this option should be matching to the default of api_info's include_builtin option so both modules behave the same.

[therfert]

If it affects any path supported yet, this would be a breaking change and we cannot merge it.

I'll I'll go through all the supported paths and let you know.

For api_info it should definitely be configurable, so you can also list builtin items with it. The option here should probably be called include_builtin, similar to the existing include_dynamic option.

Makes sense.

If any existing path supports it for api_modify, the behavior must be made configurable (maybe an option ignore_builtin with default false?). The default to this option should be matching to the default of api_info's include_builtin option so both modules behave the same.

In this case, we could maybe implement this later - perhaps in a new major version? would breaking change be allowed in such case?

The reason why I'm thinking about it is that the builtin entries are hardcoded in ROS - those are not configurable by the user. Unlike the "default" entries which are defined by default and cannot be deleted, but the user can modify them.

So defining a parametr to skip the builtin entries would really only cover the case when api_modify is used for the path with handle_absent_entries: remove. In such case the user had to define the builtin entries as well in the data in order to execute the module successfully.

[felixfontein]

If it affects any path supported yet, this would be a breaking change and we cannot merge it.

I'll I'll go through all the supported paths and let you know.

Thanks a lot!

If any existing path supports it for api_modify, the behavior must be made configurable (maybe an option ignore_builtin with default false?). The default to this option should be matching to the default of api_info's include_builtin option so both modules behave the same.

In this case, we could maybe implement this later - perhaps in a new major version? would breaking change be allowed in such case?

While a new major version allows breaking changes, doing that without a sufficiently long deprecation period is something we really try to avoid in the Ansible world. So we should add the parameter with the current default, wait a few releases, deprecate the default, and then we can switch over to the other default in the next major release (giving users some time where they can optionally state their intention, then some time where they see a deprecation warning when they don't explicitly set the value, while they can also specify that option when some colleagues are using the same playbooks with certain older versions of this collection, since the option was already there for some time).

The reason why I'm thinking about it is that the builtin entries are hardcoded in ROS - those are not configurable by the user. Unlike the "default" entries which are defined by default and cannot be deleted, but the user can modify them.

I fully understand (and I wish I would have added that code right from the beginning) :)

[therfert]

Looks good. None of the supported paths (of the released version) have any builtins. See below:

    - community.routeros.api_info:
        hostname: "{{ inventory_hostname }}"
        password: "{{ mikrotik_api_password }}"
        username: "{{ mikrotik_api_username }}"
        path: "{{ item }}"
        unfiltered: true
      register: ret
      ignore_errors: true
      loop:
        - caps-man aaa
        - caps-man access-list
        - caps-man configuration
        - caps-man datapath
        - caps-man manager
        - caps-man provisioning
        - caps-man security
        - certificate settings
        - interface bridge
        - interface bridge port
        - interface bridge port-controller
        - interface bridge port-extender
        - interface bridge settings
        - interface bridge vlan
        - interface detect-internet
        - interface ethernet
        - interface ethernet switch
        - interface ethernet switch port
        - interface l2tp-server server
        - interface list
        - interface list member
        - interface ovpn-server server
        - interface pppoe-client
        - interface pptp-server server
        - interface sstp-server server
        - interface vlan
        - interface wireless align
        - interface wireless cap
        - interface wireless sniffer
        - interface wireless snooper
        - ip accounting
        - ip accounting web-access
        - ip address
        - ip cloud
        - ip cloud advanced
        - ip dhcp-client
        - ip dhcp-client option
        - ip dhcp-server
        - ip dhcp-server config
        - ip dhcp-server lease
        - ip dhcp-server network
        - ip dns
        - ip dns static
        - ip firewall address-list
        - ip firewall connection tracking
        - ip firewall filter
        - ip firewall mangle
        - ip firewall nat
        - ip firewall service-port
        - ip hotspot service-port
        - ip ipsec identity
        - ip ipsec peer
        - ip ipsec policy
        - ip ipsec profile
        - ip ipsec proposal
        - ip ipsec settings
        - ip neighbor discovery-settings
        - ip pool
        - ip proxy
        - ip route
        - ip route vrf
        - ip service
        - ip settings
        - ip smb
        - ip socks
        - ip ssh
        - ip tftp settings
        - ip traffic-flow
        - ip traffic-flow ipfix
        - ip upnp
        - ipv6 address
        - ipv6 dhcp-client
        - ipv6 dhcp-server
        - ipv6 dhcp-server option
        - ipv6 firewall address-list
        - ipv6 firewall filter
        - ipv6 nd prefix default
        - ipv6 route
        - ipv6 settings
        - mpls
        - mpls ldp
        - port firmware
        - ppp aaa
        - queue interface
        - queue tree
        - radius incoming
        - routing bgp instance
        - routing mme
        - routing ospf area
        - routing ospf area range
        - routing ospf instance
        - routing ospf interface-template
        - routing pimsm instance
        - routing pimsm interface-template
        - routing rip
        - routing ripng
        - snmp
        - system clock
        - system clock manual
        - system identity
        - system leds settings
        - system logging
        - system logging action
        - system note
        - system ntp client
        - system ntp client servers
        - system ntp server
        - system package update
        - system routerboard settings
        - system upgrade mirror
        - system watchdog
        - tool bandwidth-server
        - tool e-mail
        - tool graphing
        - tool mac-server
        - tool mac-server mac-winbox
        - tool mac-server ping
        - tool romon
        - tool sms
        - tool sniffer
        - tool traffic-generator
        - user aaa
        - user group

    - debug:
        msg: "{{ ret.results | json_query('[?result[?builtin]].item') }}"

it gave me the same result on both ROS 6 and 7:

    "msg": [
        "interface list"
    ]

You can try yourself. Some of the paths are failing, but that's because those don't exist either on ROS6 or ROS7.

[felixfontein]

Great news! :)

[therfert]

ignore_builtin parametr has been added to the api_info module

[felixfontein]

@therfert thanks for yet another contribution :)