[PATCH] Modify SamDB default to NO_CREATE_DB prep for LMDB backend

Gary Lockyer gary at catalyst.net.nz
Wed Feb 7 21:09:36 UTC 2018


Patch modifying the default file creation behaviour of SamDB. This patch
changes the default from create if missing to do not create. This stops
TDB from overwriting any existing non TDB format file with a new TDB.

This forms part of the work Gaming and I are doing implementing an lmdb
backend for ldb based on the work done by Jakub Hrozek.

The initial plan is to add lmdb support as an experimental feature, with
the following limitations:
 - keys limited to 511 bytes
 - only the partition files will be in lmdb format.

Review and possible push appreciated.

Gary


-------------- next part --------------
From e67beccc524d4b7050524397051bd9110747b5fa Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed, 7 Feb 2018 14:59:21 +1300
Subject: [PATCH 1/3] samdb: Add tests for samdb tdb file creation.

The current defaults for SamDB are to create the database file if it
does not exist.  Most of the uses of SamDB assume the database already
exists, and so auto-creation is not the desired behaviour.

Also TDB will overwrite an existing non TDB file with a newly created
TDB file.  This becomes an issue when using alternate database file
formats i.e. lmdb.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 python/samba/tests/samdb_api.py | 158 ++++++++++++++++++++++++++++++++++++++++
 selftest/knownfail.d/samdb      |   2 +
 selftest/tests.py               |   1 +
 3 files changed, 161 insertions(+)
 create mode 100644 python/samba/tests/samdb_api.py
 create mode 100644 selftest/knownfail.d/samdb

diff --git a/python/samba/tests/samdb_api.py b/python/samba/tests/samdb_api.py
new file mode 100644
index 0000000..81c6266
--- /dev/null
+++ b/python/samba/tests/samdb_api.py
@@ -0,0 +1,158 @@
+# Tests for the samba samdb api
+#
+# Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+from samba.tests import TestCaseInTempDir
+from samba.samdb import SamDB
+from ldb import LdbError, ERR_OPERATIONS_ERROR
+import os
+import errno
+
+
+class SamDBApiTestCase(TestCaseInTempDir):
+
+    def setUp(self):
+        super(SamDBApiTestCase, self).setUp()
+
+    def tearDown(self):
+        try:
+            os.remove(self.tempdir + "/test.db")
+        except OSError as e:
+            self.assertEquals(e.errno, errno.ENOENT)
+
+        try:
+            os.remove(self.tempdir + "/existing.db")
+        except OSError as e:
+            self.assertEquals(e.errno, errno.ENOENT)
+
+        super(SamDBApiTestCase, self).tearDown()
+
+    # Attempt to open and existing non tdb file as a tdb file.
+    # Don't create new db is set, the default
+    #
+    # Should fail to open
+    # And the existing file should be left intact.
+    #
+    def test_dont_create_db_existing_non_tdb_file(self):
+        existing_name = self.tempdir + "/existing.db"
+        existing = open(existing_name, "w")
+        existing.write("This is not a tdb file!!!!!!\n")
+        existing.close()
+
+        try:
+            SamDB(url="tdb://" + existing_name)
+            self.fail("Exception not thrown ")
+        except LdbError as (err, _):
+            self.assertEquals(err, ERR_OPERATIONS_ERROR)
+
+        existing = open(existing_name, "r")
+        contents = existing.readline()
+        self.assertEquals("This is not a tdb file!!!!!!\n", contents)
+
+    # Attempt to open and existing non tdb file as a tdb file.
+    # Don't create new db is cleared
+    #
+    # Should open as a tdb file
+    # And the existing file should be over written
+    #
+    def test_create_db_existing_file_non_tdb_file(self):
+        existing_name = self.tempdir + "/existing.db"
+        existing = open(existing_name, "w")
+        existing.write("This is not a tdb file!!!!!!")
+        existing.close()
+
+        SamDB(url="tdb://" + existing_name, flags=0)
+
+        existing = open(existing_name, "r")
+        contents = existing.readline()
+        self.assertEquals("TDB file\n", contents)
+
+    #
+    # Attempt to open an existing tdb file as a tdb file.
+    # Don't create new db is set, the default
+    #
+    # Should open successfully
+    # And the existing file should be left intact.
+    #
+    def test_dont_create_db_existing_tdb_file(self):
+        existing_name = self.tempdir + "/existing.db"
+        initial = SamDB(url="tdb://" + existing_name, flags=0)
+        dn = "dn=,cn=test_dont_create_db_existing_tdb_file"
+        initial.add({
+            "dn": dn,
+            "cn": "test_dont_create_db_existing_tdb_file"
+        })
+
+        cn = initial.searchone("cn", dn)
+        self.assertEquals("test_dont_create_db_existing_tdb_file", cn)
+
+        second = SamDB(url="tdb://" + existing_name)
+        cn = second.searchone("cn", dn)
+        self.assertEquals("test_dont_create_db_existing_tdb_file", cn)
+
+    #
+    # Attempt to open an existing tdb file as a tdb file.
+    # Don't create new db is explicitly cleared
+    #
+    # Should open successfully
+    # And the existing file should be left intact.
+    #
+    def test_create_db_existing_file_tdb_file(self):
+        existing_name = self.tempdir + "/existing.db"
+        initial = SamDB(url="tdb://" + existing_name, flags=0)
+        dn = "dn=,cn=test_dont_create_db_existing_tdb_file"
+        initial.add({
+            "dn": dn,
+            "cn": "test_dont_create_db_existing_tdb_file"
+        })
+
+        cn = initial.searchone("cn", dn)
+        self.assertEquals("test_dont_create_db_existing_tdb_file", cn)
+
+        second = SamDB(url="tdb://" + existing_name, flags=0)
+        cn = second.searchone("cn", dn)
+        self.assertEquals("test_dont_create_db_existing_tdb_file", cn)
+
+    # Open a non existent TDB file.
+    # Don't create new db is set, the default
+    #
+    # Should fail
+    # and the database file should not be created
+    def test_dont_create_db_new_file(self):
+        try:
+            SamDB(url="tdb://" + self.tempdir + "/test.db")
+            self.fail("Exception not thrown ")
+        except LdbError as (err, _):
+            self.assertEquals(err, ERR_OPERATIONS_ERROR)
+
+        try:
+            file = open(self.tempdir + "/test.db", "r")
+            self.fail("New database file created")
+        except IOError as e:
+            self.assertEquals(e.errno, errno.ENOENT)
+
+
+    # Open a SamDB with the don't create new DB flag cleared.
+    # The underlying database file does not exist.
+    #
+    # Should successful open the SamDB creating a new database file.
+    #
+    def test_create_db_new_file(self):
+        SamDB(url="tdb://" + self.tempdir + "/test.db", flags=0)
+        existing = open(self.tempdir + "/test.db", "r")
+        contents = existing.readline()
+        self.assertEquals("TDB file\n", contents)
diff --git a/selftest/knownfail.d/samdb b/selftest/knownfail.d/samdb
new file mode 100644
index 0000000..6a4ab97
--- /dev/null
+++ b/selftest/knownfail.d/samdb
@@ -0,0 +1,2 @@
+^samba.tests.samdb_api.samba.tests.samdb_api.SamDBApiTestCase.test_dont_create_db_existing_non_tdb_file\(none\)
+^samba.tests.samdb_api.samba.tests.samdb_api.SamDBApiTestCase.test_dont_create_db_new_file\(none\)
diff --git a/selftest/tests.py b/selftest/tests.py
index b2d1a67..ec48b03 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -154,6 +154,7 @@ planpythontestsuite("none", "samba.tests.graph")
 plantestsuite("wafsamba.duplicate_symbols", "none", [os.path.join(srcdir(), "buildtools/wafsamba/test_duplicate_symbol.sh")])
 planpythontestsuite("none", "samba.tests.glue", py3_compatible=True)
 planpythontestsuite("none", "samba.tests.tdb_util", py3_compatible=True)
+planpythontestsuite("none", "samba.tests.samdb_api")
 
 if with_pam:
     plantestsuite("samba.tests.pam_winbind(local)", "ad_member",
-- 
2.7.4


From 8563c6c345adf519da0e4af82bbb8ec495230524 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 9 Jan 2018 07:41:32 +1300
Subject: [PATCH 2/3] pyldb: Expose extra flags

Expose the SHOW_BINARY, ENABLE_TRACING and DONT_CREATE_DB flag constants
in the python api.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 lib/ldb/pyldb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c
index 04b3f1b..4b02edb 100644
--- a/lib/ldb/pyldb.c
+++ b/lib/ldb/pyldb.c
@@ -4226,6 +4226,10 @@ static PyObject* module_init(void)
 	ADD_LDB_INT(FLG_NOSYNC);
 	ADD_LDB_INT(FLG_RECONNECT);
 	ADD_LDB_INT(FLG_NOMMAP);
+	ADD_LDB_INT(FLG_SHOW_BINARY);
+	ADD_LDB_INT(FLG_ENABLE_TRACING);
+	ADD_LDB_INT(FLG_DONT_CREATE_DB);
+
 
 	/* Historical misspelling */
 	PyModule_AddIntConstant(m, "ERR_ALIAS_DEREFERINCING_PROBLEM", LDB_ERR_ALIAS_DEREFERENCING_PROBLEM);
-- 
2.7.4


From 28cff3a540b0007bcfe946b59eba7edd2b9cf8bd Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 9 Jan 2018 07:43:18 +1300
Subject: [PATCH 3/3] python SamDB: init default flags to FLG_DONT_CREATE_DB

The current defaults for SamDB are to create the database file if it does not
exist.  Most of the uses of SamDB assume the database already exists, and so
auto-creation is not the desired behaviour.

TDB will overwrite an existing non TDB file with a newly created TDB file.
This becomes an issue when using alternate database file formats i.e. lmdb.

Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
 python/samba/samdb.py          | 3 ++-
 python/samba/upgradehelpers.py | 7 ++++++-
 selftest/knownfail.d/samdb     | 2 --
 3 files changed, 8 insertions(+), 4 deletions(-)
 delete mode 100644 selftest/knownfail.d/samdb

diff --git a/python/samba/samdb.py b/python/samba/samdb.py
index 4645629..82eb7a4 100644
--- a/python/samba/samdb.py
+++ b/python/samba/samdb.py
@@ -42,7 +42,8 @@ class SamDB(samba.Ldb):
     hash_well_known = {}
 
     def __init__(self, url=None, lp=None, modules_dir=None, session_info=None,
-                 credentials=None, flags=0, options=None, global_schema=True,
+                 credentials=None, flags=ldb.FLG_DONT_CREATE_DB,
+                 options=None, global_schema=True,
                  auto_connect=True, am_rodc=None):
         self.lp = lp
         if not auto_connect:
diff --git a/python/samba/upgradehelpers.py b/python/samba/upgradehelpers.py
index e026262..9f017bc 100644
--- a/python/samba/upgradehelpers.py
+++ b/python/samba/upgradehelpers.py
@@ -138,7 +138,12 @@ def get_ldbs(paths, creds, session, lp):
 
     ldbs = ProvisionLDB()
 
-    ldbs.sam = SamDB(paths.samdb, session_info=session, credentials=creds, lp=lp, options=["modules:samba_dsdb"])
+    ldbs.sam = SamDB(paths.samdb,
+                     session_info=session,
+                     credentials=creds,
+                     lp=lp,
+                     options=["modules:samba_dsdb"],
+                     flags=0)
     ldbs.secrets = Ldb(paths.secrets, session_info=session, credentials=creds, lp=lp)
     ldbs.idmap = Ldb(paths.idmapdb, session_info=session, credentials=creds, lp=lp)
     ldbs.privilege = Ldb(paths.privilege, session_info=session, credentials=creds, lp=lp)
diff --git a/selftest/knownfail.d/samdb b/selftest/knownfail.d/samdb
deleted file mode 100644
index 6a4ab97..0000000
--- a/selftest/knownfail.d/samdb
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.tests.samdb_api.samba.tests.samdb_api.SamDBApiTestCase.test_dont_create_db_existing_non_tdb_file\(none\)
-^samba.tests.samdb_api.samba.tests.samdb_api.SamDBApiTestCase.test_dont_create_db_new_file\(none\)
-- 
2.7.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180208/cf8afa18/signature.sig>


More information about the samba-technical mailing list