fix: support updating virtual circuit attributes other than name (#245)

Fixes #237

This marks additional attributes as `mutable` in the virtual circuit
module specs. In addition to marking the attributes as `mutable`, we
also need to add a flag to the shared `get_diff` function to tell that
function _not_ to ignore fields that are not present in the API response
but are present in the config.

This also registers the `md5` attribute as a response attribute for
virtual circuits so that the module will know about that attribute and
handle it correctly.

docs/modules/metal_virtual_circuit.md

@@ -45,16 +45,16 @@ Manage a Virtual Circuit in Equinix Metal. You can use *id* or *name* to lookup
 | `port_id` | <center>`str`</center> | <center>Optional</center> | UUID of the Connection Port where the VC is scoped to.   |
 | `nni_vlan` | <center>`int`</center> | <center>Optional</center> | Equinix Metal network-to-network VLAN ID.   |
 | `vlan_id` | <center>`str`</center> | <center>Optional</center> | UUID of the VLAN to associate.   |
-| `vnid` | <center>`str`</center> | <center>Optional</center> | VNID VLAN parameter, see the documentation for Equinix Fabric.   |
-| `description` | <center>`str`</center> | <center>Optional</center> | Description for the Virtual Circuit resource.   |
-| `tags` | <center>`str`</center> | <center>Optional</center> | Tags for the Virtual Circuit resource.   |
-| `speed` | <center>`str`</center> | <center>Optional</center> | Speed of the Virtual Circuit resource.   |
+| `vnid` | <center>`str`</center> | <center>Optional</center> | VNID VLAN parameter, see the documentation for Equinix Fabric.  **(Updatable)** |
+| `description` | <center>`str`</center> | <center>Optional</center> | Description for the Virtual Circuit resource.  **(Updatable)** |
+| `tags` | <center>`str`</center> | <center>Optional</center> | Tags for the Virtual Circuit resource.  **(Updatable)** |
+| `speed` | <center>`str`</center> | <center>Optional</center> | Speed of the Virtual Circuit resource.  **(Updatable)** |
 | `vrf` | <center>`str`</center> | <center>Optional</center> | UUID of the VRF to associate.   |
-| `peer_asn` | <center>`int`</center> | <center>Optional</center> | The BGP ASN of the peer. The same ASN may be the used across several VCs, but it cannot be the same as the local_asn of the VRF.   |
-| `subnet` | <center>`str`</center> | <center>Optional</center> | A subnet from one of the IP blocks associated with the VRF that we will help create an IP reservation for. Can only be either a /30 or /31. For a /31 block, it will only have two IP addresses, which will be used for the metal_ip and customer_ip. For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable. We will default to the first usable IP address for the metal_ip.   |
-| `metal_ip` | <center>`str`</center> | <center>Optional</center> | The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.   |
-| `customer_ip` | <center>`str`</center> | <center>Optional</center> | The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.   |
-| `md5` | <center>`str`</center> | <center>Optional</center> | The password that can be set for the VRF BGP peer   |
+| `peer_asn` | <center>`int`</center> | <center>Optional</center> | The BGP ASN of the peer. The same ASN may be the used across several VCs, but it cannot be the same as the local_asn of the VRF.  **(Updatable)** |
+| `subnet` | <center>`str`</center> | <center>Optional</center> | A subnet from one of the IP blocks associated with the VRF that we will help create an IP reservation for. Can only be either a /30 or /31. For a /31 block, it will only have two IP addresses, which will be used for the metal_ip and customer_ip. For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable. We will default to the first usable IP address for the metal_ip.  **(Updatable)** |
+| `metal_ip` | <center>`str`</center> | <center>Optional</center> | The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.  **(Updatable)** |
+| `customer_ip` | <center>`str`</center> | <center>Optional</center> | The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.  **(Updatable)** |
+| `md5` | <center>`str`</center> | <center>Optional</center> | The password that can be set for the VRF BGP peer  **(Updatable)** |
 | `timeout` | <center>`int`</center> | <center>Optional</center> | Timeout in seconds for Virtual Circuit to get to "ready" state  **(Default: `15`)** |
 
 

plugins/module_utils/equinix.py

@@ -252,9 +252,20 @@ def only_defined_mutable(params: dict, mutables: list):
     return return_dict
 
 
-def get_diff(params: dict, fetched: dict, mutables: list):
+def get_diff(params: dict, fetched: dict, mutables: list, overwrite_undefined_from_api=False):
     current_mutable = only_defined_mutable(params, mutables)
-    fetched_mutable = only_defined_mutable(fetched, mutables)
+
+    # The shared get_diff function will not report a diff if an attribute
+    # has a None value in either the config or the API response.  Setting
+    # overwrite_undefined_from_api to True tells get_diff to report a diff
+    # if an attribute is defined in the config but not the API response.
+    # TODO: Long-term we should probably move away from this centralized
+    # logic so that each module has to make its own decisions about when
+    # and how to update attributes
+    if overwrite_undefined_from_api:
+        fetched_mutable = fetched
+    else:
+        fetched_mutable = only_defined_mutable(fetched, mutables)
 
     defined_mutable_keys = current_mutable.keys() & fetched_mutable.keys()
     if defined_mutable_keys == set():

plugins/module_utils/metal/metal_api.py

@@ -349,6 +349,7 @@ def private_ipv4_subnet_size(resource: dict):
     'id': 'id',
     'name': 'name',
     'customer_ip': optional('customer_ip'),
+    'md5': optional('md5'),
     'metal_ip': optional('metal_ip'),
     'nni_vlan': 'nni_vlan',
     'peer_asn': optional('peer_asn'),

plugins/modules/metal_virtual_circuit.py

@@ -231,24 +231,28 @@
        description=[
            'VNID VLAN parameter, see the documentation for Equinix Fabric.'
        ],
+       editable=True
     ),
     description=SpecField(
         type=FieldType.string,
         description=[
             'Description for the Virtual Circuit resource.'
         ],
+        editable=True,
     ),
     tags=SpecField(
         type=FieldType.string,
         description=[
             'Tags for the Virtual Circuit resource.'
         ],
+        editable=True
     ),
     speed=SpecField(
         type=FieldType.string,
         description=[
             'Speed of the Virtual Circuit resource.'
         ],
+        editable=True
     ),
     vrf=SpecField(
         type=FieldType.string,
@@ -262,6 +266,7 @@
             'The BGP ASN of the peer.',
             'The same ASN may be the used across several VCs, but it cannot be the same as the local_asn of the VRF.'
         ],
+        editable=True,
     ),
     subnet=SpecField(
         type=FieldType.string,
@@ -272,26 +277,30 @@
             'For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable.',
             'We will default to the first usable IP address for the metal_ip.',
         ],
+        editable=True,
     ),
     metal_ip=SpecField(
         type=FieldType.string,
         description=[
             'The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit.',
             'Will default to the first usable IP in the subnet.'
         ],
+        editable=True,
     ),
     customer_ip=SpecField(
         type=FieldType.string,
         description=[
             'The Customer IP address which the CSR switch will peer with.',
             'Will default to the other usable IP in the subnet.'
         ],
+        editable=True,
     ),
     md5=SpecField(
         type=FieldType.string,
         description=[
             'The password that can be set for the VRF BGP peer'
         ],
+        editable=True,
     ),
     timeout=SpecField(
         type=FieldType.integer,
@@ -405,7 +414,8 @@ def main():
         if fetched:
             module.params['id'] = fetched['id']
             if state == "present":
-                diff = get_diff(module.params, fetched, MUTABLE_ATTRIBUTES)
+                overwrite_undefined_from_api = True
+                diff = get_diff(module.params, fetched, MUTABLE_ATTRIBUTES, overwrite_undefined_from_api)
                 if diff:
                     fetched = module.update_by_id(diff, module_route)
                     changed = True