fix: properly unset firewall filter rule `protocol` field when removed

[Toalaah]

Previously, when you attempted to update a firewall rule with a non-null protocol field by removing said field, the serializer would set protocol to an empty string instead of completely omitting the value, resulting in API errors and ultimately causing the apply job to fail.

I.e, assuming that you have applied the following:

resource "routeros_ip_firewall_filter" "rules" {
  chain    = "input"
  action   = "accept"
  protocol = "tcp"
}

and then change it to this:

resource "routeros_ip_firewall_filter" "rules" {
  chain    = "input"
  action   = "accept"
  # protocol = "tcp"
}

The apply would fail with the following API error:

2024-04-26T15:40:51.919+0200 [ERROR] vertex "routeros_ip_firewall_filter.rules" error: PATCH 'https://192.168.88.2
18/rest/ip/firewall/filter/*A' returned response code: 400, message: 'Bad Request', details: 'input does not match
 any value of protocol'

I also took the liberty of aligning the MetaSetUnsetFields field in the ipv6_filter rule with its ipv4 counterpart, as I assumed this to be a basic oversight. But if there is a deeper reason behind this discrepancy then I can of course revert that particular change and just add MetaSetUnsetFields: PropSetUnsetFields("protocol").

[vaerh]

Thank you for your fixes! I just have a quick question: have you tried these changes on your datasets? If it worked, then I won't dive into this issue. In order to properly close this issue I need to test each of the fields on my own dataset, and honestly, I just don't have the energy to do that.

[Toalaah]

I am not entirely sure what you mean with datasets. I guess you are asking how I tested these changes? If so, here was my methodology:

• Configure a fresh CHR VM (7.14.2)
• Create a routeros_ip_firewall_filter resource containing all properties referenced in MetaSetUnsetFields (i.e dst_address_list, src_address_list, in_interface_list, out_interface_list, in_bridge_port_list, out_bridge_port_list, and protocol)
• Comment out each property individually and verify that I can apply the changes
• Repeat for routeros_ipv6_firewall_filter

The rule in question I used for testing:

resource "routeros_ip_firewall_filter" "rules" {
  chain  = "forward"
  action = "accept"

  dst_address_list     = "none"
  src_address_list     = "none"
  in_interface_list    = "none"
  out_interface_list   = "none"
  in_bridge_port_list  = "none"
  out_bridge_port_list = "none"
  protocol             = "tcp"
}

Hope that answers your question.

BTW: as I was playing around with this some more I found that the same issue applies to pretty much all string-based properties for which an empty string is an invalid value (e.g out_bridge_port, packet_size, nth). I am wondering whether all these properties should be added to MetaSetUnsetFields so that they are properly removed from the request body when PATCHing the FW rule. Or maybe the serializer can deal with this internally, no idea really. I am unfortunately not really that familiar with this codebase so I can't say for sure how feasible that would be.

[vaerh]

Yes, you understood me correctly :) By dataset I meant HCL script. It's hard to quickly switch my head from other work. The serealizer will make a decision exactly according to the schema and resource configuration. It will check if there is a value in the configuration, if there is a default value (we are doing away with these for easy portability of schemas from version to version of ROS), and if there are additional actions like set/unset. For this reason, if you find attributes that are not changed in the standard way, please add them to set/unset. There may be many such attributes, as I have encountered them after adding quite a few resources without correcting existing resources.

[vaerh]

:tada: This PR is included in version 1.46.3 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: