Re: [Freeipa-devel] [PATCH 0163] server-del: handle missing server attributes when checking for last of role

2016-06-22 Thread Martin Basti



On 22.06.2016 15:47, Martin Babinsky wrote:

On 06/22/2016 03:29 PM, Martin Babinsky wrote:

On 06/22/2016 03:13 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5960





self-NACK, this check needs some more hardening.



This patch should fix the issue in more thorough way.




ACK

master:
* be3ad1ed7a34e90c7107380bb2939f737306ba77 server-del: harden check for 
last roles
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0163] server-del: handle missing server attributes when checking for last of role

2016-06-22 Thread Martin Babinsky

On 06/22/2016 03:29 PM, Martin Babinsky wrote:

On 06/22/2016 03:13 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5960





self-NACK, this check needs some more hardening.



This patch should fix the issue in more thorough way.

--
Martin^3 Babinsky
From 5aeb965494b0b5758029a524d232a4090b059713 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 22 Jun 2016 15:08:43 +0200
Subject: [PATCH] server-del: harden check for last roles

The current implementation of check for last CA/DNS server and DNSSec key
master in `server-del` is quite fragile and wroks with quite a few assumptions
which may not be always true (CA and DNS is always configured etc.).

This patch hardens the check so that it does not break when the above
assuptions do not hold.

https://fedorahosted.org/freeipa/ticket/5960
---
 ipaserver/plugins/server.py | 68 -
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index 42bcb393f21ca802c2a98a3674f9649e6ede446f..cc53a189be997dc128b78ef2b70df915985f36b3 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -469,7 +469,6 @@ class server_del(LDAPDelete):
 raise errors.ServerRemovalError(reason=_(msg))
 
 ipa_config = self.api.Command.config_show()['result']
-dns_config = self.api.Command.dnsconfig_show()['result']
 
 ipa_masters = ipa_config['ipa_master_server']
 
@@ -477,26 +476,43 @@ class server_del(LDAPDelete):
 if ipa_masters == [hostname]:
 return
 
-ca_servers = ipa_config['ca_server_server']
-ca_renewal_master = ipa_config['ca_renewal_master_server']
-dns_servers = dns_config['dns_server_server']
-dnssec_keymaster = dns_config['dnssec_key_master_server']
-
-if ca_servers == [hostname]:
-raise errors.ServerRemovalError(
-reason=_("Deleting this server is not allowed as it would "
- "leave your installation without a CA."))
-
-if dnssec_keymaster == hostname:
-handler(
-_("Replica is active DNSSEC key master. Uninstall "
-  "could break your DNS system. Please disable or "
-  "replace DNSSEC key master first."), ignore_last_of_role)
-
-if dns_servers == [hostname]:
-handler(
-_("Deleting this server will leave your installation "
-  "without a DNS."), ignore_last_of_role)
+if self.api.Command.dns_is_enabled()['result']:
+dns_config = self.api.Command.dnsconfig_show()['result']
+
+dns_servers = dns_config.get('dns_server_server', [])
+dnssec_keymaster = dns_config.get('dnssec_key_master_server', [])
+
+if dnssec_keymaster == hostname:
+handler(
+_("Replica is active DNSSEC key master. Uninstall "
+  "could break your DNS system. Please disable or "
+  "replace DNSSEC key master first."), ignore_last_of_role)
+
+if dns_servers == [hostname]:
+handler(
+_("Deleting this server will leave your installation "
+  "without a DNS."), ignore_last_of_role)
+
+if self.api.Command.ca_is_enabled()['result']:
+ca_servers = ipa_config.get('ca_server_server', [])
+ca_renewal_master = ipa_config.get(
+'ca_renewal_master_server', [])
+
+if ca_servers == [hostname]:
+raise errors.ServerRemovalError(
+reason=_("Deleting this server is not allowed as it would "
+ "leave your installation without a CA."))
+
+if ca_renewal_master == hostname:
+other_cas = [ca for ca in ca_servers if ca != hostname]
+
+# if this is the last CA there is no other server to become
+# renewal master
+if not other_cas:
+return
+
+self.api.Command.config_mod(
+ca_renewal_master_server=other_cas[0])
 
 if ignore_last_of_role:
 self.add_message(
@@ -504,16 +520,6 @@ class server_del(LDAPDelete):
 message=_("Ignoring these warnings and proceeding with "
   "removal")))
 
-if ca_renewal_master == hostname:
-other_cas = [ca for ca in ca_servers if ca != hostname]
-
-# if this is the last CA there is no other server to become renewal
-# master
-if not other_cas:
-return
-
-self.api.Command.config_mod(ca_renewal_master_server=other_cas[0])
-
 def _check_topology_connectivity(self, topology_connectivity, master_cn):
 try:
 topology_connectivity.check_current_state()
-- 
2.5.5

-- 
Manage your 

Re: [Freeipa-devel] [PATCH 0163] server-del: handle missing server attributes when checking for last of role

2016-06-22 Thread Martin Babinsky

On 06/22/2016 03:13 PM, Martin Babinsky wrote:

https://fedorahosted.org/freeipa/ticket/5960





self-NACK, this check needs some more hardening.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code