#51225 Issue 51188 - db2ldif crashes when LDIF file can't be accessed
Closed a year ago by spichugi. Opened a year ago by sgouvern.
sgouvern/389-ds-base export  into  master

@@ -0,0 +1,104 @@ 

+ # --- BEGIN COPYRIGHT BLOCK ---

+ # Copyright (C) 2020 Red Hat, Inc.

+ # All rights reserved.

+ #

+ # License: GPL (version 3 or any later version).

+ # See LICENSE for details.

+ # --- END COPYRIGHT BLOCK ---

+ 

+ 

+ import os

+ import pytest

+ 

+ from lib389.topologies import topology_st as topo

+ from lib389._constants import DEFAULT_SUFFIX, DEFAULT_BENAME

+ from lib389.dbgen import dbgen_users

+ from lib389.tasks import ImportTask

+ from lib389.utils import *

+ from lib389.idm.user import UserAccount, UserAccounts

+ from lib389.idm.account import Accounts, Account

+ from lib389.paths import Paths

+ 

+ pytestmark = pytest.mark.tier1

+ 

+ 

+ def _generate_ldif(topo, no_no):

+     """

+     Generate a test ldif file to be imported

+     """

+     ldif_dir = topo.standalone.get_ldif_dir()

+     import_ldif = ldif_dir + '/basic_import.ldif'

+     log.info("Generating ldif ...")

+     dbgen_users(topo.standalone, no_no, import_ldif, DEFAULT_SUFFIX)

+ 

+ 

+ def _import(topo, no_no):

+     """

+     Import ldif online

+     """

+     ldif_dir = topo.standalone.get_ldif_dir()

+     import_ldif = ldif_dir + '/basic_import.ldif'

+     _generate_ldif(topo, no_no)

+ 

+     log.info("Performing an online import ...")

+     import_task = ImportTask(topo.standalone)

+     import_task.import_suffix_from_ldif(ldiffile=import_ldif, suffix=DEFAULT_SUFFIX)

+ 

+     import_task.wait()

+ 

+ 

+ @pytest.fixture(scope="function")

+ def _test_setup(request, topo):

+     _import(topo, 100)

+ 

+     def fin():

+         accounts = Accounts(topo.standalone, DEFAULT_SUFFIX)

+         log.info('Cleaning the db')

+         for i in accounts.filter('(uid=*)'):

+             UserAccount(topo.standalone, i.dn).delete()

+ 

+         ldif_dir = topo.standalone.get_ldif_dir()

+         import_ldif = ldif_dir + '/import.ldif'

+ 

+         log.info('Removing the generated test ldif file')

+         if os.path.exists(import_ldif):

+             os.remove(import_ldif)

+ 

+     request.addfinalizer(fin)

+ 

+ @pytest.mark.bz1806978

+ @pytest.mark.bz1860291

+ @pytest.mark.ds51188

+ @pytest.mark.skipif(ds_is_older("1.4.3"), reason="Not implemented")

+ def test_db2ldif_with_non_accessible_ldif_file_path(topo, _test_setup):

+     """Export with db2ldif, giving a ldif file path which can't be accessed by the user (dirsrv by default)

+     :id: ca91eda7-27b1-4750-a013-531a63d3f5b0

+     :setup: Standalone Instance - entries imported in the db

+     :steps:

+         1. Stop the server

+         2. Launch db2ldif with an non accessible ldif file path 

+         3. check the error reported in the errors log

+     :expected results:

+         1. Operation successful

+         2. Operation properly fails, without crashing, and reporting an error

+         3. 'ERR - bdb_db2ldif - db2ldif: userRoot: can't open file' should be reported

+     """

+     export_ldif = '/tmp/nonexistent/export.ldif'

+ 

+     log.info("Stopping the instance...")

+     topo.standalone.stop()

+ 

+     log.info("Performing an offline export to a non accessible ldif file path - should fail properly")

+     assert not topo.standalone.db2ldif(DEFAULT_BENAME, (DEFAULT_SUFFIX,),None, None, None, export_ldif)

+       

+     # An error should be reported as output of the cli - see bz1860291

+     # This test is to be updated after bz1860291 evaluation

+ 

+     log.info("parsing the errors log to search for the error reported")

+     search_str = str(topo.standalone.ds_error_log.match(r".*ERR - bdb_db2ldif - db2ldif: userRoot: can't open*"))[1:-1]

+     assert len(search_str) > 0

+     log.info("error string : %s" % search_str)

+ 

+     log.info("Restarting the instance...")

+     topo.standalone.start()

+ 

Description:
Automated test to verify that db2ldif exits properly when the ldif file path provided cannot be accessed

Relates: https://pagure.io/389-ds-base/issue/51188
Relates: https://bugzilla.redhat.com/show_bug.cgi?id=1806978

Reviewed by: ???

All of this boiler plate is not needed, it's part of topologies now.

If the point is to test an offline import with a non-accessible ldif, whdy do you need to generate an ldif at all in _test_setup?

It may be better to choose a more "guaranteed" to not exist name, because this seems like a common pattern to put an export.ldif in /root. Perhaps something like /tmp/<uuid>/export.ldif?

Hope that helps, I think you can actually shrink and minimise this test case while retaining the goals behind it.

The newline after """ usually introduced trouble for betelgeuse to correctly generate xml for importing tests to Polarion.

Please modify the docstring to this format:
"""Export with db2ldif ...

:id: ca91eda7-27b1-4750-a013-531a63d3f5b0

(there is no space between """ and Export just for clarification)

Thanks!

rebased onto fa88636

a year ago

All of this boiler plate is not needed, it's part of topologies now.
Yes, right, removed.

If the point is to test an offline import with a non-accessible ldif, whdy do you need to generate an ldif at all in _test_setup?
The point is to test an offline export, my mistake the log.info comment was wrong. So the ldif generation and loading at _test_setup is to have fresh entries to be exported.

It may be better to choose a more "guaranteed" to not exist name, because this seems like a common pattern to put an export.ldif in /root. Perhaps something like /tmp/<uuid>/export.ldif?
I chose /root initially because it is not accessible by dirsrv, owner of the export task by default. But you're right, a more doubtless non existent path is better. I changed this.

I also removed the newline after """.

Could you please review, thanks !

Thanks for applying the feedback @sgouvern I'll be reviewing this again today :)

If the point is to test an offline import with a non-accessible ldif, whdy do you need to generate an ldif at all in _test_setup?
The point is to test an offline export, my mistake the log.info comment was wrong. So the ldif generation and loading at _test_setup is to have fresh entries to be exported.

In topologies there is already a backend (userRoot) that is populated with sample entries, so the number of entries won't affect the test of the export function - so I think you don't need the ldif generation at all, you can just immediately attempt the export to the non-existant location, which should reproduce the issue too shouldn't it?

Thanks again! I think after that comment I'm happy to accept this :)

@sgouvern
This is better, but please add the newline before id.
Like here:
https://pagure.io/389-ds-base/blob/master/f/dirsrvtests/tests/suites/clu/repl_monitor_test.py

Sorry about the confusion :)

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/4278

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
Metadata