voxpupuli/puppetboard

Facts with int values which are strings internally are not findable

ryaner opened this issue · 5 comments

Describe the bug

Currently puppetboard is able to detect the facts value type correctly from the puppetdb calls, but the generated links do not differentiate between string and int, resulting in the call to parse_python switching it back to an int.

An example will hopefully make this clearer

Using Centos operating system facts, which are stored into puppetdb as strings.

https://puppetdb/fact/operatingsystemmajrelease
This page will list all the hosts with this fact present. The generated html is showing type string here too

<td><a title="6" href="/fact/operatingsystemmajrelease/6"><span class="string">6</span></a></td>

Following the link - https://puppetdb/fact/operatingsystemmajrelease/6 - will display no entries.

The facts code is calling parse_python which is returning the int type to the query.

value = parse_python(value)
# ...to know if it should be quoted or not in the query to PuppetDB
# (f.e. a string should, while a number should not)
query.add(EqualsOperator('value', value))

If I manually quote the value - https://puppetdb/fact/operatingsystemmajrelease/"6" - the search works and displays results.

Values which are int internally, and displayed as int on the facts page do search correctly.

Puppetboard version

F.e. 3.6.1 (verified code has not changed for this flow since)

Environment and installation method

F.e.:

  • Centos 7 and Python 3.8,
  • PyPI package.
  • puppetdb-6.21.0-1

While quoting all strings does work as a solution here, that just feels wrong which is why I haven't attached a PR here. If you have a better solution that could work, I'm happy to PR things.
Pretty print explicitly tries to not quote things too

// Print plain string as-is to avoid making it surrounded with ""
to_show = '<span class="string">' + to_show + '</span>';

Hi @ryaner! Thanks for reporting this.

I don’t know when I will have the time to dig into this as I will be pretty busy over the next ~2 weeks so even more than always, PR with a fix is very welcome.

My current patch is below, but I keep thinking there must be a better solution that this. So far it does work on all the int stored as string values though.

diff --git a/puppetboard/views/facts.py b/puppetboard/views/facts.py
index 714eba9..2715cd0 100644
--- a/puppetboard/views/facts.py
+++ b/puppetboard/views/facts.py
@@ -93,7 +93,7 @@ def fact_ajax(env, node, fact, value):
                 fact_h.node))
         if value is None:
             if isinstance(fact_h.value, str):
-                value_for_url = quote_plus(fact_h.value)
+                value_for_url = '"' + quote_plus(fact_h.value) + '"'
             else:
                 value_for_url = fact_h.value

If it works and there are no side effects, then let’s create a PR and merge it. Let’s be pragmatic, @ryaner. :)

The fix has been released in v4.0.3 today.

Nice thanks for that @gdubicki