From 0ba33abc01c2a538c8965aa3b8c1552fcd652d37 Mon Sep 17 00:00:00 2001 From: Matt Proud Date: Fri, 10 Nov 2023 17:06:36 +0000 Subject: [PATCH 1/2] fix recusrive paginaton bug Signed-off-by: Matt Proud --- plugins/module_utils/vmware_nsxt.py | 61 +++++++++++++++++------------ 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/plugins/module_utils/vmware_nsxt.py b/plugins/module_utils/vmware_nsxt.py index 40df0ce0..631e1b20 100644 --- a/plugins/module_utils/vmware_nsxt.py +++ b/plugins/module_utils/vmware_nsxt.py @@ -12,7 +12,7 @@ # WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, # EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import json, os, re +import copy, json, os, re from ansible.module_utils.urls import open_url, fetch_url from ansible.module_utils.six.moves.urllib.error import HTTPError from ansible.module_utils._text import to_native @@ -30,11 +30,46 @@ def vmware_argument_spec(): def request(url, data=None, headers=None, method='GET', use_proxy=True, force=False, last_mod_time=None, timeout=300, validate_certs=True, url_username=None, url_password=None, http_agent=None, force_basic_auth=True, ignore_errors=False): + + collated_response = {} + while True: + (resp_code, resp_data) = request_api(url, data, headers=headers, method=method, use_proxy=use_proxy, + force=force, last_mod_time=last_mod_time, timeout=timeout, validate_certs=validate_certs, + url_username=url_username, url_password=url_password, http_agent=http_agent, + force_basic_auth=force_basic_auth, ignore_errors=ignore_errors) + + if not method == "GET": + return resp_code, resp_data + + if not collated_response: + # Deep copy to all updates in further loops + collated_response = copy.deepcopy(resp_data) + elif resp_data['results']: + collated_response['results'].extend(resp_data.get('results', [])) + + if 'cursor' in resp_data: + parsed_url = urlparse(url) + query_params = parse_qs(parsed_url.query) + # Add or update the query parameter + query_params['cursor'] = resp_data["cursor"] + # Reconstruct the URL with updated query parameters + updated_query = urlencode(query_params, doseq=True) + url = unquote(urlunparse(parsed_url._replace(query=updated_query))) + continue + + return resp_code, collated_response + + +def request_api(url, data=None, headers=None, method='GET', use_proxy=True, + force=False, last_mod_time=None, timeout=300, validate_certs=True, + url_username=None, url_password=None, http_agent=None, force_basic_auth=True, ignore_errors=False): ''' The main function which hits the request to the manager. Username and password are given the topmost priority. In case username and password are not provided if the environment variable is set. Authentication fails if the details are not correct. ''' + if method == "get": + print("request = get") if url_username is None or url_password is None: force_basic_auth = False client_cert = get_certificate_file_path('NSX_MANAGER_CERT_PATH') @@ -75,31 +110,7 @@ def request(url, data=None, headers=None, method='GET', use_proxy=True, raise Exception(resp_code, resp_data) if not (resp_data is None) and resp_data.__contains__('error_code'): raise Exception (resp_data['error_code'], resp_data) - if resp_code != 200: - return resp_code, resp_data - - elif resp_code == 200 and resp_data is not None: - cursor = resp_data.get('cursor', NULL_CURSOR_PREFIX) - parsed_url = urlparse(url) - query_params = parse_qs(parsed_url.query) - while cursor and not cursor.startswith(NULL_CURSOR_PREFIX): - if page is not None and isinstance(page, dict): - resp_data['results'].extend(page.get('results', [])) - cursor = page.get('cursor', NULL_CURSOR_PREFIX) - if cursor == NULL_CURSOR_PREFIX: - return resp_code, resp_data - - # Add or update the query parameter - query_params['cursor'] = cursor - # Reconstruct the URL with updated query parameters - updated_query = urlencode(query_params, doseq=True) - updated_url = unquote(urlunparse(parsed_url._replace(query=updated_query))) - resp_code, page = request(updated_url, data=data, headers=headers, method=method, use_proxy=use_proxy, - force=force, last_mod_time=last_mod_time, timeout=timeout, validate_certs=validate_certs, - url_username=url_username, url_password=url_password, http_agent=http_agent, - force_basic_auth=force_basic_auth) - return resp_code, resp_data return resp_code, resp_data def get_certificate_string(crt_file): From 98b1a4edfc3380518304a5ba59276ad385dc2b1e Mon Sep 17 00:00:00 2001 From: Matt Proud Date: Mon, 20 Nov 2023 11:01:05 +0000 Subject: [PATCH 2/2] add pagination integration test Signed-off-by: Matt Proud --- plugins/module_utils/vmware_nsxt.py | 1 - tests/playbooks/mp/pagination_test.yml | 60 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 tests/playbooks/mp/pagination_test.yml diff --git a/plugins/module_utils/vmware_nsxt.py b/plugins/module_utils/vmware_nsxt.py index 631e1b20..c5ca592d 100644 --- a/plugins/module_utils/vmware_nsxt.py +++ b/plugins/module_utils/vmware_nsxt.py @@ -58,7 +58,6 @@ def request(url, data=None, headers=None, method='GET', use_proxy=True, continue return resp_code, collated_response - def request_api(url, data=None, headers=None, method='GET', use_proxy=True, force=False, last_mod_time=None, timeout=300, validate_certs=True, diff --git a/tests/playbooks/mp/pagination_test.yml b/tests/playbooks/mp/pagination_test.yml new file mode 100644 index 00000000..3e614098 --- /dev/null +++ b/tests/playbooks/mp/pagination_test.yml @@ -0,0 +1,60 @@ + +# +--- +- name: Pagination test + hosts: localhost + gather_facts: false + collections: + - vmware.ansible_for_nsxt + vars: + hostname: "{{ lookup('ansible.builtin.env', 'NSX_MANAGER_IP') }}" + username: "{{ lookup('ansible.builtin.env', 'NSX_USERNAME') }}" + password: "{{ lookup('ansible.builtin.env', 'NSX_PASSWORD') }}" + sequence: "1-1001" + label_prefix: "ANSIBLE_INTEGRATION_TESTING" + tasks: + + - name: Create switches and test they are all visible + block: + - name: Create logical switches + nsxt_logical_switches: + hostname: "{{ hostname }}" + username: "{{ username }}" + password: "{{ password }}" + validate_certs: False + display_name: "{{ label_prefix }}-ls-{{ item }}" + admin_state: "UP" + transport_zone_name: "nsx-overlay-transportzone" + state: "present" + with_sequence: + - "{{ sequence }}" + + - name: Create logical switch + nsxt_logical_switches_facts: + hostname: "{{ hostname }}" + username: "{{ username }}" + password: "{{ password }}" + validate_certs: False + register: result_switches + + - name: Check length of get requests + ansible.builtin.assert: + that: + - result_switches.results | length > 1000 + fail_msg: | + After creating 1001 switches facts only returns {{ result_switches.results | length }}. + Pagination failure! + success_msg: "Pagination of get requests OK" + always: + - name: Delete logical switches + nsxt_logical_switches: + hostname: "{{ hostname }}" + username: "{{ username }}" + password: "{{ password }}" + validate_certs: False + display_name: "{{ label_prefix }}-ls-{{ item }}" + state: "absent" + admin_state: "UP" + transport_zone_name: "nsx-overlay-transportzone" + with_sequence: + - "{{ sequence }}" \ No newline at end of file