Re: [Freeipa-devel] [PATCH] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 12:33 PM, David Kupka wrote:
> Currently, when there is unhanded exception without any message,
> installer ends with error code but without any error message.
> 
> Adding line stating that some error occurred and where are logs located
> should help with debugging.
> 
> 
> 

The default value for the log_file_name is None, so we probably need to
handle this correctly in case the AdminTool class instance logs to
stderr only.

Tomas

-- 
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

[Freeipa-devel] [PATCH] 922 topology: add realm suffix to master entry on update

[Petr Vobornik]

This patch was extracted from replica promotion patches.
--
Petr Vobornik
From d523ddec1cdc3efc4e4f2d66a8fb9162cdb78f02 Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 15 Oct 2015 13:58:46 +0200
Subject: [PATCH] topology: add realm suffix to master entry on update

Realm suffix was set only during installation but not on update.
---
 install/updates/20-replication.update | 5 +
 1 file changed, 5 insertions(+)

diff --git a/install/updates/20-replication.update b/install/updates/20-replication.update
index 83d5d1f0c7c083e0c55c3e38a5be729d55b4e828..43f4edc0d44886c46cea498050ec7ea2793adc91 100644
--- a/install/updates/20-replication.update
+++ b/install/updates/20-replication.update
@@ -28,6 +28,11 @@ default: objectclass: iparepltopoconf
 default: ipaReplTopoConfRoot: $SUFFIX
 default: cn: realm
 
+# add IPA realm managed suffix to master entry
+dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
+add: objectclass: ipaReplTopoManagedServer
+add: ipaReplTopoManagedSuffix: $SUFFIX
+
 # Set replication changelog limit (#5086)
 dn: cn=changelog5,cn=config
 addifnew: nsslapd-changelogmaxage: 7d
-- 
2.4.3

-- 
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] 922 topology: add realm suffix to master entry on update

[Jan Cholasta]

On 15.10.2015 14:04, Petr Vobornik wrote:

This patch was extracted from replica promotion patches.


(reference: 
)


Thanks, ACK.

Pushed to master: ba22999cefb57f344acdc63a553d569ab6249099

--
Jan Cholasta

--
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] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 01:30 PM, Tomas Babej wrote:
> 
> 
> On 10/15/2015 01:24 PM, David Kupka wrote:
>> On 15/10/15 13:02, Tomas Babej wrote:
>>>
>>>
>>> On 10/15/2015 12:33 PM, David Kupka wrote:
 Currently, when there is unhanded exception without any message,
 installer ends with error code but without any error message.

 Adding line stating that some error occurred and where are logs located
 should help with debugging.



>>>
>>> The default value for the log_file_name is None, so we probably need to
>>> handle this correctly in case the AdminTool class instance logs to
>>> stderr only.
>>>
>>> Tomas
>>>
>>
>> Thanks for good catch, fixed patch attached.
>> I guess the same is true for command_name but it is used quite
>> extensively it the code so it's probably safe to assume that it will be
>> always set.
>>
> 
> Yes, such assumption is reasonable. Unlike log_file_name, there is no
> use case for command_name being set to None intentionally.
> 
> ACK.
> 
> Tomas
> 

Pushed to master: 5aa118d14905daa8c124be1cee02cfeaf26be3e4

-- 
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] admintool: Add error message with path to log on failure.

[Tomas Babej]

On 10/15/2015 01:24 PM, David Kupka wrote:
> On 15/10/15 13:02, Tomas Babej wrote:
>>
>>
>> On 10/15/2015 12:33 PM, David Kupka wrote:
>>> Currently, when there is unhanded exception without any message,
>>> installer ends with error code but without any error message.
>>>
>>> Adding line stating that some error occurred and where are logs located
>>> should help with debugging.
>>>
>>>
>>>
>>
>> The default value for the log_file_name is None, so we probably need to
>> handle this correctly in case the AdminTool class instance logs to
>> stderr only.
>>
>> Tomas
>>
> 
> Thanks for good catch, fixed patch attached.
> I guess the same is true for command_name but it is used quite
> extensively it the code so it's probably safe to assume that it will be
> always set.
> 

Yes, such assumption is reasonable. Unlike log_file_name, there is no
use case for command_name being set to None intentionally.

ACK.

Tomas

-- 
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] admintool: Add error message with path to log on failure.

[David Kupka]

On 15/10/15 13:02, Tomas Babej wrote:



On 10/15/2015 12:33 PM, David Kupka wrote:

Currently, when there is unhanded exception without any message,
installer ends with error code but without any error message.

Adding line stating that some error occurred and where are logs located
should help with debugging.





The default value for the log_file_name is None, so we probably need to
handle this correctly in case the AdminTool class instance logs to
stderr only.

Tomas



Thanks for good catch, fixed patch attached.
I guess the same is true for command_name but it is used quite 
extensively it the code so it's probably safe to assume that it will be 
always set.


--
David Kupka
From afa43f20f8e517ce67f1c196f01c604e795e42ef Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 15 Oct 2015 12:24:34 +0200
Subject: [PATCH] admintool: Add error message with path to log on failure.

---
 ipapython/admintool.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 5aa1c19bb70f9d9049130d1e2a253abb4b86677b..e40f300b1318b7325eb2b39ec83970753d39c406 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -291,6 +291,10 @@ class AdminTool(object):
 self.command_name, type(exception).__name__, exception)
 if error_message:
 self.log.error(error_message)
+message = "The %s command failed." % self.command_name
+if self.log_file_name:
+message += " See %s for more information" % self.log_file_name
+self.log.error(message)
 
 def log_success(self):
 self.log.info('The %s command was successful', self.command_name)
-- 
2.4.3

-- 
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] [PATCHSET] Replica promotion patches

[Jan Cholasta]

On 15.10.2015 12:00, Petr Vobornik wrote:

On 10/15/2015 10:45 AM, Jan Cholasta wrote:

On 23.9.2015 19:47, Simo Sorce wrote:

On Wed, 2015-09-23 at 08:35 +0200, Jan Cholasta wrote:

What I mean is that installing a replica using an already existing
replica file should be prevented at level 1 as well:

root@ipa1# ipa-server-install --domain-level=0
root@ipa1# ipa-replica-prepare ipa2.example.com
root@ipa1# ipa domainlevel-set 1

root@ipa2# ipa-replica-install replica-info-ipa2.example.com.gpg
ERROR: Can't install replica from a replica file at domain level > 0


Ok I rebased the patchset with a modification to assume promotion if no
file was provided, and then raise appropriate RuntimeErrors if
conditions about the domain level are not met.

This change also prevents installing with a replica file if domain level
is currently at 1.

They are in the usual custodia-review branch.


"Add ipa-custodia service": functional ACK

1) freeipa-python is still missing BuildRequires and Requires on
python-jwcrypto:

On 23.9.2015 08:35, Jan Cholasta wrote:

On 23.9.2015 02:47, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:57 -0400, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:45 +0200, Jan Cholasta wrote:

1) python-jwcrypto dependency is missing in the spec file.


It shouldn't be necessary as custodia already depends on it.


IMO it is a good practice to require all direct dependencies, because
you can't control indirect dependencies. For example, if one day
custodia switched from jwcrypto to something different, ipa would lose
the jwcrypto dependency without us knowing.



"Require a DS version that has working DNA plugin": ACK


"Implement replica promotion functionality":

1) You should handle NotFound for the find_entries() call in
cainstance.find_ca_server().

2) You can remove ReplicaCA and ReplicaDNS classes as they are unused.

3) I'm getting this on domain level 0 client:

# ipa-replica-install
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major (851968):
Unspecified GSS failure. Minor code may provide more information, Minor
(2529639053): No Kerberos credentials available

It comes from the "Try out authentication" conn.connect() in
promote_check(), because it is missing the ccache kwarg.


"Change DNS installer code to use passed in api": ACK


"Allow ipa-replica-conncheck to use default creds":

1) ipa-replica-install prompts for admin password twice during
connection check:

 Get credentials to log in to remote master
 Check SSH connection to remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:
 Execute check on remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:


"Add function to extract CA certs for install": ACK


"topology: manage ca replication agreements": functional ACK

1) This 20-replication.update bit does not seem to be related to the
patch:

# add IPA realm managed suffix to master entry
dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
add: objectclass: ipaReplTopoManagedServer
add: ipaReplTopoManagedSuffix: $SUFFIX

Why is it included? (Petr?)


I believe this could be send as a separate patch. It was included during
tuning of update and is not related to replica promotion nor managing of
CA agreements.


(reference: 
)






2) In update_ca_topology, call CAInstance.__update_topology() instead of
copy & pasting the code.


"enable topology plugin on upgrade": ACK


"topology plugin configuration workaround": ACK


"handle multiple managed suffixes": ACK


"prevent operation on tombstones": ACK


"Allow to setup the CA when promoting a replica": ACK


"Make checks for existing credentials reusable": ACK


"Add low level helper to get domain level": ACK


To speed things up, I fixed the issues I found in the patches above, to 
be able to push them.


Pushed to master: 6a0087aea176d1e1154b359fa262066896d663e3




"Allow ipa-ca-install to use the new promotion code":

1) The --replica option was not removed:

On 22.9.2015 10:45, Jan Cholasta wrote:

1) The --replica option is redundant. You can safely decide whether this
is the first CA master or not based on information in cn=masters.


2) ipa-ca-install prompts for both admin and DM password:

# ipa-ca-install -r
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Directory Manager (existing master) password:

DM password should not be required, right?

3) ipa-ca-install fails with:

Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 445, in start_creation
 run_step(full_msg, method)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 435, in run_step
 method()
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py", line
631, in __spawn_instance
 

[Freeipa-devel] [PATCH] admintool: Add error message with path to log on failure.

[David Kupka]

Currently, when there is unhanded exception without any message, 
installer ends with error code but without any error message.


Adding line stating that some error occurred and where are logs located 
should help with debugging.


--
David Kupka
From 15f98f44bf936434f9cbf8ab81b124cd783d3ebf Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 15 Oct 2015 12:24:34 +0200
Subject: [PATCH] admintool: Add error message with path to log on failure.

---
 ipapython/admintool.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ipapython/admintool.py b/ipapython/admintool.py
index 5aa1c19bb70f9d9049130d1e2a253abb4b86677b..a46c13e052d01caf8a53d5ac6b89dc3873085674 100644
--- a/ipapython/admintool.py
+++ b/ipapython/admintool.py
@@ -291,6 +291,8 @@ class AdminTool(object):
 self.command_name, type(exception).__name__, exception)
 if error_message:
 self.log.error(error_message)
+self.log.error("The %s command failed. See %s for more information",
+self.command_name, self.log_file_name)
 
 def log_success(self):
 self.log.info('The %s command was successful', self.command_name)
-- 
2.4.3

-- 
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

[Freeipa-devel] [PATCH 0086] disable ipa-replica prepare in non-zero domain levels

[Martin Babinsky]

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

--
Martin^3 Babinsky
From 4c344b832432e59dcfe7a32bb7c4ea31470d26af Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 15 Oct 2015 16:07:48 +0200
Subject: [PATCH] disable ipa-replica-prepare in non-zero IPA domain level

the original replica installation path (ipa-replica-prepare +
ipa-replica-install) remains valid only when IPA domain level is zero. When
this is not the case, ipa-replica-prepare will print out an error message which
instructs the user to use the new replica promotion machinery to setup
replicas.

https://fedorahosted.org/freeipa/ticket/5175
---
 ipaserver/install/ipa_replica_prepare.py | 37 +++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index 2b4a60e16bd23f9d4c8e0135708950a6cc40db9a..7f762efa58ed2636777fe29f92ba9c02be5974eb 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -41,7 +41,20 @@ from ipapython import version
 from ipalib import api
 from ipalib import errors
 from ipaplatform.paths import paths
-from ipalib.constants import CACERT
+from ipalib.constants import CACERT, MIN_DOMAIN_LEVEL
+
+
+UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE = """
+Replica creation using '{}' to generate replica file is supported only
+in {}-level IPA domain.
+
+The current IPA domain level is {} and thus the replica must be created by
+promoting an existing IPA client.
+
+To set up a replica use the following precedure:
+1.) set up a client on the host using `ipa-client-install`
+2.) promote the client to replica running `ipa-replica-install --promote`
+"""
 
 
 class ReplicaPrepare(admintool.AdminTool):
@@ -161,6 +174,8 @@ class ReplicaPrepare(admintool.AdminTool):
 api.bootstrap(in_server=True)
 api.finalize()
 
+self.check_domainlevel(api)
+
 if api.env.host == self.replica_fqdn:
 raise admintool.ScriptError("You can't create a replica on itself")
 
@@ -673,3 +688,23 @@ class ReplicaPrepare(admintool.AdminTool):
 '-w', dm_pwd_fd.name,
 '-o', ca_file
 ])
+
+def check_domainlevel(self, api):
+connected = api.Backend.ldap2.isconnected()
+try:
+if not connected:
+api.Backend.ldap2.connect()
+
+domain_level = api.Command.domainlevel_get()['result']
+except Exception as e:
+raise RuntimeError(
+"Cannot determine current domain level: {}".format(e))
+finally:
+if connected:
+api.Backend.ldap2.disconnect()
+
+if domain_level > MIN_DOMAIN_LEVEL:
+raise RuntimeError(
+UNSUPPORTED_DOMAIN_LEVEL_TEMPLATE.format(
+self.command_name, MIN_DOMAIN_LEVEL, domain_level)
+)
-- 
2.4.3

-- 
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] [PATCHSET] Replica promotion patches

[Simo Sorce]

Commenting only on the 2 remaining patches that need to be committed, 
inline.


On 15/10/15 04:45, Jan Cholasta wrote:

On 23.9.2015 19:47, Simo Sorce wrote:



"Allow ipa-ca-install to use the new promotion code":

1) The --replica option was not removed:


Will do, thanks for spotting.


On 22.9.2015 10:45, Jan Cholasta wrote:

1) The --replica option is redundant. You can safely decide whether this
is the first CA master or not based on information in cn=masters.


2) ipa-ca-install prompts for both admin and DM password:

# ipa-ca-install -r
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Directory Manager (existing master) password:

DM password should not be required, right?


Unfortunately if you install the CA in a separate step we still need to 
ask for the DM password because dogtag uses simple binds over ldaps:// 
and not ldapi://, we do not need that if you pass --setup-ca because we 
generate a random DM password and replace it with the hash obtained by 
the existing master only after all components are installed.



3) ipa-ca-install fails with:

Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 445, in start_creation
 run_step(full_msg, method)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line 435, in run_step
 method()
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/cainstance.py", line
631, in __spawn_instance
 DogtagInstance.spawn_instance(self, cfg_file)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 185, in spawn_instance
 self.handle_setup_error(e)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/dogtaginstance.py",
line 448, in handle_setup_error
 raise RuntimeError("%s configuration failed." % self.subsystem)
RuntimeError: CA configuration failed.

I guess I'm hitting the authentication bug in Dogtag. It is supposed to
be fixed in pki-core-10.2.6-10, but is it fixed in pki-core-10.2.7-0.2?
We might need a new 10.2.7 build.


I am not sure which version has it fixed, Endi ?



1) ipa-kra-install fails with:

Traceback (most recent call last):
   File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line
171, in execute
 return_value = self.run()
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_kra_install.py",
line 220, in run
 self._run()
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/ipa_kra_install.py",
line 200, in _run
 if config.subject_base is None:
AttributeError: 'NoneType' object has no attribute 'subject_base'



I need to find out why this stopped working, will post a patch asap.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

--
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] [PATCHSET] Replica promotion patches

[Martin Basti]

On 15.10.2015 14:29, Jan Cholasta wrote:

On 15.10.2015 12:00, Petr Vobornik wrote:

On 10/15/2015 10:45 AM, Jan Cholasta wrote:

On 23.9.2015 19:47, Simo Sorce wrote:

On Wed, 2015-09-23 at 08:35 +0200, Jan Cholasta wrote:

What I mean is that installing a replica using an already existing
replica file should be prevented at level 1 as well:

root@ipa1# ipa-server-install --domain-level=0
root@ipa1# ipa-replica-prepare ipa2.example.com
root@ipa1# ipa domainlevel-set 1

root@ipa2# ipa-replica-install replica-info-ipa2.example.com.gpg
ERROR: Can't install replica from a replica file at domain level > 0


Ok I rebased the patchset with a modification to assume promotion 
if no

file was provided, and then raise appropriate RuntimeErrors if
conditions about the domain level are not met.

This change also prevents installing with a replica file if domain 
level

is currently at 1.

They are in the usual custodia-review branch.


"Add ipa-custodia service": functional ACK

1) freeipa-python is still missing BuildRequires and Requires on
python-jwcrypto:

On 23.9.2015 08:35, Jan Cholasta wrote:

On 23.9.2015 02:47, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:57 -0400, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:45 +0200, Jan Cholasta wrote:

1) python-jwcrypto dependency is missing in the spec file.


It shouldn't be necessary as custodia already depends on it.


IMO it is a good practice to require all direct dependencies, because
you can't control indirect dependencies. For example, if one day
custodia switched from jwcrypto to something different, ipa would lose
the jwcrypto dependency without us knowing.



"Require a DS version that has working DNA plugin": ACK


"Implement replica promotion functionality":

1) You should handle NotFound for the find_entries() call in
cainstance.find_ca_server().

2) You can remove ReplicaCA and ReplicaDNS classes as they are unused.

3) I'm getting this on domain level 0 client:

# ipa-replica-install
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major (851968):
Unspecified GSS failure. Minor code may provide more information, Minor
(2529639053): No Kerberos credentials available

It comes from the "Try out authentication" conn.connect() in
promote_check(), because it is missing the ccache kwarg.


"Change DNS installer code to use passed in api": ACK


"Allow ipa-replica-conncheck to use default creds":

1) ipa-replica-install prompts for admin password twice during
connection check:

 Get credentials to log in to remote master
 Check SSH connection to remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:
 Execute check on remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:


"Add function to extract CA certs for install": ACK


"topology: manage ca replication agreements": functional ACK

1) This 20-replication.update bit does not seem to be related to the
patch:

# add IPA realm managed suffix to master entry
dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
add: objectclass: ipaReplTopoManagedServer
add: ipaReplTopoManagedSuffix: $SUFFIX

Why is it included? (Petr?)


I believe this could be send as a separate patch. It was included during
tuning of update and is not related to replica promotion nor managing of
CA agreements.


(reference: 
)






2) In update_ca_topology, call CAInstance.__update_topology() 
instead of

copy & pasting the code.


"enable topology plugin on upgrade": ACK


"topology plugin configuration workaround": ACK


"handle multiple managed suffixes": ACK


"prevent operation on tombstones": ACK


"Allow to setup the CA when promoting a replica": ACK


"Make checks for existing credentials reusable": ACK


"Add low level helper to get domain level": ACK


To speed things up, I fixed the issues I found in the patches above, 
to be able to push them.


Pushed to master: 6a0087aea176d1e1154b359fa262066896d663e3




"Allow ipa-ca-install to use the new promotion code":

1) The --replica option was not removed:

On 22.9.2015 10:45, Jan Cholasta wrote:
1) The --replica option is redundant. You can safely decide whether 
this

is the first CA master or not based on information in cn=masters.


2) ipa-ca-install prompts for both admin and DM password:

# ipa-ca-install -r
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Directory Manager (existing master) password:

DM password should not be required, right?

3) ipa-ca-install fails with:

Traceback (most recent call last):
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",

line 445, in start_creation
 run_step(full_msg, method)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",

line 435, in run_step
 method()
   File

Re: [Freeipa-devel] [PATCHSET] Replica promotion patches

[Martin Basti]

On 15.10.2015 14:29, Jan Cholasta wrote:

On 15.10.2015 12:00, Petr Vobornik wrote:

On 10/15/2015 10:45 AM, Jan Cholasta wrote:

On 23.9.2015 19:47, Simo Sorce wrote:

On Wed, 2015-09-23 at 08:35 +0200, Jan Cholasta wrote:

What I mean is that installing a replica using an already existing
replica file should be prevented at level 1 as well:

root@ipa1# ipa-server-install --domain-level=0
root@ipa1# ipa-replica-prepare ipa2.example.com
root@ipa1# ipa domainlevel-set 1

root@ipa2# ipa-replica-install replica-info-ipa2.example.com.gpg
ERROR: Can't install replica from a replica file at domain level > 0


Ok I rebased the patchset with a modification to assume promotion 
if no

file was provided, and then raise appropriate RuntimeErrors if
conditions about the domain level are not met.

This change also prevents installing with a replica file if domain 
level

is currently at 1.

They are in the usual custodia-review branch.


"Add ipa-custodia service": functional ACK

1) freeipa-python is still missing BuildRequires and Requires on
python-jwcrypto:

On 23.9.2015 08:35, Jan Cholasta wrote:

On 23.9.2015 02:47, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:57 -0400, Simo Sorce wrote:

On Tue, 2015-09-22 at 10:45 +0200, Jan Cholasta wrote:

1) python-jwcrypto dependency is missing in the spec file.


It shouldn't be necessary as custodia already depends on it.


IMO it is a good practice to require all direct dependencies, because
you can't control indirect dependencies. For example, if one day
custodia switched from jwcrypto to something different, ipa would lose
the jwcrypto dependency without us knowing.



"Require a DS version that has working DNA plugin": ACK


"Implement replica promotion functionality":

1) You should handle NotFound for the find_entries() call in
cainstance.find_ca_server().

2) You can remove ReplicaCA and ReplicaDNS classes as they are unused.

3) I'm getting this on domain level 0 client:

# ipa-replica-install
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Your system may be partly configured.
Run /usr/sbin/ipa-server-install --uninstall to clean up.

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major (851968):
Unspecified GSS failure. Minor code may provide more information, Minor
(2529639053): No Kerberos credentials available

It comes from the "Try out authentication" conn.connect() in
promote_check(), because it is missing the ccache kwarg.


"Change DNS installer code to use passed in api": ACK


"Allow ipa-replica-conncheck to use default creds":

1) ipa-replica-install prompts for admin password twice during
connection check:

 Get credentials to log in to remote master
 Check SSH connection to remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:
 Execute check on remote master
 ad...@vm-137.abc.idm.lab.eng.brq.redhat.com's password:


"Add function to extract CA certs for install": ACK


"topology: manage ca replication agreements": functional ACK

1) This 20-replication.update bit does not seem to be related to the
patch:

# add IPA realm managed suffix to master entry
dn: cn=$FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX
add: objectclass: ipaReplTopoManagedServer
add: ipaReplTopoManagedSuffix: $SUFFIX

Why is it included? (Petr?)


I believe this could be send as a separate patch. It was included during
tuning of update and is not related to replica promotion nor managing of
CA agreements.


(reference: 
)






2) In update_ca_topology, call CAInstance.__update_topology() 
instead of

copy & pasting the code.


"enable topology plugin on upgrade": ACK


"topology plugin configuration workaround": ACK


"handle multiple managed suffixes": ACK


"prevent operation on tombstones": ACK


"Allow to setup the CA when promoting a replica": ACK


"Make checks for existing credentials reusable": ACK


"Add low level helper to get domain level": ACK


To speed things up, I fixed the issues I found in the patches above, 
to be able to push them.


Pushed to master: 6a0087aea176d1e1154b359fa262066896d663e3


Upgrade does not work:

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

Martin





"Allow ipa-ca-install to use the new promotion code":

1) The --replica option was not removed:

On 22.9.2015 10:45, Jan Cholasta wrote:
1) The --replica option is redundant. You can safely decide whether 
this

is the first CA master or not based on information in cn=masters.


2) ipa-ca-install prompts for both admin and DM password:

# ipa-ca-install -r
Password for ad...@abc.idm.lab.eng.brq.redhat.com:
Directory Manager (existing master) password:

DM password should not be required, right?

3) ipa-ca-install fails with:

Traceback (most recent call last):
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",

line 445, in start_creation
 run_step(full_msg, method)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/install/service.py",

line 435, 

Re: [Freeipa-devel] [PATCH] 0001 cert-show: Remove check if hostname != CN

[Jan Orel]

> Anything bound to IPA can potentially retrieve a certificate. This code
> adds special handling for hosts and probably should cover services as
> well now that I think about it. I don't think services could be included
> in ACIs when this was originally written.
>
> The idea was that hosts have no need to be able to query random serial
> numbers so it should be limited to viewing its own. Removing the if
> hostname: applies this logic to ALL retrieval which is by far overkill
> and limits all non-admin entries to only be able to view certs they own
> (or can write) which sort of kills the reason for the 'retrieve
> certificate' permission.

OK, anyway I don't think I am able to refactor right now to include
also the services.

I am attaching new simple patch.
From de2f71384e72cacf3f8b1deb864518246835942b Mon Sep 17 00:00:00 2001
From: Jan Orel 
Date: Thu, 15 Oct 2015 16:59:14 +0200
Subject: [PATCH] cert-show: verify write access to usercertificate

---
 ipalib/plugins/cert.py | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index e459320..55f9484 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -625,9 +625,12 @@ class cert_show(VirtualCommand):
 result['md5_fingerprint'] = unicode(nss.data_to_hex(nss.md5_digest(cert.der_data), 64)[0])
 result['sha1_fingerprint'] = unicode(nss.data_to_hex(nss.sha1_digest(cert.der_data), 64)[0])
 if hostname:
-# If we have a hostname we want to verify that the subject
-# of the certificate matches it, otherwise raise an error
-if hostname != cert.subject.common_name:#pylint: disable=E1101
+# If we have a hostname we want to verify that we can
+# write to the usercertificate attr of the target
+ldap = self.api.Backend.ldap2
+entry = ldap.find_entry_by_attr("cn", cert.subject.common_name,
+"ipahost", base_dn=api.env.basedn)
+if not ldap.can_write(entry.dn, 'usercertificate'):
 raise acierr
 
 return dict(result=result)
-- 
2.4.3
-- 
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] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Martin Basti]

On 14.10.2015 16:10, Martin Basti wrote:



On 14.10.2015 15:51, Martin Babinsky wrote:

On 10/13/2015 06:38 PM, Martin Basti wrote:



On 12.10.2015 12:30, Martin Babinsky wrote:

On 10/08/2015 05:58 PM, Martin Basti wrote:

The attached patches fix following tickets:
 https://fedorahosted.org/freeipa/ticket/4949
 https://fedorahosted.org/freeipa/ticket/4048
 https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that 
contains

modifications to be applied to dse.ldif right after creation of DS
instance.



Hi,

Functionally the paches work as expected. However I have following
nitpicks:

in patch 318:

1.) there is a typo in ModifyLDIF class docstring:

+Operations keep the order in whihc were specified per DN.

in patch 320:

1.) you should use 'os.path.join' to construct FS paths:


-dse_filename = '%s/%s' % (
+dse_filename = os.path.join(
 paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % self.serverid,
-'dse.ldif',
+'dse.ldif'
 )

2.) IIUC the 'config_ldif_file' knob in BaseServer holds the path to
LDIF containing the mod operations to dse.ldif. However, the knob name
sounds like the option accepts the path of dse.ldif itself. I propose
to rename the knob to something more in-line with the supposed
function, like 'dse_mods_file'.



Updated patches + CI test attached


Patches work as expected and CI tests are OK.

However it seems that warning level messages are not logged to 
installer output but only to log file (*waves hand* magic).


Maybe it would be a good idea to raise the message level to "error", 
so that it is immediately obvious to the user that his DSE mods 
contain an error and cannot be parsed.


Also you have a typo in the commit message of patch 320 
(s/chages/changes/).



Updated patches attached.




Rebased patches for master branch.
From 158ea6f79f4930362e749703e15e19ee0f48f6c4 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 5 Oct 2015 14:37:05 +0200
Subject: [PATCH 1/3] Make offline LDIF modify more robust

* move code to installutils
* add replace_value method
* use lists instead of single values for add_value, remove_value methods

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

Also fixes:
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930
---
 ipaserver/install/installutils.py|  98 ++
 ipaserver/install/upgradeinstance.py | 112 ---
 2 files changed, 109 insertions(+), 101 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 52a110bb96b074f6a92e8e1f2434c3235b27f65d..8e9e43d15e14eed1cfe1030d83e6e16ddfe25530 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -23,6 +23,7 @@ from __future__ import print_function
 import socket
 import getpass
 import gssapi
+import ldif
 import os
 import re
 import fileinput
@@ -1215,3 +1216,100 @@ def check_creds(options, realm_name):
 raise ScriptError("Invalid credentials: %s" % e)
 
 os.environ['KRB5CCNAME'] = ccache_name
+
+
+class ModifyLDIF(ldif.LDIFParser):
+"""
+Allows to modify LDIF file.
+
+Operations keep the order in which were specified per DN.
+Warning: only modifications of existing DNs are supported
+"""
+def __init__(self, input_file, output_file):
+"""
+:param input_file: an LDIF
+:param output_file: an LDIF file
+"""
+ldif.LDIFParser.__init__(self, input_file)
+self.writer = ldif.LDIFWriter(output_file)
+self.dn_updated = set()
+
+self.modifications = {}  # keep modify operations in original order
+
+def add_value(self, dn, attr, values):
+"""
+Add value to LDIF.
+:param dn: DN of entry (must exists)
+:param attr: attribute name
+:param value: value to be added
+"""
+assert isinstance(values, list)
+self.modifications.setdefault(dn, []).append(
+dict(
+op="add",
+attr=attr,
+values=values,
+)
+)
+
+def remove_value(self, dn, attr, values=None):
+"""
+Remove value from LDIF.
+:param dn: DN of entry
+:param attr: attribute name
+:param value: value to be removed, if value is None, attribute will
+be removed
+"""
+assert values is None or isinstance(values, list)
+self.modifications.setdefault(dn, []).append(
+dict(
+op="del",
+attr=attr,
+values=values,
+)
+)
+
+def replace_value(self, dn, attr, values):
+"""
+Replace values in LDIF with new value.
+:param dn: DN of entry
+:param attr: attribute name
+:param value: new value for atribute
+

Re: [Freeipa-devel] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Jan Cholasta]

On 15.10.2015 19:47, Martin Basti wrote:



On 15.10.2015 18:47, Martin Basti wrote:



On 15.10.2015 18:36, Martin Babinsky wrote:

On 10/15/2015 04:50 PM, Martin Basti wrote:



On 14.10.2015 16:10, Martin Basti wrote:



On 14.10.2015 15:51, Martin Babinsky wrote:

On 10/13/2015 06:38 PM, Martin Basti wrote:



On 12.10.2015 12:30, Martin Babinsky wrote:

On 10/08/2015 05:58 PM, Martin Basti wrote:

The attached patches fix following tickets:
https://fedorahosted.org/freeipa/ticket/4949
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that
contains
modifications to be applied to dse.ldif right after creation of DS
instance.



Hi,

Functionally the paches work as expected. However I have following
nitpicks:

in patch 318:

1.) there is a typo in ModifyLDIF class docstring:

+Operations keep the order in whihc were specified per DN.

in patch 320:

1.) you should use 'os.path.join' to construct FS paths:


-dse_filename = '%s/%s' % (
+dse_filename = os.path.join(
 paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE %
self.serverid,
-'dse.ldif',
+'dse.ldif'
 )

2.) IIUC the 'config_ldif_file' knob in BaseServer holds the
path to
LDIF containing the mod operations to dse.ldif. However, the
knob name
sounds like the option accepts the path of dse.ldif itself. I
propose
to rename the knob to something more in-line with the supposed
function, like 'dse_mods_file'.



Updated patches + CI test attached


Patches work as expected and CI tests are OK.

However it seems that warning level messages are not logged to
installer output but only to log file (*waves hand* magic).

Maybe it would be a good idea to raise the message level to "error",
so that it is immediately obvious to the user that his DSE mods
contain an error and cannot be parsed.

Also you have a typo in the commit message of patch 320
(s/chages/changes/).


Updated patches attached.




Rebased patches for master branch.

ACK


Pushed to master: 5233165ce7062bb7aa649bf95a029103c375207b
Pushed to ipa-4-2: 94412b81c1c09b56ee2900942a1b804f21c264c5


These tickets are not for ipa-4-2,
reverted  a17936a1aad139236f18f0e5ad0b066f7747ca60


Can we use a better option name? --dirsrv-config-mods sounds weird, as 
you need to specify a file, not mods...


--
Jan Cholasta

--
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] 0001 cert-show: Remove check if hostname != CN

[Jan Orel]

2015-10-13 19:26 GMT+02:00 Rob Crittenden :
> Jan Orel wrote:
>>> The restriction was there so that hosts had limited visibility. This
>>> applies that limitation to all users. I think the host check needs to be
>>> re-added.
>>
>> I am confused, correct me if I am wrong, but the "if hostname:" check
>> seems always redundat because it would raise exception before
>> either here:
>>
>> 615 if not bind_principal.startswith('host/'):
>> 616 raise acierr
>>
>> or in validate_principal()
>
> Anything bound to IPA can potentially retrieve a certificate. This code
> adds special handling for hosts and probably should cover services as
> well now that I think about it. I don't think services could be included
> in ACIs when this was originally written.
>
> The idea was that hosts have no need to be able to query random serial
> numbers so it should be limited to viewing its own. Removing the if
> hostname: applies this logic to ALL retrieval which is by far overkill
> and limits all non-admin entries to only be able to view certs they own
> (or can write) which sort of kills the reason for the 'retrieve
> certificate' permission.
>
>>
>>> Also, every host is not guaranteed to have a krbPrincipalAux (it can be
>>> unenrolled). I assume you used this to cover managed services as well,
>>> that's why the broad search base?
>>
>> Checking it, even host which is not enrolled have objectClass: 
>> krbprincipalaux,
>> but advise me if different search should be used.
>
> If a host is added with a password (random or otherwise) it won't have
> this objectclass. I'd make the search filter something like
> (|(objectclass=ipahost)(objectclass=ipaservice)).
>
> rob

-- 
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] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Martin Basti]

On 15.10.2015 18:36, Martin Babinsky wrote:

On 10/15/2015 04:50 PM, Martin Basti wrote:



On 14.10.2015 16:10, Martin Basti wrote:



On 14.10.2015 15:51, Martin Babinsky wrote:

On 10/13/2015 06:38 PM, Martin Basti wrote:



On 12.10.2015 12:30, Martin Babinsky wrote:

On 10/08/2015 05:58 PM, Martin Basti wrote:

The attached patches fix following tickets:
https://fedorahosted.org/freeipa/ticket/4949
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that
contains
modifications to be applied to dse.ldif right after creation of DS
instance.



Hi,

Functionally the paches work as expected. However I have following
nitpicks:

in patch 318:

1.) there is a typo in ModifyLDIF class docstring:

+Operations keep the order in whihc were specified per DN.

in patch 320:

1.) you should use 'os.path.join' to construct FS paths:


-dse_filename = '%s/%s' % (
+dse_filename = os.path.join(
 paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % 
self.serverid,

-'dse.ldif',
+'dse.ldif'
 )

2.) IIUC the 'config_ldif_file' knob in BaseServer holds the path to
LDIF containing the mod operations to dse.ldif. However, the knob 
name
sounds like the option accepts the path of dse.ldif itself. I 
propose

to rename the knob to something more in-line with the supposed
function, like 'dse_mods_file'.



Updated patches + CI test attached


Patches work as expected and CI tests are OK.

However it seems that warning level messages are not logged to
installer output but only to log file (*waves hand* magic).

Maybe it would be a good idea to raise the message level to "error",
so that it is immediately obvious to the user that his DSE mods
contain an error and cannot be parsed.

Also you have a typo in the commit message of patch 320
(s/chages/changes/).


Updated patches attached.




Rebased patches for master branch.

ACK


Pushed to master: 5233165ce7062bb7aa649bf95a029103c375207b
Pushed to ipa-4-2: 94412b81c1c09b56ee2900942a1b804f21c264c5

--
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] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Martin Babinsky]

On 10/15/2015 04:50 PM, Martin Basti wrote:



On 14.10.2015 16:10, Martin Basti wrote:



On 14.10.2015 15:51, Martin Babinsky wrote:

On 10/13/2015 06:38 PM, Martin Basti wrote:



On 12.10.2015 12:30, Martin Babinsky wrote:

On 10/08/2015 05:58 PM, Martin Basti wrote:

The attached patches fix following tickets:
https://fedorahosted.org/freeipa/ticket/4949
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that
contains
modifications to be applied to dse.ldif right after creation of DS
instance.



Hi,

Functionally the paches work as expected. However I have following
nitpicks:

in patch 318:

1.) there is a typo in ModifyLDIF class docstring:

+Operations keep the order in whihc were specified per DN.

in patch 320:

1.) you should use 'os.path.join' to construct FS paths:


-dse_filename = '%s/%s' % (
+dse_filename = os.path.join(
 paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % self.serverid,
-'dse.ldif',
+'dse.ldif'
 )

2.) IIUC the 'config_ldif_file' knob in BaseServer holds the path to
LDIF containing the mod operations to dse.ldif. However, the knob name
sounds like the option accepts the path of dse.ldif itself. I propose
to rename the knob to something more in-line with the supposed
function, like 'dse_mods_file'.



Updated patches + CI test attached


Patches work as expected and CI tests are OK.

However it seems that warning level messages are not logged to
installer output but only to log file (*waves hand* magic).

Maybe it would be a good idea to raise the message level to "error",
so that it is immediately obvious to the user that his DSE mods
contain an error and cannot be parsed.

Also you have a typo in the commit message of patch 320
(s/chages/changes/).


Updated patches attached.




Rebased patches for master branch.

ACK

--
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

Re: [Freeipa-devel] [PATCHSET] Replica promotion patches

[Simo Sorce]

On 15/10/15 11:39, Martin Basti wrote:

Without this patch the ipa-ca-install is broken in current master.
Unexpected error - see /var/log/ipareplica-ca-install.log for details:
AttributeError: Values instance has no attribute 'promote'


Should be fixed with the attached patches.

--
Simo Sorce * Red Hat, Inc * New York
>From 78ddd9c8866713de182e65595fbf9e428ba67880 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Thu, 20 Aug 2015 17:10:23 -0400
Subject: [PATCH 1/2] Allow ipa-ca-install to use the new promotion code

This makes it possible to install a CA after-the-fact on a server
that has been promoted (and has no replica file available).

Signed-off-by: Simo Sorce 
---
 install/tools/ipa-ca-install | 92 
 ipaserver/install/ca.py  |  2 -
 2 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/install/tools/ipa-ca-install b/install/tools/ipa-ca-install
index 6564e4d0304d4e189b133c495b75f200b04e2988..e59f016e43c76d4bd40ad4535596a37b5edaeb6e 100755
--- a/install/tools/ipa-ca-install
+++ b/install/tools/ipa-ca-install
@@ -21,12 +21,16 @@
 import sys
 import os
 import shutil
+import tempfile
 from ipapython import ipautil
 
 from ipaserver.install import installutils
 from ipaserver.install import certs
 from ipaserver.install.installutils import create_replica_config
+from ipaserver.install.installutils import check_creds, ReplicaConfig
 from ipaserver.install import dsinstance, ca
+from ipaserver.install import cainstance, custodiainstance
+from ipapython import dogtag
 from ipapython import version
 from ipalib import api
 from ipapython.dn import DN
@@ -67,6 +71,8 @@ def parse_options():
   type="choice",
   choices=('SHA1withRSA', 'SHA256withRSA', 'SHA512withRSA'),
   help="Signing algorithm of the IPA CA certificate")
+parser.add_option("-P", "--principal", dest="principal", sensitive=True,
+  default=None, help="User allowed to manage replicas")
 
 options, args = parser.parse_args()
 safe_options = parser.get_safe_opts(options)
@@ -107,15 +113,24 @@ def install_replica(safe_options, options, filename):
 sys.argv[0], filename, safe_options)
 root_logger.debug('IPA version %s', version.VENDOR_VERSION)
 
-if not ipautil.file_exists(filename):
-sys.exit("Replica file %s does not exist" % filename)
-
 if not dsinstance.DsInstance().is_configured():
 sys.exit("IPA server is not configured on this system.\n")
 
 api.bootstrap(in_server=True)
 api.finalize()
 
+domain_level = dsinstance.get_domain_level(api)
+if domain_level > 0:
+options.promote = True
+else:
+options.promote = False
+if not ipautil.file_exists(filename):
+sys.exit("Replica file %s does not exist" % filename)
+
+
+# Check if we have admin creds already, otherwise acquire them
+check_creds(options, api.env.realm)
+
 # get the directory manager password
 dirman_password = options.password
 if not dirman_password:
@@ -132,13 +147,36 @@ def install_replica(safe_options, options, filename):
 options.unattended:
 sys.exit('admin password required')
 
-config = create_replica_config(dirman_password, filename, options)
+if options.promote:
+config = ReplicaConfig()
+config.master_host_name = None
+config.realm_name = api.env.realm
+config.host_name = api.env.host
+config.domain_name = api.env.domain
+config.dirman_password = dirman_password
+config.ca_ds_port = dogtag.install_constants.DS_PORT
+config.top_dir = tempfile.mkdtemp("ipa")
+config.dir = config.top_dir
+else:
+config = create_replica_config(dirman_password, filename, options)
+
 global REPLICA_INFO_TOP_DIR
 REPLICA_INFO_TOP_DIR = config.top_dir
 config.setup_ca = True
 
-api.Backend.ldap2.connect(bind_dn=DN(('cn', 'Directory Manager')),
-  bind_pw=dirman_password)
+conn = api.Backend.ldap2
+conn.connect(bind_dn=DN(('cn', 'Directory Manager')),
+bind_pw=dirman_password)
+
+if config.subject_base is None:
+attrs = conn.get_ipa_config()
+config.subject_base = attrs.get('ipacertificatesubjectbase')[0]
+
+if config.master_host_name is None:
+config.ca_host_name = cainstance.find_ca_server(api.env.ca_host, conn)
+config.master_host_name = config.ca_host_name
+else:
+config.ca_host_name = config.master_host_name
 
 options.realm_name = config.realm_name
 options.domain_name = config.domain_name
@@ -147,7 +185,22 @@ def install_replica(safe_options, options, filename):
 options.subject = config.subject_base
 
 ca.install_check(True, config, options)
-ca.install(True, config, options)
+if options.promote:
+ca_data = 

Re: [Freeipa-devel] [PATCHES 0318 - 0320, 0323] installer: allow to modify dse.ldif during installation

[Martin Basti]

On 15.10.2015 18:47, Martin Basti wrote:



On 15.10.2015 18:36, Martin Babinsky wrote:

On 10/15/2015 04:50 PM, Martin Basti wrote:



On 14.10.2015 16:10, Martin Basti wrote:



On 14.10.2015 15:51, Martin Babinsky wrote:

On 10/13/2015 06:38 PM, Martin Basti wrote:



On 12.10.2015 12:30, Martin Babinsky wrote:

On 10/08/2015 05:58 PM, Martin Basti wrote:

The attached patches fix following tickets:
https://fedorahosted.org/freeipa/ticket/4949
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that
contains
modifications to be applied to dse.ldif right after creation of DS
instance.



Hi,

Functionally the paches work as expected. However I have following
nitpicks:

in patch 318:

1.) there is a typo in ModifyLDIF class docstring:

+Operations keep the order in whihc were specified per DN.

in patch 320:

1.) you should use 'os.path.join' to construct FS paths:


-dse_filename = '%s/%s' % (
+dse_filename = os.path.join(
 paths.ETC_DIRSRV_SLAPD_INSTANCE_TEMPLATE % 
self.serverid,

-'dse.ldif',
+'dse.ldif'
 )

2.) IIUC the 'config_ldif_file' knob in BaseServer holds the 
path to
LDIF containing the mod operations to dse.ldif. However, the 
knob name
sounds like the option accepts the path of dse.ldif itself. I 
propose

to rename the knob to something more in-line with the supposed
function, like 'dse_mods_file'.



Updated patches + CI test attached


Patches work as expected and CI tests are OK.

However it seems that warning level messages are not logged to
installer output but only to log file (*waves hand* magic).

Maybe it would be a good idea to raise the message level to "error",
so that it is immediately obvious to the user that his DSE mods
contain an error and cannot be parsed.

Also you have a typo in the commit message of patch 320
(s/chages/changes/).


Updated patches attached.




Rebased patches for master branch.

ACK


Pushed to master: 5233165ce7062bb7aa649bf95a029103c375207b
Pushed to ipa-4-2: 94412b81c1c09b56ee2900942a1b804f21c264c5


These tickets are not for ipa-4-2,
reverted  a17936a1aad139236f18f0e5ad0b066f7747ca60

--
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