Feat: Interface config

[kdhlab]

Draft for #157

[kdhlab]

Summary of changes:

1. Renamed the following files:

• plugins/modules/interfaces_assignments.py to interfaces_configuration.py
• plugins/module_utils/interfaces_assignments.py to interfaces_configuration.py

2. Created a alias in meta/runtime.yml to ensure that if the old interfaces_assignments is called it will use the new module.
3. Updated all references to interfaces_assignments to the new name.
4. The updated ansible module allows you to set any valid key for an interface, it also allows you to delete interfaces.

Due to the possible permutations of configurations here I did not write any real tests or error checking outside of the basics.

[DonGiovanni83]

Thank you very much for your contribution!

Sorry for the late response. I've been reviewing your changes carefully, and I've created a PR (kdhlab#1) on your branch with some suggested modifications. These changes address the following:

1. Renaming InterfaceAssignment to InterfaceConfiguration in other unit tests and docstrings.
2. Fixing the unit tests (this required some modifications to InterfaceConfiguration:save, InterfaceConfiguration:add_or_update, and the handling of InterfaceConfiguration:extra_attr).

Would you mind taking a look at my PR? I think once we resolve the issues there, we'll be very close to merging this. The remaining items I see are:

• Molecule Tests
• Fixing Sanity Tests (see output of make test-sanity)
• Writing a changelog fragment for the release

Let me know if you have any questions about my changes or the remaining tasks. I'm happy to discuss them further.

[kdhlab]

I have been out of packet for a while. I merged your suggestions, renamed all the molecule tests, and fixed the sanity. Did you want me to write the changelog fragment?

[DonGiovanni83]

Review Round 2

Thanks for taking the time to review and merge my suggestions in kdhlab#1.

@KiLLuuuhh and I tested your module with more molecule tests, and we were astonished by how many configuration we were able to apply to our test interfaces.
All of our initial tests ran successfully out of the gate!

Your work will be a great addition to the collection!

Pushing your module further however, we were able to find some semantic edge cases.

1. Module Parameters immutability

The module parameters must be passed as a copy to the InterfaceConfiguration.from_ansible_module_params factory method since a pop will be performed, changing the original dictionary. (see kdhlab#2)

2. Semantic validation of configuration types

During testing with molecule we have tried different combinations of parameters, and we have found that the current implementation allows conflicting parameters to coexist in the XML configuration.
Take this task for example:

---
- name: Breaking task
  puzzle.opnsense.interfaces_configuration:
      identifier: "lan"
      device: "em1"
      description: "LAN with DHCPv6 an Tracking"
      dhcp6_vlan_prio: 2
      dhcp6_ia_pd_len: 56
      dhcp6_prefix_id: 1
      track6_interface: "wan"
      track6_prefix_id: 2
      track6_ifid: "::1"
      enabled: true

While it ran successfully on the first run, the second run failed with an Exception. Also, the OPNsense instance had to revert to a backup config.xml file.

We suspect that the configuration types have mutually exclusive settings.The UI allows to set the following configuration types:

• IPv4:
  - None (Not sure if relevant for the module)
  - Static IPv4
  - DHCP
• IPv6:
  - None (Not sure if relevant for the module)
  - Static IPv6
  - DHCPv6
  - SLAAC
  - 6rd Tunnel
  - 6to4 Tunnel
  - Track Interface

Most of these configuration types have their own settings in the XML.

We suggest handling combinations of these settings at two levels:

1. Ansible Module Parameter dependencies

2. InterfaceConfiguration class validation
   To validate instances of the InterfaceConfiguration class you could use the same validation approach as in the FirewallAliasSet.validate_content method.
   Instead of content_type you could use ipv4_configuration_type and ipv6_configuration_type for example (just a suggestion).

We have opened a pull request on to your changes with some molecule tests that cover some cases that we have found here.
Feel free to add more tests in the same fashion to validate more cases.

3. Validation of DHCP alias parameters

The alias_address and alias_subnet parameters are not validated and could be validated using validate_ipaddr_and_subnet as well.

If you want us to help you out by e.g. adding some unit tests don't hesitate to let us know. We are happy to help!

[DonGiovanni83]

I have been out of packet for a while. I merged your suggestions, renamed all the molecule tests, and fixed the sanity. Did you want me to write the changelog fragment?

Yes sure, the changelog fragment is also needed for a future release 👍🏼

[kdhlab]

@DonGiovanni83 In reference to your second point, I ran into this as well when testing. I think we can probably add some guardrails for the basic gotchas. When I setup the module parameters I literally copied every possible configuration that could be done in XML. Therefore due the spiderweb of configurations that can be done within the module, the amount of time that would spent trying to catch all possible misconfigurations is not worth it IMO. What are your thoughts?

Also I made on a comment on your PR, kdhlab#2 (review), double check me on that.

[DonGiovanni83]

I understand the concern about the complexity of validating all possible interface configurations. However, I'd like to advocate for a more focused approach where we support a well-tested subset of configurations rather than trying to handle everything with minimal validation.

I propose we model our validation after OPNsense's own implementation in their PHP code (https://github.com/opnsense/core/blob/master/src/www/interfaces.php#L1000-L1128). This way, we can ensure our module behaves consistently with the OPNsense UI and catches invalid configurations early on.

The OPNsense implementation handeles different interface types (static, DHCP, PPP, etc.) with specific validation rules for each. We can start by implementing validation for the most common configuration types and expand from there. This approach would:

• Provide better error messages to users
• Catch configuration issues early
• Ensure consistency with OPNsense's behavior
• Be more maintainable in the long run

Would you be open to this suggested approach? We could start with the most common configuration types and add more as needed, with proper test coverage for each.

[restena-imo]

Hi all,
May I ask for any chance to have the module interfaces_configuration available soon ?

Thank you,
Isidore

[KiLLuuuhh]

Hi all, May I ask for any chance to have the module interfaces_configuration available soon ?

Thank you, Isidore

Hi @restena-imo

Since there was no update from @kdhlab, we are planning to implement a very basic version of this feature in the coming weeks (not months).

Have a great week and thank you for collaborating with our collection!

Cheers
Kilian