Skip to content

Commit

Permalink
Fix inventory crash when query is used
Browse files Browse the repository at this point in the history
Query is defined as a list of dictionaries but the inventory is
considering the query as a str when constructing the cache suffix value.

This commit fixes this bug by returing the query as a b64 string as
"key1_value1_key2_value2"
  • Loading branch information
tupyy committed May 21, 2024
1 parent 582a81d commit 94bcf61
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 15 deletions.
29 changes: 27 additions & 2 deletions plugins/inventory/now.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,18 +520,43 @@ def _get_instance(self):
instance_env = self._get_instance_from_env()
return self._merge_instance_config(instance_config, instance_env)

def _construct_cache_suffix(self):
"""
Return the cache suffix constructued from either query or sysparm_query.
As the query can be a list of dict elements, key and values are encoded in base64.
The result is base64 encoded.
"""

def __encode(s):
from base64 import b64encode

return b64encode(s.encode()).decode()

suffix = ""
if self.get_option("query"):
for query in self.get_option("query"):
for k, v in query.items():
if suffix:
suffix = "{0}_{1}_{2}".format(suffix, k, v)
else:
suffix = "{0}_{1}".format(k, v)
elif self.get_option("sysparm_query"):
suffix = self.get_option("sysparm_query")
else:
return ""
return __encode(suffix)

def parse(self, inventory, loader, path, cache=True):
super(InventoryModule, self).parse(inventory, loader, path)

self._read_config_data(path)
self.cache_key = self.get_cache_key(path)
suffix = self.get_option("query") or self.get_option("sysparm_query") or ""
cache_sub_key = "/".join(
[
self._get_instance()["host"].rstrip("/"),
"table",
self.get_option("table"),
suffix,
self._construct_cache_suffix(),
]
)

Expand Down
56 changes: 56 additions & 0 deletions tests/unit/plugins/inventory/test_now.py
Original file line number Diff line number Diff line change
Expand Up @@ -852,3 +852,59 @@ def test_construction_enhanced(self, inventory_plugin):
)

assert a2.vars == dict(inventory_file=None, inventory_dir=None)


class TestConstructCacheSuffix:
def test_from_query(self, inventory_plugin, mocker):
from base64 import b64encode

real_suffix = b64encode("opt1_a_opt2_b".encode()).decode()

def get_option(*args):
return [dict(opt1="a"), dict(opt2="b")]

mocker.patch.object(inventory_plugin, "get_option", new=get_option)

suffix = inventory_plugin._construct_cache_suffix()

assert suffix == real_suffix

def test_from_query_2(self, inventory_plugin, mocker):
from base64 import b64encode

real_suffix = b64encode("opt1_a".encode()).decode()

def get_option(*args):
return [dict(opt1="a")]

mocker.patch.object(inventory_plugin, "get_option", new=get_option)

suffix = inventory_plugin._construct_cache_suffix()

assert suffix == real_suffix

def test_from_sysparm_query(self, inventory_plugin, mocker):
from base64 import b64encode

real_suffix = b64encode("a".encode()).decode()

def get_option(*args):
if args[0] == "sysparm_query":
return "a"
return []

mocker.patch.object(inventory_plugin, "get_option", new=get_option)

suffix = inventory_plugin._construct_cache_suffix()

assert suffix == real_suffix

def test_from_sysparm_query_2(self, inventory_plugin, mocker):
def get_option(*args):
return None

mocker.patch.object(inventory_plugin, "get_option", new=get_option)

suffix = inventory_plugin._construct_cache_suffix()

assert suffix == ""
37 changes: 24 additions & 13 deletions tests/unit/plugins/modules/test_service_catalog_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
import sys

import pytest
from ansible_collections.servicenow.itsm.plugins.module_utils import (
service_catalog
)
from ansible_collections.servicenow.itsm.plugins.module_utils import service_catalog
from ansible_collections.servicenow.itsm.plugins.modules import (
service_catalog_info,
)
Expand All @@ -33,11 +31,15 @@ def list_records(self, api, query=None):
self.call_params.append(query)
if "categories" in api:
if not isinstance(self.retured_data, tuple):
raise ValueError("expected tuple in retured_data when looking for categories")
raise ValueError(
"expected tuple in retured_data when looking for categories"
)
return self.retured_data[1]
if "items" in api:
if not isinstance(self.retured_data, tuple):
raise ValueError("expected tuple in retured_data when looking for categories")
raise ValueError(
"expected tuple in retured_data when looking for categories"
)
return self.retured_data[2]
if isinstance(self.retured_data, tuple):
return self.retured_data[0]
Expand All @@ -59,7 +61,7 @@ def test_get_without_categories(self, create_module):
),
categories=False,
items_info="none",
items_query=None
items_query=None,
)
)

Expand All @@ -85,7 +87,7 @@ def test_get_by_sys_id(self, create_module):
sys_id="catalog_sys_id",
categories=False,
items_info="none",
items_query=None
items_query=None,
)
)

Expand All @@ -110,7 +112,7 @@ def test_get_with_categories(self, create_module):
),
categories=True,
items_info="none",
items_query=None
items_query=None,
)
)

Expand All @@ -123,11 +125,16 @@ def test_get_with_categories(self, create_module):
)
category = dict(sys_id="category_sys_id")

records = service_catalog_info.run(module, self.get_sc_client(([catalog], [category])))
records = service_catalog_info.run(
module, self.get_sc_client(([catalog], [category]))
)

assert len(records) == 1
assert len(records[0]["categories"]) == 1
assert records[0]["categories"][0] == service_catalog.Category(category).to_ansible()
assert (
records[0]["categories"][0]
== service_catalog.Category(category).to_ansible()
)
assert records[0]["sys_id"] == "1"

def test_get_with_categories_and_items(self, create_module):
Expand All @@ -138,7 +145,7 @@ def test_get_with_categories_and_items(self, create_module):
),
categories=True,
items_info="brief",
items_query=None
items_query=None,
)
)

Expand All @@ -153,11 +160,15 @@ def test_get_with_categories_and_items(self, create_module):
item = dict(sys_id="item_sys_id")

records = service_catalog_info.run(
module, self.get_sc_client(([catalog], [category], [item])))
module, self.get_sc_client(([catalog], [category], [item]))
)

assert len(records) == 1
assert len(records[0]["categories"]) == 1
assert len(records[0]["sn_items"]) == 1
assert records[0]["categories"][0] == service_catalog.Category(category).to_ansible()
assert (
records[0]["categories"][0]
== service_catalog.Category(category).to_ansible()
)
assert records[0]["sn_items"][0] == service_catalog.Category(item).to_ansible()
assert records[0]["sys_id"] == "1"

0 comments on commit 94bcf61

Please sign in to comment.