#49841 Ticket 49840 - ds-replcheck command returns traceback errors against ldif files having garbage content when run in offline mode
Closed a year ago by spichugi. Opened 3 years ago by mreynolds.
mreynolds/389-ds-base ticket49840  into  master

@@ -10,18 +10,19 @@ 



  import os

+ import sys

  import re

  import time

  import ldap

  import ldapurl

  import argparse

  import getpass


+ from ldif import LDIFRecordList

  from ldap.ldapobject import SimpleLDAPObject

  from ldap.cidict import cidict

  from ldap.controls import SimplePagedResultsControl


- VERSION = "1.3"

+ VERSION = "1.4"

  RUV_FILTER = '(&(nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff)(objectclass=nstombstone))'

  LDAP = 'ldap'

  LDAPS = 'ldaps'
@@ -394,14 +395,17 @@ 

      return result



- def get_dns(LDIF, opts):

+ def get_dns(LDIF, filename, opts):

      ''' Get all the DN's from an LDIF file


      dns = []

      found = False

+     found_ruv = False

+     LDIF.seek(0)

      for line in LDIF:

          if line.startswith('dn: ') and line[4:].startswith('nsuniqueid=ffffffff-ffffffff-ffffffff-ffffffff'):

              opts['ruv_dn'] = line[4:].lower().strip()

+             found_ruv = True

          elif line.startswith('dn: '):

              found = True

              dn = line[4:].lower().strip()
@@ -415,6 +419,14 @@ 

              found = False



+     if not found_ruv:

+         print('Failed to find the database RUV in the LDIF file: ' + filename + ', the LDIF ' +

+               'file must contain replication state information.')

+         dns = None

+     else:

+         # All good, reset cursor

+         LDIF.seek(0)


      return dns


@@ -423,6 +435,7 @@ 



      result = ldif_search(LDIF, opts['ruv_dn'])

+     LDIF.seek(0)  # Reset cursor

      return result['entry'].data['nsds50ruv']


@@ -557,6 +570,7 @@ 

      rconflicts = []

      rtombstones = 0

      mtombstones = 0

+     idx = 0


      # Open LDIF files

@@ -569,12 +583,36 @@ 

          RLDIF = open(opts['rldif'], "r")

      except Exception as e:

          print('Failed to open Replica LDIF: ' + str(e))

+         MLDIF.close()

+         return None


+     # Verify LDIF Files

+     try:

+         print("Validating Master ldif file ({})...".format(opts['mldif']))

+         LDIFRecordList(MLDIF).parse()

+     except ValueError:

+         print('Master LDIF file in invalid, aborting...')

+         MLDIF.close()

+         RLDIF.close()

+         return None

+     try:

+         print("Validating Replica ldif file ({})...".format(opts['rldif']))

+         LDIFRecordList(RLDIF).parse()

+     except ValueError:

+         print('Replica LDIF file is invalid, aborting...')

+         MLDIF.close()

+         RLDIF.close()

          return None


      # Get all the dn's, and entry counts

      print ("Gathering all the DN's...")

-     master_dns = get_dns(MLDIF, opts)

-     replica_dns = get_dns(RLDIF, opts)

+     master_dns = get_dns(MLDIF, opts['mldif'], opts)

+     replica_dns = get_dns(RLDIF, opts['rldif'], opts)

+     if master_dns is None or replica_dns is None:

+         print("Aborting scan...")

+         MLDIF.close()

+         RLDIF.close()

+         sys.exit(1)

      m_count = len(master_dns)

      r_count = len(replica_dns)

@@ -583,11 +621,6 @@ 

      opts['master_ruv'] = get_ldif_ruv(MLDIF, opts)

      opts['replica_ruv'] = get_ldif_ruv(RLDIF, opts)


-     # Reset the cursors

-     idx = 0

-     MLDIF.seek(idx)

-     RLDIF.seek(idx)


      """ Compare the master entries with the replica's.  Take our list of dn's from

      the master ldif and get that entry( dn) from the master and replica ldif.  In

      this phase we keep keep track of conflict/tombstone counts, and we check for

Description: Added a basic check to see if the LDIF files are actually
LDIF files. Also added checks that the database RUV are
present as well.


Reviewed by: ?

I think it will be better if verify_ldif_file will check the whole file. It doesn't cost us much but saves us from many issues in the future.

We can improve it with something like this (ldif is part of python-ldap):

with open("/tmp/export_master2.ldif", 'rb') as f:

It will throw an error if the ldif file is bad:

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/usr/lib64/python3.6/site-packages/ldif.py", line 461, in parse
    return self.parse_entry_records() # parse()
  File "/usr/lib64/python3.6/site-packages/ldif.py", line 425, in parse_entry_records
    raise ValueError('Line %d: First line of record does not start with "dn:": %s' %     (self.line_counter,repr(k)))
ValueError: Line 28: First line of record does not start with "dn:": 'modifyTimestamp'

If you'll catch it like this, we can have a clean logged error:

with open("/tmp/export_master2.ldif", 'rb') as f:
    except ValueError as e:

It will print Line 28: First line of record does not start with "dn:": 'modifyTimestamp'

rebased onto a8dd789b1d18105536aca622edc250ddf2273491

3 years ago

Couple of small issues here...
First, I think you need to reset the cursor somewhere here, because 'get_dns(MLDIF, opts)' fails overwise. LDIFRecordList will read the data from MLDIF and RLDIF and the cursor will be in the end of the file.

Second, if the LDIFRecordList().parse() fails then we don't close() the MLDIF and RLDIF files...

Overwise, looks good to me!

rebased onto 8ed778bee9a76d812eea89a25904dc52154c017d

3 years ago

rebased onto 5d2c2a198e4a54fd8c092bde28e1829e97f36296

3 years ago

rebased onto 60cb520

3 years ago

Pull-Request has been merged by mreynolds

3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/2900

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

a year ago