Re: [Freeipa-devel] [PATCH] Fix DNS plugin: proper output definitions, --all, dns-add-rr overwritting
Pavel Zůna wrote: On 4/13/2010 10:51 PM, Rob Crittenden wrote: Pavel Zuna wrote: The DNS plugin is getting old, tired and already looking forward to his pension in the Carribean. It will be replaced soon by a younger, faster, safer, shorter (in terms of code) and more maintainable version. Until that happens, here's some medicine for the old guy: - proper output definitions: the DNS plugin was created before we had the has_output attribute in place - --all: this is related to the output definitions as Command.get_options() adds the --all and --raw options automatically if has_output contains entries - dns-add-rr overwritting: missing .lower() caused records to be overwritten every time a new one was added from the CLI Pavel This looks ok but I wonder why you are defining your own Output definition instead of using the standard? The only difference seems to be that your custom one doesn't have a summary. rob Because the standard output definitions with entries make Command plugins automatically add the --all and --raw options. dns-*-rr commands aren't comfortable with it. Can you be more specific? What doesn't work? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix DNS plugin: proper output definitions, --all, dns-add-rr overwritting
On 4/14/2010 5:36 PM, Rob Crittenden wrote:
Pavel Zůna wrote:
On 4/13/2010 10:51 PM, Rob Crittenden wrote:
Pavel Zuna wrote:
The DNS plugin is getting old, tired and already looking forward to his
pension in the Carribean. It will be replaced soon by a younger,
faster,
safer, shorter (in terms of code) and more maintainable version.
Until that happens, here's some medicine for the old guy:
- proper output definitions: the DNS plugin was created before we
had the has_output attribute in place
- --all: this is related to the output definitions as
Command.get_options() adds the --all and --raw options automatically
if has_output contains entries
- dns-add-rr overwritting: missing .lower() caused records to be
overwritten every time a new one was added from the CLI
Pavel
This looks ok but I wonder why you are defining your own Output
definition instead of using the standard? The only difference seems to
be that your custom one doesn't have a summary.
rob
Because the standard output definitions with entries make Command
plugins automatically add the --all and --raw options. dns-*-rr
commands aren't comfortable with it.
Can you be more specific? What doesn't work?
rob
There were conflicts with --all being defined explicitly by some of the
commands. Also, dns-del-rr didn't expect any options and raised an
exception when it received the automatically added --all/--raw.
Anyway, I fixed those issues, so that we can use the standard
definitions from ipalib/output.py. I guess I got lazy before or just
wasn't thinking about it too much. :) Modified patch attached.
Pavel
From 6073a12c78c4702916c7de4c5115a7ea1c62cdca Mon Sep 17 00:00:00 2001
From: Pavel Zuna pz...@redhat.com
Date: Tue, 30 Mar 2010 18:56:02 +0200
Subject: [PATCH] Fix DNS plugin: proper output definitions, --all, dns-add-rr
overwritting
The DNS plugin is getting old, tired and already looking forward to his
pension in the Carribean. It will be replaced soon by a younger, faster,
safer, shorter (in terms of code) and more maintainable version.
Until that happens, here's some medicine for the old guy:
- proper output definitions: the DNS plugin was created before we
had the has_output attribute in place
- --all: this is related to the output definitions as
Command.get_options() adds the --all and --raw options automatically
if has_output contains entries
- dns-add-rr overwritting: missing .lower() caused records to be
overwritten everytime a new one was added from the CLI
---
ipalib/plugins/dns.py | 29 +++--
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 5f6949a..4c81a8e 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -67,6 +67,7 @@ from ipalib import api, crud, errors, output
from ipalib import Object, Command
from ipalib import Flag, Int, Str, StrEnum
from ipalib import _, ngettext
+from ipalib.output import Output, standard_entry, standard_list_of_entries
# parent DN
_zone_container_dn = api.env.container_dns
@@ -310,7 +311,7 @@ class dns_find(crud.Search):
filter = ldap.make_filter_from_attr('idnsname', term, exact=False)
# select attributes we want to retrieve
-if options['all']:
+if options.get('all', False):
attrs_list = ['*']
else:
attrs_list = _zone_default_attributes
@@ -362,7 +363,7 @@ class dns_show(crud.Retrieve):
dn = _get_zone_dn(ldap, idnsname)
# select attributes we want to retrieve
-if options['all']:
+if options.get('all', False):
attrs_list = ['*']
else:
attrs_list = _zone_default_attributes
@@ -492,11 +493,11 @@ class dns_add_rr(Command):
),
)
-has_output = output.standard_entry
+has_output = standard_entry
def execute(self, zone, idnsname, type, data, **options):
ldap = self.api.Backend.ldap2
-attr = '%srecord' % type
+attr = ('%srecord' % type).lower()
# build entry DN
dn = _get_record_dn(ldap, zone, idnsname)
@@ -593,11 +594,11 @@ class dns_del_rr(Command):
),
)
-has_output = output.standard_entry
+has_output = standard_entry
-def execute(self, zone, idnsname, type, data):
+def execute(self, zone, idnsname, type, data, **options):
ldap = self.api.Backend.ldap2
-attr = '%srecord' % type
+attr = ('%srecord' % type).lower()
# build entry DN
dn = _get_record_dn(ldap, zone, idnsname)
@@ -635,9 +636,9 @@ class dns_del_rr(Command):
(dn, entry_attrs) = ldap.get_entry(dn, ['idnsname', attr])
entry_attrs['dn'] = dn
-return dict(result=result, value=idnsname)
+return dict(result=entry_attrs, value=idnsname)
-def output_for_cli(self, textui, result, zone, idnsname, type, data):
+def output_for_cli(self, textui, result, zone, idnsname, type, data,
Re: [Freeipa-devel] Use ldap2 instead of legacy LDAP code from v1 in installer scripts.
On 4/14/2010 4:35 PM, Rob Crittenden wrote:
Pavel Zuna wrote:
On 03/30/2010 10:27 PM, Rob Crittenden wrote:
Pavel Zuna wrote:
On 03/23/2010 09:40 PM, Rob Crittenden wrote:
Pavel Zuna wrote:
This is the first in a series of patches, that replace all the legacy
code from v1 related to LDAP. I did some limited testing of the
installer after this patch and nothing seems to break, but I
didn't do
replicas etc...
Pavel
nack. This breaks at least ipa-replica-manage, ipa-replica-prepare,
ipa-server-certinstall and ipa-replica-install.
rob
Fixed patch attached.
Pavel
I'm not sure if you attached the wrong patch or not (it's dated 3/24)
but things are still not working:
# ipa-replica-install replica-info-tiger.example.com.gpg
Directory Manager (existing master) password:
creation of replica failed: 'Env' object has no attribute 'basedn'
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.
rob
Sorry for a late reply. Here's a patch that should finally work. I did
a lot more testing and setting up a replica went smoothly every time.
Pavel
Lots better. I was able to create and manage replicas but
ipa-dns-install isn't working:
# ipa-dns-install
The log file for this installation can be found in
/var/log/ipaserver-install.log
==
This program will setup DNS for the FreeIPA Server.
This includes:
* Configure DNS (bind)
To accept the default shown in brackets, press the Enter key.
Existing BIND configuration detected, overwrite? [no]: y
Do you wish to configure DNS forwarders? [no]:
No DNS forwarders configured
Directory Manager password:
Unexpected error - see ipaserver-install.log for details:
'API' object has no attribute 'env_host'
Ouch, sorry about that. New patch attached.
Pavel
From 6f1e71d1ad926b827d43c4dbcab768ecaa675389 Mon Sep 17 00:00:00 2001
From: Pavel Zuna pz...@redhat.com
Date: Wed, 24 Mar 2010 15:51:31 +0100
Subject: [PATCH] Use ldap2 instead of legacy LDAP code from v1 in installer
scripts.
---
install/tools/ipa-compat-manage | 38 ++--
install/tools/ipa-dns-install| 18 +-
install/tools/ipa-fix-CVE-2008-3274 | 63 +++--
install/tools/ipa-ldap-updater |2 -
install/tools/ipa-nis-manage | 44 +++
install/tools/ipa-replica-install| 22 ++--
install/tools/ipa-replica-manage |8 ++--
install/tools/ipa-replica-prepare| 33 -
install/tools/ipa-server-certinstall | 18 -
install/tools/ipa-server-install | 24 ++---
ipaserver/plugins/ldap2.py | 22 +---
11 files changed, 144 insertions(+), 148 deletions(-)
diff --git a/install/tools/ipa-compat-manage b/install/tools/ipa-compat-manage
index 09a06ca..b22ce77 100755
--- a/install/tools/ipa-compat-manage
+++ b/install/tools/ipa-compat-manage
@@ -22,12 +22,11 @@
import sys
try:
from optparse import OptionParser
-from ipaserver import ipaldap
from ipapython import entity, ipautil, config
from ipaserver.install import installutils
from ipaserver.install.ldapupdate import LDAPUpdate, BadSyntax, UPDATES_DIR
+from ipaserver.plugins.ldap2 import ldap2
from ipalib import errors
-import ldap
import logging
import re
import krbV
@@ -95,26 +94,29 @@ def main():
else:
dirman_password = get_dirman_password()
+conn = None
try:
+ldapuri = 'ldap://%s' % installutils.get_fqdn()
try:
-conn = ipaldap.IPAdmin(installutils.get_fqdn())
-conn.do_simple_bind(bindpw=dirman_password)
-except ldap.LDAPError, e:
+conn = ldap2(shared_instance=False, ldap_uri=ldapuri, base_dn='')
+conn.connect(
+bind_dn='cn=directory manager', bind_pw=dirman_password
+)
+except errors.LDAPError, e:
print An error occurred while connecting to the server.
-print %s % e[0]['desc']
+print e
return 1
if args[0] == enable:
try:
-conn.getEntry(cn=Schema Compatibility,cn=plugins,cn=config,
- ldap.SCOPE_BASE, (objectclass=*))
+conn.get_entry('cn=Schema Compatibility,cn=plugins,cn=config')
print Plugin already Enabled
retval = 2
except errors.NotFound:
print Enabling plugin
-except ldap.LDAPError, e:
+except errors.LDAPError, e:
print An error occurred while talking to the server.
-print %s % e[0]['desc']
+print e
retval = 1
if retval == 0:
@@ -127,17 +129,15 @@ def main():
# Make a quick hack foir now, directly delete the entries by name,
# In future we should add
[Freeipa-devel] [PATCH] Fix ipa-dns-install. It was failing when DNS was reinstalling.
I noticed a few bugs when DNS was reinstalling: - Service.move_service returned None, because the service entry was already in the right place - BindInstance didn't expect that. - We were passing a unicode string to python-ldap although we know it hates that. - We were catching all exception alike when modifying the dnsserver role group. It's no longer an error if the DNS principal is already present. I think Martin has some work in progess on the bindinstance.py file, so please don't push until he acks it. He might want to included these changes in his own patch. I had to fix these to test my own code in the installer and posted the patch to point out the bugs. Pavel From 2deba7ac45bb8dc2c52afb9fa7ecedb1d867fcbf Mon Sep 17 00:00:00 2001 From: Pavel Zuna pz...@redhat.com Date: Wed, 14 Apr 2010 18:52:12 +0200 Subject: [PATCH] Fix ipa-dns-install. It was failing when DNS was reinstalling. --- ipaserver/install/bindinstance.py | 11 +-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 105cf4e..ff1e4e4 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -263,7 +263,12 @@ class BindInstance(service.Service): # Store the keytab on disk self.fstore.backup_file(/etc/named.keytab) installutils.create_keytab(/etc/named.keytab, dns_principal) -dns_principal = self.move_service(dns_principal) +p = self.move_service(dns_principal) +if p is None: +# the service has already been moved, perhaps we're doing a DNS reinstall +dns_principal = krbprincipalname=%s,cn=services,cn=accounts,%s % (dns_principal, self.suffix) +else: +dns_principal = p # Make sure access is strictly reserved to the named user pent = pwd.getpwnam(self.named_user) @@ -284,10 +289,12 @@ class BindInstance(service.Service): raise e dns_group = cn=dnsserver,cn=rolegroups,cn=accounts,%s % self.suffix -mod = [(ldap.MOD_ADD, 'member', dns_principal)] +mod = [(ldap.MOD_ADD, 'member', str(dns_principal))] try: conn.modify_s(dns_group, mod) +except ldap.TYPE_OR_VALUE_EXISTS: +pass except Exception, e: logging.critical(Could not modify principal's %s entry % dns_principal) raise e -- 1.6.6 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fix ipa-dns-install. It was failing when DNS was reinstalling.
Pavel Zůna wrote: I noticed a few bugs when DNS was reinstalling: - Service.move_service returned None, because the service entry was already in the right place - BindInstance didn't expect that. - We were passing a unicode string to python-ldap although we know it hates that. - We were catching all exception alike when modifying the dnsserver role group. It's no longer an error if the DNS principal is already present. I think Martin has some work in progess on the bindinstance.py file, so please don't push until he acks it. He might want to included these changes in his own patch. I had to fix these to test my own code in the installer and posted the patch to point out the bugs. Interesting. Do we want to support re-installing the DNS server? Or should we catch it and exit? Not crashing is definitely a good way to start though :-) rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
