[PATCH] Re: [Samba] SYSVOL ACLs and GPOs

[Jelmer Vernooij]

On Mon, Nov 05, 2012 at 01:10:13PM +1100, Andrew Bartlett wrote:
> On Thu, 2012-11-01 at 14:54 +0000, Alex Matthews wrote:
> > On 30/10/2012 00:08, Jeremy Allison wrote:
> > > On Tue, Oct 30, 2012 at 11:00:31AM +1100, Andrew Bartlett wrote:
[...]
> 
> It is certainly very helpful to have this happen with samba-tool.  Can
> you remind me the history of this domain, is it the upgrade I was trying
> to suggest you do, or a fresh provision?
> 
> If you can tell me what provision command-line you run, if it was
> provisioned with an older version, which branch and git revision that
> was and what branch and git revision as you running now?
> 
> I've tried to replicate this in 'make test' but failed (the tests pass).
> The patch for that is attached for review.
Thanks. Yay for more tests; please find some minor notes below:

> diff --git a/source4/scripting/python/samba/tests/samba_tool/gpo.py b/source4/scripting/python/samba/tests/samba_tool/gpo.py
> new file mode 100644
> index 0000000..0cd39dc
> --- /dev/null
> +++ b/source4/scripting/python/samba/tests/samba_tool/gpo.py
> @@ -0,0 +1,59 @@
> +# Unix SMB/CIFS implementation.
> +# Copyright (C) Andrew Bartlett 2012
> +#
> +# based on time.py:
> +# Copyright (C) Sean Dague <sdague at linux.vnet.ibm.com> 2011
> +#
> +# 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/>.
> +#
> +
> +import os
> +from samba.tests.samba_tool.base import SambaToolCmdTest
> +
> +class GpoCmdTestCase(SambaToolCmdTest):
> +    """Tests for samba-tool time subcommands"""
> +
> +    gpo_name = "testgpo"
> +
> +    def test_gpo_list(self):
> +        """Run gpo list against the server and make sure it looks accurate"""
> +        (result, out, err) = self.runsubcmd("gpo", "listall", "-H", "ldap://%s" % os.environ["SERVER"])
> +        self.assertCmdSuccess(result, "Ensuring gpo listall ran successfully")
> +
> +    def test_fetchfail(self):
> +        """Run against a non-existent GPO, and make sure it fails (this hard-coded UUID is very unlikely to exist"""
> +        (result, out, err) = self.runsubcmd("gpo", "fetch", "c25cac17-a02a-4151-835d-fae17446ee43", "-H", "ldap://%s" % 
> +os.environ["SERVER"])
> +        self.assertEquals(result, -1, "check for result code")
> +
> +    def test_fetch(self):
> +        """Run against a real GPO, and make sure it passes"""
> +        (result, out, err) = self.runsubcmd("gpo", "fetch", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"], "--tmpdir", os.environ['SELFTEST_PREFIX'])
> +        self.assertCmdSuccess(result, "Ensuring gpo fetched successfully")
> +
> +    def setUp(self):
> +        """set up a tempoary GPO to work with"""
^^^ temporary :-)


> +        super(GpoCmdTestCase, self).setUp()
> +        (result, out, err) = self.runsubcmd("gpo", "create", self.gpo_name, "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
> +        self.gpo_guid = "{%s}" % out.split("{")[1].split("}")[0]
> +
> +        self.assertCmdSuccess(result, "Ensuring gpo created successfully")
> +
> +    def tearDown(self):
> +        """remote the tempoary GPO to work with"""
^^^ remove, temporary :-)

> +        (result, out, err) = self.runsubcmd("gpo", "del", self.gpo_guid, "-H", "ldap://%s" % os.environ["SERVER"], "-U%s%%%s" % (os.environ["USERNAME"], os.environ["PASSWORD"]))
> +        self.assertCmdSuccess(result, "Ensuring gpo deleted successfully")
> +        super(GpoCmdTestCase, self).tearDown()
> +
> +
> diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
> index ca5bdd3..61540d0 100755
> --- a/source4/selftest/tests.py
> +++ b/source4/selftest/tests.py
> @@ -405,6 +405,8 @@ planpythontestsuite("dc:local", "samba.tests.dcerpc.bare")
>  planpythontestsuite("dc:local", "samba.tests.dcerpc.unix")
>  planpythontestsuite("dc:local", "samba.tests.dcerpc.srvsvc")
>  planpythontestsuite("dc:local", "samba.tests.samba_tool.timecmd")
> +planpythontestsuite("dc:local", "samba.tests.samba_tool.gpo")
> +planpythontestsuite("plugin_s4_dc:local", "samba.tests.samba_tool.gpo")
Do we really need to run these tests against both environments? These
tests ought to be testing that the samba-tool gpo subcommand works
well, not our server side GPO support.

As far as I know the gpo subcommands don't have any different
behaviour for these two environments.

>  planpythontestsuite("dc:local", "samba.tests.samba_tool.processes")
>  planpythontestsuite("dc:local", "samba.tests.samba_tool.user")
>  planpythontestsuite("dc:local", "samba.tests.samba_tool.group")

Cheers,

Jelmer