fix: Add conditional update logic for the ip_rdns module (#505)

* Add conditional update logic for the ip_rdns module

* make gendocs

* Add IPv6 support

* fix lint

docs/modules/ip_rdns.md

@@ -38,7 +38,7 @@ Manage a Linode IP address's rDNS.
 |-----------|------|----------|------------------------------------------------------------------------------|
 | `address` | <center>`str`</center> | <center>**Required**</center> | The IP address.   |
 | `state` | <center>`str`</center> | <center>Optional</center> | The state of this rDNS of the IP address.  **(Choices: `present`, `absent`)** |
-| `rdns` | <center>`str`</center> | <center>Optional</center> | The desired rDNS value.  **(Updatable)** |
+| `rdns` | <center>`str`</center> | <center>Optional</center> | The desired RDNS value.  **(Updatable)** |
 
 ## Return Values
 

plugins/module_utils/linode_common.py

@@ -247,7 +247,7 @@ def _get_resource_by_id(
             )
             return self.fail(
                 msg=f"failed to get {resource_name} "
-                "with id {resource_id}: {exception}"
+                f"with id {resource_id}: {exception}"
             )
 
     @property

plugins/modules/ip_rdns.py

@@ -5,7 +5,8 @@
 
 from __future__ import absolute_import, division, print_function
 
-from typing import Any, Optional
+from ipaddress import IPv6Address, ip_address
+from typing import Any, Optional, Union
 
 import ansible_collections.linode.cloud.plugins.module_utils.doc_fragments.ip_info as ip_docs
 import ansible_collections.linode.cloud.plugins.module_utils.doc_fragments.ip_rdns as ip_rdns_docs
@@ -40,7 +41,7 @@
     "rdns": SpecField(
         type=FieldType.string,
         editable=True,
-        description=["The desired rDNS value."],
+        description=["The desired RDNS value."],
     ),
 }
 
@@ -82,28 +83,68 @@ def __init__(self) -> None:
             required_if=[["state", "present", ["rdns"]]],
         )
 
-    def update_rdns(self, rdns: str) -> None:
+    @staticmethod
+    def _build_default_rdns(
+        address: str,
+    ) -> Optional[str]:
+        """
+        Builds the default RDNS address for the given IPv4/IPv6 address.
+        This is only used for local diffing purposes.
+        """
+        parsed_address = ip_address(address)
+
+        # IPv6 addresses have no default RDNS
+        if isinstance(parsed_address, IPv6Address):
+            return None
+
+        return f"{address.replace('.', '-')}.ip.linodeusercontent.com"
+
+    def _should_update_rdns(
+        self, old_rdns: str, new_rdns: Union[str, ExplicitNullValue]
+    ) -> bool:
+        """
+        Returns whether the old RDNS value and the proposed new RDNS for the
+        IP address differ.
+        """
+
+        # If the RDNS address is being reset, compare the old RDNS against
+        # the Linode API default
+        if isinstance(new_rdns, ExplicitNullValue):
+            new_rdns = self._build_default_rdns(
+                self.module.params.get("address")
+            )
+
+        return new_rdns != old_rdns
+
+    def _attempt_update_rdns(self, rdns: Union[str, ExplicitNullValue]) -> None:
         """
         Update the reverse DNS of the IP address.
         """
         ip_str = self.module.params.get("address")
         ip_obj = self._get_resource_by_id(IPAddress, ip_str)
-        ip_obj.rdns = rdns
-        ip_obj.save()
-        ip_obj._api_get()
-        self.register_action(
-            f"Updated reverse DNS of the IP address {ip_str} to be {rdns}"
-        )
+
+        old_rdns = ip_obj.rdns
+
+        if self._should_update_rdns(old_rdns, rdns):
+            ip_obj.rdns = rdns
+
+            ip_obj.save()
+            ip_obj._api_get()
+            self.register_action(
+                f"Updated reverse DNS of the IP address {ip_str} from {old_rdns} to {ip_obj.rdns}"
+            )
         self.results["ip"] = ip_obj._raw_json
 
     def _handle_present(self) -> None:
         rdns = self.module.params.get("rdns")
+
         if not rdns:
             self.fail("`rdns` attribute is required to update the IP address")
-        self.update_rdns(rdns)
+
+        self._attempt_update_rdns(rdns)
 
     def _handle_absent(self) -> None:
-        self.update_rdns(ExplicitNullValue())
+        self._attempt_update_rdns(ExplicitNullValue())
 
     def exec_module(self, **kwargs: Any) -> Optional[dict]:
         """Entrypoint for reverse DNS module"""

plugins/modules/ssh_key_info.py

@@ -92,12 +92,15 @@ def exec_module(self, **kwargs: Any) -> Optional[dict]:
 
         params = filter_null_values(self.module.params)
 
+        ssh_key = None
+
         if "id" in params:
             ssh_key = self._get_ssh_key_by_id(params.get("id"))
         elif "label" in params:
             ssh_key = self._get_ssh_key_by_label(params.get("label"))
 
         self.results["ssh_key"] = ssh_key._raw_json
+
         return self.results
 
 

tests/integration/targets/ip_rdns/tasks/main.yaml

@@ -27,7 +27,25 @@
         address: '{{ instance_create.instance.ipv4[0] }}'
         rdns: '{{ new_rdns }}'
       register: ip_rdns_modified
-
+
+    - name: Assert RDNS has been updated for the IP
+      assert:
+        that:
+          - ip_rdns_modified.changed
+          - ip_rdns_modified.ip.rdns == new_rdns
+
+    - name: Attempt to modify reverse DNS of the IP again
+      linode.cloud.ip_rdns:
+        state: present
+        address: '{{ instance_create.instance.ipv4[0] }}'
+        rdns: '{{ new_rdns }}'
+      register: ip_rdns_modified
+
+    - name: Assert RDNS has not been updated for the IP
+      assert:
+        that:
+          - not ip_rdns_modified.changed
+
     - name: Remove reverse DNS of the IP
       linode.cloud.ip_rdns:
         state: absent
@@ -41,6 +59,17 @@
           - ip_rdns_modified.ip.rdns == new_rdns
           - ip_rdns_removed.ip.rdns != new_rdns
 
+    - name: Attempt to remove RDNS of the IP again
+      linode.cloud.ip_rdns:
+        state: absent
+        address: '{{ instance_create.instance.ipv4[0] }}'
+      register: ip_rdns_removed_again
+
+    - name: Assert RDNS of the IP is unchanged
+      assert:
+        that:
+          - not ip_rdns_removed_again.changed
+
   always:
     - ignore_errors: true
       block:

tests/integration/targets/ip_rdns_ipv6/tasks/main.yaml

@@ -0,0 +1,87 @@
+- name: ip_rdns_ipv6
+  block:
+    - set_fact:
+        r: "{{ 1000000000 | random }}"
+
+    - name: Create an instance
+      linode.cloud.instance:
+        label: 'ansible-test-{{ r }}'
+        region: us-ord
+        type: g6-standard-1
+        image: linode/ubuntu22.04
+        wait: no
+        state: present
+      register: instance_create
+
+    - name: Extract the IPv6 SLAAC address
+      set_fact:
+        slaac: '{{ instance_create.networking.ipv6.slaac.address }}'
+
+    - name: Compute a new RDNS address
+      set_fact:
+        new_rdns: '{{ slaac | replace(":", "-") }}.sslip.io'
+
+    - name: Modify reverse DNS of the IP
+      linode.cloud.ip_rdns:
+        state: present
+        address: '{{ slaac }}'
+        rdns: '{{ new_rdns }}'
+      register: ip_rdns_modified
+
+    - name: Assert RDNS has been updated for the IP
+      assert:
+        that:
+          - ip_rdns_modified.changed
+          - ip_rdns_modified.ip.rdns == new_rdns
+
+    - name: Attempt to modify reverse DNS of the IP again
+      linode.cloud.ip_rdns:
+        state: present
+        address: '{{ slaac }}'
+        rdns: '{{ new_rdns }}'
+      register: ip_rdns_modified
+
+    - name: Assert RDNS has not been updated for the IP
+      assert:
+        that:
+          - not ip_rdns_modified.changed
+
+    - name: Remove reverse DNS of the IP
+      linode.cloud.ip_rdns:
+        state: absent
+        address: '{{ slaac }}'
+      register: ip_rdns_removed
+
+    - name: Assert reverse DNS of IP is removed
+      assert:
+        that:
+          - ip_rdns_removed.ip.address == slaac
+          - ip_rdns_modified.ip.rdns == new_rdns
+          - ip_rdns_removed.ip.rdns != new_rdns
+
+    - name: Attempt to remove RDNS of the IP again
+      linode.cloud.ip_rdns:
+        state: absent
+        address: '{{ slaac }}'
+      register: ip_rdns_removed_again
+
+    - name: Assert RDNS of the IP is unchanged
+      assert:
+        that:
+          - not ip_rdns_removed_again.changed
+
+  always:
+    - ignore_errors: true
+      block:
+        - name: Delete instance
+          linode.cloud.instance:
+            label: '{{ instance_create.instance.label }}'
+            state: absent
+
+  environment:
+    LINODE_UA_PREFIX: '{{ ua_prefix }}'
+    LINODE_API_TOKEN: '{{ api_token }}'
+    LINODE_API_URL: '{{ api_url }}'
+    LINODE_API_VERSION: '{{ api_version }}'
+    LINODE_CA: '{{ ca_file or "" }}'
+