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.
Will follow with anaconda patches, which make the implementation clearer. The basic procedure is: a) discover realm before install and add needed packages b) after install and configuration, join realm.
Should I file RH bugzilla bugs to track this? Happy to do this differently if necessary.
Cheers,
Stef
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@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 = Noneself.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 argsopts, 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 commandself.join_realm = remaining[0]self.join_args = args# Build a discovery commandself.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 selfdiff --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 argumentsself.assert_parse_error("realm", KickstartValueError)# Unsupported realmcommandself.assert_parse_error("realm unknown --args", KickstartValueError)# pass for joinrealm = 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-softwarerealm = 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 joinself.assert_parse_error("realm join one two", KickstartValueError)# Bad arguments, unsupported argumentself.assert_parse_error("realm join --user=blah one.example.com", KickstartValueError)# Bad arguments, unsupported argumentself.assert_parse_error("realm join --user=blah one.example.com", KickstartValueError)# passrealm = 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 denyself.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).
On 11/30/2012 01:02 PM, Vratislav Podzimek wrote:
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:
Thanks for the review. Fixed the various issues you pointed out in the attached patch. A few comments below.
# We only support these argsopts, 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?
No I guess not. We should always force things to be verbose, the output goes to the logs.
- 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 retvalCould 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.
Done. Although this is slightly different because there are multiple possible lines involved. However I believe the updated patch addresses the case you mention above.
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).
In principle I'm fine with this, as long as it's possible to include the addon along with Anaconda for many (most?) distributions in which Anaconda is used.
What's the next step to get this going? How can I help?
Cheers,
Stef
anaconda-patches@lists.fedorahosted.org
-
Stef Walter -
Vratislav Podzimek