[PATCH] Tidy up env vars used by up auth_log test + fix test hang

Tim Beale timbeale at catalyst.net.nz
Thu Feb 28 23:36:01 UTC 2019


These patches tidy up the auth_log/audit_log tests a bit. Mostly so we
don't need to pass in so many environment variables from tests.py.

There's also a nifty fix from Aaron for the problem of the auth_log
tests occasionally hanging during CI runs.

CI pass: https://gitlab.com/catalyst-samba/samba/pipelines/49751704

Review appreciated, thanks.

-------------- next part --------------
From fbe4e50a205c26504441e3a29e14295d55175109 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 28 Jan 2019 14:11:09 +1300
Subject: [PATCH 1/8] tests: Remove redundant credentials from auth_log tests

The LDB connection in these tests is to the direct sam.ldb file on disk,
so the credentials are not actually needed (and in fact, weren't event
initialized correctly). These tests always need to run on the DC itself
(i.e. :local testenv) because they use ncalrpc connections.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/auth_log_netlogon.py           | 3 ---
 python/samba/tests/auth_log_netlogon_bad_creds.py | 3 ---
 python/samba/tests/auth_log_samlogon.py           | 3 ---
 3 files changed, 9 deletions(-)

diff --git a/python/samba/tests/auth_log_netlogon.py b/python/samba/tests/auth_log_netlogon.py
index 83ffd33..36920ee 100644
--- a/python/samba/tests/auth_log_netlogon.py
+++ b/python/samba/tests/auth_log_netlogon.py
@@ -47,12 +47,9 @@ class AuthLogTestsNetLogon(samba.tests.auth_log_base.AuthLogTestBase):
     def setUp(self):
         super(AuthLogTestsNetLogon, self).setUp()
         self.lp = samba.tests.env_loadparm()
-        self.creds = Credentials()
-
         self.session = system_session()
         self.ldb = SamDB(
             session_info=self.session,
-            credentials=self.creds,
             lp=self.lp)
 
         self.domain = os.environ["DOMAIN"]
diff --git a/python/samba/tests/auth_log_netlogon_bad_creds.py b/python/samba/tests/auth_log_netlogon_bad_creds.py
index 3b699bb..bd8b497 100644
--- a/python/samba/tests/auth_log_netlogon_bad_creds.py
+++ b/python/samba/tests/auth_log_netlogon_bad_creds.py
@@ -49,12 +49,9 @@ class AuthLogTestsNetLogonBadCreds(samba.tests.auth_log_base.AuthLogTestBase):
     def setUp(self):
         super(AuthLogTestsNetLogonBadCreds, self).setUp()
         self.lp = samba.tests.env_loadparm()
-        self.creds = Credentials()
-
         self.session = system_session()
         self.ldb = SamDB(
             session_info=self.session,
-            credentials=self.creds,
             lp=self.lp)
 
         self.domain = os.environ["DOMAIN"]
diff --git a/python/samba/tests/auth_log_samlogon.py b/python/samba/tests/auth_log_samlogon.py
index eeb64df..4959543 100644
--- a/python/samba/tests/auth_log_samlogon.py
+++ b/python/samba/tests/auth_log_samlogon.py
@@ -47,12 +47,9 @@ class AuthLogTestsSamLogon(samba.tests.auth_log_base.AuthLogTestBase):
     def setUp(self):
         super(AuthLogTestsSamLogon, self).setUp()
         self.lp = samba.tests.env_loadparm()
-        self.creds = Credentials()
-
         self.session = system_session()
         self.ldb = SamDB(
             session_info=self.session,
-            credentials=self.creds,
             lp=self.lp)
 
         self.domain = os.environ["DOMAIN"]
-- 
2.7.4


From e9de76c228e09e90009c8d70f302e85fda5b461c Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 10:17:21 +1300
Subject: [PATCH 2/8] s4:tests: Avoid passing unnecessary env variables to
 auth_log tests

These tests all use the ncalrpc connection, so they're always testing a
connection that's local to the server-side. Therefore passing in the
CLIENT_IP and SOCKET_WRAPPER_DEFAULT_IFACE variables (in order to try to
simulate a client connecting) is unnecessary.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/selftest/tests.py | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index e83bbac..0ec2fcb 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -724,34 +724,26 @@ if have_heimdal_support:
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
                            environ={'CLIENT_IP': '127.0.0.11',
                                     'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+
+    # these tests use a NCA local RPC connection, so always run on the
+    # :local testenv, and so don't need to fake a client connection
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_ncalrpc", extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_ncalrpc", extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_samlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_samlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_netlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_netlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc:local",
                            "samba.tests.auth_log_netlogon_bad_creds",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc_ntvfs:local",
                            "samba.tests.auth_log_netlogon_bad_creds",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
+
     planoldpythontestsuite("ad_member:local",
                            "samba.tests.auth_log_winbind",
                            extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"'],
-- 
2.7.4


From ffa7f7c0c850c80a83576f1dc7f67e77da51e763 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 10:19:06 +1300
Subject: [PATCH 3/8] s4:tests: Move duplicated test cases into loop

This is more consistent with how we run tests elsewhere.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/selftest/tests.py | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 0ec2fcb..2613a23 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -727,22 +727,14 @@ if have_heimdal_support:
 
     # these tests use a NCA local RPC connection, so always run on the
     # :local testenv, and so don't need to fake a client connection
-    planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_ncalrpc", extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_ncalrpc", extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_samlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_samlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_netlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_netlogon",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc:local",
-                           "samba.tests.auth_log_netlogon_bad_creds",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
-    planoldpythontestsuite("ad_dc_ntvfs:local",
-                           "samba.tests.auth_log_netlogon_bad_creds",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'])
+    for env in ["ad_dc_ntvfs:local", "ad_dc:local"]:
+        planoldpythontestsuite(env, "samba.tests.auth_log_ncalrpc", extra_args=['-U"$USERNAME%$PASSWORD"'])
+        planoldpythontestsuite(env, "samba.tests.auth_log_samlogon",
+                               extra_args=['-U"$USERNAME%$PASSWORD"'])
+        planoldpythontestsuite(env, "samba.tests.auth_log_netlogon",
+                               extra_args=['-U"$USERNAME%$PASSWORD"'])
+        planoldpythontestsuite(env, "samba.tests.auth_log_netlogon_bad_creds",
+                               extra_args=['-U"$USERNAME%$PASSWORD"'])
 
     planoldpythontestsuite("ad_member:local",
                            "samba.tests.auth_log_winbind",
-- 
2.7.4


From 55e33db26f055766e61a17f9e0ff980bdcb508c8 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 10:21:37 +1300
Subject: [PATCH 4/8] s4:tests: Remove unused DC_ENV variable

I believe this was a leftover remnant from an earlier patch revision -
it's now been replaced by the DC_SERVERCONFFILE variable.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/selftest/tests.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 2613a23..56aa5ba 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -738,8 +738,7 @@ if have_heimdal_support:
 
     planoldpythontestsuite("ad_member:local",
                            "samba.tests.auth_log_winbind",
-                           extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"'],
-                           environ={'DC_ENV': 'ad_dc_ntvfs'})
+                           extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"'])
     planoldpythontestsuite("ad_dc:local", "samba.tests.audit_log_pass_change",
                            extra_args=['-U"$USERNAME%$PASSWORD"'],
                            environ={'CLIENT_IP': '127.0.0.11',
-- 
2.7.4


From 13dee74f7b97639c37b886facf68b9956f7d37df Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 10:53:43 +1300
Subject: [PATCH 5/8] tests: Remove explicit SOCKET_WRAPPER usage from auth_log
 tests

The auth-logging tests are an odd combination of server and client
behaviour. On the one hand we want a IRPC connection to see the auth
events being logged on the server. On the other hand, we want the auth
events to appear to be happening on a client. Currently we hardcode in
the use of a SOCKET_WRAPPER interface to make this happen.

We can avoid this explicit socket wrapper usage by using the server
smb.conf instead in the one place we actually want to act like the
server (creating the IRPC connection). Then we can switch from using
the 'ad_dc*:local' testenvs to use 'ad_dc*', in order to act like a
client by default. The SERVERCONFFILE environment variable has already
been added for the few cases where a test needs explicit access to the
server's smb.conf.

However, for samba.tests.auth_log, the samlogon test cases are still
reliant on being run on the :local testenv, and so we can't switch them
over just yet. This is because the samlogon is using the DC's machine
creds underneath, which will fail on the non-local testenv. We could
create separate machine creds for the client and use those, but this is
a non-trivial rework of the test code.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/audit_log_base.py | 11 ++++++++++-
 python/samba/tests/auth_log_base.py  | 10 +++++++++-
 selftest/knownfail                   |  4 ++--
 source4/selftest/tests.py            | 25 ++++++++++---------------
 4 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/python/samba/tests/audit_log_base.py b/python/samba/tests/audit_log_base.py
index e91c414..d3bc3da 100644
--- a/python/samba/tests/audit_log_base.py
+++ b/python/samba/tests/audit_log_base.py
@@ -22,6 +22,7 @@ from __future__ import print_function
 import samba.tests
 from samba.messaging import Messaging
 from samba.dcerpc.messaging import MSG_AUTH_LOG, AUTH_EVENT_NAME
+from samba.param import LoadParm
 import time
 import json
 import os
@@ -41,7 +42,15 @@ class AuditLogTestBase(samba.tests.TestCase):
 
     def setUp(self):
         super(AuditLogTestBase, self).setUp()
-        lp_ctx = self.get_loadparm()
+
+        # connect to the server's messaging bus (we need to explicitly load a
+        # different smb.conf here, because in all other respects this test
+        # wants to act as a separate remote client)
+        server_conf = os.getenv('SERVERCONFFILE')
+        if server_conf:
+            lp_ctx = LoadParm(filename_for_non_global_lp=server_conf)
+        else:
+            lp_ctx = self.get_loadparm()
         self.msg_ctx = Messaging((1,), lp_ctx=lp_ctx)
         self.msg_ctx.irpc_add_name(self.event_type)
 
diff --git a/python/samba/tests/auth_log_base.py b/python/samba/tests/auth_log_base.py
index c139108..bd09c44 100644
--- a/python/samba/tests/auth_log_base.py
+++ b/python/samba/tests/auth_log_base.py
@@ -22,6 +22,7 @@ from __future__ import print_function
 import samba.tests
 from samba.messaging import Messaging
 from samba.dcerpc.messaging import MSG_AUTH_LOG, AUTH_EVENT_NAME
+from samba.param import LoadParm
 import time
 import json
 import os
@@ -34,7 +35,14 @@ class AuthLogTestBase(samba.tests.TestCase):
 
     def setUp(self):
         super(AuthLogTestBase, self).setUp()
-        lp_ctx = self.get_loadparm()
+        # connect to the server's messaging bus (we need to explicitly load a
+        # different smb.conf here, because in all other respects this test
+        # wants to act as a separate remote client)
+        server_conf = os.getenv('SERVERCONFFILE')
+        if server_conf:
+            lp_ctx = LoadParm(filename_for_non_global_lp=server_conf)
+        else:
+            lp_ctx = self.get_loadparm()
         self.msg_ctx = Messaging((1,), lp_ctx=lp_ctx)
         global msg_ctxs
         msg_ctxs.append(self.msg_ctx)
diff --git a/selftest/knownfail b/selftest/knownfail
index dc78838..750b5f5 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -339,9 +339,9 @@
 ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dbcheck_dangling_multi_valued_clean
 ^samba4.blackbox.dbcheck-links.release-4-5-0-pre1.dangling_multi_valued_check_missing
 #
-# rap password tests don't function in the ad_dc_ntvfs:local environment
+# rap password tests don't function in the ad_dc_ntvfs environment
 #
-^samba.tests.auth_log_pass_change.samba.tests.auth_log_pass_change.AuthLogPassChangeTests.test_rap_change_password\(ad_dc_ntvfs:local\)
+^samba.tests.auth_log_pass_change.samba.tests.auth_log_pass_change.AuthLogPassChangeTests.test_rap_change_password\(ad_dc_ntvfs\)
 # We currently don't send referrals for LDAP modify of non-replicated attrs
 ^samba4.ldap.rodc.python\(rodc\).__main__.RodcTests.test_modify_nonreplicated.*
 ^samba4.ldap.rodc_rwdc.python.*.__main__.RodcRwdcTests.test_change_password_reveal_on_demand_kerberos
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 56aa5ba..0cc3441 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -718,12 +718,10 @@ if have_heimdal_support:
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log", extra_args=['-U"$USERNAME%$PASSWORD"'],
                            environ={'CLIENT_IP': '127.0.0.11',
                                     'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
-    planoldpythontestsuite("ad_dc:local", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
-    planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+    planoldpythontestsuite("ad_dc", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
+                           environ={'CLIENT_IP': '127.0.0.11'})
+    planoldpythontestsuite("ad_dc_ntvfs", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
+                           environ={'CLIENT_IP': '127.0.0.11'})
 
     # these tests use a NCA local RPC connection, so always run on the
     # :local testenv, and so don't need to fake a client connection
@@ -739,18 +737,15 @@ if have_heimdal_support:
     planoldpythontestsuite("ad_member:local",
                            "samba.tests.auth_log_winbind",
                            extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"'])
-    planoldpythontestsuite("ad_dc:local", "samba.tests.audit_log_pass_change",
+    planoldpythontestsuite("ad_dc", "samba.tests.audit_log_pass_change",
                            extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
-    planoldpythontestsuite("ad_dc:local", "samba.tests.audit_log_dsdb",
+                           environ={'CLIENT_IP': '127.0.0.11'})
+    planoldpythontestsuite("ad_dc", "samba.tests.audit_log_dsdb",
                            extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
-    planoldpythontestsuite("ad_dc:local", "samba.tests.group_audit",
+                           environ={'CLIENT_IP': '127.0.0.11'})
+    planoldpythontestsuite("ad_dc", "samba.tests.group_audit",
                            extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11',
-                                    'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
+                           environ={'CLIENT_IP': '127.0.0.11'})
 
 planoldpythontestsuite("fl2008r2dc:local",
                        "samba.tests.getdcname",
-- 
2.7.4


From 350fd7a341f04fa15f57a7bc3b47ef73a99e569a Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 11:06:52 +1300
Subject: [PATCH 6/8] tests: Work audit_log CLIENT_IP out from config instead
 of env var

Instead of passing the CLIENT_IP to the audit_log tests, we can just
work out the source-IP that the client will use from its smb.conf file.
Because the audit_log tests are all run on the non-local testenv,
they'll already use the client.conf and the 127.0.0.11 address.

The main advantage of this change is it avoids having hardcoded IP
addresses in the selftest framework.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/audit_log_base.py        | 11 +++++++++++
 python/samba/tests/audit_log_dsdb.py        |  1 -
 python/samba/tests/audit_log_pass_change.py |  1 -
 python/samba/tests/group_audit.py           |  1 -
 source4/selftest/tests.py                   |  9 +++------
 5 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/python/samba/tests/audit_log_base.py b/python/samba/tests/audit_log_base.py
index d3bc3da..c1223ed 100644
--- a/python/samba/tests/audit_log_base.py
+++ b/python/samba/tests/audit_log_base.py
@@ -54,6 +54,17 @@ class AuditLogTestBase(samba.tests.TestCase):
         self.msg_ctx = Messaging((1,), lp_ctx=lp_ctx)
         self.msg_ctx.irpc_add_name(self.event_type)
 
+        # Now switch back to using the client-side smb.conf. The tests will
+        # use the first interface in the client.conf (we need to strip off
+        # the subnet mask portion)
+        lp_ctx = self.get_loadparm()
+        client_ip_and_mask = lp_ctx.get('interfaces')[0]
+        client_ip = client_ip_and_mask.split('/')[0]
+
+        # the messaging ctx is the server's view of the world, so our own
+        # client IP will be the remoteAddress when connections are logged
+        self.remoteAddress = client_ip
+
         #
         # Check the remote address of a message against the one beimg used
         # for the tests.
diff --git a/python/samba/tests/audit_log_dsdb.py b/python/samba/tests/audit_log_dsdb.py
index 0471c22..2986133 100644
--- a/python/samba/tests/audit_log_dsdb.py
+++ b/python/samba/tests/audit_log_dsdb.py
@@ -43,7 +43,6 @@ class AuditLogDsdbTests(AuditLogTestBase):
         self.event_type = DSDB_EVENT_NAME
         super(AuditLogDsdbTests, self).setUp()
 
-        self.remoteAddress = os.environ["CLIENT_IP"]
         self.server_ip = os.environ["SERVER_IP"]
 
         host = "ldap://%s" % os.environ["SERVER"]
diff --git a/python/samba/tests/audit_log_pass_change.py b/python/samba/tests/audit_log_pass_change.py
index d580698..4e25c87 100644
--- a/python/samba/tests/audit_log_pass_change.py
+++ b/python/samba/tests/audit_log_pass_change.py
@@ -48,7 +48,6 @@ class AuditLogPassChangeTests(AuditLogTestBase):
         self.event_type = DSDB_PWD_EVENT_NAME
         super(AuditLogPassChangeTests, self).setUp()
 
-        self.remoteAddress = os.environ["CLIENT_IP"]
         self.server_ip = os.environ["SERVER_IP"]
 
         host = "ldap://%s" % os.environ["SERVER"]
diff --git a/python/samba/tests/group_audit.py b/python/samba/tests/group_audit.py
index b8c90a3..7163c50 100644
--- a/python/samba/tests/group_audit.py
+++ b/python/samba/tests/group_audit.py
@@ -50,7 +50,6 @@ class GroupAuditTests(AuditLogTestBase):
         self.event_type = DSDB_GROUP_EVENT_NAME
         super(GroupAuditTests, self).setUp()
 
-        self.remoteAddress = os.environ["CLIENT_IP"]
         self.server_ip = os.environ["SERVER_IP"]
 
         host = "ldap://%s" % os.environ["SERVER"]
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 0cc3441..7197a3b 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -738,14 +738,11 @@ if have_heimdal_support:
                            "samba.tests.auth_log_winbind",
                            extra_args=['-U"$DC_USERNAME%$DC_PASSWORD"'])
     planoldpythontestsuite("ad_dc", "samba.tests.audit_log_pass_change",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11'})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc", "samba.tests.audit_log_dsdb",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11'})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
     planoldpythontestsuite("ad_dc", "samba.tests.group_audit",
-                           extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11'})
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
 
 planoldpythontestsuite("fl2008r2dc:local",
                        "samba.tests.getdcname",
-- 
2.7.4


From e3cea2716b392f05032ae53274cf4f0e7f4b5ea1 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 11:10:46 +1300
Subject: [PATCH 7/8] tests: Work auth_log CLIENT_IP out from config instead of
 env var

Instead of passing the CLIENT_IP to the auth_log tests, we can just
work out the source-IP that the client will use from its smb.conf file.

This only works for auth_log_pass_change, but not auth_log.py - the
latter still needs to be run on the :local testenv for other reasons, so
it doesn't use the client.conf. However, we can still update the base
code to use the client.conf IP, as auth_log.py overrides
self.remoteAddress anyway.

The main advantage of this change is it avoids having hardcoded IP
addresses in the selftest framework.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/auth_log_base.py        | 11 +++++++++++
 python/samba/tests/auth_log_pass_change.py |  1 -
 source4/selftest/tests.py                  |  8 ++++----
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/python/samba/tests/auth_log_base.py b/python/samba/tests/auth_log_base.py
index bd09c44..d6a3976 100644
--- a/python/samba/tests/auth_log_base.py
+++ b/python/samba/tests/auth_log_base.py
@@ -48,6 +48,17 @@ class AuthLogTestBase(samba.tests.TestCase):
         msg_ctxs.append(self.msg_ctx)
         self.msg_ctx.irpc_add_name(AUTH_EVENT_NAME)
 
+        # Now switch back to using the client-side smb.conf. The tests will
+        # use the first interface in the client.conf (we need to strip off
+        # the subnet mask portion)
+        lp_ctx = self.get_loadparm()
+        client_ip_and_mask = lp_ctx.get('interfaces')[0]
+        client_ip = client_ip_and_mask.split('/')[0]
+
+        # the messaging ctx is the server's view of the world, so our own
+        # client IP will be the remoteAddress when connections are logged
+        self.remoteAddress = client_ip
+
         def messageHandler(context, msgType, src, message):
             # This does not look like sub unit output and it
             # makes these tests much easier to debug.
diff --git a/python/samba/tests/auth_log_pass_change.py b/python/samba/tests/auth_log_pass_change.py
index f0a0ee6..9954867 100644
--- a/python/samba/tests/auth_log_pass_change.py
+++ b/python/samba/tests/auth_log_pass_change.py
@@ -45,7 +45,6 @@ class AuthLogPassChangeTests(samba.tests.auth_log_base.AuthLogTestBase):
     def setUp(self):
         super(AuthLogPassChangeTests, self).setUp()
 
-        self.remoteAddress = os.environ["CLIENT_IP"]
         self.server_ip = os.environ["SERVER_IP"]
 
         host = "ldap://%s" % os.environ["SERVER"]
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 7197a3b..f165087 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -718,10 +718,10 @@ if have_heimdal_support:
     planoldpythontestsuite("ad_dc_ntvfs:local", "samba.tests.auth_log", extra_args=['-U"$USERNAME%$PASSWORD"'],
                            environ={'CLIENT_IP': '127.0.0.11',
                                     'SOCKET_WRAPPER_DEFAULT_IFACE': 11})
-    planoldpythontestsuite("ad_dc", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11'})
-    planoldpythontestsuite("ad_dc_ntvfs", "samba.tests.auth_log_pass_change", extra_args=['-U"$USERNAME%$PASSWORD"'],
-                           environ={'CLIENT_IP': '127.0.0.11'})
+    planoldpythontestsuite("ad_dc", "samba.tests.auth_log_pass_change",
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
+    planoldpythontestsuite("ad_dc_ntvfs", "samba.tests.auth_log_pass_change",
+                           extra_args=['-U"$USERNAME%$PASSWORD"'])
 
     # these tests use a NCA local RPC connection, so always run on the
     # :local testenv, and so don't need to fake a client connection
-- 
2.7.4


From 151b175c12385bd73c9c44e664d87762888d2cae Mon Sep 17 00:00:00 2001
From: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Date: Thu, 28 Feb 2019 16:55:31 +1300
Subject: [PATCH 8/8] tests: Reduce likelihood of auth_log test locking up
 during CI

We would sometimes see the auth_log test hang during a CI run. The CI
job would eventually fail after consuming a costly 10 hours of CI
runtime.

We believe the problem is around the test creating multiple instances of
the Messaging() context. This is a similar race condition to what was
seen in 19f34b2161dee26.

Currently a new Messaging() context is created for every test case. By
using classmethods instead, the Messaging context is only created once
per python test file execution (i.e. creation of the python class,
rather than initialization of the python object, which happens for every
test-case).

This means the test will only create one Messaging() context, which
should avoid any race conditions.

Changes:
+ removed msg_ctxs - this wasn't actually used for anything.
+ use classmethods to setup and tear-down the Messaging() context (and
tweak lp initialization accordingly).
+ fix discardMessages() - the loop wasn't actually discarding any
messages previously (this may also have been the cause of the test
hanging).

Signed-off-by: Aaron Haslett <aaronhaslett at catalyst.net.nz>
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/auth_log_base.py | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/python/samba/tests/auth_log_base.py b/python/samba/tests/auth_log_base.py
index d6a3976..09a8df0 100644
--- a/python/samba/tests/auth_log_base.py
+++ b/python/samba/tests/auth_log_base.py
@@ -27,14 +27,13 @@ import time
 import json
 import os
 import re
-
-msg_ctxs = []
+from samba import param
 
 
 class AuthLogTestBase(samba.tests.TestCase):
 
-    def setUp(self):
-        super(AuthLogTestBase, self).setUp()
+    @classmethod
+    def setUpClass(self):
         # connect to the server's messaging bus (we need to explicitly load a
         # different smb.conf here, because in all other respects this test
         # wants to act as a separate remote client)
@@ -42,16 +41,14 @@ class AuthLogTestBase(samba.tests.TestCase):
         if server_conf:
             lp_ctx = LoadParm(filename_for_non_global_lp=server_conf)
         else:
-            lp_ctx = self.get_loadparm()
+            samba.tests.env_loadparm()
         self.msg_ctx = Messaging((1,), lp_ctx=lp_ctx)
-        global msg_ctxs
-        msg_ctxs.append(self.msg_ctx)
         self.msg_ctx.irpc_add_name(AUTH_EVENT_NAME)
 
         # Now switch back to using the client-side smb.conf. The tests will
         # use the first interface in the client.conf (we need to strip off
         # the subnet mask portion)
-        lp_ctx = self.get_loadparm()
+        lp_ctx = samba.tests.env_loadparm()
         client_ip_and_mask = lp_ctx.get('interfaces')[0]
         client_ip = client_ip_and_mask.split('/')[0]
 
@@ -71,18 +68,21 @@ class AuthLogTestBase(samba.tests.TestCase):
         self.msg_ctx.register(self.msg_handler_and_context,
                               msg_type=MSG_AUTH_LOG)
 
-        self.discardMessages()
-
         self.remoteAddress = None
         self.server = os.environ["SERVER"]
         self.connection = None
 
-    def tearDown(self):
+    @classmethod
+    def tearDownClass(self):
         if self.msg_handler_and_context:
             self.msg_ctx.deregister(self.msg_handler_and_context,
                                     msg_type=MSG_AUTH_LOG)
             self.msg_ctx.irpc_remove_name(AUTH_EVENT_NAME)
 
+    def setUp(self):
+        super(AuthLogTestBase, self).setUp()
+        self.discardMessages()
+
     def waitForMessages(self, isLastExpectedMessage, connection=None):
         """Wait for all the expected messages to arrive
         The connection is passed through to keep the connection alive
@@ -126,11 +126,12 @@ class AuthLogTestBase(samba.tests.TestCase):
         return list(filter(isRemote, self.context["messages"]))
 
     # Discard any previously queued messages.
+    @classmethod
     def discardMessages(self):
         self.msg_ctx.loop_once(0.001)
         while len(self.context["messages"]):
+            self.context["messages"] = []
             self.msg_ctx.loop_once(0.001)
-        self.context["messages"] = []
 
     # Remove any NETLOGON authentication messages
     # NETLOGON is only performed once per session, so to avoid ordering
-- 
2.7.4



More information about the samba-technical mailing list