Re: [Freeipa-devel] [PATCH 003] Use 'remove-ds.pl' to remove DS instance during server uninstall
On 22/01/15 15:03, Martin Babinsky wrote:
On 01/22/2015 12:38 PM, Martin Babinsky wrote:
On 01/22/2015 12:19 PM, Martin Kosek wrote:
On 01/22/2015 11:58 AM, Martin Babinsky wrote:
On 01/22/2015 11:04 AM, Martin Babinsky wrote:
The attached patch addresses
https://fedorahosted.org/freeipa/ticket/4487.
Using 'remove-ds.pl' script from 389-ds during server/replica
uninstallation should allow for cleaner removal of DS instance
with no
leftovers (opened ports etc).
Martin^3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Thanks to Martin^2 for explaining the rules governing the placement
of new
attributes into BasePathNamespace class.
Attaching updated patch.
Martin^3
I see you kept erase_ds_instance_data still in the dsinstance. What is
the
motivation? I thought that just like with PKI, with DS we will also
have
uninstall based solely only on the DS uninstall script and remove our
custom
removal.
We can keep the manual removal somewhere in wiki, but I would
personally not
keep/maintain it in FreeIPA code.
I originally thought that I will keep erase_ds_instance-data as a
failsafe method to clean up the dirsrv data in th case that remove-ds.pl
fails.
But I will remove the method altogether and change the code so that it
prints out some warning about manual removal of DS data when
remove-ds.pl fails.
Martin^2
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Updated patch attached
Martin^3
Hi,
thank you for patch, but I have a few nitpicks:
1) Extra parenthesis
+root_logger.error((Failed to remove CA DS instance.
You may
+ need to remove instance data
manually))
and (twice)
+root_logger.error((Failed to remove DS instance. You may
+ need to remove instance data
manually))
and
+root_logger.debug(('%s' failed.
+ Attempting to force removal %
paths.REMOVE_DS_PL))
2) Remove unused paths:
USR_LIB_DIRSRV_SLAPD_INSTANCE_DIR_TEMPLATE
SLAPD_INSTANCE_LOCK_TEMPLATE
USR_LIB_SLAPD_INSTANCE_TEMPLATE
Please do double check.
3) IMO we should avoid hardcoded value 'slapd'
instance_name = '-'.join(['slapd', serverid])
you may create module variable DS_INSTANCE_PREFIX and use it. Dont
forget to change it in following place as well.
ipaserver/install/dsinstance.py:117:instance_prefix = 'slapd-'
4)
DS keytab hasn't been removed, it is okay?
It is created by IPA (krbinstance), not by DS install script.
Martin^2
--
Martin Basti
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
Dne 23.1.2015 v 10:13 Martin Basti napsal(a): On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Makes sense. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Please do. It's an ACK then. Is this supposed to go in ipa-4-1 too or is it master only? -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
Dne 23.1.2015 v 10:25 Martin Basti napsal(a): On 23/01/15 10:23, Jan Cholasta wrote: Dne 23.1.2015 v 10:13 Martin Basti napsal(a): On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Makes sense. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Please do. It's an ACK then. Is this supposed to go in ipa-4-1 too or is it master only? Both please, thank you. Pushed to: master: 46c12159e6c27082e7bc46e96d3738eea68dba91 ipa-4-1: 64cf3071ca908b22e5ad402585d9690c1a7fc518 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Honza -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 0187, 0188] DNSSEC ipa-dnskeysyncd fixes
On 23/01/15 10:23, Jan Cholasta wrote: Dne 23.1.2015 v 10:13 Martin Basti napsal(a): On 23/01/15 08:04, Jan Cholasta wrote: Hi, Dne 21.1.2015 v 13:39 Martin Basti napsal(a): Patch 188 catch ldap exceptions to prevent false positive abrt reports Patch 187 fixes issues with removing root zone Patches attached. Patch 187: Is there a reason only SERVER_DOWN and CONNECT_ERROR are handled, instead of any LDAPError? These are expected during IPA restart/start etc. Other ldap exceptions should not happen, so we can get abrt reports from users, if something is wrong. Makes sense. Patch 188: IMO it would be slightly better to do it like this: -name = name.relativize(dns.name.root) +if name != dns.name.root: +name = name.relativize(dns.name.root) This will not work. There is relativization for some zones before this step. I will try to clean the mess I found now in a new patch. Please do. It's an ACK then. Is this supposed to go in ipa-4-1 too or is it master only? Both please, thank you. -- Martin Basti ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0184] Always return absolute idnsname in dnszone commands
On 23/01/15 08:22, Jan Cholasta wrote:
Dne 20.1.2015 v 12:49 Martin Basti napsal(a):
On 15/01/15 16:07, Jan Cholasta wrote:
Dne 15.1.2015 v 15:39 Martin Basti napsal(a):
On 15/01/15 15:07, Jan Cholasta wrote:
Dne 15.1.2015 v 14:58 Martin Basti napsal(a):
On 15/01/15 14:25, Jan Cholasta wrote:
Hi,
Dne 15.1.2015 v 13:27 Martin Basti napsal(a):
On 15/01/15 13:17, Martin Basti wrote:
https://fedorahosted.org/freeipa/ticket/4722
Patch attached.
Fast fix.
Updated patch attached.
1) Forward zone commands are not fixed.
FWzones are new and always normalized to absolute name in ldap
Would you bet your money on that? Better be safe than sorry,
especially when it's just a matter of moving the code around (right?)
2) It seems that the primary key returned by -mod, -del and -show
(.result.value) is made absolute somewhere else in the code.
Would it
be possible to do it in one place?
IMO it is not possible.
Value is generated from key, and key is normalized to absolute zone
before calling execute.
LDAPUpdate:
...
if self.obj.primary_key:
pkey = keys[-1]
else:
pkey = None
return dict(result=entry_attrs, value=pkey_to_value(pkey,
options))
The idnsname attribute is just taken from LDAP without any
normalization
Right.
3) Attribute values returned from LDAP are never None, so the if
should be if 'idnsname' in entry_attrs:.
Ok I will revert the change I made.
4) If idnsname always has only single value, use
entry_attrs.single_value['idnsname'] =
entry_attrs.single_value['idnsname'].make_absolute()
Thanks
Honza
Updated patch attached.
Updated patch attached.
Thanks.
Is there a reason why you put the _make_zone_absolute calls in
dnszone_* and dnsforwardzone_* instead of DNSZoneBase_*?
I moved callback into Base classes.
Patch attached.
1) Multi-line statements should generally use parentheses for implicit
line continuation rather than backslashes:
+entry_attrs.single_value['idnsname'] = (
+ entry_attrs.single_value['idnsname'].make_absolute())
2) You can remove the DN asserts from dnszone_*, they are already
asserted in DNSZoneBase_*.
3) Do not just ignore return values of super().post_callback():
def post_callback(self, something, ...):
something = super(...).post_callback(something, ...)
...
return something
Updated patch attached.
--
Martin Basti
From 2cfa5856aa5276569856acefb7dd03c1feced04e Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 15 Jan 2015 13:13:55 +0100
Subject: [PATCH] Always return absolute idnsname in dnszone commands
Ticket: https://fedorahosted.org/freeipa/ticket/4722
---
ipalib/plugins/dns.py | 36 ++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 7a80036c94432a01ea8781101712ea1135134948..9dc3ed0b021b7d9bb42053a48690047bd7a244a2 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2081,6 +2081,18 @@ class DNSZoneBase(LDAPObject):
except errors.NotFound:
raise e # re-raise original exception
+def _make_zonename_absolute(self, entry_attrs, **options):
+
+Zone names can be relative in IPA 4.0, make sure we always return
+absolute zone name from ldap
+
+if options.get('raw'):
+return
+
+if idnsname in entry_attrs:
+entry_attrs.single_value['idnsname'] = (
+entry_attrs.single_value['idnsname'].make_absolute())
+
class DNSZoneBase_add(LDAPCreate):
@@ -2128,6 +2140,11 @@ class DNSZoneBase_del(LDAPDelete):
class DNSZoneBase_mod(LDAPUpdate):
has_output_params = LDAPUpdate.has_output_params + dnszone_output_params
+def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+assert isinstance(dn, DN)
+self.obj._make_zonename_absolute(entry_attrs, **options)
+return dn
+
class DNSZoneBase_find(LDAPSearch):
__doc__ = _('Search for DNS zones (SOA records).')
@@ -2162,6 +2179,11 @@ class DNSZoneBase_find(LDAPSearch):
filter = _create_idn_filter(self, ldap, *args, **options)
return (filter, base_dn, scope)
+def post_callback(self, ldap, entries, truncated, *args, **options):
+for entry_attrs in entries:
+self.obj._make_zonename_absolute(entry_attrs, **options)
+return truncated
+
class DNSZoneBase_show(LDAPRetrieve):
has_output_params = LDAPRetrieve.has_output_params + dnszone_output_params
@@ -2172,6 +2194,11 @@ class DNSZoneBase_show(LDAPRetrieve):
self.obj.handle_not_found(*keys)
return dn
+def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+assert isinstance(dn, DN)
+self.obj._make_zonename_absolute(entry_attrs, **options)
+return dn
+
class DNSZoneBase_disable(LDAPQuery):
has_output =
