atlas-bi / LDAP-ETL

⚙️ Atlas ETL | LDAP User Profiles
https://www.atlas.bi/docs/bi-library/etl/supplementary-etls/ldap-user-import/
GNU General Public License v3.0
2 stars 1 forks source link

pyodbc.ProgrammingError: The second parameter to executemany must not be empty. #29

Closed ImNtReal closed 1 year ago

ImNtReal commented 1 year ago

System Info

Issue

I'm attempting to run the LDAP ETL via poetry run ldap.py. It's returning:

Traceback (most recent call last:
  File "C:\ETLs\LDAP-ETL-1.1.0\ldap.py", line 180, in <module>
    cursor.executemany(
pyodbc.ProgrammingError: The second parameter to executemany must not be empty.

How To Reproduce

Download the v1.1.0 of the ETL Create a .env file (passwords redacted):

SERVERURI=adldap.nghs.com
ADUSERNAME='NGHS\ldap_user'
ADPASSWORD=*
DATABASE='DRIVER={ODBC Driver 17 for SQL Server};SERVER=vmdbatlasbi1.nghs.com;DATABASE=LDAP;UID=ldap_etl;PWD=*'
ADDOMAIN=NGHS
DC=NGHS
SEARCHBASES='OU=ITS,OU=People,DC=NGHS,DC=COM'
GROUPSEARCHBASES='OU=Groups,DC=NGHS,DC=COM'

Run poetry install Run poetry run ldap.py

Based on the error, I'm thinking that the LDAP queries are not returning information, so the ODBC call is missing parameters. trace.log

christopherpickering commented 1 year ago

Yep, it seems like you are not getting EmployeeID's from your ldap. You should probably validate that they are actually no employeeID's.

If thats the case, does adding a default value on line 117 work?

get_attribute("employeeID", data) or "",

ImNtReal commented 1 year ago

I do have data in my employeeID field, but I'm not sure all users do. I still got the error after changing 117. I also tried:

get_attribute("employeeID", data) or "a",
ImNtReal commented 1 year ago

Is this stuff always added to search bases, even if I specify a full DN for them?

search_base="OU=" + base + ",dc=" + DC + ",dc=net",
ImNtReal commented 1 year ago

Yeah. After adjusting my variables so that SEARCHBASES and GROUPSEARCHBASES to only be the name of an OU, and the dc=net to dc=com, I'm able to get further. There's a different error, but it seems like expecting LDAP directory structures to always end in dc=net, and that one OU wouldn't be nested under others seems like a bad assumption.

ImNtReal commented 1 year ago

Right now, I'm having trouble getting memberOf parsed. I wonder if it's because some of our groups have spaces in the name:

Traceback (most recent call last):
  File "C:\ETLs\LDAP-ETL-1.1.0\ldap.py", line 134, in <module>
    memberdict = dict([x.split("=") for x in member_set.split(",")])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: dictionary update sequence element #1 has length 1; 2 is required
christopherpickering commented 1 year ago

Thanks! Yeah, I built it based on my AD config (not necessarily the best one..) and AD possibilities are limitless.

If you can find a way to make it more flexible, please send over another pr.

For your last error, it is expecting something along the lines of "one=two,three=four" and converting it to [["one","two",[ "three","four"]]. Spaces in the name shouldn't matter, but I wonder if your names are setup differently? try adding a print(member_set) on the line above to see what is coming in.

ImNtReal commented 1 year ago

It looks like this is the one it's choking on: CN=ITS Primary\, Specialty\, and Women & Children,OU=DistributionGroups,OU=Groups,DC=nghs,DC=com

I had thought about printing the whole list of groups before the loop, but didn't even think to debug inside the loop.

christopherpickering commented 1 year ago

Ah, it is dying on the comma in the first name... we'll have to find a better way to parse those. Probably by using the AD tags CN, OU and DC.

ImNtReal commented 1 year ago

print(member_set.split("=")) show's it's escaped, but from what I've found there isn't an easy way to ignore escaped characters when doing a string split in python.

['CN', 'ITS Primary\\, Specialty\\, and Women & Children,OU', 'DistributionGroups,OU', 'Groups,DC', 'nghs,DC', 'com']
christopherpickering commented 1 year ago

@ImNtReal I just pushed a fix for this onto the dev branch, do you mind to check it out and see if it fixes your problem? It should also help you out where you have multiple OU's in the list.

christopherpickering commented 1 year ago

Sorry your last comment just popped up here! I've changed to use regex to find the strings to get around the splitting issue.

ImNtReal commented 1 year ago

@christopherpickering You could also consider using something like ldap3.utils.dn.parse_dn(ldap3.utils.dn.safe_dn(member_set), but I'm pretty sure the dict will be a different format.

christopherpickering commented 1 year ago

I'll check that out!

ImNtReal commented 1 year ago

On the dev branch, I'm getting

Traceback (most recent call last):
    File "C:\Users\jmpugh_dladm\git\LDAP-ETL\ldap.py", line 147, in <module>
        get_attribute("OU", ou),
        ^^^^^^^^^^^^^^^^^^^^^^^
    File "C:\Users\jmpugh_dladm\git\LDAP-ETL\ldap.py", line 86, in get_attribute
        if my_data.get(current_attrib):
        ^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'get'                                                                     
christopherpickering commented 1 year ago

shucks my bad :) Its not a dict any more. I'll change that.

christopherpickering commented 1 year ago

better luck now?

ImNtReal commented 1 year ago

That appears to have got me through a run!

If I would like to contribute changes to help make it less org dependent, should I make those against the dev branch?

christopherpickering commented 1 year ago

cool! Yes, sending a pr against dev is great. The branches are normally in sync so if you do send against master I can swap it.

christopherpickering commented 1 year ago

:tada: This issue has been resolved in version 1.1.1-alpha.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

ImNtReal commented 1 year ago

All of the tables of my LDAP database seem to be populated.

christopherpickering commented 1 year ago

:tada: This issue has been resolved in version 1.1.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: