issue 350 cipher group idempotency

Makefile

@@ -80,7 +80,7 @@ galaxy_importer: build
 # skip the playbook which contains "password" in the file name
 run_examples:
 	@for playbook in examples/*.yaml; do \
-		if [[ $$playbook == *"password"* || $$playbook == *"login"* || $$playbook == *"logout"* || $$playbook == *"route"* || $$playbook == locationfile.yaml || $$playbook == nsip6.yaml ]]; then \
+		if [[ $$playbook == *"password"* || $$playbook == *"login"* || $$playbook == *"logout"* || $$playbook == *"route"* || $$playbook == locationfile.yaml || $$playbook == nsip6.yaml || $$playbook == hanode.yaml ]]; then \
 			continue; \
 		fi; \
 		echo "Running $$playbook"; \

plugins/module_utils/common.py

@@ -21,7 +21,10 @@
 
 @trace
 def get_netscaler_version(client):
-    response = get_resource(client, "nsversion")
+    is_exist, response = get_resource(client, "nsversion")
+    if not is_exist:
+        log("ERROR: Failed to get NetScaler version")
+        return (0.0, 0.0)
     # response = [{'installedversion': True, 'version': 'NetScaler NS13.0: Build 79.1002.nc, Date: Jul  7 2021, 10:31:36   (64-bit)', 'mode': '1'}]
     try:
         # send a tuple of (major, minor) version. i.e. (13.0, 79.1002)
@@ -46,17 +49,22 @@ def get_resource(
         filter=filter,
     )
     if status_code in {HTTP_RESOURCE_NOT_FOUND}:
-        return []
-    try:
-        # `update-only` resources return a dict instead of a list.
-        return_response = response_body[resource_name]
-        return (
-            return_response if isinstance(return_response, list) else [return_response]
-        )
-    except (
-        KeyError
-    ):  # for zero bindings, the response_body will be {'errorcode': 0, 'message': 'Done', 'severity': 'NONE'}
-        return []
+        return False, []
+    if status_code in HTTP_SUCCESS_CODES:
+        try:
+            # `update-only` resources return a dict instead of a list.
+            return_response = response_body[resource_name]
+            return (
+                True,
+                return_response
+                if isinstance(return_response, list)
+                else [return_response],
+            )
+        except (
+            KeyError
+        ):  # for zero bindings, the response_body will be {'errorcode': 0, 'message': 'Done', 'severity': 'NONE'}
+            return True, []
+    return False, []
 
 
 @trace

plugins/module_utils/constants.py

@@ -18,12 +18,6 @@
     409: "Resource already exists",
 }
 
-NETSCALER_NO_GET_RESOURCE = [
-    "login",
-    "logout",
-    "save_config",
-]
-
 NETSCALER_EMPTY_ADD_PAYLOAD_RESOURCES = [
     "logout",
 ]

plugins/module_utils/module_executor.py

@@ -35,7 +35,6 @@
     ATTRIBUTES_NOT_PRESENT_IN_GET_RESPONSE,
     HTTP_RESOURCE_ALREADY_EXISTS,
     NETSCALER_COMMON_ARGUMENTS,
-    NETSCALER_NO_GET_RESOURCE,
 )
 from .decorators import trace
 from .logger import log, loglines
@@ -120,7 +119,7 @@ def __init__(self, resource_name, supports_check_mode=True):
         self._filter_resource_module_params()
         if not self.resource_name.endswith("_binding"):
             self._filter_desired_bindings()
-        if self.resource_name not in NETSCALER_NO_GET_RESOURCE:
+        if {"get", "get-byname", "get-all"} & set(self.supported_operations):
             self.get_existing_resource()
 
     @trace
@@ -208,13 +207,25 @@ def get_existing_resource(self):
                 del get_args["ciphername"]
 
         # binding resources require `filter` instead of `args` to uniquely identify a resource
-        existing_resource = get_resource(
+        is_exist, existing_resource = get_resource(
             self.client,
             resource_name=self.resource_name,
             resource_id=self.resource_id,
             args=get_args if not self.resource_name.endswith("_binding") else {},
             filter=get_args if self.resource_name.endswith("_binding") else {},
         )
+        if is_exist is False:
+            return {}
+        if is_exist and not existing_resource:
+            if self.resource_name == "sslcipher":
+                # FIXME: NITRO-BUG: Some resources like `sslcipher` sends only
+                # { "errorcode": 0, "message": "Done", "severity": "NONE" },
+                # without any resource details
+                self.existing_resource[self.resource_primary_key] = self.resource_id
+            else:
+                self.existing_resource = {}
+            return self.existing_resource
+
         if len(existing_resource) > 1:
             msg = (
                 "ERROR: Found more than one resource with the same primary key %s and get arguments %s"
@@ -601,7 +612,7 @@ def sync_single_binding(self, binding_name):
             err = "`%s.binding_members` should be a `list`" % binding_name
             self.return_failure(err)
 
-        existing_bindings = get_bindings(
+        is_exists, existing_bindings = get_bindings(
             self.client,
             binding_name=binding_name,
             binding_id=self.resource_id,
@@ -672,7 +683,7 @@ def sync_single_binding(self, binding_name):
 
             # If there is any default bindings, after adding the custom bindings, the default bindings will be deleted automatically
             # Hence GET the existing bindings again and construct the `to_be_deleted_bindings` list
-            existing_bindings = get_bindings(
+            is_exists, existing_bindings = get_bindings(
                 self.client,
                 binding_name=binding_name,
                 binding_id=self.resource_id,

tests/integration/targets/sslcipher/aliases

@@ -0,0 +1,3 @@
+gather_facts/no
+netscaler/cpx/
+netscaler/vpx/

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

@@ -0,0 +1,119 @@
+---
+- name: SSLCIPHER | ADD | --check
+  delegate_to: localhost
+  register: result
+  check_mode: true
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | ADD | --check
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: SSLCIPHER | ADD
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | ADD
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: SSLCIPHER | ADD | idempotent
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: present
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | ADD | idempotent
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==false"
+- name: SSLCIPHER | DELETE | --check
+  delegate_to: localhost
+  register: result
+  check_mode: true
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | DELETE | --check
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: SSLCIPHER | DELETE
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | DELETE
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==true"
+- name: SSLCIPHER | DELETE | idempotent
+  delegate_to: localhost
+  register: result
+  check_mode: false
+  tags: test
+  netscaler.adc.sslcipher:
+    nsip: "{{ nsip }}"
+    nitro_user: "{{ nitro_user }}"
+    nitro_pass: "{{ nitro_pass }}"
+    nitro_protocol: "{{ nitro_protocol }}"
+    validate_certs: "{{ validate_certs }}"
+    save_config: "{{ save_config }}"
+    state: absent
+    ciphergroupname: test_ciphergroup
+- name: Assert | SSLCIPHER | DELETE | idempotent
+  tags: test
+  ansible.builtin.assert:
+    that:
+      - "result.failed==false"
+      - "result.changed==false"