[PATCH] pykickstart realm support

Vratislav Podzimek vpodzime at redhat.com
Fri Nov 30 12:02:51 UTC 2012


On Wed, 2012-11-21 at 09:46 +0100, Stef Walter wrote:
> I've been working on more tight integration between realmd and
> kickstart. The big use case is automatic AD domain joins by adding a
> line to a kickstart file.
> 
> The line in the kickstart file looks like:
> 
> realm join domain.example.com
> 
> realm join --one-time-password domain.example.com
> 
> Attached is the first set of patches to pykickstart which implements
> support for the realm command.
> 
> I'd like to see this feature make it into RHEL7. As I understand it that
> would mean inclusion in Fedora 19 and backporting to RHEL 7. I'm happy
> to help with that, and I don't mean to just drop this off in your guys
> lap :)
> 
> So attached are two patches. One which adds support for Fedora 19 to
> pykickstart. I hope I've done this right. I'm pretty new to this. The
> other patch adds the realm command, and tests.
0001-Bump-for-Fedora-19-development.patch looks good to me.

Comments for 0002-Add-realm-command-to-join-domains-and-realms.patch
follow as inline comments:
> diff --git a/pykickstart/commands/__init__.py b/pykickstart/commands/__init__.py
> index 0e7d0d1..d8c71f3 100644
> --- a/pykickstart/commands/__init__.py
> +++ b/pykickstart/commands/__init__.py
> @@ -21,6 +21,6 @@ import authconfig, autopart, autostep, bootloader, btrfs, clearpart, device
>  import deviceprobe, displaymode, dmraid, driverdisk, fcoe, firewall, firstboot
>  import group, ignoredisk, interactive, iscsi, iscsiname, key, keyboard, lang
>  import langsupport, lilocheck, logging, logvol, mediacheck, method, monitor
> -import mouse, multipath, network, partition, raid, reboot, repo, rescue, rootpw
> +import mouse, multipath, network, partition, raid, realm, reboot, repo, rescue, rootpw
>  import selinux, services, skipx, sshpw, timezone, updates, upgrade, user
>  import unsupported_hardware, vnc, volgroup, xconfig, zerombr, zfcp
> diff --git a/pykickstart/commands/realm.py b/pykickstart/commands/realm.py
> new file mode 100644
> index 0000000..cd9726a
> --- /dev/null
> +++ b/pykickstart/commands/realm.py
> @@ -0,0 +1,99 @@
> +#
> +# Stef Walter <stefw at redhat.com>
> +#
> +# Copyright 2012 Red Hat, Inc.
> +#
> +# This copyrighted material is made available to anyone wishing to use, modify,
> +# copy, or redistribute it subject to the terms and conditions of the GNU
> +# General Public License v.2.  This program is distributed in the hope that it
> +# will be useful, but WITHOUT ANY WARRANTY expressed or implied, including the
> +# implied warranties 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, write to the Free Software Foundation, Inc., 51
> +# Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.  Any Red Hat
> +# trademarks that are incorporated in the source code or documentation are not
> +# subject to the GNU General Public License and may only be used or replicated
> +# with the express permission of Red Hat, Inc.
> +#
> +
> +from pykickstart.base import *
> +
> +import getopt
> +import pipes
> +import shlex
> +
> +_ = lambda x: gettext.ldgettext("pykickstart", x)
> +
> +class F19_Realm(KickstartCommand):
> +    removedKeywords = KickstartCommand.removedKeywords
> +    removedAttrs = KickstartCommand.removedAttrs
> +
> +    def __init__(self, writePriority=0, *args, **kwargs):
> +        KickstartCommand.__init__(self, *args, **kwargs)
> +        self.join_realm = None
> +        self.join_args = [ ]
> +        self.discover_options = [ ]
> +        self.after = [ ]
We use [] for empty lists.

> +
> +    def _parseArguments(self, string):
> +        args = shlex.split(string)
> +        if not args:
> +            raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Missing realm command arguments"))
> +        command = args and args.pop(0)
You don't have to test 'args' twice.

> +        if command in ("permit", "deny"):
> +            self._parsePermitOrDeny(command, args)
> +        elif command in ("join"):
> +            self._parseJoin(args)
This could be just 'if command == "join"'.

> +        else:
> +            raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Unsupported realm '%s' command" % command))
> +
> +    def _parsePermitOrDeny(self, command, args):
> +        try:
> +            opts, remaining = getopt.getopt(args, "av", ("all", "verbose"))
> +            self.after.append((command, args))
> +        except getopt.GetoptError, ex:
> +            raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Invalid realm arguments: %s") % str(ex))
> +
> +    def _parseJoin(self, args):
> +        if self.join_realm:
> +            raise KickstartParseError, formatErrorMsg(self.lineno, msg=_("The realm command 'join' should only be specified once"))
> +
> +        try:
> +            # We only support these args
> +            opts, remaining = getopt.getopt(args, "v", ("client-software=", "server-software=", "membership-software=",
> +                                                        "one-time-password=", "no-password=", "verbose", "computer-ou="))
Have 'v' and 'verbose' any sense for use in kickstart/anaconda?

> +        except getopt.GetoptError, ex:
> +            raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Invalid realm arguments: %s") % str(ex))
> +
> +        if len(remaining) != 1:
> +            raise KickstartValueError, formatErrorMsg(self.lineno, msg=_("Specify one realm to join"))
> +
> +        # Parse successful, just use this as the join command
> +        self.join_realm = remaining[0]
> +        self.join_args = args
> +
> +        # Build a discovery command
> +        self.discover_options = []
> +        for (o, a) in opts:
> +            if o in ("--client-software", "--server-software", "--membership-software"):
> +                self.discover_options.append("%s=%s" % (o, a))
> +
> +    def __str__(self):
> +        retval = KickstartCommand.__str__(self)
> +
> +        if self.join_args or self.after:
> +            retval += "# Realm or domain membership\n"
> +        if self.join_args:
> +            args = [pipes.quote(arg) for arg in self.join_args]
> +            retval += "realm join " + " ".join(self.join_args) + "\n"
> +        for (command, args) in self.after:
> +            args = [pipes.quote(arg) for arg in args]
> +            retval += "realm " + command + " " + " ".join(args) + "\n"
> +
> +        return retval
Could you please split the __str__ function to the code adding the comment and
the "realm" string and adding and calling _getArgsAsStr method as we do in many
other commands (e.g. timezone)? It's much better in case you want to add the
F20_Realm class inheriting from F19_Realm later.

> +
> +    def parse(self, args):
> +        self._parseArguments(self.currentLine[len(self.currentCmd):].strip())
> +        return self
> diff --git a/pykickstart/handlers/control.py b/pykickstart/handlers/control.py
> index 85f499c..3cda30c 100644
> --- a/pykickstart/handlers/control.py
> +++ b/pykickstart/handlers/control.py
> @@ -982,6 +982,7 @@ commandMap = {
>          "partition": partition.F18_Partition,
>          "poweroff": reboot.F18_Reboot,
>          "raid": raid.F18_Raid,
> +        "realm": realm.F19_Realm,
>          "reboot": reboot.F18_Reboot,
>          "repo": repo.F15_Repo,
>          "rescue": rescue.F10_Rescue,
> diff --git a/tests/commands/realm.py b/tests/commands/realm.py
> new file mode 100644
> index 0000000..3f47235
> --- /dev/null
> +++ b/tests/commands/realm.py
> @@ -0,0 +1,54 @@
> +import unittest, shlex
> +import warnings
> +from tests.baseclass import *
> +
> +from pykickstart.errors import *
> +from pykickstart.commands.realm import *
> +
> +class F19_TestCase(CommandTest):
> +    command = "realm"
> +
> +    def runTest(self):
> +
> +        # No realm command arguments
> +        self.assert_parse_error("realm", KickstartValueError)
> +
> +        # Unsupported realmcommand
> +        self.assert_parse_error("realm unknown --args", KickstartValueError)
> +
> +        # pass for join
> +        realm = self.assert_parse("realm join blah")
> +        self.assertEquals(realm.join_realm, "blah")
> +        self.assertEquals(realm.join_args, ["blah"])
> +        self.assertEquals(realm.discover_options, [])
> +        self.assertEquals(realm.after, [])
> +        self.assertEquals(str(realm), "# Realm or domain membership\nrealm join blah\n")
> +
> +        # pass for join with client-software
> +        realm = self.assert_parse("realm join --client-software=sssd --verbose domain.example.com")
> +        self.assertEquals(realm.join_realm, "domain.example.com")
> +        self.assertEquals(realm.join_args, ["--client-software=sssd", "--verbose", "domain.example.com"])
> +        self.assertEquals(realm.discover_options, ["--client-software=sssd"])
> +        self.assertEquals(realm.after, [])
> +        self.assertEquals(str(realm), "# Realm or domain membership\nrealm join --client-software=sssd --verbose domain.example.com\n")
> +
> +        # Bad arguments, only one domain for join
> +        self.assert_parse_error("realm join one two", KickstartValueError)
> +
> +        # Bad arguments, unsupported argument
> +        self.assert_parse_error("realm join --user=blah one.example.com", KickstartValueError)
> +
> +        # Bad arguments, unsupported argument
> +        self.assert_parse_error("realm join --user=blah one.example.com", KickstartValueError)
> +
> +        # pass
> +        realm = self.assert_parse("realm permit -a")
> +        self.assertEquals(realm.join_realm, None)
> +        self.assertEquals(realm.after, [("permit", ["-a"])])
> +        self.assertEquals(str(realm), "# Realm or domain membership\nrealm permit -a\n")
> +
> +        # Bad arguments to permit or deny
> +        self.assert_parse_error("realm permit --bad-argument", KickstartValueError)
> +
> +if __name__ == "__main__":
> +    unittest.main()
> 
Otherwise it looks good to me, but as I'm discussing with Stef and Martin Sivák, we could probably
have this as a separate addon not part of pykickstart (and the same goes for the anaconda part).

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list