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

group_by number strip the first number and query problem #119

Closed
metabsd opened this issue Oct 13, 2021 · 8 comments
Closed

group_by number strip the first number and query problem #119

metabsd opened this issue Oct 13, 2021 · 8 comments
Labels
needs_info needs_research Further research is needed to determine the feasibility and scope of the issue. priority/low type/bug Something isn't working verified

Comments

@metabsd
Copy link

metabsd commented Oct 13, 2021

SUMMARY

When using the group_by with a Servicenow field that contains a number. The first digit is replaced by _

ISSUE TYPE
  • Bug Report
COMPONENT NAME

servicenow.itsm.now

ANSIBLE VERSION
ansible [core 2.11.6] 
  config file = None
  configured module search path = ['/home/marbe020/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/marbe020/.pyenv/versions/3.8.3/lib/python3.8/site-packages/ansible
  ansible collection location = /home/marbe020/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/marbe020/.pyenv/versions/3.8.3/bin/ansible
  python version = 3.8.3 (default, May  6 2021, 17:06:59) [GCC 9.3.0]
  jinja version = 2.11.3
  libyaml = True
COLLECTION VERSION
Collection      Version
--------------- -------
servicenow.itsm 1.2.0  
CONFIGURATION
Nothing
OS / ENVIRONMENT

Ubuntu 20.04 running the inventory script on ServiceNOW Quebec version

STEPS TO REPRODUCE
plugin: servicenow.itsm.now
columns:
  - name
  - install_status
  - category
  - location.u_number
table: cmdb_ci_ip_router
query:
  - category: = Infrastructure succ
  - install_status: IN 1,101
group_by:
  location.u_number:
EXPECTED RESULTS
  1. Query work as expected so no retired device
  2. group_by work but 1 digit is stripped / replace by _
ACTUAL RESULTS
  1. I found in the result all category and all install_status instead of the one I want.

image

Normally I expecte to see 5 digit. Like the first one in ServiceNOW is 21004 ans not _1004

Thx in advance!!!

@tadeboro
Copy link
Contributor

The problem you are describing here looks like a group name sanitization issue. Non-deprecated parts of the ServiceNow inventory plugin respects the TRANSFORM_INVALID_GROUP_CHARS configuration option. But older parts (things like group_by and named_groups) do not and this is what you are hitting here.

Now, I have some bad news. The replacement functionality does not know how to handle data with dots in the name. The old functionality does know how to work with such data by sheer luck. We never anticipated that plugins and modules will need to handle "nested" data. This will need some work to implement properly and make it accessible to Ansible.

As an ugly ugly workaround, you can locally edit your copy of the inventory plugin and replace the

group_name = to_safe_group_name(record[category])
line with

group_name = orig_safe(record[category])

We cannot do this change in the plugin itself because it would break backward compatibility, but for local installations, this should be safe.

To properly resolve the issue, we will need to fix #100. Once a fix for that issue is in place, you will be able to migrate to non-deprecated functionality.

@metabsd
Copy link
Author

metabsd commented Oct 15, 2021

Hi @tadeboro First, Big Thx for you time.

Do you think it would be possible to respect the variable in ansible.cfg?

[defaults]
force_valid_group_names = always

or

[defaults]
force_valid_group_names = never

As for nested variable, I think it still works except when I use keyed_groups.

I'm going to experiment using a custom version of now.py in my project with the recommendations you gave me.

A huge thank you for that.

Regarding the fact that the query: section doesn't seem to be respected, do you have any advice for me.

Thanks in advance!

@metabsd
Copy link
Author

metabsd commented Oct 15, 2021

After following your advice to use a fork version of your code to apply a workaround. I also took the opportunity to add some print to finally find how to display the query sent to ServiceNow when building the inventory via now.py.

url = "{0}?{1}".format(url, urlencode(query))

I add that.

        if query:
            url = "{0}?{1}".format(url, urlencode(query))
            print(url)
        headers = dict(headers or DEFAULT_HEADERS, **self.auth_header)

What I see on my terminal is that the sysparm_query=category%3DInfrastructure%20succ%5Einstall_statusIN1%2C101 is not used.

https://mydev.service-now.com/api/now/table/cmdb_ci_ip_router?sysparm_fields=ip_address%2Cinstall_status%2Ccategory%2Clocation.u_number%2Csys_id%2Clocation%2Cname&sysparm_display_value=True&sysparm_exclude_reference_link=true&sysparm_limit=1000&sysparm_offset=0

Can you confirm that you have the same result as me please.

@metabsd
Copy link
Author

metabsd commented Dec 1, 2021

@tadeboro Can you please help me with this problem.

@Akasurde
Copy link
Member

@tadeboro ^^

@tima tima added type/bug Something isn't working verified needs_research Further research is needed to determine the feasibility and scope of the issue. labels Apr 7, 2022
@tima tima added this to To do in servicenow.itsm v2.0 via automation Apr 22, 2022
@tima tima removed this from To do in servicenow.itsm v2.0 Aug 24, 2022
@tima tima added this to To do in servicenow.itsm v2.0.1 via automation Aug 24, 2022
@Akasurde Akasurde removed this from To do in servicenow.itsm v2.0.1 Feb 21, 2024
@Akasurde
Copy link
Member

@metabsd Is this issue still present? Thanks in advance.

@tupyy
Copy link
Contributor

tupyy commented Mar 19, 2024

@metabsd I can't reproduce it in Tokyo.

@Akasurde
Copy link
Member

Closing due to inactivity. Please feel free to re-open the issue. Thanks.

@Akasurde Akasurde closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_info needs_research Further research is needed to determine the feasibility and scope of the issue. priority/low type/bug Something isn't working verified
Projects
None yet
Development

No branches or pull requests

5 participants