Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Facts with int values which are strings internally are not findable #706

Closed
ryaner opened this issue Jul 22, 2022 · 5 comments
Closed

Facts with int values which are strings internally are not findable #706

ryaner opened this issue Jul 22, 2022 · 5 comments
Labels

Comments

@ryaner
Copy link
Contributor

ryaner commented Jul 22, 2022

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>';

@ryaner ryaner added the bug label Jul 22, 2022
@gdubicki
Copy link
Contributor

gdubicki commented Jul 22, 2022

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.

@ryaner
Copy link
Contributor Author

ryaner commented Jul 23, 2022

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

@gdubicki
Copy link
Contributor

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

ryaner added a commit to ryaner/puppetboard that referenced this issue Jul 25, 2022
gdubicki added a commit that referenced this issue Jul 26, 2022
Force quote strings for the facts page - #706
@ryaner ryaner closed this as completed Jul 26, 2022
@gdubicki
Copy link
Contributor

The fix has been released in v4.0.3 today.

@ryaner
Copy link
Contributor Author

ryaner commented Jul 29, 2022

Nice thanks for that @gdubicki

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants