[PATCH] Clean up 'interfaces' smb.conf in selftest + remove unused client IPs
Tim Beale
timbeale at catalyst.net.nz
Wed Mar 13 02:43:49 UTC 2019
These are some small tidy-ups so that the 'interface' smb.conf setting
for the testenvs is generated in one place. This keeps assumptions about
the IP subnets in one place.
The main change is that the extra client.conf IPs have been removed, as
they did not appear to be used. (The IPs are still reserved, if we need
to simulate multiple different clients in future). The client.conf also
now has an IPv6 address, to make it more consistent with the server
smb.conf (although the client didn't need this IPv6 address, as all the
tests worked fine without it).
CI pass: https://gitlab.com/catalyst-samba/samba/pipelines/51526393
Merge request: https://gitlab.com/samba-team/samba/merge_requests/300
Review appreciated. Thanks.
-------------- next part --------------
From a2bc966312104341d4cd30c1508b66b5b4fd206b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 26 Feb 2019 12:14:55 +1300
Subject: [PATCH 1/6] selftest: Remove secondary client interfaces
I can't see anything in the tests that ever tries to use these other IP
addresses. While it makes sense that we might want the tests to simulate
multiple different clients (with different IPs), we don't appear to do
this currently.
Removing the spare client addresses minimizes the number of hard-coded
IP addresses in selftest.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/selftest.pl | 7 +------
selftest/target/Samba.pm | 5 ++++-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index 773b284..37e288e 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -511,12 +511,7 @@ foreach (@opt_include) {
push (@includes, read_test_regexes($_));
}
-my $interfaces = join(',', ("127.0.0.11/8",
- "127.0.0.12/8",
- "127.0.0.13/8",
- "127.0.0.14/8",
- "127.0.0.15/8",
- "127.0.0.16/8"));
+my $interfaces = "127.0.0.11/8";
my $clientdir = "$prefix_abs/client";
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 3a23862..2d8cbc2 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -437,9 +437,12 @@ sub get_interface($)
localnt4dc9 => 9,
# 10 is spare
- # 11-16 used by selftest.pl for client interfaces
+ # 11 is used by selftest.pl for the client interface
client => 11,
+ # 12-16 have been historically reserved for the client, although
+ # aren't actually used
+
addc_no_nss => 17,
addc_no_ntlm => 18,
idmapadmember => 19,
--
2.7.4
From c33aec0febe3dc75404f004a603aad90839c297e Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 12 Mar 2019 14:00:55 +1300
Subject: [PATCH 2/6] selftest: Add helper function to get interfaces config
Add a helper function to return the IPv4/IPv6 addresses for the
smb.conf. This keeps the netmask assumptions in the same places as
the IP subnet assumptions.
This refactor means we no longer need to store $ctx->{interfaces}, as it
was only used in one place.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/target/Samba.pm | 11 +++++++++++
selftest/target/Samba3.pm | 5 ++++-
selftest/target/Samba4.pm | 5 +++--
3 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index 2d8cbc2..f7332da 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -505,6 +505,17 @@ sub get_ipv6_addr
return sprintf("fd00:0000:0000:0000:0000:0000:5357:5f%02x", $swiface);
}
+# returns the 'interfaces' setting for smb.conf, i.e. the IPv4/IPv6
+# addresses for testenv
+sub get_interfaces_config
+{
+ (my $hostname) = @_;
+ my $ipv4_addr = Samba::get_ipv4_addr($hostname);
+ my $ipv6_addr = Samba::get_ipv6_addr($hostname);
+
+ return "$ipv4_addr/8 $ipv6_addr/64";
+}
+
sub cleanup_child($$)
{
my ($pid, $name) = @_;
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index e176e31..cbf5309 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1734,10 +1734,13 @@ sub provision($$$$$$$$$)
warn("Unable to open $conffile");
return undef;
}
+
+ my $interfaces = Samba::get_interfaces_config($server);
+
print CONF "
[global]
netbios name = $server
- interfaces = $server_ip/8 $server_ipv6/64
+ interfaces = $interfaces
bind interfaces only = yes
panic action = cd $self->{srcdir} && $self->{srcdir}/selftest/gdb_backtrace %d %\$(MAKE_TEST_BINARY)
smbd:suicide mode = yes
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 12f1a60..d8722dd 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -675,7 +675,6 @@ sub provision_raw_prepare($$$$$$$$$$$$)
$ctx->{ipv4} = Samba::get_ipv4_addr($hostname);
$ctx->{ipv6} = Samba::get_ipv6_addr($hostname);
- $ctx->{interfaces} = "$ctx->{ipv4}/8 $ctx->{ipv6}/64";
push(@{$ctx->{directories}}, $ctx->{privatedir});
push(@{$ctx->{directories}}, $ctx->{binddnsdir});
@@ -783,6 +782,8 @@ sub provision_raw_step1($$)
$services = "+smb -s3fs";
}
+ my $interfaces = Samba::get_interfaces_config($ctx->{netbiosname});
+
print CONFFILE "
[global]
netbios name = $ctx->{netbiosname}
@@ -799,7 +800,7 @@ sub provision_raw_step1($$)
winbindd socket directory = $ctx->{winbindd_socket_dir}
ntp signd socket directory = $ctx->{ntp_signd_socket_dir}
winbind separator = /
- interfaces = $ctx->{interfaces}
+ interfaces = $interfaces
tls dh params file = $ctx->{tlsdir}/dhparms.pem
tls crlfile = ${crlfile}
tls verify peer = no_check
--
2.7.4
From 55f99fd8330d719cb2aa92e5f12604cc3a52ab71 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 13 Mar 2019 10:18:41 +1300
Subject: [PATCH 3/6] tests: Make IPv4 assumption explicit
This test is asserting the expected number of *IPv4* addresses, not any
interface address (including IPv6). It works currently because the
selftest client doesn't have an IPv6 address in its smb.conf.
This patch makes the IPv4 assumption explicit by importing
interface_ips_v4() from the provision code. We need to tweak this to
pass through an 'all_interfaces' flag, otherwise it filters out the
loopback IP addresses that the testenv is using.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
python/samba/provision/__init__.py | 4 ++--
python/samba/tests/join.py | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/python/samba/provision/__init__.py b/python/samba/provision/__init__.py
index 1b7762eb..0a3a7b8 100644
--- a/python/samba/provision/__init__.py
+++ b/python/samba/provision/__init__.py
@@ -1853,9 +1853,9 @@ def checksysvolacl(samdb, netlogon, sysvol, domainsid, dnsdomain, domaindn,
direct_db_access)
-def interface_ips_v4(lp):
+def interface_ips_v4(lp, all_interfaces=False):
"""return only IPv4 IPs"""
- ips = samba.interface_ips(lp, False)
+ ips = samba.interface_ips(lp, all_interfaces)
ret = []
for i in ips:
if i.find(':') == -1:
diff --git a/python/samba/tests/join.py b/python/samba/tests/join.py
index 09a102e..e0ad34e 100644
--- a/python/samba/tests/join.py
+++ b/python/samba/tests/join.py
@@ -24,6 +24,7 @@ from samba.tests.dns_base import DNSTKeyTest
from samba.join import DCJoinContext
from samba.dcerpc import drsuapi, misc, dns
from samba.credentials import Credentials
+from samba.provision import interface_ips_v4
def get_logger(name="subunit"):
@@ -92,7 +93,7 @@ class JoinTestCase(DNSTKeyTest):
questions.append(q)
# Get expected IPs
- IPs = samba.interface_ips(self.lp)
+ IPs = interface_ips_v4(self.lp, all_interfaces=True)
self.finish_name_packet(p, questions)
(response, response_packet) = self.dns_transaction_tcp(p, host=self.server_ip)
@@ -132,7 +133,7 @@ class JoinTestCase(DNSTKeyTest):
updates = []
# Delete the old expected IPs
- IPs = samba.interface_ips(self.lp)
+ IPs = interface_ips_v4(self.lp, all_interfaces=True)
for IP in IPs[1:]:
if ":" in IP:
r = dns.res_rec()
--
2.7.4
From 5abcaa512ab402afddb4213254efaeec44f54edb Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 12 Mar 2019 14:04:21 +1300
Subject: [PATCH 4/6] selftest: Use new helper function for client's smb.conf
IP
This has the side-effect of giving the client an IPv6 address, which it
hasn't had up until now. But it at least makes the client and server
interfaces settings consistent, and gets rid of a hard-coded IP address.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/selftest.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index 37e288e..919a9d5 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -511,7 +511,7 @@ foreach (@opt_include) {
push (@includes, read_test_regexes($_));
}
-my $interfaces = "127.0.0.11/8";
+my $interfaces = Samba::get_interfaces_config("client");
my $clientdir = "$prefix_abs/client";
--
2.7.4
From ed02eb4002dcd785192230b1e16dc417195a43d4 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 12 Mar 2019 17:39:50 +1300
Subject: [PATCH 5/6] selftest: Use consistent env variables for dns_hub
Setting up a testenv involves populating 2 different hashmaps - an
intermediary one (usually called 'ctx') and one that is used to populate
the testenv environment variables (usually called 'env_vars' or
'dcvars').
Because the dns_hub setup is very simple, it doesn't need two different
hashmaps. However, the variable names are still a mix of the two
hashmaps.
This patch updates dns_hub to use the second, more finalized hashmap
variable-names.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/target/Samba4.pm | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index d8722dd..3a468fa 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -390,18 +390,18 @@ sub setup_dns_hub_internal($$$)
$env->{prefix} = $prefix;
$env->{prefix_abs} = $prefix_abs;
- $env->{hostname} = $hostname;
+ $env->{NETBIOSNAME} = $hostname;
- $env->{ipv4} = Samba::get_ipv4_addr($hostname);
- $env->{ipv6} = Samba::get_ipv6_addr($hostname);
+ $env->{SERVER_IP} = Samba::get_ipv4_addr($hostname);
+ $env->{SERVER_IPV6} = Samba::get_ipv6_addr($hostname);
$env->{DNS_HUB_LOG} = "$prefix_abs/dns_hub.log";
$env->{RESOLV_CONF} = "$prefix_abs/resolv.conf";
open(RESOLV_CONF, ">$env->{RESOLV_CONF}");
- print RESOLV_CONF "nameserver $env->{ipv4}\n";
- print RESOLV_CONF "nameserver $env->{ipv6}\n";
+ print RESOLV_CONF "nameserver $env->{SERVER_IP}\n";
+ print RESOLV_CONF "nameserver $env->{SERVER_IPV6}\n";
close(RESOLV_CONF);
# use a pipe for stdin in the child processes. This allows
@@ -434,7 +434,7 @@ sub setup_dns_hub_internal($$$)
}
$ENV{MAKE_TEST_BINARY} = "$self->{srcdir}/selftest/target/dns_hub.py";
push (@args, "$self->{server_maxtime}");
- push (@args, "$env->{ipv4}");
+ push (@args, "$env->{SERVER_IP}");
push (@args, Samba::realm_to_ip_mappings());
close($env->{STDIN_PIPE});
open STDIN, ">&", $STDIN_READER or die "can't dup STDIN_READER to STDIN: $!";
--
2.7.4
From a5985536b333345f6d8f02d7f5eb744c12000567 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 13 Mar 2019 10:30:25 +1300
Subject: [PATCH 6/6] selftest: Remove unnecessary dns_hub hashmap entries
These are only used within the function, and there's already a local
variable that stores the same info.
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
selftest/target/Samba4.pm | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 3a468fa..50835ef 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -387,9 +387,6 @@ sub setup_dns_hub_internal($$$)
}
my $env = undef;
- $env->{prefix} = $prefix;
- $env->{prefix_abs} = $prefix_abs;
-
$env->{NETBIOSNAME} = $hostname;
$env->{SERVER_IP} = Samba::get_ipv4_addr($hostname);
@@ -443,7 +440,7 @@ sub setup_dns_hub_internal($$$)
or die("Unable to start $ENV{MAKE_TEST_BINARY}: $!");
}
$env->{SAMBA_PID} = $pid;
- $env->{KRB5_CONFIG} = "${prefix_abs}/no_krb5.conf";
+ $env->{KRB5_CONFIG} = "$prefix_abs/no_krb5.conf";
close($STDIN_READER);
print "DONE\n";
--
2.7.4
More information about the samba-technical
mailing list