[Feature]: netbox_circuit_termination module should not expose low-level parameters

[benoit-ferre]

NetBox version

Devel Branch

Feature type

Change to existing Module

Proposed functionality

I think netbox ansible module should not expose this type of parameters.
As said in those comments : #1421 (review) and #1421 (comment)

ansible_modules/plugins/modules/netbox_circuit_termination.py

Lines 52 to 71 in 77e28f8

	termination_id:
	description:
	- The ProviderNetwork, Location, Site, Region, or SiteGroup ID of the circuit termination will be assigned to.
	- This parameter is used with NetBox versions >= 4.2.0.
	required: false
	type: int
	version_added: 4.2.0
	termination_type:
	description:
	- The type the circuit termination will be assigned to.
	- This parameter is used with NetBox versions >= 4.2.0.
	choices:
	- dcim.site
	- dcim.location
	- dcim.region
	- dcim.sitegroup
	- circuits.providernetwork
	required: false
	type: str
	version_added: 4.2.0

Instead module should check netbox version and dynamically create those parameters if running compatible netbox version.
Like in the coed below but with netbox version check.

ansible_modules/plugins/module_utils/netbox_ipam.py

Lines 219 to 227 in 77e28f8

	if self.endpoint == "services":
	if "device" in data:
	data["parent_object_type"] = "dcim.device"
	data["parent_object_id"] = data["device"]
	del data["device"]
	elif "virtual_machine" in data:
	data["parent_object_type"] = "virtualization.virtualmachine"
	data["parent_object_id"] = data["virtual_machine"]
	del data["virtual_machine"]

So I would be

        if self.endpoint == "services" and self._version_check_greater(
            self.version, "4.3", greater_or_equal=True
        ):
            if "device" in data:
                data["parent_object_type"] = "dcim.device"
                data["parent_object_id"] = data["device"]
                del data["device"]
            elif "virtual_machine" in data:
                data["parent_object_type"] = "virtualization.virtualmachine"
                data["parent_object_id"] = data["virtual_machine"]
                del data["virtual_machine"]

So I suggest to :

• Add parameters mutually exclusive : site, location, region, sitegroup, providernetwork.
• Remove termination_type and termination_id.
• Add code similar to the one above.
• For site, location, region, sitegroup, providernetwork the parameter value I don't kown how to handle it, But it should be similar on how "circuit" parameter is handled to convert display name into id.
  I may be related to this code:
  

  ansible_modules/plugins/module_utils/netbox_utils.py

  Lines 233 to 242 in 77e28f8

  	# Specifies keys within data that need to be converted to ID and the endpoint to be used when queried
  	CONVERT_TO_ID = {
  	"assigned_object": "assigned_object",
  	"bridge": "interfaces",
  	"circuit": "circuits",
  	"circuit_type": "circuit_types",
  	"circuit_termination": "circuit_terminations",
  	"circuits.circuittermination": "circuit_terminations",
  	"cluster": "clusters",
  	"clusters": "clusters",

WDYT?

Use case

Other modules use this approach.
It would be more consitent and userfriendly.

External dependencies

N/A

[benoit-ferre]

If no one else is planning to take this on, I’d be glad to work on a PR based on your feedback.