[PATCH] Fixes for wait_for_start in s3:selftest

Ralph Böhme slow at samba.org
Tue Jan 9 19:58:16 UTC 2018


Hi!

Here are a few patches for s3/selftest. Already reviewed by Andreas and
skimmed-over-by: metze, so I'm going to push later if noone objects. They
already passed a full private autobuild.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
-------------- next part --------------
From 32ebbb58cdd6ef27858c2d1a57c0245f3204faf6 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Jan 2018 18:38:08 +0100
Subject: [PATCH 1/6] selftest: fix creation of builtin users in wait_for_start

If "BUILTIN\Users" already exists, attempting to create it would fail,
so we should check for the existence prior to the creation.

It is unclear *why* the mapping sometimes already exist and sometime
not. There are two places where they would have been created:

1. libnet_join_add_dom_rids_to_builtins tries to add the mapping when
joining a domain, but at that point winbindd isn't running

2. when a user is authenticated in smbd, which clearly can't have
happended when in the function wait_for_start

Go figure...

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index f5e64725acf..18f9efbf28f 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2332,6 +2332,7 @@ userdup:x:$gid_userdup:$unix_name
 sub wait_for_start($$$$$)
 {
 	my ($self, $envvars, $nmbd, $winbindd, $smbd) = @_;
+	my $cmd;
 	my $ret;
 
 	if ($nmbd eq "yes") {
@@ -2365,8 +2366,7 @@ sub wait_for_start($$$$$)
 	if ($winbindd eq "yes") {
 	    print "checking for winbindd\n";
 	    my $count = 0;
-	    my $cmd = "";
-	    $cmd .= "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' ";
+	    $cmd = "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' ";
 	    $cmd .= "NSS_WRAPPER_PASSWD='$envvars->{NSS_WRAPPER_PASSWD}' ";
 	    $cmd .= "NSS_WRAPPER_GROUP='$envvars->{NSS_WRAPPER_GROUP}' ";
 	    $cmd .= Samba::bindir_path($self, "wbinfo") . " --ping-dc";
@@ -2418,9 +2418,28 @@ sub wait_for_start($$$$$)
 	    return 1;
 	}
 
+	# note: creating builtin groups requires winbindd for the
+	# unix id allocator
+	my $create_builtin_users = "no";
 	if ($winbindd eq "yes") {
-	    # note: creating builtin groups requires winbindd for the
-	    # unix id allocator
+		$cmd = "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' ";
+		$cmd .= "NSS_WRAPPER_PASSWD='$envvars->{NSS_WRAPPER_PASSWD}' ";
+		$cmd .= "NSS_WRAPPER_GROUP='$envvars->{NSS_WRAPPER_GROUP}' ";
+		$cmd .= Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545";
+		my $wbinfo_out = qx($cmd 2>&1);
+		if ($? != 0) {
+			# wbinfo doesn't give us a better error code then
+			# WBC_ERR_DOMAIN_NOT_FOUND, but at least that's
+			# different then WBC_ERR_WINBIND_NOT_AVAILABLE
+			if ($wbinfo_out !~ /WBC_ERR_DOMAIN_NOT_FOUND/) {
+				print("Failed to run \"wbinfo --sid-to-gid=S-1-5-32-545\": $wbinfo_out");
+				teardown_env($self, $envvars);
+				return 0;
+			}
+			$create_builtin_users = "yes";
+		}
+	}
+	if ($create_builtin_users eq "yes") {
 	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} sam createbuiltingroup Users");
 	    if ($ret != 0) {
 	        print "Failed to create BUILTIN\\Users group\n";
-- 
2.13.6


From 0e77da2152a405f01d8145d8bfeb271951623c56 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Jan 2018 18:45:01 +0100
Subject: [PATCH 2/6] selftest: remove second loop waiting for winbindd from
 wait_for_start()

A few lines above we already checked that winbindd is running.

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 18f9efbf28f..5f27dc7d9b6 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2443,19 +2443,13 @@ sub wait_for_start($$$$$)
 	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} sam createbuiltingroup Users");
 	    if ($ret != 0) {
 	        print "Failed to create BUILTIN\\Users group\n";
+		teardown_env($self, $envvars);
 	        return 0;
 	    }
-	    my $count = 0;
-	    do {
-		system(Samba::bindir_path($self, "net") . " $envvars->{CONFIGURATION} cache del IDMAP/SID2XID/S-1-5-32-545");
-		$ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545");
-		if ($ret != 0) {
-		    sleep(2);
-		}
-		$count++;
-	    } while ($ret != 0 && $count < 10);
-	    if ($count == 10) {
-		print "WINBINDD not reachable after 20 seconds\n";
+	    system(Samba::bindir_path($self, "net") . " $envvars->{CONFIGURATION} cache del IDMAP/SID2XID/S-1-5-32-545");
+	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545");
+	    if ($ret != 0) {
+		print "Missing \"BUILTIN\\Users\", did net sam createbuiltingroup Users fail?\n";
 		teardown_env($self, $envvars);
 		return 0;
 	    }
-- 
2.13.6


From 1472a496cc38d6508297823bc357d313b207d5ff Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Jan 2018 14:28:40 +0100
Subject: [PATCH 3/6] selftest: set wrapper env variables when running net
 groupmap

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5f27dc7d9b6..19b21368a3e 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2333,6 +2333,7 @@ sub wait_for_start($$$$$)
 {
 	my ($self, $envvars, $nmbd, $winbindd, $smbd) = @_;
 	my $cmd;
+	my $netcmd;
 	my $ret;
 
 	if ($nmbd eq "yes") {
@@ -2405,17 +2406,29 @@ sub wait_for_start($$$$$)
 	}
 
 	# Ensure we have domain users mapped.
-	$ret = system(Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} groupmap add rid=513 unixgroup=domusers type=domain");
+	$netcmd = "NSS_WRAPPER_PASSWD='$envvars->{NSS_WRAPPER_PASSWD}' ";
+	$netcmd .= "NSS_WRAPPER_GROUP='$envvars->{NSS_WRAPPER_GROUP}' ";
+	$netcmd .= Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} ";
+
+	$cmd = $netcmd . "groupmap add rid=513 unixgroup=domusers type=domain";
+	$ret = system($cmd);
 	if ($ret != 0) {
-	    return 1;
+		print("\"$cmd\" failed\n");
+		return 1;
 	}
-	$ret = system(Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} groupmap add rid=512 unixgroup=domadmins type=domain");
+
+	$cmd = $netcmd . "groupmap add rid=512 unixgroup=domadmins type=domain";
+	$ret = system($cmd);
 	if ($ret != 0) {
-	    return 1;
+		print("\"$cmd\" failed\n");
+		return 1;
 	}
-	$ret = system(Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} groupmap add sid=S-1-1-0 unixgroup=everyone type=builtin");
+
+	$cmd = $netcmd . "groupmap add sid=S-1-1-0 unixgroup=everyone type=builtin";
+	$ret = system($cmd);
 	if ($ret != 0) {
-	    return 1;
+		print("\"$cmd\" failed\n");
+		return 1;
 	}
 
 	# note: creating builtin groups requires winbindd for the
-- 
2.13.6


From 629526054753efb3148685686bc4d33a320a78a0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Jan 2018 10:40:41 +0100
Subject: [PATCH 4/6] selftest: split a large system invocation line

Small cleanup for better code readability, no change in behaviour.

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 19b21368a3e..00f7b72620f 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2453,7 +2453,10 @@ sub wait_for_start($$$$$)
 		}
 	}
 	if ($create_builtin_users eq "yes") {
-	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "net") ." $envvars->{CONFIGURATION} sam createbuiltingroup Users");
+	    $cmd = "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' ";
+	    $cmd .= Samba::bindir_path($self, "net") . " $envvars->{CONFIGURATION} ";
+	    $cmd .= "sam createbuiltingroup Users";
+	    $ret = system($cmd);
 	    if ($ret != 0) {
 	        print "Failed to create BUILTIN\\Users group\n";
 		teardown_env($self, $envvars);
-- 
2.13.6


From 4639bf8e5cc4880100a41d2c693423be7755e8c0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Jan 2018 10:45:59 +0100
Subject: [PATCH 5/6] selftest: split a large system invocation line

Small cleanup for better code readability, no change in behaviour.

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 00f7b72620f..a9439444601 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2462,7 +2462,11 @@ sub wait_for_start($$$$$)
 		teardown_env($self, $envvars);
 	        return 0;
 	    }
-	    system(Samba::bindir_path($self, "net") . " $envvars->{CONFIGURATION} cache del IDMAP/SID2XID/S-1-5-32-545");
+
+	    $cmd = Samba::bindir_path($self, "net") . " $envvars->{CONFIGURATION} ";
+	    $cmd .= "cache del IDMAP/SID2XID/S-1-5-32-545";
+	    system($cmd);
+
 	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545");
 	    if ($ret != 0) {
 		print "Missing \"BUILTIN\\Users\", did net sam createbuiltingroup Users fail?\n";
-- 
2.13.6


From 330505d182b7e322326f3130aaf916240b55a4b1 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Jan 2018 10:46:40 +0100
Subject: [PATCH 6/6] selftest: split a large system invocation line

Small cleanup for better code readability, no change in behaviour.

Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Andreas Schneider <asn at samba.org>
---
 selftest/target/Samba3.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index a9439444601..3017270e313 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2467,7 +2467,9 @@ sub wait_for_start($$$$$)
 	    $cmd .= "cache del IDMAP/SID2XID/S-1-5-32-545";
 	    system($cmd);
 
-	    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545");
+	    $cmd = "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' ";
+	    $cmd .= Samba::bindir_path($self, "wbinfo") . " --sid-to-gid=S-1-5-32-545";
+	    $ret = system($cmd);
 	    if ($ret != 0) {
 		print "Missing \"BUILTIN\\Users\", did net sam createbuiltingroup Users fail?\n";
 		teardown_env($self, $envvars);
-- 
2.13.6



More information about the samba-technical mailing list