Fix validate interface for firewall rule creation to support interface groups #242
Conversation
jaredhendrickson13
added
the
blocked
There was a problem hiding this comment.
I've left some context on #241. Essentially this pull request will be considered blocked until an interface group endpoint is developed. You are welcome to leave this pull request open until then so it can be merged in at the same time. There are a few things I'd like to see addressed on this pull request first however.
| @@ -56,7 +56,10 @@ class APIFirewallRuleCreate extends APIModel { | |||
| private function __validate_interface() { | |||
| # Check for our required 'interface' payload value | |||
| if (isset($this->initial_data['interface'])) { | |||
| $this->initial_data['interface'] = APITools\get_pfsense_if_id($this->initial_data['interface']); | |||
| # only check for if id if the interface name is not a valid group name | |||
| if (!APITools\is_ifgroup($this->initial_data['interface'])) { | |||
There was a problem hiding this comment.
Rather than calling the is_ifgroup function directly, it would be preferred to incorporate this lookup into the get_pfsense_if_id function with a conditional flag similar to the $include_carp flag currently present on that function. This would allow all interface lookups to be done from a central location and also allow lookups to be configured for each endpoint's specific needs.
Additionally, any change made to the creation model would also need to be made to the update model (APIFireallRuleUpdate).
There was a problem hiding this comment.
I understand and agree! The support for firewall rules on interface groups is now moved to the APITools\get_pfsense_if_id function and can be used by an optional flag.
I have also created the Endpoint APIInterfaceGroup it supports all the request methods GET, POST, PUT and DELETE.
While doing so I spotted that pfSense supports deleting an interface group while firewall rules are still present. In this case, the rules are still viewable via the API, so they must be lurking around somewhere but not visible through the pfSense ui itself.
I have decided to not included any logic to avoid deleting interface groups with active firewall rules, as pfSense itself supports this behaviour.
…get_pfsense_if_id for create and update requests
…get_pfsense_if_id for create and update requests
|
This looks great! Thanks for your work. I'll merge these changes into my v1.5.0 branch (#243) so they can be tested along side alongside other features being staged for that release. I will include development builds in that PR periodically that can be used to test until the release is ready. |
See #241