Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-26 Thread Martin Kosek

On 09/25/2014 05:14 PM, Jan Cholasta wrote:

Dne 25.9.2014 v 16:15 Martin Basti napsal(a):

On 22/09/14 19:30, Martin Basti wrote:

On 19/09/14 14:47, Jan Cholasta wrote:

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service
to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It
should enable the service no matter if the entry existed before or
not.

Similarly, in ldap_disable you should not raise an error when the
entry is not found, because that already makes the service disabled.

Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti


1) ipaConfigString is case-insensitive, so you need to look for the
string enabledService in a case-insensitive way.


2) Don't say failed to enable ... when the service is already
enabled. It is not a failure. Same for disabled.


3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say except
errors.EmptyModlist.



Thank you,

updated patch attached


Updated patch attached. I modified ldap_disable to use search function.



ACK.



Pushed to:
master: 66ce71f17a689bcad03022e3df6bbdf0fada2ab8
ipa-4-1: df9086c938963fdf59309cd8af69f4f5d8a7a34e

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-25 Thread Martin Basti

On 22/09/14 19:30, Martin Basti wrote:

On 19/09/14 14:47, Jan Cholasta wrote:

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):
This patch allows to disable service in LDAP to prevents service 
to be

started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It
should enable the service no matter if the entry existed before or 
not.


Similarly, in ldap_disable you should not raise an error when the
entry is not found, because that already makes the service disabled.

Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti


1) ipaConfigString is case-insensitive, so you need to look for the 
string enabledService in a case-insensitive way.



2) Don't say failed to enable ... when the service is already 
enabled. It is not a failure. Same for disabled.



3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say except 
errors.EmptyModlist.




Thank you,

updated patch attached


Updated patch attached. I modified ldap_disable to use search function.

--
Martin Basti

From 045137a10a7b003f92132c96fa8b341eeaf2d95c Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 28 Aug 2014 19:27:44 +0200
Subject: [PATCH] LDAP disable service

This patch allows to disable service in LDAP (ipactl will not start it)
---
 ipaserver/install/service.py | 67 +++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 8fb802099d7e58a10fd8fb17ff5dc7511eb98c7f..d095fe041bec8098937bf5b99b301a41842ce53e 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -392,6 +392,32 @@ class Service(object):
 self.ldap_connect()
 
 entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+
+# enable disabled service
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+pass
+else:
+if any(u'enabledservice' == val.lower()
+   for val in entry.get('ipaConfigString', [])):
+root_logger.debug(service %s startup entry already enabled, name)
+return
+
+entry.setdefault('ipaConfigString', []).append(u'enabledService')
+
+try:
+self.admin_conn.update_entry(entry)
+except errors.EmptyModlist:
+root_logger.debug(service %s startup entry already enabled, name)
+return
+except:
+root_logger.debug(failed to enable service %s startup entry, name)
+raise
+
+root_logger.debug(service %s startup entry enabled, name)
+return
+
 order = SERVICE_LIST[name][1]
 entry = self.admin_conn.make_entry(
 entry_name,
@@ -404,9 +430,48 @@ class Service(object):
 try:
 self.admin_conn.add_entry(entry)
 except (errors.DuplicateEntry), e:
-root_logger.debug(failed to add %s Service startup entry % name)
+root_logger.debug(failed to add service %s startup entry, name)
 raise e
 
+def ldap_disable(self, name, fqdn, ldap_suffix):
+assert isinstance(ldap_suffix, DN)
+if not self.admin_conn:
+self.ldap_connect()
+
+entry_dn = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'),
+('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+search_kw = {'ipaConfigString': u'enabledService'}
+filter = self.admin_conn.make_filter(search_kw)
+try:
+entries, truncated = self.admin_conn.find_entries(
+filter=filter,
+attrs_list=['ipaConfigString'],
+base_dn=entry_dn,
+scope=self.admin_conn.SCOPE_BASE)
+except errors.NotFound:
+root_logger.debug(service %s startup entry already disabled, name)
+return
+
+assert len(entries) == 1  # only one entry is expected
+entry = entries[0]
+
+# case insensitive
+for value in entry.get('ipaConfigString', []):
+if value.lower() == u'enabledservice':
+entry['ipaConfigString'].remove(value)
+break
+
+try:
+self.admin_conn.update_entry(entry)
+except errors.EmptyModlist:
+pass
+except:
+root_logger.debug(failed to disable service %s startup 

Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-25 Thread Jan Cholasta

Dne 25.9.2014 v 16:15 Martin Basti napsal(a):

On 22/09/14 19:30, Martin Basti wrote:

On 19/09/14 14:47, Jan Cholasta wrote:

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service
to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It
should enable the service no matter if the entry existed before or
not.

Similarly, in ldap_disable you should not raise an error when the
entry is not found, because that already makes the service disabled.

Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti


1) ipaConfigString is case-insensitive, so you need to look for the
string enabledService in a case-insensitive way.


2) Don't say failed to enable ... when the service is already
enabled. It is not a failure. Same for disabled.


3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say except
errors.EmptyModlist.



Thank you,

updated patch attached


Updated patch attached. I modified ldap_disable to use search function.



ACK.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-22 Thread Martin Basti

On 19/09/14 14:47, Jan Cholasta wrote:

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):
This patch allows to disable service in LDAP to prevents service 
to be

started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It
should enable the service no matter if the entry existed before or 
not.


Similarly, in ldap_disable you should not raise an error when the
entry is not found, because that already makes the service disabled.

Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti


1) ipaConfigString is case-insensitive, so you need to look for the 
string enabledService in a case-insensitive way.



2) Don't say failed to enable ... when the service is already 
enabled. It is not a failure. Same for disabled.



3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say except errors.EmptyModlist.



Thank you,

updated patch attached

--
Martin Basti

From 89d54ab02a3a8db846cce8bbea96925dacbe3156 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 28 Aug 2014 19:27:44 +0200
Subject: [PATCH] LDAP disable service

This patch allows to disable service in LDAP (ipactl will not start it)
---
 ipaserver/install/service.py | 55 +++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 7f812a99c557ae567cd1d8a0db30eba434c507f9..b2b343c6b0a1f23ec7b369384ab4c15d29b73053 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -374,6 +374,30 @@ class Service(object):
 self.ldap_connect()
 
 entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+
+# enable disabled service
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+pass
+else:
+if any(u'enabledservice' == x.lower() for x in entry.get('ipaConfigString', [])):
+root_logger.debug(service %s startup entry already enabled, name)
+return
+
+if 'ipaConfigString' in entry and entry['ipaConfigString'] is not None:
+entry['ipaConfigString'].append(u'enabledService')
+else:
+entry['ipaConfigString'] = [u'enabledService']
+
+try:
+self.admin_conn.update_entry(entry)
+except errors.EmptyModlist:
+root_logger.debug(service %s startup entry already enabled, name)
+
+root_logger.debug(service %s startup entry enabled, name)
+return
+
 order = SERVICE_LIST[name][1]
 entry = self.admin_conn.make_entry(
 entry_name,
@@ -386,9 +410,38 @@ class Service(object):
 try:
 self.admin_conn.add_entry(entry)
 except (errors.DuplicateEntry), e:
-root_logger.debug(failed to add %s Service startup entry % name)
+root_logger.debug(failed to add service %s startup entry, name)
 raise e
 
+def ldap_disable(self, name, fqdn, ldap_suffix):
+assert isinstance(ldap_suffix, DN)
+if not self.admin_conn:
+self.ldap_connect()
+
+entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+root_logger.debug(service %s startup entry already disabled (entry not found), name)
+return
+
+if not any(u'enabledservice' == x.lower() for x in entry.get('ipaConfigString', [])):
+root_logger.debug(service %s startup entry already disabled, name)
+return
+
+entry['ipaConfigString'].remove(u'enabledService')
+
+try:
+self.admin_conn.update_entry(entry)
+except errors.EmptyModlist:
+pass
+except:
+root_logger.debug(failed to disable service %s startup entry, name)
+raise
+
+root_logger.debug(service %s startup entry disabled, name)
+
+
 class SimpleServiceInstance(Service):
 def create_instance(self, gensvc_name=None, fqdn=None, dm_password=None, ldap_suffix=None, realm=None):
 self.gensvc_name = gensvc_name
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-19 Thread Martin Basti

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It 
should enable the service no matter if the entry existed before or not.


Similarly, in ldap_disable you should not raise an error when the 
entry is not found, because that already makes the service disabled.


Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-19 Thread Jan Cholasta

Dne 19.9.2014 v 13:33 Martin Basti napsal(a):

On 02/09/14 11:59, Martin Basti wrote:

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It
should enable the service no matter if the entry existed before or not.

Similarly, in ldap_disable you should not raise an error when the
entry is not found, because that already makes the service disabled.

Honza


Updated patch attached



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Please review, this should be in 4.1

--
Martin Basti


1) ipaConfigString is case-insensitive, so you need to look for the 
string enabledService in a case-insensitive way.



2) Don't say failed to enable ... when the service is already enabled. 
It is not a failure. Same for disabled.



3) Missing ipaConfigString is nothing to warn about.


4) You should log success in both ldap_enable/ldap_disable.


5) The except below update_entry should say except errors.EmptyModlist.


--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-02 Thread Jan Cholasta

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It should 
enable the service no matter if the entry existed before or not.


Similarly, in ldap_disable you should not raise an error when the entry 
is not found, because that already makes the service disabled.


Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-02 Thread Martin Basti

On 02/09/14 09:10, Jan Cholasta wrote:

Hi,

Dne 1.9.2014 v 16:57 Martin Basti napsal(a):

This patch allows to disable service in LDAP to prevents service to be
started by ipactl restart

Required by DNSSEC

Patch attached


I don't think the extra argument in ldap_enable is necessary. It 
should enable the service no matter if the entry existed before or not.


Similarly, in ldap_disable you should not raise an error when the 
entry is not found, because that already makes the service disabled.


Honza


Updated patch attached

--
Martin Basti

From 43fb8d981cc02b60c76b0a7040d0232bdf2165bc Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 28 Aug 2014 19:27:44 +0200
Subject: [PATCH] LDAP disable service

This patch allows to disable service in LDAP (ipactl will not start it)
---
 ipaserver/install/service.py | 49 
 1 file changed, 49 insertions(+)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 1f01b275135173b7d0bfdb4d56729438a0853142..370f86fe308607162e9bd8b41144e3557ab0a7ab 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -375,6 +375,30 @@ class Service(object):
 self.ldap_connect()
 
 entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+
+# enable disabled service
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+pass
+else:
+if 'enabledService' in entry.get('ipaConfigString', []):
+root_logger.debug(failed to enable %s Service startup entry (already enabled) % name)
+return
+
+if 'ipaConfigString' in entry and entry['ipaConfigString'] is not None:
+entry['ipaConfigString'].append('enabledService')
+else:
+entry['ipaConfigString'] = ['enabledService']
+root_logger.warning(%s Service startup entry has no 'ipaConfigString' attributes % name)
+
+try:
+self.admin_conn.update_entry(entry)
+except:
+root_logger.debug(failed to re-enable %s Service startup entry (already enabled) % name)
+
+return
+
 order = SERVICE_LIST[name][1]
 entry = self.admin_conn.make_entry(
 entry_name,
@@ -390,6 +414,31 @@ class Service(object):
 root_logger.debug(failed to add %s Service startup entry % name)
 raise e
 
+def ldap_disable(self, name, fqdn, ldap_suffix):
+assert isinstance(ldap_suffix, DN)
+if not self.admin_conn:
+self.ldap_connect()
+
+entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+root_logger.debug(failed to disable %s Service startup entry (service not found) % name)
+return
+
+if 'enabledService' not in entry.get('ipaConfigString', []):
+root_logger.debug(failed to disable %s Service startup entry (Service already disabled) % name)
+return
+
+entry['ipaConfigString'].remove('enabledService')
+
+try:
+self.admin_conn.update_entry(entry)
+except:
+root_logger.debug(failed to disable %s Service startup entry % name)
+raise
+
+
 class SimpleServiceInstance(Service):
 def create_instance(self, gensvc_name=None, fqdn=None, dm_password=None, ldap_suffix=None, realm=None):
 self.gensvc_name = gensvc_name
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH 0118] Allow to disable service (in LDAP)

2014-09-01 Thread Martin Basti
This patch allows to disable service in LDAP to prevents service to be 
started by ipactl restart


Required by DNSSEC

Patch attached

--
Martin Basti

From df330b6b2d630982a25ceaf7c7f6af79327f9089 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Thu, 28 Aug 2014 19:27:44 +0200
Subject: [PATCH] LDAP disable service

This patch allows to disable service in LDAP (ipactl will not start it)
---
 ipaserver/install/service.py | 54 +++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 1f01b275135173b7d0bfdb4d56729438a0853142..f008c7b8f94f3c8489b2c69f8d7cac52c2172a82 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -368,13 +368,40 @@ class Service(object):
 
 self.steps = []
 
-def ldap_enable(self, name, fqdn, dm_password, ldap_suffix, config=[]):
+def ldap_enable(self, name, fqdn, dm_password, ldap_suffix, config=[], enable_if_exists=False):
 assert isinstance(ldap_suffix, DN)
 self.disable()
 if not self.admin_conn:
 self.ldap_connect()
 
 entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+
+# enable disabled service
+if enable_if_exists:
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+pass
+else:
+if 'enabledService' in entry.get('ipaConfigString', []):
+root_logger.debug(failed to re-enable %s Service startup entry (already enabled) % name)
+return
+
+if 'ipaConfigString' in entry and entry['ipaConfigString'] is not None:
+entry['ipaConfigString'].append('enabledService')
+else:
+entry['ipaConfigString'] = ['serviceEnabled']
+root_logger.warning(%s Service startup entry has no 'ipaConfigString' attributes % name)
+
+try:
+self.admin_conn.update_entry(entry)
+except:
+root_logger.debug(failed to re-enable %s Service startup entry (already enabled) % name)
+raise
+else:
+return
+
+
 order = SERVICE_LIST[name][1]
 entry = self.admin_conn.make_entry(
 entry_name,
@@ -390,6 +417,31 @@ class Service(object):
 root_logger.debug(failed to add %s Service startup entry % name)
 raise e
 
+def ldap_disable(self, name, fqdn, ldap_suffix):
+assert isinstance(ldap_suffix, DN)
+if not self.admin_conn:
+self.ldap_connect()
+
+entry_name = DN(('cn', name), ('cn', fqdn), ('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), ldap_suffix)
+try:
+entry = self.admin_conn.get_entry(entry_name, ['ipaConfigString'])
+except errors.NotFound:
+root_logger.debug(failed to disable %s Service startup entry (service not found) % name)
+raise
+
+if 'enabledService' not in entry.get('ipaConfigString', []):
+root_logger.debug(failed to disable %s Service startup entry (Service already disabled) % name)
+return
+
+entry['ipaConfigString'].remove('enabledService')
+
+try:
+self.admin_conn.update_entry(entry)
+except:
+root_logger.debug(failed to disable %s Service startup entry % name)
+raise
+
+
 class SimpleServiceInstance(Service):
 def create_instance(self, gensvc_name=None, fqdn=None, dm_password=None, ldap_suffix=None, realm=None):
 self.gensvc_name = gensvc_name
-- 
1.8.3.1

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel