Re: [Freeipa-devel] Stage users - inconsistent permission names
On 06/10/2015 09:12 AM, Martin Kosek wrote: Hello Thierry/David, I saw the new privileges and permissions for the Staged Users functionality and found couple spelling/English issues that I think we should fix before Alpha/GA so that we can just rename them and not care about upgrade changes. Namely: # ipa permission-find stage | grep -i Permission name Permission name: System: Add Stage Users by Provisioning and Administrators Should be System: Add Stage User Permission should not care who will do it, it is privilege/role's job. Permission name: System: Delete modify Stage Users by administrators Why is Modify and Delete combined in 1 permission? Should be System: Modify Stage User and System: Remove Stage User Permission name: System: Preserve an active user to a delete Users Maybe System: Preserve User? We do not use deleted users bur rather preserved users anyway Permission name: System: Reactive delete users System: Undelete User to reflect the command name. Permission name: System: Read Stage User kerberos principal key and password Rather System: Read Stage User password - I do not think we need to call out the principal key explicitly, but this is negotiable. Permission name: System: Read Stage Users by administrators System: Read Stage Users Permission name: System: Read/Write delete Users by administrators This needs to be 2 permissions: System: Read Preserved Users System: Modify Preserved Users Permission name: System: Reset userPassord and kerberos keys of delete users by administrator Rather System: Reset Preserved User password Permission name: System: Write Active Users RDN by administrators Rather System: Modify User RDN Permission name: System: Write Delete Users RDN by administrators Why is this permission needed, isn't System: Modify Preserved Users enough? Hello, it's probably my fault, I should have paid more attention when reviewing the patch set. I created ticket https://fedorahosted.org/freeipa/ticket/5057 and can fix it. -- David Kupka -- 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] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a):
Dne 18.5.2015 v 10:33 thierry bordaz napsal(a):
On 05/15/2015 04:44 PM, David Kupka wrote:
Hello Thierry,
thanks for the patch set. Overall functionality of ULC feature looks
good to
me and is definitely alpha ready.
I found following issues but don't insist on fixing it right now:
1) When stageuser-activate fails due to already existent
active/deleted user.
DN is show instead of user name that's used in other commands (user-add,
stageuser-add).
$ ipa user-add tuser --first Test --last User
$ ipa stageuser-add tuser --first Test --last User
$ ipa stageuser-activate tuser
ipa: ERROR: Active user
uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
already exists
Hi David, Jan,
Thanks you so much for all those tests and feedback. I agree, some minor
bugs can be fixed separatly from this main patches.
You are right, It should return the user ID not the DN.
2) According to the design there should be '--only-delete' and
'--also-delete'
options for user-find command instead there is '--preserved' option.
Honza proposed adding virtual boolean attribute 'deleted' to user
entry and
filter on it.
The 'deleted' attribute would be useful also in user-show where is no
way to
tell if the displayed user is active or deleted. (Except running with
--all
and looking on the dn).
Yes a bit late to resynch the design.
The final option is 'preserved' for user-find and 'preserve' for
user-del. '--only-delete' or 'also-delete' are old name that I need to
replace in the design.
About the 'deleted' attribute, do you think adding a DS cos virtual
attribute ?
See the attached patch.
3) uidNumber and gidNumber can't be set back to '-1' once set to other
value.
This would be useful when admin changes its mind and want IPA to
assign them.
IIUC, there should be no validation in cn=staged user container. All
validation should be done during stageuser-activate.
Yes that comes from user plugin that enforce the number to be 0.
That is a good point giving the ability to reset uidNumber/gidNumber.
I will check if it is possible, how (give a value or an option to
reset), and also if it would not create other issue.
4) Support for deleted - stage workflow is still missing. But I'm
unsure if we
agreed to finish it now or later.
Yes thanks
5) Twice deleting user with '--preserve' deletes him permanently.
$ ipa user-add tuser --first Test --last User
$ ipa user-del tuser --preserve
$ ipa user-del tuser --preserve
$ ipa user-find --preserved
0 (delete) users matched
Number of entries returned 0
Deleting a deleted (preserved) entry, should permanently remove the
entry.
Now if the second time the preserve option is present, it makes sense to
not delete it.
BTW: I might be stating the obvious here, but it would be better to use
one boolean parameter rather than two mutually exclusive flags in user-del.
thanks
theirry
Overall, LGTM,
Just 2 nitpicks:
1) preserved attribute label: 'Preserved deleted user' - 'Preserved user'
2) 'preserved' attribute should be shown in user-{find,show} when
'--all' is specified
Updated patch attached.
--
David Kupka
From 7c6e3869ceb64177169b360b21b0af5d73e0405c Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 20 May 2015 08:12:07 +
Subject: [PATCH] User life cycle: provide preserved user virtual attribute
https://fedorahosted.org/freeipa/ticket/3813
---
API.txt| 2 +-
VERSION| 4 +--
ipalib/plugins/user.py | 78 +++---
3 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/API.txt b/API.txt
index 9e3f223b7ac338840d7090299f9108e951ea920a..b892eff8bf95c79b7ffdb98738710a7c54000f93 100644
--- a/API.txt
+++ b/API.txt
@@ -5023,7 +5023,7 @@ option: Str('pager', attribute=True, autofill=False, cli_name='pager', multivalu
option: Flag('pkey_only?', autofill=True, default=False)
option: Str('postalcode', attribute=True, autofill=False, cli_name='postalcode', multivalue=False, query=True, required=False)
option: Str('preferredlanguage', attribute=True, autofill=False, cli_name='preferredlanguage', multivalue=False, pattern='^(([a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?(\\s*,\\s*[a-zA-Z]{1,8}(-[a-zA-Z]{1,8})?(;q\\=((0(\\.[0-9]{0,3})?)|(1(\\.0{0,3})?)))?)*)|(\\*))$', query=True, required=False)
-option: Flag('preserved?', autofill=True, cli_name='preserved', default=False)
+option: Bool('preserved', attribute=False, autofill=False, cli_name='preserved', default=False, multivalue=False, query=True, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Int('sizelimit?', autofill=False, minvalue=0)
option: Str('sn', attribute=True, autofill=False, cli_name='last', multivalue=False, query=True
[Freeipa-devel] [PATCH 0050] Allow to skip lint when building FreeIPA.
-- David Kupka From f68607e9a3db4cd8893c465d804615aac34afc29 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 4 Jun 2015 12:10:37 +0200 Subject: [PATCH] Allow to skip lint when building FreeIPA. Target 'lint' does nothing when SKIP_LINT is set to anything else than no. By default the variable is unset and lint is executed as always was. --- Makefile | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index abf58382960099a54b8920dd0e741b9fda17682f..4ad1d69dfc330c3a48a13a0b525e1f533183236d 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,8 @@ ifneq ($(DEVELOPER_MODE),0) LINT_OPTIONS=--no-fail endif +SKIP_LINT ?= no + PYTHON ?= $(shell rpm -E %__python || echo /usr/bin/python2) CFLAGS := -g -O2 -Wall -Wextra -Wformat-security -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers $(CFLAGS) @@ -116,9 +118,10 @@ client-dirs: fi lint: bootstrap-autogen - ./make-lint $(LINT_OPTIONS) - $(MAKE) -C install/po validate-src-strings - + if [ $(SKIP_LINT) == no ]; then \ + ./make-lint $(LINT_OPTIONS); \ + $(MAKE) -C install/po validate-src-strings; \ + fi test: ./make-test -- 2.3.5 -- 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] 0005 User life cycle: del/mod/find/show stageuser commands
Dne 18.6.2015 v 13:22 Petr Vobornik napsal(a): On 06/18/2015 12:52 PM, Jan Cholasta wrote: Dne 18.6.2015 v 09:30 Jan Cholasta napsal(a): Dne 15.6.2015 v 17:29 thierry bordaz napsal(a): On 06/15/2015 05:00 PM, Simo Sorce wrote: On Mon, 2015-06-15 at 16:48 +0200, Petr Vobornik wrote: On 06/09/2015 02:02 PM, Jan Cholasta wrote: Dne 20.5.2015 v 11:26 Jan Cholasta napsal(a): Dne 18.5.2015 v 10:33 thierry bordaz napsal(a): On 05/15/2015 04:44 PM, David Kupka wrote: Hello Thierry, thanks for the patch set. Overall functionality of ULC feature looks good to me and is definitely alpha ready. I found following issues but don't insist on fixing it right now: 1) When stageuser-activate fails due to already existent active/deleted user. DN is show instead of user name that's used in other commands (user-add, stageuser-add). $ ipa user-add tuser --first Test --last User $ ipa stageuser-add tuser --first Test --last User $ ipa stageuser-activate tuser ipa: ERROR: Active user uid=tuser,cn=users,cn=accounts,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com already exists Hi David, Jan, Thanks you so much for all those tests and feedback. I agree, some minor bugs can be fixed separatly from this main patches. You are right, It should return the user ID not the DN. 2) According to the design there should be '--only-delete' and '--also-delete' options for user-find command instead there is '--preserved' option. Honza proposed adding virtual boolean attribute 'deleted' to user entry and filter on it. The 'deleted' attribute would be useful also in user-show where is no way to tell if the displayed user is active or deleted. (Except running with --all and looking on the dn). Yes a bit late to resynch the design. The final option is 'preserved' for user-find and 'preserve' for user-del. '--only-delete' or 'also-delete' are old name that I need to replace in the design. About the 'deleted' attribute, do you think adding a DS cos virtual attribute ? See the attached patch. Can someone please review the patch? 3) uidNumber and gidNumber can't be set back to '-1' once set to other value. This would be useful when admin changes its mind and want IPA to assign them. IIUC, there should be no validation in cn=staged user container. All validation should be done during stageuser-activate. Yes that comes from user plugin that enforce the number to be 0. That is a good point giving the ability to reset uidNumber/gidNumber. I will check if it is possible, how (give a value or an option to reset), and also if it would not create other issue. 4) Support for deleted - stage workflow is still missing. But I'm unsure if we agreed to finish it now or later. Yes thanks 5) Twice deleting user with '--preserve' deletes him permanently. $ ipa user-add tuser --first Test --last User $ ipa user-del tuser --preserve $ ipa user-del tuser --preserve $ ipa user-find --preserved 0 (delete) users matched Number of entries returned 0 Deleting a deleted (preserved) entry, should permanently remove the entry. +1, but no-op if default behavior is preserve Now if the second time the preserve option is present, it makes sense to not delete it. +1, should be no-op BTW: I might be stating the obvious here, but it would be better to use one boolean parameter rather than two mutually exclusive flags in user-del. I would like an opinion on this as well. So the proposal is, e.g.,: Replace: ipa user del fbar --preserve ipa user del fbar --permanently with: ipa user del fbar --permanently=False ipa user del fbar --permanently=True and ipa user del fbar uses the default behavior(permanently atm.) I don't think there is a big difference. A boolean is easier for scripting. 2 options are more descriptive for humans. With a single boolean, I would be afraid that omitting it would imply False to some users which is not always the same as the default behavior [1]. With Web UI developer hat I would vote for single boolean but as a CLI user I would like the current options. Given that Web UI or any other API client should not define CLI, I would keep the current options. my 2c [1] http://www.freeipa.org/page/V4/User_Life-Cycle_Management#Delete_User -- Petr Vobornik +1 --preserve is 100x better for a human than --permanently=False I also prefere --preserve for usability of 'user del'. In addition we have 'user find|show --preserved' to retrieve users that have been preserved. So it seems to me better that the action that preserved the user uses the option '--preserve' rather '--permanently=False'. It's ridiculous that the CLI taints the RPC API and it should be fixed. Also on a more nitpicky side, I think the flag should be called --no-preserve rather than --permanently. There is plenty of commands (rm, cp, ...) which have --no-preserve as opposite of --preserve. The attached patch fixes both
[Freeipa-devel] [PATCH 0053] upgrade: Raise error when certmonger is not running.
https://fedorahosted.org/freeipa/ticket/5080
--
David Kupka
From f5467b5a338647a20aef5e5657b9e21be5b0a2f5 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Fri, 26 Jun 2015 10:42:23 +0200
Subject: [PATCH] upgrade: Raise error when certmonger is not running.
Certmonger should be running (should be started on system boot).
Either user decided to stop it or it crashed. We should just error out and
let user check fix it.
https://fedorahosted.org/freeipa/ticket/5080
---
ipaserver/install/server/upgrade.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 43beb6799befcad8d512d15409b363f02c3bad08..784a03b195ab99c865935b6e51cc86a3b81842ee 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1477,6 +1477,9 @@ def upgrade_check(options):
print unicode(e)
sys.exit(1)
+if not services.knownservices.certmonger.is_running():
+raise RuntimeError('Certmonger is not running. Start certmonger and run upgrade again.')
+
if not options.skip_version_check:
# check IPA version and data version
try:
--
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 434, 443, 444] vault: Fix ipa-kra-install
Dne 10.6.2015 v 18:08 David Kupka napsal(a):
Dne 10.6.2015 v 13:25 Jan Cholasta napsal(a):
Hi,
the attached patches fix several shortcomings in ipa-kra-install, see
commit messages.
https://fedorahosted.org/freeipa/ticket/3872
(Patch 434 was introduced in
https://www.redhat.com/archives/freeipa-devel/2015-June/msg00035.html.)
Honza
There are two issues:
1) https://fedorahosted.org/freeipa/ticket/5059 but it is just missing
check and can be fixed later.
2) kra.install() was called before http_install() but kra installation
needs httpd running. This is fixed in attached patch.
I accidentally included change in Makefile, updated patch attached.
Also I forget to explicitly write 'ACK' to fulfill the process
requirements, so: Works for me, ACK.
--
David Kupka
From a56cee4c6e0fc9fa246f5d7c053218a21819eae7 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 10 Jun 2015 08:50:42 +
Subject: [PATCH] vault: Fix ipa-kra-install
Use state in LDAP rather than local state to check if KRA is installed.
Use correct log file names.
https://fedorahosted.org/freeipa/ticket/3872
---
API.txt| 6 +++
VERSION| 4 +-
ipalib/plugins/vault.py| 38 -
ipaplatform/base/paths.py | 4 +-
ipaserver/install/installutils.py | 16
ipaserver/install/ipa_kra_install.py | 22 ++
ipaserver/install/kra.py | 65 +-
ipaserver/install/server/install.py| 7 ++--
ipaserver/install/server/replicainstall.py | 33 +++
ipaserver/install/service.py | 1 +
ipaserver/plugins/dogtag.py| 2 +-
11 files changed, 102 insertions(+), 96 deletions(-)
diff --git a/API.txt b/API.txt
index 9e3f223b7ac338840d7090299f9108e951ea920a..9e41ece74c94d5d1f9ee2900461b02b56a6f562b 100644
--- a/API.txt
+++ b/API.txt
@@ -2487,6 +2487,12 @@ option: Str('version?', exclude='webui')
output: Output('commands', type 'dict', None)
output: Output('methods', type 'dict', None)
output: Output('objects', type 'dict', None)
+command: kra_is_enabled
+args: 0,1,3
+option: Str('version?', exclude='webui')
+output: Output('result', type 'bool', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
command: krbtpolicy_mod
args: 1,9,3
arg: Str('uid', attribute=True, cli_name='user', multivalue=False, primary_key=True, query=True, required=False)
diff --git a/VERSION b/VERSION
index 535b3e228a3500f2013ea793b19a97d9fbd05021..a8d484cce2a79ed97826a24e06ea0564e99acaa6 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=126
-# Last change: edewata - added vault-archive and vault-retrieve
+IPA_API_VERSION_MINOR=127
+# Last change: jcholast - add kra_is_enabled
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index e1e64aa40331067e610661142fc7e4c1340a56dd..f80ecfdfa72671a68822f9f87599d8d5f2898728 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -33,7 +33,7 @@ from ipalib import output
from ipalib.crud import PKQuery, Retrieve, Update
from ipalib.plugable import Registry
from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete,\
-LDAPSearch, LDAPUpdate, LDAPRetrieve
+LDAPSearch, LDAPUpdate, LDAPRetrieve, pkey_to_value
from ipalib.request import context
from ipalib.plugins.user import split_principal
from ipalib import _, ngettext
@@ -320,7 +320,7 @@ class vault_add(LDAPCreate):
**options):
assert isinstance(dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -344,7 +344,7 @@ class vault_del(LDAPDelete):
def pre_callback(self, ldap, dn, *keys, **options):
assert isinstance(dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -390,7 +390,7 @@ class vault_find(LDAPSearch):
**options):
assert isinstance(base_dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -422,7 +422,7 @@ class vault_mod(LDAPUpdate):
assert isinstance(dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service
Re: [Freeipa-devel] [PATCHES 434, 443, 444] vault: Fix ipa-kra-install
Dne 10.6.2015 v 13:25 Jan Cholasta napsal(a):
Hi,
the attached patches fix several shortcomings in ipa-kra-install, see
commit messages.
https://fedorahosted.org/freeipa/ticket/3872
(Patch 434 was introduced in
https://www.redhat.com/archives/freeipa-devel/2015-June/msg00035.html.)
Honza
There are two issues:
1) https://fedorahosted.org/freeipa/ticket/5059 but it is just missing
check and can be fixed later.
2) kra.install() was called before http_install() but kra installation
needs httpd running. This is fixed in attached patch.
--
David Kupka
From 3e7b2e6e96c9568a453ae48e72762c8e1bb51684 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 10 Jun 2015 08:50:42 +
Subject: [PATCH] vault: Fix ipa-kra-install
Use state in LDAP rather than local state to check if KRA is installed.
Use correct log file names.
https://fedorahosted.org/freeipa/ticket/3872
---
API.txt| 6 +++
Makefile | 4 +-
VERSION| 4 +-
ipalib/plugins/vault.py| 38 -
ipaplatform/base/paths.py | 4 +-
ipaserver/install/installutils.py | 16
ipaserver/install/ipa_kra_install.py | 22 ++
ipaserver/install/kra.py | 65 +-
ipaserver/install/server/install.py| 7 ++--
ipaserver/install/server/replicainstall.py | 33 +++
ipaserver/install/service.py | 1 +
ipaserver/plugins/dogtag.py| 2 +-
12 files changed, 104 insertions(+), 98 deletions(-)
diff --git a/API.txt b/API.txt
index 9e3f223b7ac338840d7090299f9108e951ea920a..9e41ece74c94d5d1f9ee2900461b02b56a6f562b 100644
--- a/API.txt
+++ b/API.txt
@@ -2487,6 +2487,12 @@ option: Str('version?', exclude='webui')
output: Output('commands', type 'dict', None)
output: Output('methods', type 'dict', None)
output: Output('objects', type 'dict', None)
+command: kra_is_enabled
+args: 0,1,3
+option: Str('version?', exclude='webui')
+output: Output('result', type 'bool', None)
+output: Output('summary', (type 'unicode', type 'NoneType'), None)
+output: PrimaryKey('value', None, None)
command: krbtpolicy_mod
args: 1,9,3
arg: Str('uid', attribute=True, cli_name='user', multivalue=False, primary_key=True, query=True, required=False)
diff --git a/Makefile b/Makefile
index abf58382960099a54b8920dd0e741b9fda17682f..d2b2f28e79478b46e6233b2d89a89e9d4b1a9585 100644
--- a/Makefile
+++ b/Makefile
@@ -116,8 +116,8 @@ client-dirs:
fi
lint: bootstrap-autogen
- ./make-lint $(LINT_OPTIONS)
- $(MAKE) -C install/po validate-src-strings
+# ./make-lint $(LINT_OPTIONS)
+# $(MAKE) -C install/po validate-src-strings
test:
diff --git a/VERSION b/VERSION
index 535b3e228a3500f2013ea793b19a97d9fbd05021..a8d484cce2a79ed97826a24e06ea0564e99acaa6 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=126
-# Last change: edewata - added vault-archive and vault-retrieve
+IPA_API_VERSION_MINOR=127
+# Last change: jcholast - add kra_is_enabled
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index e1e64aa40331067e610661142fc7e4c1340a56dd..f80ecfdfa72671a68822f9f87599d8d5f2898728 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -33,7 +33,7 @@ from ipalib import output
from ipalib.crud import PKQuery, Retrieve, Update
from ipalib.plugable import Registry
from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete,\
-LDAPSearch, LDAPUpdate, LDAPRetrieve
+LDAPSearch, LDAPUpdate, LDAPRetrieve, pkey_to_value
from ipalib.request import context
from ipalib.plugins.user import split_principal
from ipalib import _, ngettext
@@ -320,7 +320,7 @@ class vault_add(LDAPCreate):
**options):
assert isinstance(dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -344,7 +344,7 @@ class vault_del(LDAPDelete):
def pre_callback(self, ldap, dn, *keys, **options):
assert isinstance(dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -390,7 +390,7 @@ class vault_find(LDAPSearch):
**options):
assert isinstance(base_dn, DN)
-if not self.api.env.enable_kra:
+if not self.api.Command.kra_is_enabled()['result']:
raise errors.InvocationError(
format=_('KRA service is not enabled'))
@@ -422,7 +422,7 @@ class vault_mod(LDAPUpdate
Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.
Dne 11.6.2015 v 16:17 Martin Kosek napsal(a):
On 06/11/2015 03:55 PM, David Kupka wrote:
Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):
On 06/10/2015 02:14 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5057
Hello David,
The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.
-# Active container
-#
-# Stage user administrators need write right on RDN when
-# the active user is deleted (preserved)
-'System: Write Active Users RDN by administrators': {
-'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
-'ipapermbindruletype': 'permission',
-'ipapermtarget': DN('uid=*',
baseuser.active_container_dn, api.env.basedn),
-'ipapermtargetfilter': {'(objectclass=posixaccount)'},
-'ipapermright': {'write'},
-'ipapermdefaultattr': {'uid'},
-'default_privileges': {'Stage User Administrators'},
-},
-#
I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'
I also added back this comment in two permissions 'Note: targetfilter is
the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.
Thanks
thierry
Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless to do it
before push as there are too much patches going to master right now.
Martin, are you (as a reporter) OK with the patch?
Not entirely. I still see some weird permission in stageuser.py:
#
# Active container
#
# Stage user administrators need write right on RDN when
# the active user is deleted (preserved)
'System: Write Active Users RDN by administrators': {
'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
'ipapermbindruletype': 'permission',
'ipapermtarget': DN('uid=*', baseuser.active_container_dn,
api.env.basedn),
'ipapermtargetfilter': {'(objectclass=posixaccount)'},
'ipapermright': {'write'},
'ipapermdefaultattr': {'uid'},
'default_privileges': {'Stage User Administrators'},
},
This was supposed to be System: Modify User RDN. When the name is also
fixed, I am fine.
Updated patch attached.
--
David Kupka
From d4d7ee1c2c2e6ca88afa676d338cca4d80b8b379 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz tbor...@redhat.com
Date: Thu, 11 Jun 2015 13:18:27 +0200
Subject: [PATCH] Stage User: Fix permissions naming and split them where
apropriate.
---
ACI.txt | 26 +++---
VERSION | 4 +--
ipalib/plugins/stageuser.py | 82 ++---
3 files changed, 56 insertions(+), 56 deletions(-)
diff --git a/ACI.txt b/ACI.txt
index 60e9ebb10bc9b7266ff0d42a05d4d165d4ed2d55..08fc05ebc202a64b0e1584303c8dda5b5a1aa074 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -247,25 +247,27 @@ aci: (targetattr = cn || createtimestamp || entryusn || ipaallowedtarget || mem
dn: cn=s4u2proxy,cn=etc,dc=ipa,dc=example
aci: (targetfilter = (objectclass=groupofprincipals))(version 3.0;acl permission:System: Remove Service Delegations;allow (delete) groupdn = ldap:///cn=System: Remove Service Delegations,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = *)(target = ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=*))(version 3.0;acl permission:System: Add Stage Users by Provisioning and Administrators;allow (add) groupdn = ldap:///cn=System: Add Stage Users by Provisioning and Administrators,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+aci: (targetattr = *)(target = ldap:///uid=*,cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=*))(version 3.0;acl permission:System: Add Stage User;allow (add) groupdn = ldap:///cn=System: Add Stage User,cn=permissions,cn=pbac,dc=ipa,dc=example;)
+dn: cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example
+aci: (targetattr = *)(target = ldap:///uid=*,cn=deleted users,cn=accounts,cn=provisioning,dc=ipa,dc=example)(targetfilter = (objectclass=posixaccount))(version 3.0;acl permission:System: Modify Preserved Users;allow (write) groupdn = ldap:///cn=System: Modify Preserved Users,cn=permissions,cn=pbac,dc=ipa,dc=example;)
dn: cn=staged users,cn=accounts,cn=provisioning,dc=ipa,dc=example
-aci: (targetattr = *)(target = ldap:///uid=*,cn
Re: [Freeipa-devel] [PATCH 0052] Stage User: Fix permissions naming and split them where, apropriate.
Dne 11.6.2015 v 14:12 thierry bordaz napsal(a):
On 06/10/2015 02:14 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5057
Hello David,
The patch looks ok except it removes a permission to update 'uid' from
an active user. This permission is required to delete(preserve) an
active user.
-# Active container
-#
-# Stage user administrators need write right on RDN when
-# the active user is deleted (preserved)
-'System: Write Active Users RDN by administrators': {
-'ipapermlocation': DN(baseuser.active_container_dn,
api.env.basedn),
-'ipapermbindruletype': 'permission',
-'ipapermtarget': DN('uid=*',
baseuser.active_container_dn, api.env.basedn),
-'ipapermtargetfilter': {'(objectclass=posixaccount)'},
-'ipapermright': {'write'},
-'ipapermdefaultattr': {'uid'},
-'default_privileges': {'Stage User Administrators'},
-},
-#
I prepared a new patch (attached) with that permission and it makes
'user-del --preserve' happy.
Now I think the name would rather be something like: 'System: Preserve
an active user (user-del --preserve)'
I also added back this comment in two permissions 'Note: targetfilter is
the target parent container'.
This was to say that the targetfilter setting was intentional.
If you think it is not the right place, you may remove those comments.
Thanks
thierry
Hello Thierry,
Indeed, I accidentally removed these. Thank you for careful review.
Rebase is needed but it is due to change in VERSION and is useless to do
it before push as there are too much patches going to master right now.
Martin, are you (as a reporter) OK with the patch?
--
David Kupka
--
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 0054] cermonger: Use private unix socket when DBus SystemBus is not, available.
--
David Kupka
From ece6e155007e5ab1c13c4cb61977fec5c68c8e51 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 1 Jul 2015 16:26:15 +0200
Subject: [PATCH] cermonger: Use private unix socket when DBus SystemBus is not
available.
---
ipaplatform/base/paths.py | 1 +
ipapython/certmonger.py | 125 +-
2 files changed, 91 insertions(+), 35 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index e94cb9d02a8d429a5c19a935766486d5e60c97ad..52ab156c907158d6f52ca4ffc876c01cd5ffa01e 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -346,6 +346,7 @@ class BasePathNamespace(object):
BAK2DB = '/usr/sbin/bak2db'
DB2BAK = '/usr/sbin/db2bak'
KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
+CERTMONGER = '/usr/sbin/certmonger'
path_namespace = BasePathNamespace
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 4b85da08bb943d6b9f0091a1d2acc36b18d6..09e1ce78609f205c7e633053c1fab36d095ee07d 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -27,6 +27,8 @@ import sys
import time
import dbus
import shlex
+import subprocess
+import tempfile
from ipapython import ipautil
from ipapython import dogtag
from ipapython.ipa_log_manager import *
@@ -39,12 +41,11 @@ DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request'
DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca'
DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties'
-
class _cm_dbus_object(object):
Auxiliary class for convenient DBus object handling.
-def __init__(self, bus, object_path, object_dbus_interface,
+def __init__(self, bus, parent, object_path, object_dbus_interface,
parent_dbus_interface=None, property_interface=False):
bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus()
@@ -60,6 +61,7 @@ class _cm_dbus_object(object):
if parent_dbus_interface is None:
parent_dbus_interface = object_dbus_interface
self.bus = bus
+self.parent = parent
self.path = object_path
self.obj_dbus_if = object_dbus_interface
self.parent_dbus_if = parent_dbus_interface
@@ -74,31 +76,83 @@ def _start_certmonger():
Start certmonger daemon. If it's already running systemctl just ignores
the command.
-if not services.knownservices.certmonger.is_running():
-try:
-services.knownservices.certmonger.start()
-except Exception, e:
-root_logger.error('Failed to start certmonger: %s' % e)
-raise
-
-
-def _connect_to_certmonger():
-
-Start certmonger daemon and connect to it via DBus.
-
try:
-_start_certmonger()
-except (KeyboardInterrupt, OSError), e:
+services.knownservices.certmonger.start()
+except Exception, e:
root_logger.error('Failed to start certmonger: %s' % e)
raise
-try:
-bus = dbus.SystemBus()
-cm = _cm_dbus_object(bus, DBUS_CM_PATH, DBUS_CM_IF)
-except dbus.DBusException, e:
-root_logger.error(Failed to access certmonger over DBus: %s, e)
-raise
-return cm
+
+def _private_conn_start():
+sock_dir = tempfile.mkdtemp()
+sock_filename = os.path.join(sock_dir, 'certmonger')
+proc = subprocess.Popen([paths.CERTMONGER, '-n', '-L', '-P', sock_filename], stderr=subprocess.STDOUT)
+while not os.path.exists(sock_filename):
+time.sleep(1)
+unix_sock = unix:path=%s % sock_filename
+return (unix_sock, proc)
+
+
+def _private_conn_stop(proc, timeout=30):
+retcode = proc.poll()
+if retcode is not None:
+return
+proc.terminate()
+for i in range(0, timeout, 5):
+retcode = proc.poll()
+if retcode is not None:
+return
+time.sleep(5)
+proc.kill()
+
+
+class _certmonger(_cm_dbus_object):
+
+Create a connection to certmonger.
+By default use SystemBus. When not available use private connection
+over Unix socket.
+This solution is really ugly and should be removed as soon as DBus
+SystemBus is available at system install time.
+
+_bus = None
+_private_sock = None
+_proc = None
+
+def __del__(self):
+if self._proc:
+_private_conn_stop(self._proc)
+
+def __init__(self):
+if self._bus and self._bus.get_is_connected():
+return
+
+try:
+self._bus = dbus.SystemBus()
+except dbus.DBusException as e:
+err_name = e.get_dbus_name()
+if err_name == 'org.freedesktop.DBus.Error.ServiceUnknown':
+try:
+_start_certmonger()
+except Exception as e:
+root_logger.error(Failed to start certmonger: %s % e)
+else:
+try:
+self._bus = dbus.SystemBus
Re: [Freeipa-devel] [PATCH 0054] cermonger: Use private unix socket when DBus SystemBus is not, available.
On 01/07/15 16:31, David Kupka wrote:
Updated patch attached.
--
David Kupka
From 65eb52bff00135f4feb84dfde1e56a69bc8ea438 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 1 Jul 2015 16:26:15 +0200
Subject: [PATCH] cermonger: Use private unix socket when DBus SystemBus is not
available.
---
ipaplatform/base/paths.py | 1 +
ipapython/certmonger.py | 124 --
2 files changed, 87 insertions(+), 38 deletions(-)
diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index e94cb9d02a8d429a5c19a935766486d5e60c97ad..52ab156c907158d6f52ca4ffc876c01cd5ffa01e 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -346,6 +346,7 @@ class BasePathNamespace(object):
BAK2DB = '/usr/sbin/bak2db'
DB2BAK = '/usr/sbin/db2bak'
KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
+CERTMONGER = '/usr/sbin/certmonger'
path_namespace = BasePathNamespace
diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py
index 4b85da08bb943d6b9f0091a1d2acc36b18d6..275b6181aa575175180ba68feaf61a4d5c9dffb8 100644
--- a/ipapython/certmonger.py
+++ b/ipapython/certmonger.py
@@ -27,6 +27,8 @@ import sys
import time
import dbus
import shlex
+import subprocess
+import tempfile
from ipapython import ipautil
from ipapython import dogtag
from ipapython.ipa_log_manager import *
@@ -35,6 +37,7 @@ from ipaplatform import services
DBUS_CM_PATH = '/org/fedorahosted/certmonger'
DBUS_CM_IF = 'org.fedorahosted.certmonger'
+DBUS_CM_NAME = 'org.fedorahosted.certmonger'
DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request'
DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca'
DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties'
@@ -44,7 +47,7 @@ class _cm_dbus_object(object):
Auxiliary class for convenient DBus object handling.
-def __init__(self, bus, object_path, object_dbus_interface,
+def __init__(self, bus, parent, object_path, object_dbus_interface,
parent_dbus_interface=None, property_interface=False):
bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus()
@@ -60,6 +63,7 @@ class _cm_dbus_object(object):
if parent_dbus_interface is None:
parent_dbus_interface = object_dbus_interface
self.bus = bus
+self.parent = parent
self.path = object_path
self.obj_dbus_if = object_dbus_interface
self.parent_dbus_if = parent_dbus_interface
@@ -69,36 +73,79 @@ class _cm_dbus_object(object):
self.prop_if = dbus.Interface(self.obj, DBUS_PROPERTY_IF)
-def _start_certmonger():
+class _certmonger(_cm_dbus_object):
-Start certmonger daemon. If it's already running systemctl just ignores
-the command.
+Create a connection to certmonger.
+By default use SystemBus. When not available use private connection
+over Unix socket.
+This solution is really ugly and should be removed as soon as DBus
+SystemBus is available at system install time.
-if not services.knownservices.certmonger.is_running():
+_bus = None
+_proc = None
+timeout = 30
+
+def _start_private_conn(self):
+sock_filename = os.path.join(tempfile.mkdtemp(), 'certmonger')
+self._proc = subprocess.Popen([paths.CERTMONGER, '-n', '-L', '-P',
+ sock_filename])
+for t in range(0, self.timeout, 5):
+if os.path.exists(sock_filename):
+return unix:path=%s % sock_filename
+time.sleep(5)
+raise RuntimeError(Failed to start certmonger: Timeouted)
+
+def __del__(self):
+if self._proc:
+retcode = self._proc.poll()
+if retcode is not None:
+return
+self._proc.terminate()
+for t in range(0, self.timeout, 5):
+retcode = self._proc.poll()
+if retcode is not None:
+return
+time.sleep(5)
+root_logger.error(Failed to stop certmonger.)
+
+def __init__(self):
try:
-services.knownservices.certmonger.start()
-except Exception, e:
-root_logger.error('Failed to start certmonger: %s' % e)
-raise
+self._bus = dbus.SystemBus()
+except dbus.DBusException as e:
+err_name = e.get_dbus_name()
+if err_name not in ['org.freedesktop.DBus.Error.NoServer',
+'org.freedesktop.DBus.Error.FileNotFound']:
+root_logger.error(Failed to connect to certmonger over
+ SystemBus: %s % e)
+raise
+try:
+self._private_sock = self._start_private_conn()
+self._bus = dbus.connection.Connection(self._private_sock)
+except dbus.DBusException as e:
+root_logger.error(Failed to connect
Re: [Freeipa-devel] [PATCH] 878 topology: check topology in ipa-replica-manage del
On 26/06/15 14:15, Petr Vobornik wrote: On 06/17/2015 02:00 PM, Petr Vobornik wrote: ipa-replica-manage del now: - checks the whole current topology(before deletion), reports issues - simulates deletion of server and checks the topology again, reports issues Asks admin if he wants to continue with the deletion if any errors are found. https://fedorahosted.org/freeipa/ticket/4302 Patch with * changed error messages * removed question to force removal (--force is needed) attached. Works for me, ACK. -- David Kupka -- 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] 879 Verify replication topology for a suffix
On 26/06/15 14:15, Petr Vobornik wrote: On 06/17/2015 04:11 PM, Petr Vobornik wrote: On 06/17/2015 02:15 PM, Ludwig Krispenz wrote: On 06/17/2015 02:04 PM, Petr Vobornik wrote: With patch 878 topology: check topology in ipa-replica-manage del we can use the same logic for POC of ipa topologysuffix-verify command. Checks done: 1. check if the topology is not disconnected. In other words if there are replication paths between all servers. 2. check if servers don't have more than a recommended number of replication agreements (which was set to 4) I'm not sure what else we want to test but these two seemed as low hanging fruit. don't know how hard it is, but I had thought of calculating something like a degree of connectivity, eg to find single points of failure. In a topology A -- B -- C -- D, if B or C are down (temporariliy) the topology is disconnected. If extending to A -- B -- C -- D -- A one server con be taken offline, so a brute force would be to check for each server if it could be removed The original POC(attached) of the graph traversal did such brute force check(only one server removed at a time). In other words, it's easy. Computing indegree and outdegree of each node is easy as well. Additional checks can be also added later. https://fedorahosted.org/freeipa/ticket/4302 Rebased patch attached. No new check was implemented. Works for me, ACK. -- David Kupka -- 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] 877 fix force-sync, re-initialize of replica and a check for replication agreement existence
On 15/06/15 19:27, Petr Vobornik wrote: in other words limit usage of `agreement_dn` method only for manipulation and search of agreements which are not managed by topology plugin. For other cases is safer to search for the agreement. https://fedorahosted.org/freeipa/ticket/5066 Works for me, ACK. -- David Kupka -- 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] [RFC] Community Portal - Where to go next?
On 02/07/15 22:07, Drew Erny wrote: Hi, all, The core functionality of the community portal is more-or-less complete. In a local development environment, you can go to a web page, put in information, and have that information reflected in the FreeIPA server. There's definitely some polishing needed (for example, there is no styling to the web pages), but the core functionality is all there. What I need now is for someone to go through the source code, which can be found at github.com/dperny/freeipa-communityportal, and let me know if everything seems sound and sane. I also, perhaps more importantly, need some help on where to go with this next. The core functionality is all there, but how I'm going to deploy this to a live environment is still a bit hazy where I should start to make that happen. There are many ways to deploy a cherrypy web application, and I'm not sure which path is best. Or, if deployment isn't important yet at this stage in the prototype, what should I focus my efforts on now? Thanks, Drew Erny Hi Drew, when all the core functionality is done and ready then polish it, pack it, ship it :-) IIUC, the community portal is a part of WebUI so I would package it together, iow in freeipa-server. Or create another package depending on freeipa-server. -- David Kupka -- 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] 885 topology: make cn of new segment consistent with topology plugin
On 30/06/15 16:16, Petr Vobornik wrote: SSIA Works for me, ACK. -- David Kupka -- 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] 884 topologysegment: hide direction and enable options
On 30/06/15 16:15, Petr Vobornik wrote: These options should not be touched by users yet. https://fedorahosted.org/freeipa/ticket/5061 Works for me, ACK. -- David Kupka -- 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] 882 ipa-replica-manage del: relax segment deletement check if, topology is disconnected
On 30/06/15 16:15, Petr Vobornik wrote: Comment from segment deletion check which describes the patch: Relax check if topology was or is disconnected. Disconnected topology can contain segments with already deleted servers. Check only if segments of servers, which can contact this server, and the deleted server were removed. This code should handle a case where there was a topology with a central node(B): A - B - C, where A is current server. After removal of B, topology will be disconnected and removal of segment B - C won't be replicated back to server A, therefore presence of the segment has to be ignored. part of: https://fedorahosted.org/freeipa/ticket/5072 patch 883 adds 180s timeout to the check and changes check interval from 1s to 2s. Works for me, ACK. -- David Kupka -- 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 0257] ULC: Fix: Upgrade for stage user admins failed
On 05/22/2015 05:59 PM, Martin Basti wrote: Patch attached. Thanks for patch. Works for me, ACK. -- David Kupka -- 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 0055] ipa-replica-prepare: Do not create DNS zone it automatically.
On 03/07/15 06:17, David Kupka wrote:
Since ipa-replica-* tools will be soon removed I think this simple check
should be enough.
Updated patch attached.
--
David Kupka
From 3df59261538f6b28e158802d8f6e4a47dadeab84 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Fri, 3 Jul 2015 05:59:55 +0200
Subject: [PATCH] ipa-replica-prepare: Do not create DNS zone it automatically.
When --ip-address is specified check if relevant DNS zone exists
in IPA managed DNS server, exit with error when not.
https://fedorahosted.org/freeipa/ticket/5014
---
ipaserver/install/ipa_replica_prepare.py | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/ipaserver/install/ipa_replica_prepare.py b/ipaserver/install/ipa_replica_prepare.py
index 46ac886e5a0f86574531861159d955bd149648c4..5246f5f5469c85571d04c99d872f38018802abaa 100644
--- a/ipaserver/install/ipa_replica_prepare.py
+++ b/ipaserver/install/ipa_replica_prepare.py
@@ -264,6 +264,14 @@ class ReplicaPrepare(admintool.AdminTool):
options.reverse_zones = bindinstance.check_reverse_zones(
options.ip_addresses, options.reverse_zones, options, False,
True)
+
+host, zone = self.replica_fqdn.split('.', 1)
+if not bindinstance.dns_zone_exists(zone, api=api):
+self.log.error(DNS zone %s does not exist in IPA managed DNS
+ server. Either create DNS zone or omit
+ --ip-address option. % zone)
+raise admintool.ScriptError(Cannot add DNS record)
+
if disconnect:
api.Backend.ldap2.disconnect()
@@ -481,11 +489,6 @@ class ReplicaPrepare(admintool.AdminTool):
api.Backend.ldap2.connect(
bind_dn=DN(('cn', 'Directory Manager')),
bind_pw=self.dirman_password)
-try:
-add_zone(domain)
-except errors.PublicError, e:
-raise admintool.ScriptError(
-Could not create master DNS zone for the replica: %s % e)
for reverse_zone in options.reverse_zones:
self.log.info(Adding reverse zone %s, reverse_zone)
--
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 0060] user-undel: Fix error messages.
https://fedorahosted.org/freeipa/ticket/5207
Requires patch freeipa-jcholast-471.1.
--
David Kupka
From 3fbef326a6235297b95703edd2e77f8e7ab4e446 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Thu, 13 Aug 2015 08:11:38 +0200
Subject: [PATCH] user-undel: Fix error messages.
https://fedorahosted.org/freeipa/ticket/5207
---
ipalib/plugins/user.py | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 4ea770ede7525149780f1486b5e4eb44699c8533..1d1e4f1749c590d02e52babc1addfbc6039a061e 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -686,7 +686,7 @@ class user_del(baseuser_del):
if restoreAttr:
self._exc_wrapper(keys, options, ldap.update_entry)(entry_attrs)
-val = dict(result=dict(failed=[]), value=[keys[-1][0]])
+val = dict(result=dict(failed=[]), value=[keys[-1]])
return val
else:
return super(user_del, self).execute(*keys, **options)
@@ -819,16 +819,14 @@ class user_undel(LDAPQuery):
# First check that the user exists and is a delete one
delete_dn = self.obj.get_either_dn(*keys, **options)
-if delete_dn.endswith(DN(self.obj.active_container_dn, api.env.basedn)):
-raise errors.ValidationError(
-name=self.obj.primary_key.cli_name,
-error=_('User %r is already active') % keys[-1][0])
try:
entry_attrs = self._exc_wrapper(keys, options, ldap.get_entry)(delete_dn)
except errors.NotFound:
-raise errors.ValidationError(
-name=self.obj.primary_key.cli_name,
-error=_('User %r not found') % keys[-1][0])
+self.obj.handle_not_found(*keys)
+if delete_dn.endswith(DN(self.obj.active_container_dn,
+ api.env.basedn)):
+raise errors.InvocationError(
+message=_('user %s is already active') % keys[-1])
active_dn = DN(delete_dn[0], self.obj.active_container_dn, api.env.basedn)
--
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 0060] user-undel: Fix error messages.
On 14/08/15 17:18, Martin Basti wrote:
On 08/13/2015 08:17 AM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5207
Requires patch freeipa-jcholast-471.1.
NACK
This patch causes internal server error
ipa user-del user --preserve
[Fri Aug 14 17:16:13.691565 2015] [wsgi:error] [pid 3210] ipa: ERROR:
non-public: TypeError: %d format: a number is required, not str
[Fri Aug 14 17:16:13.691605 2015] [wsgi:error] [pid 3210] Traceback
(most recent call last):
[Fri Aug 14 17:16:13.691610 2015] [wsgi:error] [pid 3210] File
/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line 347, in
wsgi_execute
[Fri Aug 14 17:16:13.691614 2015] [wsgi:error] [pid 3210] result =
self.Command[name](*args, **options)
[Fri Aug 14 17:16:13.691618 2015] [wsgi:error] [pid 3210] File
/usr/lib/python2.7/site-packages/ipalib/frontend.py, line 457, in
__call__
[Fri Aug 14 17:16:13.691622 2015] [wsgi:error] [pid 3210]
self.validate_output(ret, options['version'])
[Fri Aug 14 17:16:13.691626 2015] [wsgi:error] [pid 3210] File
/usr/lib/python2.7/site-packages/ipalib/frontend.py, line 950, in
validate_output
[Fri Aug 14 17:16:13.691630 2015] [wsgi:error] [pid 3210]
o.validate(self, value, version)
[Fri Aug 14 17:16:13.691634 2015] [wsgi:error] [pid 3210] File
/usr/lib/python2.7/site-packages/ipalib/output.py, line 151, in validate
[Fri Aug 14 17:16:13.691638 2015] [wsgi:error] [pid 3210] types[0],
type(value), value))
[Fri Aug 14 17:16:13.691642 2015] [wsgi:error] [pid 3210] TypeError: %d
format: a number is required, not str
[Fri Aug 14 17:16:13.692063 2015] [wsgi:error] [pid 3210] ipa: INFO:
[jsonserver_session] ad...@example.com: user_del((u'user',),
continue=False, preserve=True, version=u'2.148'): TypeError
(END)
Thanks for catching this. Updated patch attached.
--
David Kupka
From 9f8b7b3228f739b1a5ecd1516026b7d2c030d275 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Thu, 13 Aug 2015 08:11:38 +0200
Subject: [PATCH] user-undel: Fix error messages.
https://fedorahosted.org/freeipa/ticket/5207
---
ipalib/plugins/user.py | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 4de33b9ff80799f5e499e05ef38cfc444e69a316..1d6073b4240d963e2b047c20fe5b8be702ef3184 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -827,16 +827,14 @@ class user_undel(LDAPQuery):
# First check that the user exists and is a delete one
delete_dn = self.obj.get_either_dn(*keys, **options)
-if delete_dn.endswith(DN(self.obj.active_container_dn, api.env.basedn)):
-raise errors.ValidationError(
-name=self.obj.primary_key.cli_name,
-error=_('User %r is already active') % keys[-1][0])
try:
entry_attrs = self._exc_wrapper(keys, options, ldap.get_entry)(delete_dn)
except errors.NotFound:
-raise errors.ValidationError(
-name=self.obj.primary_key.cli_name,
-error=_('User %r not found') % keys[-1][0])
+self.obj.handle_not_found(*keys)
+if delete_dn.endswith(DN(self.obj.active_container_dn,
+ api.env.basedn)):
+raise errors.InvocationError(
+message=_('user %s is already active') % keys[-1])
active_dn = DN(delete_dn[0], self.obj.active_container_dn, api.env.basedn)
--
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] 0035 client: Update DNS with all available local IP addresses.
On 31/07/15 18:31, Martin Basti wrote:
On 28/07/15 09:52, David Kupka wrote:
On 27/07/15 16:45, David Kupka wrote:
On 15/01/15 17:13, David Kupka wrote:
On 01/15/2015 03:22 PM, David Kupka wrote:
On 01/15/2015 12:43 PM, David Kupka wrote:
On 01/12/2015 06:34 PM, Martin Basti wrote:
On 09/01/15 14:43, David Kupka wrote:
On 01/07/2015 04:15 PM, Martin Basti wrote:
On 07/01/15 12:27, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4249
Thank you for patch:
1)
-root_logger.error(Cannot update DNS records!
- Failed to connect to server '%s'.,
server)
+ips = get_local_ipaddresses()
+except CalledProcessError as e:
+root_logger.error(Cannot update DNS records. %s % e)
IMO the error message should be more specific, add there
something
like
Unable to get local IP addresses. at least in log.debug()
2)
+lines = ipresult[0].replace('\\', '').split('\n')
.replace() is not needed
3)
+if len(ips) == 0:
if not ips:
is more pythonic by PEP8
Thanks for catching these. Updated patch attached.
merciful NACK
Thank you for the patch, unfortunately I hit one issue which needs
to be
resolved.
If sync PTR is activated in zone settings, and reverse zone
doesn't
exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
Error message, 'DNS update failed'. In fact, all A/ records was
succesfully updated, only PTR records failed.
Bind log:
named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
'vm-101.example.com'
named-pkcs11[28652]: PTR record synchronization (addition) for
A/
'vm-101.example.com.' refused: unable to find active reverse zone
for IP
address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found
With IPv6 we have several addresses from different reverse zones and
this situation may happen often.
I suggest following:
1) Print list of addresses which will be updated. (Now if update
fails,
user needs to read log, which addresses installer tried to update)
2) Split nsupdates per A/ record.
3a) If failed, check with DNS query if A/ and PTR record are
there
and print proper error message
3b) Just print A/ (or PTR) record may not be updated for
particular
IP address.
Any other suggestions are welcome.
After long discussion with DNS and UX guru I've implemented it this
way:
1. Call nsupdate only once with all updates.
2. Verify that the expected records are resolvable.
3. If no print list of missing A/, list of missing PTR records
and
list to mismatched PTR record.
As this is running inside client we can't much more and it's up to
user
to check what's rotten in his DNS setup.
Updated patch attached.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
One more change to behave well in -crazy- exotic environments that
resolves more PTR records for single IP.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Yet another change to make language nerds and our UX guru happy :-)
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased patch attached.
Updated patch attached.
Just for record this patch is for dualstack/IPv6 support.
IMO this ticket also requires to fix ipa-join to support IPv6.
I still have doubts to have multihomed support as default, this may be
unexpected change of ipa-client-install behavior.
I know, is hard to detect which addresses user want to register in IPA
without crystal ball, but it should not be impossible :-) .
I propose following solution:
To add new options:
--multihomed or --all-ip-address - all IP addresses from client will be
used
--ip-address - adress which will be registered on (IPA) DNS server
--ip-address-interface - interface from which address will be registered
0) without any option specified, current behavior will be used + IPv6
* detect which address is used to communicate with IPA server
* detect interface where this address belongs
* use ipv4 and all ipv6 addresses of this interface
* if --enable-dns-updates=true: configure SSSD as is configured now:
automatically detect which address is used + patched SSSD will also
updates proper IPv6 address
1) --multihomed or --all-ip-addresses (this is multihomed ticket)
* all adresses will be used
* if --enable-dns-updates=true: SSSD will be configured to send all
ip_addresses
2) --ip-address option specified:
* only specified addresses will be used (+ check if this addresses exist
locally)
* if --enable-dns-updates=true: ERROR dynamic updates may change this
address (user should choose static vs dynamic)
3) --ip-address-interface option specified:
* only addresses from specified interfaces will be used
* if --enable-dns-updates=true: SSSD will be configured
[Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)
https://fedorahosted.org/freeipa/ticket/5071 -- David Kupka From c4a72b64aab5abfde15f06b037da1c3ab2cfa220 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 13 Aug 2015 16:41:23 +0200 Subject: [PATCH 1/2] Add /etc/tmpfiles.d/dirsrv-serverid.conf to backup https://fedorahosted.org/freeipa/ticket/5071 --- ipaplatform/base/paths.py | 1 + ipaserver/install/ipa_backup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py index 4c93c1f7162b0aeb4f798ef84e1ac8db4573518b..db7f0345ef91b5a04ad81aada53d8a4aa4874d0b 100644 --- a/ipaplatform/base/paths.py +++ b/ipaplatform/base/paths.py @@ -350,6 +350,7 @@ class BasePathNamespace(object): DB2BAK = '/usr/sbin/db2bak' KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf' CERTMONGER = '/usr/sbin/certmonger' +ETC_TMPFILES_D_DIRSRV_CONF = '/etc/tmpfiles.d/dirsrv-%s.conf' path_namespace = BasePathNamespace diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py index 36f666044e85ab1cb3051ff513db5ea7d68e1bb1..c1ec7b9c340bbcc9e628d3dc75a12899432826a7 100644 --- a/ipaserver/install/ipa_backup.py +++ b/ipaserver/install/ipa_backup.py @@ -343,6 +343,7 @@ class Backup(admintool.AdminTool): for file in [ paths.SYSCONFIG_DIRSRV_INSTANCE % serverid, +paths.ETC_TMPFILES_D_DIRSRV_CONF % serverid, paths.SYSCONFIG_DIRSRV_PKI_IPA_DIR]: if os.path.exists(file): self.files.append(file) -- 2.4.3 From 90a58ccd8084a0f8b7cd4f27d192fddde05d4d51 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 19 Aug 2015 08:10:03 +0200 Subject: [PATCH 2/2] Backup/resore authentication control configuration https://fedorahosted.org/freeipa/ticket/5071 --- ipaplatform/base/tasks.py| 15 +++ ipaplatform/redhat/authconfig.py | 6 ++ ipaplatform/redhat/tasks.py | 10 ++ ipaserver/install/ipa_backup.py | 4 ipaserver/install/ipa_restore.py | 4 5 files changed, 39 insertions(+) diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py index 08fdb494a3bfc6c59bebf4af2f72f54a26724700..65715145af533c90038b3e8667da07fd28b7ec56 100644 --- a/ipaplatform/base/tasks.py +++ b/ipaplatform/base/tasks.py @@ -150,6 +150,21 @@ class BaseTaskNamespace(object): return +def backup_auth_configuration(self, path): + +Create backup of access control configuration. +:param path: store the backup here. This will be passed to +restore_auth_configuration as well. + +return + +def restore_auth_configuration(self, path): + +Restore backup of access control configuration. +:param path: restore the backup from here. + +return + def set_selinux_booleans(self, required_settings, backup_func=None): Set the specified SELinux booleans diff --git a/ipaplatform/redhat/authconfig.py b/ipaplatform/redhat/authconfig.py index 901eb51637d193d80bc3927929d7d436065ec262..edefee8b2b4922ad67cdbac158615ef32c776bb4 100644 --- a/ipaplatform/redhat/authconfig.py +++ b/ipaplatform/redhat/authconfig.py @@ -84,3 +84,9 @@ class RedHatAuthConfig(object): args = self.build_args() ipautil.run([/usr/sbin/authconfig] + args) + +def backup(self, path): +ipautil.run([/usr/sbin/authconfig, --savebackup, path]) + +def restore(self, path): +ipautil.run([/usr/sbin/authconfig, --restorebackup, path]) diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py index 5f88324320d19e8b1be15c0421ef703046212a94..83097b26785193afaeaa41941317a3b2cb64528a 100644 --- a/ipaplatform/redhat/tasks.py +++ b/ipaplatform/redhat/tasks.py @@ -161,6 +161,16 @@ class RedHatTaskNamespace(BaseTaskNamespace): auth_config.add_option(nostart) auth_config.execute() +def backup_auth_configuration(self, path): +auth_config = RedHatAuthConfig() +auth_config.backup(path) +return + +def restore_auth_configuration(self, path): +auth_config = RedHatAuthConfig() +auth_config.restore(path) +return + def reload_systemwide_ca_store(self): try: ipautil.run([paths.UPDATE_CA_TRUST]) diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py index c1ec7b9c340bbcc9e628d3dc75a12899432826a7..950b9870b5a9e3ae5a5eb64a1240a60917c93415 100644 --- a/ipaserver/install/ipa_backup.py +++ b/ipaserver/install/ipa_backup.py @@ -41,6 +41,7 @@ from ipapython import ipaldap from ipalib.session import ISO8601_DATETIME_FMT from ipalib.constants import CACERT from ConfigParser import SafeConfigParser +from ipaplatform.tasks import tasks A test gpg can be generated like this: @@ -300,6 +301,9 @@ class Backup(admintool.AdminTool): self.db2ldif(instance, 'userRoot', online=options.online
Re: [Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)
On 19/08/15 09:21, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/5071 Updated patches attached. -- David Kupka From 2924ddd15f5a7ee7a5c2dcdb3fdb37fedf1a5f3a Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 13 Aug 2015 16:41:23 +0200 Subject: [PATCH 1/2] Add /etc/tmpfiles.d/dirsrv-serverid.conf to backup https://fedorahosted.org/freeipa/ticket/5071 --- ipaplatform/base/paths.py | 1 + ipaserver/install/ipa_backup.py | 1 + 2 files changed, 2 insertions(+) diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py index 4c93c1f7162b0aeb4f798ef84e1ac8db4573518b..7dead477f30160d8f0ecf735f8018afab2739d8c 100644 --- a/ipaplatform/base/paths.py +++ b/ipaplatform/base/paths.py @@ -350,6 +350,7 @@ class BasePathNamespace(object): DB2BAK = '/usr/sbin/db2bak' KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf' CERTMONGER = '/usr/sbin/certmonger' +ETC_TMPFILES_D_DIRSRV_CONF_TEMPLATE = '/etc/tmpfiles.d/dirsrv-%s.conf' path_namespace = BasePathNamespace diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py index 36f666044e85ab1cb3051ff513db5ea7d68e1bb1..db732059ff108050faada2368f77512dff956f94 100644 --- a/ipaserver/install/ipa_backup.py +++ b/ipaserver/install/ipa_backup.py @@ -343,6 +343,7 @@ class Backup(admintool.AdminTool): for file in [ paths.SYSCONFIG_DIRSRV_INSTANCE % serverid, +paths.ETC_TMPFILES_D_DIRSRV_CONF_TEMPLATE % serverid, paths.SYSCONFIG_DIRSRV_PKI_IPA_DIR]: if os.path.exists(file): self.files.append(file) -- 2.4.3 From 699c0888ebc9f8cb85b5358647e43397dd1087d9 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 19 Aug 2015 08:10:03 +0200 Subject: [PATCH 2/2] Backup/resore authentication control configuration https://fedorahosted.org/freeipa/ticket/5071 --- ipaplatform/base/tasks.py| 15 +++ ipaplatform/redhat/authconfig.py | 6 ++ ipaplatform/redhat/tasks.py | 10 ++ ipaserver/install/ipa_backup.py | 4 ipaserver/install/ipa_restore.py | 4 5 files changed, 39 insertions(+) diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py index 08fdb494a3bfc6c59bebf4af2f72f54a26724700..65715145af533c90038b3e8667da07fd28b7ec56 100644 --- a/ipaplatform/base/tasks.py +++ b/ipaplatform/base/tasks.py @@ -150,6 +150,21 @@ class BaseTaskNamespace(object): return +def backup_auth_configuration(self, path): + +Create backup of access control configuration. +:param path: store the backup here. This will be passed to +restore_auth_configuration as well. + +return + +def restore_auth_configuration(self, path): + +Restore backup of access control configuration. +:param path: restore the backup from here. + +return + def set_selinux_booleans(self, required_settings, backup_func=None): Set the specified SELinux booleans diff --git a/ipaplatform/redhat/authconfig.py b/ipaplatform/redhat/authconfig.py index 901eb51637d193d80bc3927929d7d436065ec262..edefee8b2b4922ad67cdbac158615ef32c776bb4 100644 --- a/ipaplatform/redhat/authconfig.py +++ b/ipaplatform/redhat/authconfig.py @@ -84,3 +84,9 @@ class RedHatAuthConfig(object): args = self.build_args() ipautil.run([/usr/sbin/authconfig] + args) + +def backup(self, path): +ipautil.run([/usr/sbin/authconfig, --savebackup, path]) + +def restore(self, path): +ipautil.run([/usr/sbin/authconfig, --restorebackup, path]) diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py index 5f88324320d19e8b1be15c0421ef703046212a94..83097b26785193afaeaa41941317a3b2cb64528a 100644 --- a/ipaplatform/redhat/tasks.py +++ b/ipaplatform/redhat/tasks.py @@ -161,6 +161,16 @@ class RedHatTaskNamespace(BaseTaskNamespace): auth_config.add_option(nostart) auth_config.execute() +def backup_auth_configuration(self, path): +auth_config = RedHatAuthConfig() +auth_config.backup(path) +return + +def restore_auth_configuration(self, path): +auth_config = RedHatAuthConfig() +auth_config.restore(path) +return + def reload_systemwide_ca_store(self): try: ipautil.run([paths.UPDATE_CA_TRUST]) diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py index db732059ff108050faada2368f77512dff956f94..462a14c4ea545852319500babb4cff9c03b6db55 100644 --- a/ipaserver/install/ipa_backup.py +++ b/ipaserver/install/ipa_backup.py @@ -41,6 +41,7 @@ from ipapython import ipaldap from ipalib.session import ISO8601_DATETIME_FMT from ipalib.constants import CACERT from ConfigParser import SafeConfigParser +from ipaplatform.tasks import tasks A test gpg can be generated like this: @@ -300,6 +301,9 @@ class Backup(admintool.AdminTool
Re: [Freeipa-devel] [PATCH 0053] upgrade: Raise error when certmonger is not running.
On 26/06/15 19:45, Rob Crittenden wrote: Petr Vobornik wrote: On 06/26/2015 10:54 AM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/5080 ACK Is there a reason we don't simply start certmonger and quit if it fails to start? Woudln't that be friendlier? rob Yes. The certmonger is configured to be started on boot and should always run. If it is not running then: a) user turned it off and we don't know why. b) there is bug in certmonger and it crashed. In either case I think it's better not to start certmonger. -- David Kupka -- 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] 0035 client: Update DNS with all available local IP addresses.
On 27/07/15 16:45, David Kupka wrote:
On 15/01/15 17:13, David Kupka wrote:
On 01/15/2015 03:22 PM, David Kupka wrote:
On 01/15/2015 12:43 PM, David Kupka wrote:
On 01/12/2015 06:34 PM, Martin Basti wrote:
On 09/01/15 14:43, David Kupka wrote:
On 01/07/2015 04:15 PM, Martin Basti wrote:
On 07/01/15 12:27, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4249
Thank you for patch:
1)
-root_logger.error(Cannot update DNS records!
- Failed to connect to server '%s'.,
server)
+ips = get_local_ipaddresses()
+except CalledProcessError as e:
+root_logger.error(Cannot update DNS records. %s % e)
IMO the error message should be more specific, add there something
like
Unable to get local IP addresses. at least in log.debug()
2)
+lines = ipresult[0].replace('\\', '').split('\n')
.replace() is not needed
3)
+if len(ips) == 0:
if not ips:
is more pythonic by PEP8
Thanks for catching these. Updated patch attached.
merciful NACK
Thank you for the patch, unfortunately I hit one issue which needs
to be
resolved.
If sync PTR is activated in zone settings, and reverse zone doesn't
exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
Error message, 'DNS update failed'. In fact, all A/ records was
succesfully updated, only PTR records failed.
Bind log:
named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
'vm-101.example.com'
named-pkcs11[28652]: PTR record synchronization (addition) for A/
'vm-101.example.com.' refused: unable to find active reverse zone
for IP
address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found
With IPv6 we have several addresses from different reverse zones and
this situation may happen often.
I suggest following:
1) Print list of addresses which will be updated. (Now if update
fails,
user needs to read log, which addresses installer tried to update)
2) Split nsupdates per A/ record.
3a) If failed, check with DNS query if A/ and PTR record are there
and print proper error message
3b) Just print A/ (or PTR) record may not be updated for
particular
IP address.
Any other suggestions are welcome.
After long discussion with DNS and UX guru I've implemented it this
way:
1. Call nsupdate only once with all updates.
2. Verify that the expected records are resolvable.
3. If no print list of missing A/, list of missing PTR records and
list to mismatched PTR record.
As this is running inside client we can't much more and it's up to user
to check what's rotten in his DNS setup.
Updated patch attached.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
One more change to behave well in -crazy- exotic environments that
resolves more PTR records for single IP.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Yet another change to make language nerds and our UX guru happy :-)
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased patch attached.
Updated patch attached.
--
David Kupka
From 5c923daf7ce662e19b144e338557e1b8df7a061d Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Sun, 4 Jan 2015 15:04:18 -0500
Subject: [PATCH] client: Update DNS with all available local IP addresses.
Detect all usable IP addresses assigned to any interface and create
coresponding DNS records on server.
https://fedorahosted.org/freeipa/ticket/4249
---
ipa-client/ipa-install/ipa-client-install | 174 +++---
1 file changed, 113 insertions(+), 61 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 91323ae115a27d221bcbc43fee887c56d99c8635..947cb10d98e950498b9ea1e4a3b715de1ee33e3b 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -32,6 +32,7 @@ try:
from optparse import SUPPRESS_HELP, OptionGroup, OptionValueError
import shutil
from krbV import Krb5Error
+import dns
import nss.nss as nss
import SSSDConfig
@@ -1285,6 +1286,7 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie
if options.dns_updates:
domain.set_option('dyndns_update', True)
+domain.set_option('dyndns_iface', '*')
if options.krb5_offline_passwords:
domain.set_option('krb5_store_password_if_offline', True)
@@ -1500,40 +1502,22 @@ def unconfigure_nisdomain():
if not enabled:
services.knownservices.domainname.disable()
-
-def resolve_ipaddress(server):
- Connect to the server's LDAP port in order to determine what ip
-address this machine uses as public ip (relative to the server
Re: [Freeipa-devel] [PATCH 0294] ULC: fix stageuser-add --from-delete command
On 23/07/15 13:46, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5145 Patch attached. This patch fixes only first part of problem -- the traceback. Removing promt for name and surname requires too big hacks in internal API, and I'm not sure if we will be able to do that. IMO this should be separate command, I will open a discussion. Works for me, ACK. It would be better to leave the ticket open until the issue is fully resolved. -- David Kupka -- 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 0286, 0290] Sysrestore: copy files instead of moving them to avoid SELinux issues
On 17/07/15 16:33, Martin Basti wrote: On 17/07/15 13:57, Petr Vobornik wrote: On 07/17/2015 01:46 PM, Petr Vobornik wrote: On 07/17/2015 01:44 PM, Alexander Bokovoy wrote: On Fri, 17 Jul 2015, Martin Basti wrote: From b05f4a2e17ae00e5c20e5eb7bd046472f100e0ad Mon Sep 17 00:00:00 2001 From: Martin Basti mba...@redhat.com Date: Wed, 15 Jul 2015 16:20:59 +0200 Subject: [PATCH] sysrestore: copy files instead of moving them to avoind SELinux issues ACK. Pushed to: master: 9f701283534745bf93b41a1886183e9ef1d06566 ipa-4-2: 92a73e8b2a5f26744b036a36de4b9956e8883f61 Does it really fix the whole ticket? There is also in freeipa.spec.in %post client (i.e. upgrade): cat /etc/krb5.conf /etc/krb5.conf.ipanew mv /etc/krb5.conf.ipanew /etc/krb5.conf /sbin/restorecon /etc/krb5.conf + some others. Between the mv and restorecon, SSSD tries to access the file and raises AVC. In this case we can freely use mv -z since target platforms are Fedora and newest RHEL. The new patch fixing specfile attached. Works for me, ACK. -- David Kupka -- 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] Replace stageuser-add --from-delete with user-undel --to-staged
are creating object A from object B using operation X, you should be creating object B from object A using the reverse of operation X, otherwise there *will* be inconsistencies, and we don't want that. +1 I agree with this So my understanding is that you think a new verb should be created to allow: 'Active' - 'Stage' I do not recall why this was not discussed or if it as already been rejected. I like the idea and I think it could be useful to Support Engineer. Now I am not sure we want to make 'easy' the action to 'unactivate' a user (currently it requires two operations). In your opinion, does it replace 'stageuser-add --from-delete' or 'user-undel --to-stage' ? or is it an additional subcommand. Also, activate/unactivate is not a NULL operation. Some values has been changed (uid,gid,uniqueuiid...) and some values will be lost (membership). About the verb, this is not because the previous action is 'activate' that we should use 'unactivate'. For example, Thomas raised the point that after 'user-del', 'user-restore' would have been more user friendly than 'user-undel' Thanks thierry We had discussion off-list discussion, and result is following proposal: * remove 'stageuser-add --from-delete' * add new command: 'user-stage' the user-stage command will move both deleted or active users to staged area. $ user-stage deleted_user replaces the stage-user --from-delete, keeps the same behavior $ user-stage active_user this is stretch goal, nice to have, but I don't know how easy is to implement this For better visualization, here is link to our board screen https://pvoborni.fedorapeople.org/images/user-lifecycle.jpg Thierry, do you agree with this? Martin^2 Hello, I really like the idea (as well as the drawing) of having the same cli for both active/deleted user. About the exact verb 'user-stage', I am always bad at this exercise and it would be great to have Thomas ack on that. Just a question about the other verbs user-disable/user-enable. I know they are doing something different but do you think there is a risk of confusion for admin when he should do user-stage or user-disable ? thanks thierry Adding Tomas to the loop. -- David Kupka -- 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 471] ULC: Prevent preserved users from being assigned membership
On 12/08/15 12:22, Jan Cholasta wrote: Hi, the attached patch fixes https://fedorahosted.org/freeipa/ticket/5170. Honza Works for me, ACK. -- David Kupka -- 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] 907 webui: add LDAP vs Kerberos behavior description to user auth types
On 10/08/15 13:05, Petr Vobornik wrote: Text in the ticket is IMHO wrong. Patch uses different text.: If you choose the password and two-factor authentication types at once, Kerberos still enforces authentication with both password and OTP. LDAP allows authentication with either one of the authentication types in this situation. One can also use only Password with kinit but must provide an armor ccache. e.g.: $ kinit admin $ klist Ticket cache: KEYRING:persistent:17127:17127 ... $ kinit -T KEYRING:persistent:17127:17127 fbar https://fedorahosted.org/freeipa/ticket/4935 Works for me, ACK. -- David Kupka -- 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 0059] dbus: Create empty dbus.Array with specified signature
I was installing freeipa-server earlier today and it failed with Unable
to guess signature from empty list.
I was unable to reproduce it but there is now harm in explicitly
specifying the signature of the empty list to prevent this issue.
--
David Kupka
From bd77546fc5844645a0813b702aa2b5de718b6e0b Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Mon, 10 Aug 2015 15:46:03 +0200
Subject: [PATCH] dbus: Create empty dbus.Array with specified signature
Python DBus binding could fail to guess the type signature from empty list.
This issue was seen but we don't have a reproducer. There is no harm in making
sure that it will not happen.
---
ipaserver/install/dogtaginstance.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py
index 33f39f7930b4151200f2880d02a0bc2c152c0025..70f11ee09a6ac427322bb069b0851bc7c93fe04d 100644
--- a/ipaserver/install/dogtaginstance.py
+++ b/ipaserver/install/dogtaginstance.py
@@ -304,7 +304,8 @@ class DogtagInstance(service.Service):
if not path:
iface.add_known_ca(
'dogtag-ipa-ca-renew-agent',
-paths.DOGTAG_IPA_CA_RENEW_AGENT_SUBMIT, [])
+paths.DOGTAG_IPA_CA_RENEW_AGENT_SUBMIT,
+dbus.Array([], dbus.Signature('s')))
def __get_pin(self):
try:
--
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] 0035 client: Update DNS with all available local IP addresses.
On 15/01/15 17:13, David Kupka wrote:
On 01/15/2015 03:22 PM, David Kupka wrote:
On 01/15/2015 12:43 PM, David Kupka wrote:
On 01/12/2015 06:34 PM, Martin Basti wrote:
On 09/01/15 14:43, David Kupka wrote:
On 01/07/2015 04:15 PM, Martin Basti wrote:
On 07/01/15 12:27, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4249
Thank you for patch:
1)
-root_logger.error(Cannot update DNS records!
- Failed to connect to server '%s'.,
server)
+ips = get_local_ipaddresses()
+except CalledProcessError as e:
+root_logger.error(Cannot update DNS records. %s % e)
IMO the error message should be more specific, add there something
like
Unable to get local IP addresses. at least in log.debug()
2)
+lines = ipresult[0].replace('\\', '').split('\n')
.replace() is not needed
3)
+if len(ips) == 0:
if not ips:
is more pythonic by PEP8
Thanks for catching these. Updated patch attached.
merciful NACK
Thank you for the patch, unfortunately I hit one issue which needs
to be
resolved.
If sync PTR is activated in zone settings, and reverse zone doesn't
exists, nsupdate/BIND returns SERVFAIL and ipa-client-install print
Error message, 'DNS update failed'. In fact, all A/ records was
succesfully updated, only PTR records failed.
Bind log:
named-pkcs11[28652]: updating zone 'example.com/IN': adding an RR at
'vm-101.example.com'
named-pkcs11[28652]: PTR record synchronization (addition) for A/
'vm-101.example.com.' refused: unable to find active reverse zone
for IP
address '2620:52:0:104c:21a:4aff:fe10:4eaa': not found
With IPv6 we have several addresses from different reverse zones and
this situation may happen often.
I suggest following:
1) Print list of addresses which will be updated. (Now if update fails,
user needs to read log, which addresses installer tried to update)
2) Split nsupdates per A/ record.
3a) If failed, check with DNS query if A/ and PTR record are there
and print proper error message
3b) Just print A/ (or PTR) record may not be updated for particular
IP address.
Any other suggestions are welcome.
After long discussion with DNS and UX guru I've implemented it this way:
1. Call nsupdate only once with all updates.
2. Verify that the expected records are resolvable.
3. If no print list of missing A/, list of missing PTR records and
list to mismatched PTR record.
As this is running inside client we can't much more and it's up to user
to check what's rotten in his DNS setup.
Updated patch attached.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
One more change to behave well in -crazy- exotic environments that
resolves more PTR records for single IP.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Yet another change to make language nerds and our UX guru happy :-)
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Rebased patch attached.
--
David Kupka
From 3ae6959cfd08c34cfcb0eaf29d057b5ea4ebbac4 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Sun, 4 Jan 2015 15:04:18 -0500
Subject: [PATCH] client: Update DNS with all available local IP addresses.
Detect all usable IP addresses assigned to any interface and create
coresponding DNS records on server.
https://fedorahosted.org/freeipa/ticket/4249
---
ipa-client/ipa-install/ipa-client-install | 173 +++---
1 file changed, 112 insertions(+), 61 deletions(-)
diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 91323ae115a27d221bcbc43fee887c56d99c8635..eab20e6c44954834b736d3477db88c7708912002 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -32,6 +32,7 @@ try:
from optparse import SUPPRESS_HELP, OptionGroup, OptionValueError
import shutil
from krbV import Krb5Error
+import dns
import nss.nss as nss
import SSSDConfig
@@ -1500,40 +1501,22 @@ def unconfigure_nisdomain():
if not enabled:
services.knownservices.domainname.disable()
-
-def resolve_ipaddress(server):
- Connect to the server's LDAP port in order to determine what ip
-address this machine uses as public ip (relative to the server).
-
-Returns a tuple with the IP address and address family when
-connection was successful. Socket error is raised otherwise.
-
-last_socket_error = None
-
-for res in socket.getaddrinfo(server, 389, socket.AF_UNSPEC,
-socket.SOCK_STREAM):
-af, socktype, proto, canonname, sa = res
-try:
-s = socket.socket(af, socktype, proto)
-except socket.error
Re: [Freeipa-devel] [PATCH 0284] stageuser-activate: show user name in error message instead of DN
On 10/07/15 14:51, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5038 I reworded the error message to keep the same format as stageuser-add and user-add. Patch attached. Works for me, ACK. -- David Kupka -- 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 0283] copy-schema-to-ca: allow to overwrite schema files
On 10/07/15 14:31, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5034 Patch attached. Works for me, ACK. -- David Kupka -- 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 0057] Do not use anonymous bind in migration UI.
On 15/07/15 16:04, David Kupka wrote:
On 15/07/15 15:34, Jan Cholasta wrote:
Dne 15.7.2015 v 15:21 David Kupka napsal(a):
https://fedorahosted.org/freeipa/ticket/4953
To test this patch:
1. Migrate users from LDAP or other FreeIPA server
(https://www.freeipa.org/page/Howto/Migration)
2. Disable anonymous bind to Directory Server
(https://docs.fedoraproject.org/en-US/Fedora/15/html/FreeIPA_Guide/disabling-anon-binds.html)
3. Go to FreeIPA migration page (ipa.example.com/ipa/migration/) and
enter name and password of one of the migrated users.
Without this patch you will get an error page.
NACK, you are calling do_bind with wrong arguments.
Updated patch attached.
With Honza, we've found better solution. Instead of binding to the LDAP
just to get base DN we can instantiate api and use api.env.basedn
variable. In the same time we can use api.anv.ldap_uri instead of
searching filesystem for ldapi socket.
Patch attached.
--
David Kupka
From 3fa339547c580ea8dac13fd529bd8adecec0c3dc Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Thu, 16 Jul 2015 10:15:36 +0200
Subject: [PATCH] migration: Use api.env variables.
Use api.env.basedn instead of anonymously accessing LDAP to get base DN.
Use api.env.basedn instead of searching filesystem for ldapi socket.
https://fedorahosted.org/freeipa/ticket/4953
---
install/migration/migration.py | 33 +
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/install/migration/migration.py b/install/migration/migration.py
index b629b1c9ff7bd58f1ea64e4c2b2433428a939f28..8c440175a0358b01acba227ea3179318af50fa32 100644
--- a/install/migration/migration.py
+++ b/install/migration/migration.py
@@ -22,14 +22,13 @@ Password migration script
import cgi
import errno
-import glob
from wsgiref.util import request_uri
from ipapython.ipa_log_manager import root_logger
from ipapython.ipautil import get_ipa_basedn
from ipapython.dn import DN
from ipapython.ipaldap import IPAdmin
-from ipalib import errors
+from ipalib import errors, create_api
from ipaplatform.paths import paths
@@ -45,23 +44,6 @@ def get_ui_url(environ):
return full_url[:index] + /ipa/ui
-def get_base_dn(ldap_uri):
-
-Retrieve LDAP server base DN.
-
-try:
-conn = IPAdmin(ldap_uri=ldap_uri)
-conn.do_simple_bind(DN(), '')
-base_dn = get_ipa_basedn(conn)
-except Exception, e:
-root_logger.error('migration context search failed: %s' % e)
-return ''
-finally:
-conn.unbind()
-
-return base_dn
-
-
def bind(ldap_uri, base_dn, username, password):
if not base_dn:
root_logger.error('migration unable to get base dn')
@@ -90,16 +72,11 @@ def application(environ, start_response):
if not form_data.has_key('username') or not form_data.has_key('password'):
return wsgi_redirect(start_response, 'invalid.html')
-slapd_sockets = glob.glob(paths.ALL_SLAPD_INSTANCE_SOCKETS)
-if slapd_sockets:
-ldap_uri = 'ldapi://%s' % slapd_sockets[0].replace('/', '%2f')
-else:
-ldap_uri = 'ldaps://localhost:636'
-
-base_dn = get_base_dn(ldap_uri)
-
+# API object only for configuration, finalize() not needed
+api = create_api(mode=None)
+api.bootstrap(context='server', in_server=True)
try:
-bind(ldap_uri, base_dn,
+bind(api.env.ldap_uri, api.env.basedn,
form_data['username'].value, form_data['password'].value)
except IOError as err:
if err.errno == errno.EPERM:
--
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 0054] cermonger: Use private unix socket when DBus SystemBus is not, available.
On 15/07/15 13:41, Jan Cholasta wrote: Dne 7.7.2015 v 16:51 David Kupka napsal(a): On 03/07/15 08:46, Martin Kosek wrote: On 07/03/2015 08:41 AM, Jan Cholasta wrote: Dne 2.7.2015 v 14:34 David Kupka napsal(a): On 01/07/15 16:31, David Kupka wrote: Updated patch attached. Client install works, but uninstall does not: # ipa-client-install --uninstall -U certmonger failed to start: Command ''/bin/systemctl' 'start' 'certmonger.service'' returned non-zero exit status 1 certmonger failed to stop tracking certificate: Failed to start certmonger: Timeouted 2015-07-03 02:38:15 [17242] Error reading PIN from /etc/ipa/nssdb/pwdfile.txt: No such file or directory. Failed to start certmonger: Timeouted The patch needs a rebase. Also, Timeouted is not a word, try Timed out instead :-) Updated patch attached. Also attaching patch that removes unneeded certmonger (re)starting and DBus starting from ipa-client-install. NACK. When dbus is not available and ipa-client-install is run *without* --request-cert, certmonger tracks Local IPA host in /etc/ipa/nssdb. When ipa-client-install is run *with* --request-cert, the certificate is not issued, but I guess this is not caused by your patch. Updated patch attached. -- David Kupka From a01f422452902fbc0313648a74cd5d317b67faff Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Tue, 7 Jul 2015 15:49:27 +0200 Subject: [PATCH] cermonger: Use private unix socket when DBus SystemBus is not available. --- ipaplatform/base/paths.py | 4 ++ ipapython/certmonger.py | 129 -- 2 files changed, 94 insertions(+), 39 deletions(-) diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py index 9fef3e7a1351dd42895fe560bb3c1bc5a1c852b4..5756040172126438d42275b734f4d766d53048fe 100644 --- a/ipaplatform/base/paths.py +++ b/ipaplatform/base/paths.py @@ -348,3 +348,7 @@ class BasePathNamespace(object): BAK2DB = '/usr/sbin/bak2db' DB2BAK = '/usr/sbin/db2bak' KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf' +CERTMONGER = '/usr/sbin/certmonger' + + +path_namespace = BasePathNamespace diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py index 4b85da08bb943d6b9f0091a1d2acc36b18d6..b37676872a8b983636c7b2dc5590e83c8b08ea98 100644 --- a/ipapython/certmonger.py +++ b/ipapython/certmonger.py @@ -27,6 +27,8 @@ import sys import time import dbus import shlex +import subprocess +import tempfile from ipapython import ipautil from ipapython import dogtag from ipapython.ipa_log_manager import * @@ -35,6 +37,7 @@ from ipaplatform import services DBUS_CM_PATH = '/org/fedorahosted/certmonger' DBUS_CM_IF = 'org.fedorahosted.certmonger' +DBUS_CM_NAME = 'org.fedorahosted.certmonger' DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request' DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca' DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties' @@ -44,7 +47,7 @@ class _cm_dbus_object(object): Auxiliary class for convenient DBus object handling. -def __init__(self, bus, object_path, object_dbus_interface, +def __init__(self, bus, parent, object_path, object_dbus_interface, parent_dbus_interface=None, property_interface=False): bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus() @@ -60,6 +63,7 @@ class _cm_dbus_object(object): if parent_dbus_interface is None: parent_dbus_interface = object_dbus_interface self.bus = bus +self.parent = parent self.path = object_path self.obj_dbus_if = object_dbus_interface self.parent_dbus_if = parent_dbus_interface @@ -69,36 +73,83 @@ class _cm_dbus_object(object): self.prop_if = dbus.Interface(self.obj, DBUS_PROPERTY_IF) -def _start_certmonger(): +class _certmonger(_cm_dbus_object): -Start certmonger daemon. If it's already running systemctl just ignores -the command. +Create a connection to certmonger. +By default use SystemBus. When not available use private connection +over Unix socket. +This solution is really ugly and should be removed as soon as DBus +SystemBus is available at system install time. -if not services.knownservices.certmonger.is_running(): +timeout = 300 + +def _start_private_conn(self): +sock_filename = os.path.join(tempfile.mkdtemp(), 'certmonger') +self._proc = subprocess.Popen([paths.CERTMONGER, '-n', '-L', '-P', + sock_filename]) +for t in range(0, self.timeout, 5): +if os.path.exists(sock_filename): +return unix:path=%s % sock_filename +time.sleep(5) +self._stop_private_conn() +raise RuntimeError(Failed to start certmonger: Timed out) + +def _stop_private_conn(self): +if self._proc: +retcode = self._proc.poll() +if retcode is not None
Re: [Freeipa-devel] [PATCH 0057] Do not use anonymous bind in migration UI.
On 15/07/15 15:34, Jan Cholasta wrote:
Dne 15.7.2015 v 15:21 David Kupka napsal(a):
https://fedorahosted.org/freeipa/ticket/4953
To test this patch:
1. Migrate users from LDAP or other FreeIPA server
(https://www.freeipa.org/page/Howto/Migration)
2. Disable anonymous bind to Directory Server
(https://docs.fedoraproject.org/en-US/Fedora/15/html/FreeIPA_Guide/disabling-anon-binds.html)
3. Go to FreeIPA migration page (ipa.example.com/ipa/migration/) and
enter name and password of one of the migrated users.
Without this patch you will get an error page.
NACK, you are calling do_bind with wrong arguments.
Updated patch attached.
--
David Kupka
From 43d8cc79283e9cbead102bd1415ad4107f65df11 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 15 Jul 2015 14:55:28 +0200
Subject: [PATCH] Do not use anonymous bind in migration UI.
https://fedorahosted.org/freeipa/ticket/4953
---
install/migration/migration.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/install/migration/migration.py b/install/migration/migration.py
index b629b1c9ff7bd58f1ea64e4c2b2433428a939f28..4e92794e3bb386bbd9dd80e7123bfb63f2fa8dc4 100644
--- a/install/migration/migration.py
+++ b/install/migration/migration.py
@@ -51,7 +51,7 @@ def get_base_dn(ldap_uri):
try:
conn = IPAdmin(ldap_uri=ldap_uri)
-conn.do_simple_bind(DN(), '')
+conn.do_bind()
base_dn = get_ipa_basedn(conn)
except Exception, e:
root_logger.error('migration context search failed: %s' % e)
--
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 0057] Do not use anonymous bind in migration UI.
https://fedorahosted.org/freeipa/ticket/4953
To test this patch:
1. Migrate users from LDAP or other FreeIPA server
(https://www.freeipa.org/page/Howto/Migration)
2. Disable anonymous bind to Directory Server
(https://docs.fedoraproject.org/en-US/Fedora/15/html/FreeIPA_Guide/disabling-anon-binds.html)
3. Go to FreeIPA migration page (ipa.example.com/ipa/migration/) and
enter name and password of one of the migrated users.
Without this patch you will get an error page.
--
David Kupka
From a9c50987842a08eb6928bd662a1db57b85d4b3cd Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 15 Jul 2015 14:55:28 +0200
Subject: [PATCH] Do not use anonymous bind in migration UI.
https://fedorahosted.org/freeipa/ticket/4953
---
install/migration/migration.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/install/migration/migration.py b/install/migration/migration.py
index b629b1c9ff7bd58f1ea64e4c2b2433428a939f28..ec660ba5329193675826cd8ce292034fd33744b5 100644
--- a/install/migration/migration.py
+++ b/install/migration/migration.py
@@ -51,7 +51,7 @@ def get_base_dn(ldap_uri):
try:
conn = IPAdmin(ldap_uri=ldap_uri)
-conn.do_simple_bind(DN(), '')
+conn.do_bind(DN(), '')
base_dn = get_ipa_basedn(conn)
except Exception, e:
root_logger.error('migration context search failed: %s' % e)
--
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 0342] Use domain level constants in topology plugin
On 03/11/15 10:45, Martin Basti wrote: Patch attached. Works for me, ACK. -- David Kupka -- 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.
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 <dku...@redhat.com>
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
[Freeipa-devel] [PATCH] admintool: Add error message with path to log on failure.
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 <dku...@redhat.com>
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
Re: [Freeipa-devel] [PATCHES 0069-0077] support for proper Kerberos principal canonicalization
On 07/10/15 17:32, thierry bordaz wrote: On 10/07/2015 05:29 PM, Simo Sorce wrote: On 07/10/15 11:06, thierry bordaz wrote: On 10/07/2015 03:10 PM, David Kupka wrote: On 06/10/15 17:52, Jakub Hrozek wrote: On Tue, Oct 06, 2015 at 08:32:29AM -0400, Simo Sorce wrote: On 06/10/15 08:04, David Kupka wrote: On 06/10/15 13:35, Simo Sorce wrote: On 06/10/15 03:51, thierry bordaz wrote: On 10/06/2015 07:19 AM, David Kupka wrote: On 05/10/15 16:12, Simo Sorce wrote: On 05/10/15 09:00, Martin Babinsky wrote: These patches implement the plumbing required to properly support canonicalization of Kerberos principals ( https://fedorahosted.org/freeipa/ticket/3864). Setting multiple principal aliases on hosts/services is beyond the scope of this patchset and should be done after these patches are pushed. I will try to send some tests for the patches later this week. Please review the hell out of them. LGTM, I do not see any issue at quick visual inspection. What about the performance regression with the indexes ? Is that bug fixed in 389ds ? Simo. The issue is still there. Thierry investigated this in 389 DS and IIUC he is not sure if it's bug or completely missing feature. Therefore we still don't know how much time is needed there. Hi, that is correct. I can reproduce the problem. Although the matching rule (in my test caseIgnoreIA5Match) is found, it has no registered indexing function, so the setting (nsMatchingRule) is ignored. I do not know if the indexing function is missing or there is a bug so that the matching rule "forget" to register it. This feature is documented but I can not find any QA test around it, so I do not know yet if it is a regression or if it was not enabled at all. I do not expect rapid progress on it. How urgent is it ? 7.3 ? For the moment I can think to only two workarounds: * use filtered matching rule (preferred) * change the attribute syntax/matching rule, in the schema (I would discourage this one because changing the schema is risky) We can't change the syntax at this point. Well this patchset is blocked until the 389 ds bug is fixed (the performance regression is too big to just put it in and hope) so I guess we'll have to negotiate a time for the fix. Simo. I agree that we really shouldn't change schema. But I don't think the patches're necessary blocked by this issue. Canonicalization was never supported in FreeIPA and when it is not requested the performance is not effected at all. We could merge patches as soon as they're carefully reviewed and tested to avoid tedious rebasing and start using the new functionality when 389 DS gets fixed. The fact we didn't do canonicalization this way doesn't mean clients aren't asking for it. I think Windows clients ask for canonicalization by default, and in SSSD I see we turn on by default krb5_canonicalize in the IPA nd LDAP case (oddly enough not in the AD case ?) So SSSD's authentication requests would end up hitting this case all the time if I am reading the code correctly (CCed Jakub to confirm/dispel this). We ask for canonicalization always in IPA and LDAP, but also whenever enterprise principals are used, which is true for AD provider. Then SSSD will hit this every time it requests ticket on behalf of user. But to be sure what the impact would be I've once again set up FreeIPA server with 10K users and run some tests. 1) 3 LDAP searches (caseIgnoreIA5Match, caseExactIA5Match, without specifying the matching rule). Results (http://fpaste.org/275847/44221770/raw/) shows that unindexed search takes ~100 times longer than indexed. 2) kinit with and without requested canonicalization. As we use kinit to get the ticket it makes sense to check what will the performance hit be when we run kinit as a whole and not just an isolated LDAP search. The results (http://fpaste.org/275848/21793144/raw/) shows that with canonicalization it takes ~2 times longer than without it. While this is nothing to be happy about it's certainly better than I would expect. Clearly we need to make the search indexed. In your deployment you defined: dn: uid=user198,cn=users,cn=accounts,dc=example,dc=test uid: user198 givenName: Test sn: User198 cn: Test User198 initials: TU homeDirectory: /home/user198 gecos: Test User198 loginShell: /bin/sh mail: user1000...@example.test uidNumber: 761100198 gidNumber: 761100198 displayName: Test User198 *krbPrincipalName: user1000...@example.test* *krbCanonicalName: user1000...@example.test* memberOf: cn=ipausers,cn=groups,cn=accounts,dc=example,dc=test objectClass: ipaobject objectClass: person objectClass: top objectClass: ipasshuser objectClass: inetorgperson objectClass: organizationalperson objectClass: krbticketpolicyaux objectClass: krbprincipalaux objectClass: inetuser objectClass: posixaccount objectClass: ipaSshGrou
Re: [Freeipa-devel] [PATCHES 0069-0077] support for proper Kerberos principal canonicalization
On 06/10/15 17:52, Jakub Hrozek wrote: On Tue, Oct 06, 2015 at 08:32:29AM -0400, Simo Sorce wrote: On 06/10/15 08:04, David Kupka wrote: On 06/10/15 13:35, Simo Sorce wrote: On 06/10/15 03:51, thierry bordaz wrote: On 10/06/2015 07:19 AM, David Kupka wrote: On 05/10/15 16:12, Simo Sorce wrote: On 05/10/15 09:00, Martin Babinsky wrote: These patches implement the plumbing required to properly support canonicalization of Kerberos principals ( https://fedorahosted.org/freeipa/ticket/3864). Setting multiple principal aliases on hosts/services is beyond the scope of this patchset and should be done after these patches are pushed. I will try to send some tests for the patches later this week. Please review the hell out of them. LGTM, I do not see any issue at quick visual inspection. What about the performance regression with the indexes ? Is that bug fixed in 389ds ? Simo. The issue is still there. Thierry investigated this in 389 DS and IIUC he is not sure if it's bug or completely missing feature. Therefore we still don't know how much time is needed there. Hi, that is correct. I can reproduce the problem. Although the matching rule (in my test caseIgnoreIA5Match) is found, it has no registered indexing function, so the setting (nsMatchingRule) is ignored. I do not know if the indexing function is missing or there is a bug so that the matching rule "forget" to register it. This feature is documented but I can not find any QA test around it, so I do not know yet if it is a regression or if it was not enabled at all. I do not expect rapid progress on it. How urgent is it ? 7.3 ? For the moment I can think to only two workarounds: * use filtered matching rule (preferred) * change the attribute syntax/matching rule, in the schema (I would discourage this one because changing the schema is risky) We can't change the syntax at this point. Well this patchset is blocked until the 389 ds bug is fixed (the performance regression is too big to just put it in and hope) so I guess we'll have to negotiate a time for the fix. Simo. I agree that we really shouldn't change schema. But I don't think the patches're necessary blocked by this issue. Canonicalization was never supported in FreeIPA and when it is not requested the performance is not effected at all. We could merge patches as soon as they're carefully reviewed and tested to avoid tedious rebasing and start using the new functionality when 389 DS gets fixed. The fact we didn't do canonicalization this way doesn't mean clients aren't asking for it. I think Windows clients ask for canonicalization by default, and in SSSD I see we turn on by default krb5_canonicalize in the IPA nd LDAP case (oddly enough not in the AD case ?) So SSSD's authentication requests would end up hitting this case all the time if I am reading the code correctly (CCed Jakub to confirm/dispel this). We ask for canonicalization always in IPA and LDAP, but also whenever enterprise principals are used, which is true for AD provider. Then SSSD will hit this every time it requests ticket on behalf of user. But to be sure what the impact would be I've once again set up FreeIPA server with 10K users and run some tests. 1) 3 LDAP searches (caseIgnoreIA5Match, caseExactIA5Match, without specifying the matching rule). Results (http://fpaste.org/275847/44221770/raw/) shows that unindexed search takes ~100 times longer than indexed. 2) kinit with and without requested canonicalization. As we use kinit to get the ticket it makes sense to check what will the performance hit be when we run kinit as a whole and not just an isolated LDAP search. The results (http://fpaste.org/275848/21793144/raw/) shows that with canonicalization it takes ~2 times longer than without it. While this is nothing to be happy about it's certainly better than I would expect. -- David Kupka -- 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 0069-0077] support for proper Kerberos principal canonicalization
On 05/10/15 16:12, Simo Sorce wrote: On 05/10/15 09:00, Martin Babinsky wrote: These patches implement the plumbing required to properly support canonicalization of Kerberos principals ( https://fedorahosted.org/freeipa/ticket/3864). Setting multiple principal aliases on hosts/services is beyond the scope of this patchset and should be done after these patches are pushed. I will try to send some tests for the patches later this week. Please review the hell out of them. LGTM, I do not see any issue at quick visual inspection. What about the performance regression with the indexes ? Is that bug fixed in 389ds ? Simo. The issue is still there. Thierry investigated this in 389 DS and IIUC he is not sure if it's bug or completely missing feature. Therefore we still don't know how much time is needed there. -- David Kupka -- 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] 0026..0027 #5096 enforce caacl for SAN principals
On 03/07/15 16:26, Fraser Tweedale wrote: The attached patches fix: - a bug that caused caacl false negatives for hosts principals - #5096 cert-request: enforce caacl for subjectAltName principals Thanks, Fraser Works for me, ACK. -- David Kupka -- 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] Meaning of two strings in plugins/service.py
On 05/07/15 11:25, Jérôme Fenal wrote:
Hi,
I stumbled upon those two following strings while translating into
French, and just cannot figure out the meaning.
Str('ipaallowedtoperform_read_keys',
label=_('Failed allowed to retrieve keytab'),
),
Str('ipaallowedtoperform_write_keys',
label=_('Failed allowed to create keytab'),
),
Would it be that failure is allowed while retrieving or creating keytab?
Or...?
Thanks for helping,
Jérôme
Hi Jérôme,
I guess it should be Failed to allow retrieval/creation of keytab.
But Petr (added) is author of this code and should know better.
--
David Kupka
--
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 0054] cermonger: Use private unix socket when DBus SystemBus is not, available.
On 03/07/15 08:46, Martin Kosek wrote: On 07/03/2015 08:41 AM, Jan Cholasta wrote: Dne 2.7.2015 v 14:34 David Kupka napsal(a): On 01/07/15 16:31, David Kupka wrote: Updated patch attached. Client install works, but uninstall does not: # ipa-client-install --uninstall -U certmonger failed to start: Command ''/bin/systemctl' 'start' 'certmonger.service'' returned non-zero exit status 1 certmonger failed to stop tracking certificate: Failed to start certmonger: Timeouted 2015-07-03 02:38:15 [17242] Error reading PIN from /etc/ipa/nssdb/pwdfile.txt: No such file or directory. Failed to start certmonger: Timeouted The patch needs a rebase. Also, Timeouted is not a word, try Timed out instead :-) Updated patch attached. Also attaching patch that removes unneeded certmonger (re)starting and DBus starting from ipa-client-install. -- David Kupka From e4a04d2f1c6ceb73306d5c417172eba38257dd11 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Tue, 7 Jul 2015 15:49:27 +0200 Subject: [PATCH] cermonger: Use private unix socket when DBus SystemBus is not available. --- ipaplatform/base/paths.py | 4 ++ ipapython/certmonger.py | 128 -- 2 files changed, 94 insertions(+), 38 deletions(-) diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py index 9fef3e7a1351dd42895fe560bb3c1bc5a1c852b4..5756040172126438d42275b734f4d766d53048fe 100644 --- a/ipaplatform/base/paths.py +++ b/ipaplatform/base/paths.py @@ -348,3 +348,7 @@ class BasePathNamespace(object): BAK2DB = '/usr/sbin/bak2db' DB2BAK = '/usr/sbin/db2bak' KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf' +CERTMONGER = '/usr/sbin/certmonger' + + +path_namespace = BasePathNamespace diff --git a/ipapython/certmonger.py b/ipapython/certmonger.py index 4b85da08bb943d6b9f0091a1d2acc36b18d6..9914481a6c9ceccdfbfebcd294a60c827acf801f 100644 --- a/ipapython/certmonger.py +++ b/ipapython/certmonger.py @@ -27,6 +27,8 @@ import sys import time import dbus import shlex +import subprocess +import tempfile from ipapython import ipautil from ipapython import dogtag from ipapython.ipa_log_manager import * @@ -35,6 +37,7 @@ from ipaplatform import services DBUS_CM_PATH = '/org/fedorahosted/certmonger' DBUS_CM_IF = 'org.fedorahosted.certmonger' +DBUS_CM_NAME = 'org.fedorahosted.certmonger' DBUS_CM_REQUEST_IF = 'org.fedorahosted.certmonger.request' DBUS_CM_CA_IF = 'org.fedorahosted.certmonger.ca' DBUS_PROPERTY_IF = 'org.freedesktop.DBus.Properties' @@ -44,7 +47,7 @@ class _cm_dbus_object(object): Auxiliary class for convenient DBus object handling. -def __init__(self, bus, object_path, object_dbus_interface, +def __init__(self, bus, parent, object_path, object_dbus_interface, parent_dbus_interface=None, property_interface=False): bus - DBus bus object, result of dbus.SystemBus() or dbus.SessionBus() @@ -60,6 +63,7 @@ class _cm_dbus_object(object): if parent_dbus_interface is None: parent_dbus_interface = object_dbus_interface self.bus = bus +self.parent = parent self.path = object_path self.obj_dbus_if = object_dbus_interface self.parent_dbus_if = parent_dbus_interface @@ -69,36 +73,83 @@ class _cm_dbus_object(object): self.prop_if = dbus.Interface(self.obj, DBUS_PROPERTY_IF) -def _start_certmonger(): +class _certmonger(_cm_dbus_object): -Start certmonger daemon. If it's already running systemctl just ignores -the command. +Create a connection to certmonger. +By default use SystemBus. When not available use private connection +over Unix socket. +This solution is really ugly and should be removed as soon as DBus +SystemBus is available at system install time. -if not services.knownservices.certmonger.is_running(): +_bus = None +_proc = None +timeout = 300 + +def _start_private_conn(self): +sock_filename = os.path.join(tempfile.mkdtemp(), 'certmonger') +self._proc = subprocess.Popen([paths.CERTMONGER, '-n', '-L', '-P', + sock_filename]) +for t in range(0, self.timeout, 5): +if os.path.exists(sock_filename): +return unix:path=%s % sock_filename +time.sleep(5) +self._stop_private_conn() +raise RuntimeError(Failed to start certmonger: Timed out) + +def _stop_private_conn(self): +if self._proc: +retcode = self._proc.poll() +if retcode is not None: +return +self._proc.terminate() +for t in range(0, self.timeout, 5): +retcode = self._proc.poll() +if retcode is not None: +return +time.sleep(5) +root_logger.error(Failed to stop certmonger.) + +def __del__(self): +self
Re: [Freeipa-devel] [PATCH] 897 fix error message when certificate CN is invalid
On 09/07/15 00:28, Petr Vobornik wrote: The error message was probably copied from mail address check below. ACK. -- David Kupka -- 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 0065] vault: Limit size of data stored in vault
https://fedorahosted.org/freeipa/ticket/5231
--
David Kupka
From f86f4f89d1083c1474d8c470ae3b0f85ed1eb6bb Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 26 Aug 2015 14:11:21 +0200
Subject: [PATCH] vault: Limit size of data stored in vault
https://fedorahosted.org/freeipa/ticket/5231
---
ipalib/constants.py | 2 ++
ipalib/plugins/vault.py | 20 ++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/ipalib/constants.py b/ipalib/constants.py
index 53c3106cdd16fef0eba42a70518f7633b3fd95d1..5b83c7177987e95e101b2050aabfbc53d18e1b8d 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -239,3 +239,5 @@ SID_ANCHOR_PREFIX = ':SID:'
MIN_DOMAIN_LEVEL = 0
MAX_DOMAIN_LEVEL = 1
+
+MAX_VAULT_DATA_SIZE = 2**20 # = 1 MB
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 6b7d188f4c024cc6709faff33af942559ce4f8ec..4eb67438df0bb7ba3f22398d717b0be354edf893 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -47,7 +47,7 @@ from ipalib.plugins.baseldap import LDAPObject, LDAPCreate, LDAPDelete,\
from ipalib.request import context
from ipalib.plugins.user import split_principal
from ipalib.plugins.service import normalize_principal
-from ipalib import _, ngettext
+from ipalib import _, ngettext, constants
from ipaplatform.paths import paths
from ipapython.dn import DN
from ipapython.nsslib import current_dbdir
@@ -1227,10 +1227,26 @@ class vault_archive(PKQuery, Local):
raise errors.MutuallyExclusiveError(
reason=_('Input data specified multiple times'))
+elif data:
+if len(data) constants.MAX_VAULT_DATA_SIZE:
+raise errors.ValidationError(name=data, error=_(Size of data
+ exceeds the limit. Current vault data size limit is
+ %(limit)d B) % {'limit': constants.MAX_VAULT_DATA_SIZE})
+
elif input_file:
+try:
+stat = os.stat(input_file)
+except IOError as exc:
+raise errors.ValidationError(name=in, error=_(Cannot read
+ file '%(filename)s': %(exc)s) % {'filename': input_file,
+'exc': exc[1]})
+if stat.st_size constants.MAX_VAULT_DATA_SIZE:
+raise errors.ValidationError(name=in, error=_(Size of data
+ exceeds the limit. Current vault data size limit is
+ %(limit)d B) % {'limit': constants.MAX_VAULT_DATA_SIZE})
data = validated_read('in', input_file, mode='rb')
-elif not data:
+else:
data = ''
if self.api.env.in_server:
--
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 0066] ipactl: Do not start/stop/restart single service multiple times
https://fedorahosted.org/freeipa/ticket/5248 -- David Kupka From 349e8ada21526cb704d9d876a151aaa2764970f8 Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Wed, 26 Aug 2015 15:10:16 +0200 Subject: [PATCH] ipactl: Do not start/stop/restart single service multiple times In case multiple services are provided by single system daemon it is not needed to start/stop/restart it mutiple time. https://fedorahosted.org/freeipa/ticket/5248 --- install/tools/ipactl | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/install/tools/ipactl b/install/tools/ipactl index f37c4e02c44d57c93a88541192a8477cc6033a40..5782d4c424fb98c08e55614c71f3abaa6e776a68 100755 --- a/install/tools/ipactl +++ b/install/tools/ipactl @@ -45,6 +45,16 @@ def check_IPA_configuration(): raise IpactlError(IPA is not configured + (see man pages of ipa-server-install for help), 6) +def deduplicate(lst): +new_lst = [] +s = set(lst) +for i in lst: +if i in s: +s.remove(i) +new_lst.append(i) + +return new_lst + def is_dirsrv_debugging_enabled(): Check the 389-ds instance to see if debugging is enabled. @@ -283,6 +293,7 @@ def ipa_start(options): # no service to start return +svc_list = deduplicate(svc_list) for svc in svc_list: svchandle = services.service(svc) try: @@ -321,6 +332,7 @@ def ipa_stop(options): finally: raise IpactlError() +svc_list = deduplicate(svc_list) for svc in reversed(svc_list): svchandle = services.service(svc) try: @@ -398,6 +410,7 @@ def ipa_restart(options): if len(old_svc_list) != 0: # we need to definitely stop some services +old_svc_list = deduplicate(old_svc_list) for svc in reversed(old_svc_list): svchandle = services.service(svc) try: @@ -422,7 +435,7 @@ def ipa_restart(options): if len(svc_list) != 0: # there are services to restart - +svc_list = deduplicate(svc_list) for svc in svc_list: svchandle = services.service(svc) try: @@ -444,6 +457,7 @@ def ipa_restart(options): if len(new_svc_list) != 0: # we still need to start some services +new_svc_list = deduplicate(new_svc_list) for svc in new_svc_list: svchandle = services.service(svc) try: @@ -494,6 +508,7 @@ def ipa_status(options): if len(svc_list) == 0: return +svc_list = deduplicate(svc_list) for svc in svc_list: svchandle = services.service(svc) try: -- 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 0065] vault: Limit size of data stored in vault
On 26/08/15 15:45, Petr Vobornik wrote: On 08/26/2015 02:13 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/5231 Attaching updated patch. With changes discussed offline. Changes works for me, ACK. Not related to the patch: This patch limits the size to 1MB instead of proposed 10MB. Testing showed that even 10MB raises a MemoryError in archive_encrypted_data which is AFAIK a KraClient method - Endi, this sounds as something which should be also handled in PKI. Especially when it happens the subsequent vault-archive command ends with HTTPError: 503 Server Error: Service Unavailable. After restart of pki, subsequent vault-archive with 1M file took about 4mins (in vault_retrieve_internal). Next archive command with 1M file took only 18s. 10k file took 9s. Why is it so slow? -- David Kupka -- 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 0066] ipactl: Do not start/stop/restart single service multiple times
On 26/08/15 17:49, Tomas Babej wrote: On 08/26/2015 03:16 PM, David Kupka wrote: https://fedorahosted.org/freeipa/ticket/5248 +def deduplicate(lst): +new_lst = [] +s = set(lst) +for i in lst: +if i in s: +s.remove(i) +new_lst.append(i) + +return new_lst + Imho, this method deserves a docstring or at least a comment. It is not entrirely clear from the name, that its job is to remove the duplicates while preserving the order of the entries. You're right, line or two could not hurt. Patch attached. Anyway, deduplication can be implemented in a more readable way: from collections import OrderedDict sample_list = [3,2,1,2,1,5,3] OrderedDict.fromkeys(sample_list).keys() [3, 2, 1, 5] I know about this approach and it does not seem much more readable to me. I just chosen few more lines instead of an import. Tomas -- David Kupka From 8f8a97747968ae9b69b1f7d0b9deaab730feba4c Mon Sep 17 00:00:00 2001 From: David Kupka dku...@redhat.com Date: Thu, 27 Aug 2015 07:34:02 +0200 Subject: [PATCH] comment: Add Documentation string to deduplicate function --- install/tools/ipactl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/install/tools/ipactl b/install/tools/ipactl index 5782d4c424fb98c08e55614c71f3abaa6e776a68..379a8d91b2419a9708919ac9713352fcc242e79f 100755 --- a/install/tools/ipactl +++ b/install/tools/ipactl @@ -46,6 +46,9 @@ def check_IPA_configuration(): (see man pages of ipa-server-install for help), 6) def deduplicate(lst): +Remove duplicates and preserve order. +Returns copy of list with preserved order and removed duplicates. + new_lst = [] s = set(lst) for i in lst: -- 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] changes in preparation of replica promotion work
On 27/08/15 11:05, Jan Cholasta wrote: On 27.8.2015 07:56, Jan Cholasta wrote: On 25.8.2015 20:43, Simo Sorce wrote: On Wed, 2015-08-05 at 11:24 -0400, Simo Sorce wrote: On Wed, 2015-08-05 at 08:20 +0200, Jan Cholasta wrote: Hi, Dne 31.7.2015 v 12:46 Simo Sorce napsal(a): I've been carrying these patches in my tree for a while, I think it is time to put them in master as they stand on their own. Simo. Patch 530: ACK Patch 531: ACK Patch 532: The methods should be static methods: @staticmethod def setOption(name, value): ... Care to explain why ? @staticmethod is not used anywhere else in that file. Rebased patches on master, made requested change +1 more patch. Simo. Patch 532: ACK Patch 533: ACK Pushed to master: f57b687241fbc92d1138507210e87e9de465c507 Honza Actually, there is a problem with patch 531: SASL mapping are added only on replica. The attached patch fixes it. Works for me, ACK. -- David Kupka -- 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 0002] Port from python-krbV to python-gssapi
On 26/08/15 09:42, Jan Cholasta wrote:
On 25.8.2015 21:00, Simo Sorce wrote:
On Tue, 2015-08-25 at 20:45 +0200, Michael Šimáček wrote:
On 2015-08-25 18:43, Robbie Harwood wrote:
Jan Cholasta <jchol...@redhat.com> writes:
On 25.8.2015 12:46, Michael Šimáček wrote:
On 2015-08-25 12:38, Alexander Bokovoy wrote:
On Tue, 25 Aug 2015, Michael Šimáček wrote:
On 2015-08-24 20:29, Robbie Harwood wrote:
Michael Šimáček <msima...@redhat.com> writes:
On 2015-08-24 17:49, Simo Sorce wrote:
On Mon, 2015-08-24 at 17:18 +0200, Michael Šimáček wrote:
On 2015-08-24 14:50, Jan Cholasta wrote:
Fixed. python-gssapi has a display_as method that could pull
the
name
from it, but it doesn't work in current version, therefore
using
partition to split on '@'
It's actually a bug in MIT Krb5, as we noted in your bug[0].
So this:
-user =
api.Command.user_show(unicode(principal[0]))['result']
+user =
api.Command.user_show(principal.partition('@')[0])['result']
is working around a bug in specific Kerberos versions. If
people are
okay with merging such code, then I guess this is fine; I would
personally not do so because there is not a clear point at
which it can
be removed. At the very least, we should wait until we see what
versions of krb5 MIT is going to fix.
Otherwise, looks good.
[0]: https://github.com/pythongssapi/python-gssapi/issues/79
python-krbV migration is blocking support for Python 3. The bug
doesn't have any fix upstream yet and there are two bugs
actually, the
second one is in python-gssapi, which I've just reported [1].
Waiting
for two bugs to be fixed could be detrimental to py3 migration
as we
don't have much time left. And I'm no longer sure that display_as
I don't buy this.
We have plenty of time for solving these bugs. Remember, that Samba
DCE RPC bindings aren't migrated to Python 3 either and will not be
before release of Samba 4.4. For Samba 4.3 it is simply too late.
So we are still far away from full Python3 migration for FreeIPA and
waiting for solving these two bugs is OK.
If fixing them solves anything at all. I planned to use
display_as(NameType.user), but when trying it on Name object with
name_type set (which doesn't trigger the segfault), it doesn't
seem to
work either. I get:
gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The
operation or option is not available or unsupported, Minor (0):
Unknown
error
Robbie, can you clarify whether display_as could be actually used
to get
the first component of the principal reliably?
display_as should behave in accordance with its docs; anything else
is a
bug report, which you filed. I don't know what you're asking me for
beyond that.
Why I mentioned display_as at all is that I initially assumed it could
be used for this, but it was only an assumption because I couldn't get
around the segfault. Later on, the cause of the segfault was found and I
was able to try the method and I found out that it probably cannot be
used for this purpose (i. e. extracting the first component of the
principal) regardless of the two bugs. How I thought it would be used:
import gssapi
cred = gssapi.Credentials()
user = cred.name.display_as(gssapi.NameType.user)
What I got:
gssapi.raw.exceptions.OperationUnavailableError: Major (1048576): The
operation or option is not available or unsupported, Minor (0): Unknown
error
This seems more like the method is not intended to be used this way. So
I'm asking you whether it is a bug or whether there is another way to do
it. Otherwise display_as cannot be used here.
As I have written in the other thread, we use
"principal.split('@')" in
other parts of IPA, so "principal.partition('@')" should be OK as
well.
This patch works for me, so ACK.
Unless there are any further objections, I would like to push it.
I think the newest iteration of this
user =
api.Command.user_show(principal.partition('@')[0].partition('/')[0])['result']
is even worse, but if it is decided to merge, then hopefully we can be
rid of it quickly.
It is splitting a string of known format in a way that is used in other
places of freeipa. What is specifically so bad about it? What do you
suggest as an alternative?
Given display_as() currently does not work for you go ahead with this
code. We'll revisit display_as later once we figure out more about the
bug that makes it fail.
OK.
Pushed to master: aad73fad601f576dd83b758f4448839b4e8e87df
I think this patch is causing tracebacks when expired or missing
kerberos ticket (https://fedorahosted.org/freeipa/ticket/5272).
--
David Kupka
--
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] fixing Kerberos principal aliases handling in IPA
On 01/09/15 16:53, Simo Sorce wrote: On Tue, 2015-09-01 at 16:39 +0200, Martin Babinsky wrote: Hi list, I own the following ticket https://fedorahosted.org/freeipa/ticket/3864 and I would like to clarify what needs to be done in order to make IPA to fully support multiple aliases per entry. So far I have identified these task based on the ticket comments and discussion with Simo way back in the past: 1.) mark 'ipaKrbPrincipalAlias' attribute as deprecated so that it is not used in the new code. 2.) fix ACIs that do not permit setting multiple values of 'krbPrincipalName' attribute per entry (see https://fedorahosted.org/freeipa/ticket/3961) 3.) Modify KDB backend (namely 'ipadb_fetch_principal' and 'ipadb_find_principal' functions) to correctly perform lookup of krbprincipalname/krbcanonicalname, i.e. search krbprincipalname case-insensitively and krbcanonicalname case-sensitively, return krbcanonicalname when canonicalization is requested. 4.) Modify KDB backend and IPA framework to handle creation of both krbprincipalname and krbcanonicalname. I am not quite sure what cases should be covered here (I remember that we should create krbcanonicalname when we add another aliases to krbprincipalname), so it would be nice if you could comment on this. 5.) write tests which cover all this stuff so that we don't shoot ourselves in the foot. I am not very well versed in Kerberos so I might get some of this stuff wrong. If that's the case please point me to the right direction. Also please write me some additional stuff which I have fogot and needs to be done. I think the summary is correct, the only thing we need to be careful is to keep handling entries that have only a single valued krbprincipalname correctly as that will happen in upgrade paths and potentially if someone uses external tools. The tricky part for point 3 is to implement it *without* changing the schema. KrbPrincipalName is case-sensitive, however I think we can solve the issue of "searching case-insensitively" by always lower-casing the principal name components and always upper casing the realm part on storage. If we always store a krbCanonicalName we get the "correct" case there anyway so out mucking with the krbPrincipalName case will not be a problem for any new entry. Or as Honza pointed out we can use case-insensitive search like this: (krbPrincipalName:caseIgnoreMatch:=ad...@example.com). This will return all variants of casing and reduce the need for fallback code. This *may* cause issues with upgrades though, so we may need fallback code that searches with the case sent by the client if we determine the entry has no krbCanonicalName attribute (sign that it was created before we started adding krbCanonicalName and never "updated"). Note that we also need to think what will happen during and upgrade when some servers still use the current code and some servers will use the new code. So I guess it would be nice if you could write down a table with all possible forms a principal can be in on rows, and old/new server states in columns, and mark what will happen for various operations in each case. Simo. -- David Kupka -- 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] fixing Kerberos principal aliases handling in IPA
On 02/09/15 14:27, Simo Sorce wrote: On Wed, 2015-09-02 at 08:11 +0200, David Kupka wrote: On 01/09/15 16:53, Simo Sorce wrote: On Tue, 2015-09-01 at 16:39 +0200, Martin Babinsky wrote: Hi list, I own the following ticket https://fedorahosted.org/freeipa/ticket/3864 and I would like to clarify what needs to be done in order to make IPA to fully support multiple aliases per entry. So far I have identified these task based on the ticket comments and discussion with Simo way back in the past: 1.) mark 'ipaKrbPrincipalAlias' attribute as deprecated so that it is not used in the new code. 2.) fix ACIs that do not permit setting multiple values of 'krbPrincipalName' attribute per entry (see https://fedorahosted.org/freeipa/ticket/3961) 3.) Modify KDB backend (namely 'ipadb_fetch_principal' and 'ipadb_find_principal' functions) to correctly perform lookup of krbprincipalname/krbcanonicalname, i.e. search krbprincipalname case-insensitively and krbcanonicalname case-sensitively, return krbcanonicalname when canonicalization is requested. 4.) Modify KDB backend and IPA framework to handle creation of both krbprincipalname and krbcanonicalname. I am not quite sure what cases should be covered here (I remember that we should create krbcanonicalname when we add another aliases to krbprincipalname), so it would be nice if you could comment on this. 5.) write tests which cover all this stuff so that we don't shoot ourselves in the foot. I am not very well versed in Kerberos so I might get some of this stuff wrong. If that's the case please point me to the right direction. Also please write me some additional stuff which I have fogot and needs to be done. I think the summary is correct, the only thing we need to be careful is to keep handling entries that have only a single valued krbprincipalname correctly as that will happen in upgrade paths and potentially if someone uses external tools. The tricky part for point 3 is to implement it *without* changing the schema. KrbPrincipalName is case-sensitive, however I think we can solve the issue of "searching case-insensitively" by always lower-casing the principal name components and always upper casing the realm part on storage. If we always store a krbCanonicalName we get the "correct" case there anyway so out mucking with the krbPrincipalName case will not be a problem for any new entry. Or as Honza pointed out we can use case-insensitive search like this: (krbPrincipalName:caseIgnoreMatch:=ad...@example.com). This will return all variants of casing and reduce the need for fallback code. The principal name is not case insensitive, a search like that would probably cause DS to do a full search through the krbPrincipalName index, it might quickly become a performance issue. Before choosing a method please create an install with a 1 principals, and then compare the speed of a few thousand search with exact matching case and a few with specifying caseIgnoreMatch and see the difference. Simo. We will add index for caseIgnoreCaseIA5Match matching rule on krbPrincipalName and then the case insensitive match should be as quick as the case sensitive. Without the index it is indeed far slower. I've generated 10k users and compared 100 ldapsearches: The indexed ones took ~ 4 seconds and the nonindexed one ~2 minutes. That's by two orders of magnitude worse. When we tried to add the index into DS we uncovered a bug in the way DS handles nsMatchingRule attributes. Thierry investigated it and is filling the ticket for DS right now. Thierry can you please send link? Once it's fixed we should be good. David This *may* cause issues with upgrades though, so we may need fallback code that searches with the case sent by the client if we determine the entry has no krbCanonicalName attribute (sign that it was created before we started adding krbCanonicalName and never "updated"). Note that we also need to think what will happen during and upgrade when some servers still use the current code and some servers will use the new code. So I guess it would be nice if you could write down a table with all possible forms a principal can be in on rows, and old/new server states in columns, and mark what will happen for various operations in each case. Simo. -- David Kupka -- 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] fixing Kerberos principal aliases handling in IPA
On 04/09/15 12:49, thierry bordaz wrote: On 09/03/2015 04:03 PM, David Kupka wrote: On 02/09/15 14:27, Simo Sorce wrote: On Wed, 2015-09-02 at 08:11 +0200, David Kupka wrote: On 01/09/15 16:53, Simo Sorce wrote: On Tue, 2015-09-01 at 16:39 +0200, Martin Babinsky wrote: Hi list, I own the following ticket https://fedorahosted.org/freeipa/ticket/3864 and I would like to clarify what needs to be done in order to make IPA to fully support multiple aliases per entry. So far I have identified these task based on the ticket comments and discussion with Simo way back in the past: 1.) mark 'ipaKrbPrincipalAlias' attribute as deprecated so that it is not used in the new code. 2.) fix ACIs that do not permit setting multiple values of 'krbPrincipalName' attribute per entry (see https://fedorahosted.org/freeipa/ticket/3961) 3.) Modify KDB backend (namely 'ipadb_fetch_principal' and 'ipadb_find_principal' functions) to correctly perform lookup of krbprincipalname/krbcanonicalname, i.e. search krbprincipalname case-insensitively and krbcanonicalname case-sensitively, return krbcanonicalname when canonicalization is requested. 4.) Modify KDB backend and IPA framework to handle creation of both krbprincipalname and krbcanonicalname. I am not quite sure what cases should be covered here (I remember that we should create krbcanonicalname when we add another aliases to krbprincipalname), so it would be nice if you could comment on this. 5.) write tests which cover all this stuff so that we don't shoot ourselves in the foot. I am not very well versed in Kerberos so I might get some of this stuff wrong. If that's the case please point me to the right direction. Also please write me some additional stuff which I have fogot and needs to be done. I think the summary is correct, the only thing we need to be careful is to keep handling entries that have only a single valued krbprincipalname correctly as that will happen in upgrade paths and potentially if someone uses external tools. The tricky part for point 3 is to implement it *without* changing the schema. KrbPrincipalName is case-sensitive, however I think we can solve the issue of "searching case-insensitively" by always lower-casing the principal name components and always upper casing the realm part on storage. If we always store a krbCanonicalName we get the "correct" case there anyway so out mucking with the krbPrincipalName case will not be a problem for any new entry. Or as Honza pointed out we can use case-insensitive search like this: (krbPrincipalName:caseIgnoreMatch:=ad...@example.com). This will return all variants of casing and reduce the need for fallback code. The principal name is not case insensitive, a search like that would probably cause DS to do a full search through the krbPrincipalName index, it might quickly become a performance issue. Before choosing a method please create an install with a 1 principals, and then compare the speed of a few thousand search with exact matching case and a few with specifying caseIgnoreMatch and see the difference. Simo. We will add index for caseIgnoreCaseIA5Match matching rule on krbPrincipalName and then the case insensitive match should be as quick as the case sensitive. Without the index it is indeed far slower. I've generated 10k users and compared 100 ldapsearches: The indexed ones took ~ 4 seconds and the nonindexed one ~2 minutes. That's by two orders of magnitude worse. When we tried to add the index into DS we uncovered a bug in the way DS handles nsMatchingRule attributes. Thierry investigated it and is filling the ticket for DS right now. Thierry can you please send link? The ticket is https://fedorahosted.org/389/ticket/48270. I think understand where the problem is but I have no fix yet. David, when you said the unindexed search was slow, did you index 'krbPrincipalName' but added a matching rule to your filter ? I would like to also verify the fix against that perf hit. thanks thierry I've tried these 3 searches: 1) ldapsearch -h localhost -D "cn=Directory Manager" -b cn=users,cn=accounts,dc=example,dc=com -w freeipa8 "(krbprincipalname:caseIgnoreIA5Match:=user1005...@example.com)" 2) ldapsearch -h localhost -D "cn=Directory Manager" -b cn=users,cn=accounts,dc=example,dc=com -w freeipa8 "(krbprincipalname:caseExactIA5Match:=user1005...@example.com)"' 3) ldapsearch -h localhost -D "cn=Directory Manager" -b cn=users,cn=accounts,dc=example,dc=com -w freeipa8 "(krbprincipalname=user1005...@example.com)" The first two was slow and only the last one was quick. Once it's fixed we should be good. David This *may* cause issues with upgrades though, so we may need fallback code that searches with the case sent by the client if we determine the entry has no krbCanonicalName attribute (sign that it was created before we started adding krbCanonicalName and never "updated"
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 28/08/15 13:36, Martin Basti wrote:
On 08/28/2015 10:03 AM, Petr Spacek wrote:
On 27.8.2015 14:22, David Kupka wrote:
@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap
with '
+ 'existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params +
dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_overlap_check'] = True
+
try:
entry = ldap.get_entry(dn)
except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
entry_attrs['idnszoneactive'] = 'TRUE'
+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
return dn
@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
__doc__ = _('Create new DNS zone (SOA record).')
takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is
not resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is not '
+ 'resolvable.')
),
# Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_nameserver_check'] = True
Why is it in DNSZoneBase_add.pre_callback?
Shouldn't the equation force = (skip_nameserver_check +
skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know
the IPA
framework :-))
+
dn = super(dnszone_add, self).pre_callback(
ldap, dn, entry_attrs, attrs_list, *keys, **options)
@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
error=_("Nameserver for reverse zone cannot be
a relative DNS name"))
# verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
check_ns_rec_resolvable(keys[0],
entry_attrs['idnssoamname'])
# show warning about --name-server option
context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
else:
return True
+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for
domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)
Nitpick: I would say "already exists in DNS" to make it absolutely
clear. Just
'already exists' might be confusing because ipa dnszone-show will say
that the
zone does not exist ...
+if ns:
+msg += u" and is handled by server(s): {0}".format(',
'.join(ns))
+raise RuntimeError(msg)
+
def get_ipa_basedn(conn):
"""
Get base DN of IPA suffix in given LDAP server.
0064
NACK
ipa-replica-install should have the --skip-overlap-check too, because
any replica can be the first DNS server.
Thanks for the catch, added.
0064+0058
Can be the options --allow-zone-overlap and --skip-overlap-check merged
into an one name, to have just one name for same thing?
Each option has bit different behavior:
The '--skip-overlap-check' option in API call prevent the check to be
performed and thus no error or warning is raised. This is the way
'--force' option was originally working.
The '--allow-zone-overlap' options in installers do not skip the check
but change the error to warning instead and let the installation continue.
If you think that this can confuse users we need to change the names or
even the logic.
Updated patches attached.
--
David Kupka
From 816ee3102ee0603450f897f8f6bfed4d5460636c Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Fri, 21 Aug 2015 13:25:34 +0200
Subject: [PA
Re: [Freeipa-devel] fixing Kerberos principal aliases handling in IPA
On 02/09/15 14:19, Sumit Bose wrote: On Wed, Sep 02, 2015 at 02:10:52PM +0200, Martin Kosek wrote: On 09/01/2015 04:53 PM, Simo Sorce wrote: On Tue, 2015-09-01 at 16:39 +0200, Martin Babinsky wrote: Hi list, I own the following ticket https://fedorahosted.org/freeipa/ticket/3864 and I would like to clarify what needs to be done in order to make IPA to fully support multiple aliases per entry. So far I have identified these task based on the ticket comments and discussion with Simo way back in the past: 1.) mark 'ipaKrbPrincipalAlias' attribute as deprecated so that it is not used in the new code. 2.) fix ACIs that do not permit setting multiple values of 'krbPrincipalName' attribute per entry (see https://fedorahosted.org/freeipa/ticket/3961) 3.) Modify KDB backend (namely 'ipadb_fetch_principal' and 'ipadb_find_principal' functions) to correctly perform lookup of krbprincipalname/krbcanonicalname, i.e. search krbprincipalname case-insensitively and krbcanonicalname case-sensitively, return krbcanonicalname when canonicalization is requested. 4.) Modify KDB backend and IPA framework to handle creation of both krbprincipalname and krbcanonicalname. I am not quite sure what cases should be covered here (I remember that we should create krbcanonicalname when we add another aliases to krbprincipalname), so it would be nice if you could comment on this. 5.) write tests which cover all this stuff so that we don't shoot ourselves in the foot. I am not very well versed in Kerberos so I might get some of this stuff wrong. If that's the case please point me to the right direction. Also please write me some additional stuff which I have fogot and needs to be done. I think the summary is correct, the only thing we need to be careful is to keep handling entries that have only a single valued krbprincipalname correctly as that will happen in upgrade paths and potentially if someone uses external tools. The tricky part for point 3 is to implement it *without* changing the schema. KrbPrincipalName is case-sensitive, however I think we can solve the issue of "searching case-insensitively" by always lower-casing the principal name components and always upper casing the realm part on storage. If we always store a krbCanonicalName we get the "correct" case there anyway so out mucking with the krbPrincipalName case will not be a problem for any new entry. This *may* cause issues with upgrades though, so we may need fallback code that searches with the case sent by the client if we determine the entry has no krbCanonicalName attribute (sign that it was created before we started adding krbCanonicalName and never "updated"). Note that we also need to think what will happen during and upgrade when some servers still use the current code and some servers will use the new code. So I guess it would be nice if you could write down a table with all possible forms a principal can be in on rows, and old/new server states in columns, and mark what will happen for various operations in each case. Simo. The list looks OK. Do we also plan to change the default RDN for new services or other objects that use krbPrincipalName as RDN at the moment? AFAIU, we are supposed to always use krbCanonicalName as the primary RDN and then only allow krbPrincipalName to be added for the aliases. Of course, the framework needs to still work with old services having krbPrincipalName. no, I think we can/should keep krbPrincipalName e.g. becasue krbCanonicalName is only optional according to the scheme. That's right. We should keep krbprincipalname as RDN as it must be there anyway. We just need to set krbprincipalname and krbcanonicalname to the same value at entry creation and use this value in RDN. Also we won't allow removing krbprincipalname with the same value as krbcanonicalname. This allows us to maintain backwards compatibility with no additional effort. David bye, Sumit -- 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 -- David Kupka -- 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 0304] Installer: do not modify /etc/hosts before user agreement
On 02/09/15 14:12, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/4561 This also fixes: https://fedorahosted.org/freeipa/ticket/5266 Patch attached. Looks good an works for me, ACK. -- David Kupka -- 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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 25/08/15 10:37, David Kupka wrote:
On 24/08/15 16:51, Martin Basti wrote:
On 08/20/2015 10:28 AM, David Kupka wrote:
On 31/07/15 13:32, Martin Basti wrote:
On 30/07/15 14:38, Martin Basti wrote:
On 29/07/15 16:12, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5087
NACK
You forgot to update API.txt file
Thanks for catching that. Updated patch attached.
I'm just curious, what is the reason to check if forward zone exists?
IMO forwardzone must exists somewhere as the master zone. I don't think
we should check forwardzones, this may give too many false positive
errors.
AIUI if the zone exist somewhere and is resolvable there is no need to
add it as a forward zone. If user for some reason want to do it he's
hiding the original zone and we should not allow this (without --force).
Note: Petr2 agreed with David's solution
LGTM, works as expected, but this patch prevents users to add
conflicting zones via webUI (there is no --force field).
We should improve webUI together with this patch.
Martin^2
Martin^2
The '--force' option was not in WebUI before even though it was in API.
IMO we should not expose '--force' options in WebUI at all.
Added similar options to ipa-{server,dns}-install and reworked the patch
to not duplicate the code.
Updated patch and one new attached.
--
David Kupka
From ab7aa126a68bcea95f707a78ca1f776270d3b5ec Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Thu, 2 Jul 2015 15:10:40 +0200
Subject: [PATCH 1/2] dns: do not add (forward)zone if it is already
resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 8 ++--
ipalib/plugins/dns.py | 33 -
ipapython/ipautil.py | 17 +
3 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/API.txt b/API.txt
index 6ab30ddab41715fdbccb4f37aa1852621bca62b4..f6db1e99fca2acf410308c5fdaa13b40c43df933 100644
--- a/API.txt
+++ b/API.txt
@@ -960,15 +960,17 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,10,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('force', autofill=True, default=False)
option: Str('idnsforwarders', attribute=True, cli_name='forwarder', csv=True, multivalue=True, required=False)
option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first', u'none'))
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
@@ -1367,7 +1369,7 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
output: Output('summary', (type 'unicode', type 'NoneType'), None)
output: PrimaryKey('value', None, None)
command: dnszone_add
-args: 1,26,3
+args: 1,28,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1394,6 +1396,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (type 'unicode', type 'NoneType'), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 512a653c3cc8ee641debec0d20f58e17eff08266
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 28/08/15 10:03, Petr Spacek wrote:
On 27.8.2015 14:22, David Kupka wrote:
@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap with '
+ 'existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params + dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
**options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_overlap_check'] = True
+
try:
entry = ldap.get_entry(dn)
except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
entry_attrs['idnszoneactive'] = 'TRUE'
+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
return dn
@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
__doc__ = _('Create new DNS zone (SOA record).')
takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is not
resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is not '
+ 'resolvable.')
),
# Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys,
**options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_nameserver_check'] = True
Why is it in DNSZoneBase_add.pre_callback?
Shouldn't the equation force = (skip_nameserver_check + skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know the IPA
framework :-))
IIUC it is usually handled in pre_callback because framework does not
provide any other mechanism preprocess and validate options.
+
dn = super(dnszone_add, self).pre_callback(
ldap, dn, entry_attrs, attrs_list, *keys, **options)
@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
error=_("Nameserver for reverse zone cannot be a relative DNS
name"))
# verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'])
# show warning about --name-server option
context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
else:
return True
+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)
Nitpick: I would say "already exists in DNS" to make it absolutely clear. Just
'already exists' might be confusing because ipa dnszone-show will say that the
zone does not exist ...
Ok, will update this.
+if ns:
+msg += u" and is handled by server(s): {0}".format(', '.join(ns))
+raise RuntimeError(msg)
+
def get_ipa_basedn(conn):
"""
Get base DN of IPA suffix in given LDAP server.
--
David Kupka
--
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 PoC] proper support of kerberos principal aliases
On 09/09/15 15:59, Simo Sorce wrote:
On Wed, 2015-09-09 at 10:52 +0200, Martin Babinsky wrote:
if (found) {
+/* replace the incoming principal with the value got
from LDAP
+ * search. This is needed so that correctly case
principal is
+ * returned in the case when canonicalization is
switched on
+ * and no krbcanonicalname attribute is present in
the entry.
+ */
+free(*principal);
+*principal = strdup(vals[i]->bv_val);
+if (!(*principal)) {
+return KRB5_KDB_INTERNAL_ERROR;
+}
break;
This unconditionally replaces the principal even when canonicalization
is not requested. Shouldn't this replace be conditional on
KRB5_KDB_FLAGS_ALIAS_OK being set ?
Simo.
It's not obvious from first look but it actually depends on the
KRB5_KDB_FLAGS_ALIAS_OK.
When KRB5_KDB_FLAGS_ALIAS_OK is true the 'found' variable is the result
of case-insensitive comparison.
When it's false 'found' variable is the result of case-sensitive comparison.
In case of case-sensitive match we're replacing the principal with the
exactly same value though effectively not changing it.
--
David Kupka
--
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 0314] Server Upgrade: backup CS.cfg when dogtag is turnend off
On 10/09/15 18:50, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5287 Patch attached. Works for me, ACK. -- David Kupka -- 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 0291, 0292] Limit max age of replication changelog
On 22/07/15 17:03, Martin Basti wrote: On 20/07/15 19:04, Mark Reynolds wrote: On 07/20/2015 12:50 PM, Martin Basti wrote: On 20/07/15 17:48, Petr Vobornik wrote: On 07/20/2015 05:24 PM, Rob Crittenden wrote: Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5086 Patch attached. Is this going to be a shock on upgrades for people who until now may be relying on the fact that there is no limit? Not making any point, but have to note: Ludwig raised a question on users list but there was no feedback from users. https://www.redhat.com/archives/freeipa-users/2015-July/msg00022.html Should there be a way for an admin to manage this, via the config module perhaps? IMHO this is a significant change and red flags need to be raised so users are aware of it. rob IIUC there is purge delay 7 days, so if changelog max age is 7 or more days, it will not break replication. The issue is if somebody uses changelog for different purpose, right? Well the replication changelog can not be used for anything else but the multimaster replication plugin. If a customer increased the replication purge delay you could potentially run into issues, but again this only comes into play when a replica is down for a very long time. I'm not sure if IPA even provides the option to adjust the replication purge delay, but that doesn't mean a customer can not adjust these settings on their own. Mark I'm attaching new patch, that modifies behavior of 'addifnew' keyword in update files. addifnew will no create new entry if doesn't exist. This is required for proper working of patch 292 Rob are you okay with these patches, as Mark wrote, changelog is used only for replication plugins, so it should not cause any issues to users. Martin^2 Works as expected, ACK. -- David Kupka -- 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 0066] fix for regression in ipa-restore
On 25/09/15 18:13, Martin Babinsky wrote: fixes https://fedorahosted.org/freeipa/ticket/5328 Fixes the issue for me, ACK. -- David Kupka -- 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 0066] fix for regression in ipa-restore
On 01/10/15 14:18, Martin Kosek wrote: On 09/29/2015 03:27 PM, David Kupka wrote: On 25/09/15 18:13, Martin Babinsky wrote: fixes https://fedorahosted.org/freeipa/ticket/5328 Fixes the issue for me, ACK. Just checking - what is the impact here, will ipa-restore still work on a clean machine without FreeIPA installed, i.e. without dirsrv being in /etc/passwd? Yes, restore on clean machine should not be affected. The problem is in scenario such as: 1. install freeipa packages 2. install freeipa-server 3. run ipa-backup 4. *add some system user* 5. run ipa-restore After restore the newly added system user is gone. -- David Kupka -- 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 0004] Rewrap errors in get_principal to CCacheError
On 04/09/15 17:07, Michael Šimáček wrote: On 2015-09-03 14:32, Tomas Babej wrote: On 09/03/2015 12:54 PM, Michael Šimáček wrote: After porting to gssapi, the ipa command prints ugly traceback when kerberos credentials are not available. Rewrapping to CCacheError when getting the principal name results in nicer error message. https://fedorahosted.org/freeipa/ticket/5272 This fixes the issue, however, I am getting a trailing forward slash in the error message: $ ipa user-find ipa: ERROR: Kerberos error: did not receive Kerberos credentials/ Attaching updated revision. I altered more places where kerberos errors were used. Michael Thanks, patch works for me, ACK. -- David Kupka -- 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] More Python 3 porting
On 18/09/15 17:00, Petr Viktorin wrote: Hello, Here are more patches that bring IPA closer to Python 3 compatibility. Hi Petr, thanks for another batch of Python 3 compatibility patches. Unfortunately I hit a lot of pylint errors. Some of them are false positives for sure. Could you please look at them, mark the false positive with "pylint: disable=E" directive and fix the rest? http://fpaste.org/270090/92665414/ And one nitpick, I believe that the plus signs are not needed. -self.arabic_hello_utf8 = '\xd9\x85\xd9\x83\xd9\x8a\xd9\x84' + \ - '\xd8\xb9\x20\xd9\x85\xd8\xa7\xd9' + \ - '\x84\xd9\x91\xd8\xb3\xd9\x84\xd8\xa7' +self.arabic_hello_utf8 = (b'\xd9\x85\xd9\x83\xd9\x8a\xd9\x84' + + b'\xd8\xb9\x20\xd9\x85\xd8\xa7\xd9' + + b'\x84\xd9\x91\xd8\xb3\xd9\x84\xd8\xa7') -- David Kupka -- 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 0069-0077] support for proper Kerberos principal canonicalization
On 06/10/15 13:35, Simo Sorce wrote: On 06/10/15 03:51, thierry bordaz wrote: On 10/06/2015 07:19 AM, David Kupka wrote: On 05/10/15 16:12, Simo Sorce wrote: On 05/10/15 09:00, Martin Babinsky wrote: These patches implement the plumbing required to properly support canonicalization of Kerberos principals ( https://fedorahosted.org/freeipa/ticket/3864). Setting multiple principal aliases on hosts/services is beyond the scope of this patchset and should be done after these patches are pushed. I will try to send some tests for the patches later this week. Please review the hell out of them. LGTM, I do not see any issue at quick visual inspection. What about the performance regression with the indexes ? Is that bug fixed in 389ds ? Simo. The issue is still there. Thierry investigated this in 389 DS and IIUC he is not sure if it's bug or completely missing feature. Therefore we still don't know how much time is needed there. Hi, that is correct. I can reproduce the problem. Although the matching rule (in my test caseIgnoreIA5Match) is found, it has no registered indexing function, so the setting (nsMatchingRule) is ignored. I do not know if the indexing function is missing or there is a bug so that the matching rule "forget" to register it. This feature is documented but I can not find any QA test around it, so I do not know yet if it is a regression or if it was not enabled at all. I do not expect rapid progress on it. How urgent is it ? 7.3 ? For the moment I can think to only two workarounds: * use filtered matching rule (preferred) * change the attribute syntax/matching rule, in the schema (I would discourage this one because changing the schema is risky) We can't change the syntax at this point. Well this patchset is blocked until the 389 ds bug is fixed (the performance regression is too big to just put it in and hope) so I guess we'll have to negotiate a time for the fix. Simo. I agree that we really shouldn't change schema. But I don't think the patches're necessary blocked by this issue. Canonicalization was never supported in FreeIPA and when it is not requested the performance is not effected at all. We could merge patches as soon as they're carefully reviewed and tested to avoid tedious rebasing and start using the new functionality when 389 DS gets fixed. -- David Kupka -- 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 0070] install: Run all validators at once.
On 07/12/15 14:05, David Kupka wrote:
Running validators after all Knobs are set allows use of other Knob
value during validation.
Updated patch attached.
--
David Kupka
From 7f18ac0d8b78ea08ed797ceb9393c6b3121b734d Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Mon, 7 Dec 2015 13:35:49 +0100
Subject: [PATCH] install: Run all validators at once.
---
ipapython/install/core.py | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/ipapython/install/core.py b/ipapython/install/core.py
index 8e3ba58021adba263eb038c5cb70603e4e8c9352..2f62b8568fea129255e42b404789fd29b70dca7c 100644
--- a/ipapython/install/core.py
+++ b/ipapython/install/core.py
@@ -118,16 +118,6 @@ class KnobBase(PropertyBase):
def __init__(self, outer):
self.outer = outer
-def __set__(self, obj, value):
-try:
-self.validate(value)
-except KnobValueError:
-raise
-except ValueError as e:
-raise KnobValueError(self.__outer_name__, str(e))
-
-super(KnobBase, self).__set__(obj, value)
-
def validate(self, value):
pass
@@ -253,8 +243,25 @@ class Configurable(six.with_metaclass(abc.ABCMeta, object)):
except KeyError:
pass
else:
-prop = prop_cls(self)
-prop.__set__(self, value)
+setattr(self, name, value)
+
+for owner_cls, name in cls.knobs():
+if name.startswith('_'):
+continue
+if not isinstance(self, owner_cls):
+continue
+value = getattr(self, name, None)
+if value is None:
+continue
+
+prop_cls = getattr(owner_cls, name)
+prop = prop_cls(self)
+try:
+prop.validate(value)
+except KnobValueError:
+raise
+except ValueError as e:
+raise KnobValueError(name, str(e))
if kwargs:
extra = sorted(kwargs)
--
2.5.0
--
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 07/12/15 14:06, David Kupka wrote:
On 09/09/15 13:39, Petr Spacek wrote:
On 8.9.2015 16:30, David Kupka wrote:
On 28/08/15 13:36, Martin Basti wrote:
On 08/28/2015 10:03 AM, Petr Spacek wrote:
On 27.8.2015 14:22, David Kupka wrote:
@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap
with '
+ 'existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params +
dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_overlap_check'] = True
+
try:
entry = ldap.get_entry(dn)
except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
entry_attrs['idnszoneactive'] = 'TRUE'
+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
return dn
@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
__doc__ = _('Create new DNS zone (SOA record).')
takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is
not resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is
not '
+ 'resolvable.')
),
# Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_nameserver_check'] = True
Why is it in DNSZoneBase_add.pre_callback?
Shouldn't the equation force = (skip_nameserver_check +
skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know
the IPA
framework :-))
+
dn = super(dnszone_add, self).pre_callback(
ldap, dn, entry_attrs, attrs_list, *keys, **options)
@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
error=_("Nameserver for reverse zone cannot be
a relative DNS name"))
# verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
check_ns_rec_resolvable(keys[0],
entry_attrs['idnssoamname'])
# show warning about --name-server option
context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
else:
return True
+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone,
'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for
domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)
Nitpick: I would say "already exists in DNS" to make it absolutely
clear. Just
'already exists' might be confusing because ipa dnszone-show will say
that the
zone does not exist ...
+if ns:
+msg += u" and is handled by server(s): {0}".format(',
'.join(ns))
+raise RuntimeError(msg)
+
def get_ipa_basedn(conn):
"""
Get base DN of IPA suffix in given LDAP server.
0064
NACK
ipa-replica-install should have the --skip-overlap-check too, because
any replica can be the first DNS server.
Thanks for the catch, added.
0064+0058
Can be the options --allow-zone-overlap and --skip-overlap-check merged
into an one name, to have just one name for same thing?
Each option has bit different behavior:
The '--skip-overlap-check' option in API call prevent the check to be
performed and thus no error or warning is raised. This is the way
'--force'
option was originally working.
The '--allow-zone-overlap' options in installers do not skip the
check but
change the error to warning instead and let the installation continue.
If you think that this can confuse users we need to change the names
or even
the logic.
Updated patches attached.
Hello,
thank you very much
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 08/12/15 08:56, Petr Spacek wrote:
On 7.12.2015 14:41, David Kupka wrote:
+def is_host_resolvable(fqdn):
+if not isinstance(fqdn, DNSName):
+fqdn = DNSName(fqdn)
+for rdtype in (rdatatype.A, rdatatype.):
+try:
+resolver.query(fqdn.make_absolute(), rdtype)
+except DNSException:
+continue
+else:
+return True
+
+return False
NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.
Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.
Updated patches attached.
--
David Kupka
From 6927ea57fe73ad9dfd64d432aa18fd7b3ecda084 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 2 Dec 2015 13:17:13 +
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 7 +++--
ipalib/plugins/dns.py | 32 ---
ipapython/ipautil.py | 87 +--
3 files changed, 117 insertions(+), 9 deletions(-)
diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnszone_add
-args: 1,26,3
+args: 1,28,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1393,6 +1394,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 67947360eb207de31ed114bb630705c409b2f9a9..9cad9cfb8b4175cc92778b2df057621ca055e58f 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -53,8 +53,7 @@ from ipalib.util import (normalize_zonemgr,
validate_dnssec_zone_forwarder_step1,
validate_dnssec_zone_forwarder_step2,
verify_host_resolvable)
-
-from ipapython.ipautil import CheckedIPAddress
+from ipapython.ipautil import CheckedIPAddress, check_zone_overlap
from ipapython.dnsutil import DNSName
if six.PY3:
@@ -2121,6 +2120,13 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap with '
+ 'an existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params + dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -2140,6 +2146,12 @@ class DNSZoneBase_add(LDAP
[Freeipa-devel] [PATCH 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).
https://fedorahosted.org/freeipa/ticket/5531
--
David Kupka
From eee2c606aeba8aff61777cbf54fdb6c006e8c755 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 8 Dec 2015 14:22:01 +0100
Subject: [PATCH] replica: Fix ipa-replica-install with replica file (domain
level 0).
https://fedorahosted.org/freeipa/ticket/5531
---
ipaserver/install/server/replicainstall.py | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..a962ef93442c201f9df80adfb0443ab37cf9dc59 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -654,6 +654,8 @@ def install(installer):
if installer._update_hosts_file:
installutils.update_hosts_file(config.ips, config.host_name, fstore)
+ca_enabled = ipautil.file_exists(config.dir + "/cacert.p12")
+
# Create DS user/group if it doesn't exist yet
dsinstance.create_ds_user()
@@ -675,7 +677,7 @@ def install(installer):
ntp.create_instance()
# Configure dirsrv
-ds = install_replica_ds(config, options, installer._ca_enabled)
+ds = install_replica_ds(config, options, ca_enabled)
# Always try to install DNS records
install_dns_records(config, options, remote_api)
@@ -690,20 +692,20 @@ def install(installer):
options.domain_name = config.domain_name
options.host_name = config.host_name
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
options.ra_p12 = config.dir + "/ra.p12"
ca.install(False, config, options)
krb = install_krb(config, setup_pkinit=not options.no_pkinit)
http = install_http(config, auto_redirect=not options.no_ui_redirect,
-ca_is_configured=installer._ca_enabled)
+ca_is_configured=ca_enabled)
otpd = otpdinstance.OtpdInstance()
otpd.create_instance('OTPD', config.host_name, config.dirman_password,
ipautil.realm_to_suffix(config.realm_name))
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
CA = cainstance.CAInstance(config.realm_name, certs.NSS_DIR)
CA.dm_password = config.dirman_password
--
2.5.0
--
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 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).
On 08/12/15 16:33, Tomas Babej wrote:
On 12/08/2015 04:20 PM, Oleg Fayans wrote:
ACK. The initial issue is fixed.
On 12/08/2015 03:03 PM, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5531
Can we get some more love for the patch and provide at least a sentence
worth of commit message before pushing?
It's not obvious from the title what the patch does, other than it fixes
things.
Tomas
I believe it's pretty obvious from linked ticket and attached patch
changing just 5 lines.
But you're right verbosity in commit message could save time later.
Patch with changed commit message attached.
--
David Kupka
From eee2c606aeba8aff61777cbf54fdb6c006e8c755 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 8 Dec 2015 14:22:01 +0100
Subject: [PATCH] replica: Fix ipa-replica-install with replica file (domain
level 0).
Attribute _ca_enabled is set in promote_check() and is not available in
install(). When installing replica in domain level 0 we can determine existence
of CA service based on existence of cacert.p12 file in provided replica-file.
https://fedorahosted.org/freeipa/ticket/5531
---
ipaserver/install/server/replicainstall.py | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..a962ef93442c201f9df80adfb0443ab37cf9dc59 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -654,6 +654,8 @@ def install(installer):
if installer._update_hosts_file:
installutils.update_hosts_file(config.ips, config.host_name, fstore)
+ca_enabled = ipautil.file_exists(config.dir + "/cacert.p12")
+
# Create DS user/group if it doesn't exist yet
dsinstance.create_ds_user()
@@ -675,7 +677,7 @@ def install(installer):
ntp.create_instance()
# Configure dirsrv
-ds = install_replica_ds(config, options, installer._ca_enabled)
+ds = install_replica_ds(config, options, ca_enabled)
# Always try to install DNS records
install_dns_records(config, options, remote_api)
@@ -690,20 +692,20 @@ def install(installer):
options.domain_name = config.domain_name
options.host_name = config.host_name
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
options.ra_p12 = config.dir + "/ra.p12"
ca.install(False, config, options)
krb = install_krb(config, setup_pkinit=not options.no_pkinit)
http = install_http(config, auto_redirect=not options.no_ui_redirect,
-ca_is_configured=installer._ca_enabled)
+ca_is_configured=ca_enabled)
otpd = otpdinstance.OtpdInstance()
otpd.create_instance('OTPD', config.host_name, config.dirman_password,
ipautil.realm_to_suffix(config.realm_name))
-if ipautil.file_exists(config.dir + "/cacert.p12"):
+if ca_enabled:
CA = cainstance.CAInstance(config.realm_name, certs.NSS_DIR)
CA.dm_password = config.dirman_password
--
2.5.0
--
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 0069] ipa-replica-install support caless install with promotion.
On 02/12/15 07:58, Jan Cholasta wrote:
On 1.12.2015 14:27, David Kupka wrote:
On 30/11/15 17:24, Jan Cholasta wrote:
Hi,
On 27.11.2015 07:57, David Kupka wrote:
On 26/11/15 15:22, David Kupka wrote:
On 26/11/15 15:13, David Kupka wrote:
On 26/11/15 15:01, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5441
Replaced accidentally inserted tabs.
Fixed indentation I screwed up when replacing tabs :-/
1) The deprecated --*_pkcs12 and --*_pin aliases should not be supported
in ipa-replica-install.
In ServerCA, inherit the knobs from BaseServerCA rather than
BaseServer.ca. The "#pylint: disable=no-member" will no longer be
necessary.
In ipa-server-install help, there are 2 "certificate system" option
groups. This is a shortcoming in the installer framework, which will be
addressed in the future. For now, please inherit *all* knobs of
BaseServerCA in ServerCA as a workaround.
2) This check from ipa-replica-prepare should be added to
Replica.__init__() as well:
# If any of the PKCS#12 options are selected, all are required.
cert_file_req = (options.dirsrv_cert_files,
options.http_cert_files)
cert_file_opt = (options.pkinit_cert_files,)
if any(cert_file_req + cert_file_opt) and not
all(cert_file_req):
self.option_parser.error(
"--dirsrv-cert-file and --http-cert-file are required
if any "
"PKCS#12 options are used.")
The check is done when replica file is specified in the patch, but it
should be done only when replica file is *not* specified.
6) Please make the ca_is_enabled argument of install_replica_ds() and
install_http() mandatory and fill as appropriate when called, it will
make the code more readable.
This bit in install_http() is redundant now:
+if ca_is_configured is None:
+ca_is_configured = ipautil.file_exists(config.dir + "/cacert.p12")
7)
$ git diff -U0 | pep8 --diff
./ipaserver/install/server/replicainstall.py:99:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:161:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:1289:13: E265 block comment
should start with '# '
./ipaserver/install/server/replicainstall.py:1291:17: E125 continuation
line with same indent as next logical line
./ipaserver/install/server/replicainstall.py:1291:17: E128 continuation
line under-indented for visual indent
$ git diff -U0 | pep8 --diff
./ipaserver/install/server/install.py:1142:1: E302 expected 2 blank
lines, found 1
./ipaserver/install/server/install.py:1143:5: E265 block comment should
start with '# '
./ipaserver/install/server/install.py:1160:17: E222 multiple spaces
after operator
./ipaserver/install/server/install.py:1288:9: E265 block comment should
start with '# '
./ipaserver/install/server/replicainstall.py:100:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:162:80: E501 line too long
(82 > 79 characters)
./ipaserver/install/server/replicainstall.py:697:41: E251 unexpected
spaces around keyword / parameter equals
./ipaserver/install/server/replicainstall.py:697:43: E251 unexpected
spaces around keyword / parameter equals
./ipaserver/install/server/replicainstall.py:922:9: E129 visually
indented line with same indent as next logical line
./ipaserver/install/server/replicainstall.py:925:14: E131 continuation
line unaligned for hanging indent
./ipaserver/install/server/replicainstall.py:1345:9: E265 block comment
should start with '# '
./ipaserver/install/server/replicainstall.py:1389:21: E128 continuation
line under-indented for visual indent
Thanks, updated patch attached.
--
David Kupka
From c1e2259bb352e160e41deb8853bd615f1c9f3db1 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 26 Nov 2015 09:01:27 +0100
Subject: [PATCH] ipa-replica-install support caless install with promotion.
https://fedorahosted.org/freeipa/ticket/5441
---
ipaserver/install/custodiainstance.py | 6 +-
ipaserver/install/dsinstance.py| 3 +-
ipaserver/install/server/common.py | 6 --
ipaserver/install/server/install.py| 58 +-
ipaserver/install/server/replicainstall.py | 168 -
5 files changed, 199 insertions(+), 42 deletions(-)
diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index df99962a7e6e8ecac044ff4e8341a4a9913e4d4d..dbe36af6d7af23fa859dcb78f3dc24224fd8fd07 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -17,7 +17,7 @@ import tempfile
class CustodiaInstance(SimpleServiceInstance):
-def __init__(self, host_name=None, realm=None):
+def __init__(self, host_name=None, realm=None, ca_is_configured=True):
super(CustodiaInstance, self).__init__("ipa-custodia")
self.
[Freeipa-devel] [PATCH 0070] install: Run all validators at once.
Running validators after all Knobs are set allows use of other Knob
value during validation.
--
David Kupka
From b9a8ae178e770a4b84fc8d05d04218531642d3eb Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Mon, 7 Dec 2015 13:35:49 +0100
Subject: [PATCH] install: Run all validators at once.
---
ipapython/install/core.py | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/ipapython/install/core.py b/ipapython/install/core.py
index 8e3ba58021adba263eb038c5cb70603e4e8c9352..62c1fb61797863da49402d3f86d14e3c6389d932 100644
--- a/ipapython/install/core.py
+++ b/ipapython/install/core.py
@@ -118,16 +118,6 @@ class KnobBase(PropertyBase):
def __init__(self, outer):
self.outer = outer
-def __set__(self, obj, value):
-try:
-self.validate(value)
-except KnobValueError:
-raise
-except ValueError as e:
-raise KnobValueError(self.__outer_name__, str(e))
-
-super(KnobBase, self).__set__(obj, value)
-
def validate(self, value):
pass
@@ -253,8 +243,27 @@ class Configurable(six.with_metaclass(abc.ABCMeta, object)):
except KeyError:
pass
else:
-prop = prop_cls(self)
-prop.__set__(self, value)
+setattr(self, name, value)
+
+for owner_cls, name in cls.knobs():
+if name.startswith('_'):
+continue
+if not isinstance(self, owner_cls):
+continue
+if name not in self.__dict__:
+continue
+try:
+value = getattr(self, name)
+except AttributeError:
+continue
+prop_cls = getattr(owner_cls, name)
+prop = prop_cls(self)
+try:
+prop.validate(value)
+except KnobValueError:
+raise
+except ValueError as e:
+raise KnobValueError(name, str(e))
if kwargs:
extra = sorted(kwargs)
--
2.5.0
--
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 09/09/15 13:39, Petr Spacek wrote:
On 8.9.2015 16:30, David Kupka wrote:
On 28/08/15 13:36, Martin Basti wrote:
On 08/28/2015 10:03 AM, Petr Spacek wrote:
On 27.8.2015 14:22, David Kupka wrote:
@@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('force',
+ label=_('Force'),
+ doc=_('Force DNS zone creation.')
+),
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap
with '
+ 'existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params +
dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_overlap_check'] = True
+
try:
entry = ldap.get_entry(dn)
except errors.NotFound:
@@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
entry_attrs['idnszoneactive'] = 'TRUE'
+if not options['skip_overlap_check']:
+try:
+check_zone_overlap(keys[-1])
+except RuntimeError as e:
+raise errors.InvocationError(e.message)
+
return dn
@@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
__doc__ = _('Create new DNS zone (SOA record).')
takes_options = DNSZoneBase_add.takes_options + (
-Flag('force',
- label=_('Force'),
- doc=_('Force DNS zone creation even if nameserver is
not resolvable.'),
+Flag('skip_nameserver_check',
+ doc=_('Force DNS zone creation even if nameserver is not '
+ 'resolvable.')
),
# Deprecated
@@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
def pre_callback(self, ldap, dn, entry_attrs, attrs_list,
*keys, **options):
assert isinstance(dn, DN)
+if options['force']:
+options['skip_nameserver_check'] = True
Why is it in DNSZoneBase_add.pre_callback?
Shouldn't the equation force = (skip_nameserver_check +
skip_nameserver_check)
be handled in parameter parsing & validation? (Again, I do not know
the IPA
framework :-))
+
dn = super(dnszone_add, self).pre_callback(
ldap, dn, entry_attrs, attrs_list, *keys, **options)
@@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
error=_("Nameserver for reverse zone cannot be
a relative DNS name"))
# verify if user specified server is resolvable
-if not options['force']:
+if not options['skip_nameserver_check']:
check_ns_rec_resolvable(keys[0],
entry_attrs['idnssoamname'])
# show warning about --name-server option
context.show_warning_nameserver_option = True
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index
d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a
100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -924,6 +924,20 @@ def host_exists(host):
else:
return True
+def check_zone_overlap(zone):
+if resolver.zone_for_name(zone) == zone:
+try:
+ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+except DNSException as e:
+root_logger.debug("Failed to resolve nameserver(s) for
domain"
+" {0}: {1}".format(zone, e))
+ns = []
+
+msg = u"DNS zone {0} already exists".format(zone)
Nitpick: I would say "already exists in DNS" to make it absolutely
clear. Just
'already exists' might be confusing because ipa dnszone-show will say
that the
zone does not exist ...
+if ns:
+msg += u" and is handled by server(s): {0}".format(',
'.join(ns))
+raise RuntimeError(msg)
+
def get_ipa_basedn(conn):
"""
Get base DN of IPA suffix in given LDAP server.
0064
NACK
ipa-replica-install should have the --skip-overlap-check too, because
any replica can be the first DNS server.
Thanks for the catch, added.
0064+0058
Can be the options --allow-zone-overlap and --skip-overlap-check merged
into an one name, to have just one name for same thing?
Each option has bit different behavior:
The '--skip-overlap-check' option in API call prevent the check to be
performed and thus no error or warning is raised. This is the way '--force'
option was originally working.
The '--allow-zone-overlap' options in installers do not skip the check but
change the error to warning instead and let the installation continue.
If you think that this can confuse users we need to change the names or even
the logic.
Updated patches attached.
Hello,
thank you very much for updating the patch.
Unfortunately it is not yet re
Re: [Freeipa-devel] [PATCH 0113] properly add ACIs to custodia container during IPA upgrade
On 10/12/15 10:14, Martin Babinsky wrote: On 12/08/2015 10:45 AM, Martin Babinsky wrote: fixes https://fedorahosted.org/freeipa/ticket/5524 Attaching updated patch with simpler fix suggested by Jan. Thanks for the patch. Works for me, ACK. -- David Kupka -- 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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG 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/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372, in __runner self
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 08/12/15 15:24, Petr Spacek wrote:
On 8.12.2015 12:19, David Kupka wrote:
On 08/12/15 08:56, Petr Spacek wrote:
On 7.12.2015 14:41, David Kupka wrote:
+def is_host_resolvable(fqdn):
+if not isinstance(fqdn, DNSName):
+fqdn = DNSName(fqdn)
+for rdtype in (rdatatype.A, rdatatype.):
+try:
+resolver.query(fqdn.make_absolute(), rdtype)
+except DNSException:
+continue
+else:
+return True
+
+return False
NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.
Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.
Updated patches attached.
Hmm, my review script screams:
Detected base branch: origin/master
Checks will be made against following base commit: 848912a add missing
/ipaplatform/constants.py to .gitignore
Writing API to API.txt
./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
visual indent
./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
./ipaserver/install/bindinstance.py:49:9: E128 continuation line
under-indented for visual indent
./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
characters)
./ipaserver/install/bindinstance.py:438:19: E128 continuation line
under-indented for visual indent
./ipaserver/install/server/common.py:271:63: E261 at least two spaces before
inline comment
./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
characters)
ERROR: PEP8 --diff failed, continuing ...
ERROR: API.txt was changed without a change in VERSION, continuing ...
Summary of detected errors:
ERROR: PEP8 --diff failed
ERROR: API.txt was changed without a change in VERSION
Please fix it :-)
Make make this more pleasant for you, you can use (and review) my attached
patch. It adds 'review' target to Makefile, it will do the same checks as I do.
Unfortunately your tool does not work for me (output w/ -o xtrace attached).
Anyway I have incremented API version and fixed most of the pep8 errors
except for E124 twice in ipalib/plugins/dns.py as it seems to be
convention in the file and E501 twice in ipapython/ipautil.py because
IMO breaking string constant into multiple lines does not help readability.
Updated patches also attached.
--
David Kupka
From 4fb72e2b197d365b99bb426fa4b4919c60efe44b Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 2 Dec 2015 13:17:13 +
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 7 ++--
VERSION | 2 +-
ipalib/plugins/dns.py | 32 +++---
ipapython/ipautil.py | 89 +--
4 files changed, 120 insertions(+), 10 deletions(-)
diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
outpu
[Freeipa-devel] [PATCH 0074] spec file: Add dbus-python to BuildRequires
During work on ticket #5497 [0] the need for dbus-python in build time was introduced but it was not added in spec file. [0] https://fedorahosted.org/freeipa/ticket/5497 -- David Kupka From 6d1f5532de420efbe5c5f251681b8e7496ecb065 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Mon, 14 Dec 2015 12:16:33 + Subject: [PATCH] spec file: Add dbus-python to BuildRequires Commit 8d7f67e introduced the need for dbus-python during build time. https://fedorahosted.org/freeipa/ticket/5497 --- freeipa.spec.in | 1 + 1 file changed, 1 insertion(+) diff --git a/freeipa.spec.in b/freeipa.spec.in index a074c37..d8d9f11 100644 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -98,6 +98,7 @@ BuildRequires: python-six BuildRequires: python-jwcrypto BuildRequires: custodia BuildRequires: libini_config-devel >= 1.2.0 +BuildRequires: dbus-python # Build dependencies for unit tests BuildRequires: libcmocka-devel -- 2.5.0 -- 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 0376] KRA: add RA cert during replica promotion
On 10/12/15 19:40, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5512 patch attached. Hi, thanks for the patch. It works but only when WAIT_AFTER_ARCHIVE is raised. Patch attached. -- David Kupka From a209343652b8bedfcbca83c7eafc699e72c0a261 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Mon, 14 Dec 2015 10:52:44 +0100 Subject: [PATCH] test: Temporarily increase timeout in vault test. Remove this change when vault is fixed. --- ipatests/test_integration/test_vault.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_integration/test_vault.py b/ipatests/test_integration/test_vault.py index 74b554eb283278940e6ed0a93596ed194eadadcb..3b717c9cdfda30dd230d31730328cd3aa4cbdd49 100644 --- a/ipatests/test_integration/test_vault.py +++ b/ipatests/test_integration/test_vault.py @@ -7,7 +7,7 @@ import time from ipatests.test_integration.base import IntegrationTest from ipatests.test_integration import tasks -WAIT_AFTER_ARCHIVE = 30 # give some time to replication +WAIT_AFTER_ARCHIVE = 90 # give some time to replication class TestInstallKRA(IntegrationTest): -- 2.5.0 -- 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 0365] Remove unused KRA code from ipa-server-install
On 14/12/15 16:54, Alexander Bokovoy wrote:
On Mon, 14 Dec 2015, David Kupka wrote:
On 14/12/15 15:05, Alexander Bokovoy wrote:
On Mon, 14 Dec 2015, David Kupka wrote:
On 30/11/15 16:31, Martin Basti wrote:
First instance of KRA should be installed only by ipa-kra-install
Patch attached.
Hi,
patch works, but I don't like the approach.
Do we really want to remove '--setup-kra' option from
ipa-server-install?
Why do we remove '--setup-kra' while keeping '--setup-dns'? Why do we
keep installing PKI CA when it can be also installed later?
Yes, we do want. Adding the option was an error in the first place. This
patch fixes the error.
I would love if 'ipa-server-install' installed only the bare minimum
needed and then other features was added using ipa-{feature}-install
but also I don't mind one almighty installer that can install all
possible combinations of features.
But this is neither of it. This just brings another inconsistency into
FreeIPA behavior.
We don't have --with-adtrust either. And we don't want it.
Ok, then are we aiming for 'ipa-server-install' that only installs the
bare minimum and everything else should be installed later?
Or, do we decide per feature if it's appropriate to include it in
'ipa-server-install'? What are the criteria in this case?
Per feature decision is a bit better description of an ad-hoc process we
have so far.
As we had this discussion with Simo today, let me quickly capture
arguments for both. We typically allow options in ipa-server-install for
features that can be present and used very early. Certificates are
issued early in the process, DNS records are critical to exist before
hosts can be added and can start using Kerberos and so on. However,
trust to AD cannot be established to 'just deployed' FreeIPA realm, you
need to pre-configure your and that remote realm.
Certificates were in particular important part of the story before we
added support for GSSAPI authentication between replicas (4.3 release is
going to have it). This meant we were constrained by the fact that some
entity had to issue certificates before we even create a replica.
I was arguing that having KRA would require us to have full fledged CA
as we depend on ability to issue certificates for KRA feature to be
useful as well. While standard CA is somewhat optional in the case only
end-point service certs are needed (Let's Encrypt is a solution there),
having KRA means use of some CA. So to me having KRA means we have CA
already, why not to simply merge the two setting into --setup-ca at all?
Separate installers also were used in past as a gap-bridging tool to
allow people to upgrade their old deployments and gain new
functionality. So separate installer has another function than just
deploying a service.
Having --with-kra option thus makes less sense. We can consider KRA
functionality to be in a default feature set at some point, that would
still require us to have a separate installer, ipa-kra-install, to allow
extending old deployments to provide new feature.
The fact that the DNS records need to exist for some (core) IPA
functionality does not necessary mean that we need DNS server installed
as a part of FreeIPA. We even support DNS-less install and user is then
responsible for maintaining DNS records in server of his choice.
IOW 'ipa-server-install --setup-dns' results in the same as
'ipa-server-install && ipa-dns-install'.
In case of trust, would it be really impossible to preconfigure the
remote realm and then provide all necessary information to
'ipa-server-install --setup-trust'? Or is it just the way we're doing it
right now for some maybe really good reason?
So the only reason to add the '--setup-' or '--with-'
to ipa-server-install I can see is user comfort and ease of
installation. That's good reason for me but in that case I still do not
understand why do not include options that can be easily included.
--
David Kupka
--
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 14/12/15 15:25, David Kupka wrote: On 14/12/15 14:52, David Kupka wrote: On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG 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/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line
Re: [Freeipa-devel] [PATCH 0365] Remove unused KRA code from ipa-server-install
On 30/11/15 16:31, Martin Basti wrote:
First instance of KRA should be installed only by ipa-kra-install
Patch attached.
Hi,
patch works, but I don't like the approach.
Do we really want to remove '--setup-kra' option from ipa-server-install?
Why do we remove '--setup-kra' while keeping '--setup-dns'? Why do we
keep installing PKI CA when it can be also installed later?
I would love if 'ipa-server-install' installed only the bare minimum
needed and then other features was added using ipa-{feature}-install but
also I don't mind one almighty installer that can install all possible
combinations of features.
But this is neither of it. This just brings another inconsistency into
FreeIPA behavior.
--
David Kupka
--
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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG 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/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/pyt
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 14/12/15 14:52, David Kupka wrote: On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG 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/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in
Re: [Freeipa-devel] [PATCH 0365] Remove unused KRA code from ipa-server-install
On 14/12/15 15:05, Alexander Bokovoy wrote:
On Mon, 14 Dec 2015, David Kupka wrote:
On 30/11/15 16:31, Martin Basti wrote:
First instance of KRA should be installed only by ipa-kra-install
Patch attached.
Hi,
patch works, but I don't like the approach.
Do we really want to remove '--setup-kra' option from ipa-server-install?
Why do we remove '--setup-kra' while keeping '--setup-dns'? Why do we
keep installing PKI CA when it can be also installed later?
Yes, we do want. Adding the option was an error in the first place. This
patch fixes the error.
I would love if 'ipa-server-install' installed only the bare minimum
needed and then other features was added using ipa-{feature}-install
but also I don't mind one almighty installer that can install all
possible combinations of features.
But this is neither of it. This just brings another inconsistency into
FreeIPA behavior.
We don't have --with-adtrust either. And we don't want it.
Ok, then are we aiming for 'ipa-server-install' that only installs the
bare minimum and everything else should be installed later?
Or, do we decide per feature if it's appropriate to include it in
'ipa-server-install'? What are the criteria in this case?
--
David Kupka
--
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 0077] ipa-dns-install: Do not check for zone overlap when DNS, installed.
Standalone DNS installer always performed overlap check effectively
preventing installation on replica when other DNS instance was already
installed in topology.
--
David Kupka
From d9b9c861ea3090d62bbe011c402d82243a166754 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 17 Dec 2015 16:16:09 +0100
Subject: [PATCH] ipa-dns-install: Do not check for zone overlap when DNS
installed.
When DNS is already installed somewhere in topology we should not check for
zone overlap because it would always say that we are overlapping our own domain.
ipa-replica-install already does that but ipa-dns-install did not.
---
install/tools/ipa-dns-install | 2 +-
ipaserver/install/dns.py | 24
ipaserver/install/server/install.py| 2 +-
ipaserver/install/server/replicainstall.py | 4 ++--
4 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index eebc9e2ad8a6da71807b7d9d24cd41289c5b4d58..ee7ebe0bdcd2dde9ec1bcbe9f54dbe1baf3db9b2 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -134,7 +134,7 @@ def main():
options.setup_ca = None # must be None to enable autodetection
-dns_installer.install_check(True, False, options, hostname=api.env.host)
+dns_installer.install_check(True, api, False, options, hostname=api.env.host)
dns_installer.install(True, False, options)
# Restart http instance to make sure that python-dns has the right resolver
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 763b2aca475d5f5b25d2aded05bc540ce3836f81..b9a749362de79415b3f3b925b20acd9b74420195 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -99,20 +99,7 @@ def _disable_dnssec():
conn.update_entry(entry)
-def check_dns_enabled(api):
-try:
-api.Backend.rpcclient.connect()
-result = api.Backend.rpcclient.forward(
-'dns_is_enabled',
-version=u'2.112',# All the way back to 3.0 servers
-)
-return result['result']
-finally:
-if api.Backend.rpcclient.isconnected():
-api.Backend.rpcclient.disconnect()
-
-
-def install_check(standalone, replica, options, hostname):
+def install_check(standalone, api, replica, options, hostname):
global ip_addresses
global reverse_zones
fstore = sysrestore.FileStore(paths.SYSRESTORE)
@@ -121,8 +108,13 @@ def install_check(standalone, replica, options, hostname):
raise RuntimeError("Integrated DNS requires '%s' package" %
constants.IPA_DNS_PACKAGE_NAME)
-# when installing first replica with DNS we need to check zone overlap
-if not replica or not check_dns_enabled(api):
+# when installing first DNS instance we need to check zone overlap
+if replica or standalone:
+already_enabled = api.Command('dns_is_enabled')['result']
+else:
+already_enabled = False
+
+if not already_enabled:
domain = dnsutil.DNSName(util.normalize_zone(api.env.domain))
print("Checking DNS domain %s, please wait ..." % domain)
try:
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index a07ca664e263a1bb49c6f5e18bc888fa66e56b55..78cc5a4f5cfe1ee440392f9e0a7f5a508a32d4ab 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -706,7 +706,7 @@ def install_check(installer):
sys.exit(1)
if options.setup_dns:
-dns.install_check(False, False, options, host_name)
+dns.install_check(False, api, False, options, host_name)
ip_addresses = dns.ip_addresses
else:
ip_addresses = get_server_ip_address(host_name,
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4e6abde0e424aaacd06d5d782aab0c260f1b07ab..63fe02b5cbb5c8a2a4be6ffdceaaf0200a1c7969 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -653,7 +653,7 @@ def install_check(installer):
conn.disconnect()
if options.setup_dns:
-dns.install_check(False, True, options, config.host_name)
+dns.install_check(False, remote_api, True, options, config.host_name)
config.ips = dns.ip_addresses
else:
config.ips = installutils.get_server_ip_address(
@@ -1175,7 +1175,7 @@ def promote_check(installer):
conn.disconnect()
if options.setup_dns:
-dns.install_check(False, True, options, config.host_name)
+dns.install_check(False, remote_api, True, options, config.host_name)
else:
config.ips = installutils.get_server_ip_address(
config.host_name, not installer.interactive,
--
2.5.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.r
Re: [Freeipa-devel] [PATCH 0077] ipa-dns-install: Do not check for zone overlap when DNS, installed.
On 18/12/15 12:04, Petr Vobornik wrote: On 12/18/2015 11:26 AM, David Kupka wrote: Standalone DNS installer always performed overlap check effectively preventing installation on replica when other DNS instance was already installed in topology. I don't like the position of api argument in the install_check. It is not consistent with install_check in KRA plus it's between two related options: standalone and replica. Is there a reason behind it which I don't see? Right now (with my patch applied) I see: ca.install_check(standalone, replica_config, options) kra.install_check(api, replica_config, options) dns.install_check(standalone, api, replica, options, hostname) And if I needed to get common signature of the functions I would probably end up with: install_check(standalone, api, replica_config, options, ...) In KRA there is no use for option standalone so it is left out. In DNS replica_config is not needed, only information we are running on replica has some value. So it's replaced by boolean. But if you think this is important please tell me where api option should go and I'll change it... -- David Kupka -- 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 0075-0076] Fix installer regression
On 17/12/15 13:44, Jan Cholasta wrote: On 17.12.2015 13:26, David Kupka wrote: On 17/12/15 12:14, Petr Vobornik wrote: On 12/16/2015 02:31 PM, David Kupka wrote: https://www.redhat.com/archives/freeipa-users/2015-December/msg00203.html please link the patch to https://fedorahosted.org/freeipa/ticket/5556 Updated patches attached. NACK, this is the correct procedure for the __getattr__(), sorry for confusing you earlier: def __getattr__(self, name): for owner_cls, knob_name in self.knobs(): if knob_name == name: break else: raise AttributeError(name) for component in self.__components: if isinstance(component, owner_cls): break else: raise AttributeError(name) return getattr(component, name) Honza Updated patches attached. -- David Kupka From 2f4a8a968c6d25c60aeff46ed137f736726c0225 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 16 Dec 2015 12:43:13 + Subject: [PATCH 1/2] installer: Propagate option values from components instead of copying them. https://fedorahosted.org/freeipa/ticket/5556 --- ipapython/install/core.py | 13 ++--- ipaserver/install/server/common.py | 28 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/ipapython/install/core.py b/ipapython/install/core.py index 6b2da05d31d26bd3b7222e237c8646fa80ab5290..8520aaf097e7851a364e616d8843df9338cae8ad 100644 --- a/ipapython/install/core.py +++ b/ipapython/install/core.py @@ -528,6 +528,13 @@ class Composite(Configurable): for order, owner_cls, name in result: yield owner_cls, name +def __getattr__(self, name): +for owner_cls, knob_name in self.knobs(): +for component in self.__components: +if isinstance(component, owner_cls): +return getattr(component, knob_name) +raise AttributeError(name) + def _reset(self): self.__components = list(self._get_components()) @@ -545,8 +552,7 @@ class Composite(Configurable): try: validator.next() except StopIteration: -if child.done(): -self.__components.remove(child) +pass else: new_validate.append((child, validator)) if not new_validate: @@ -560,7 +566,8 @@ class Composite(Configurable): yield from_(super(Composite, self)._configure()) -execute = [(c, c._executor()) for c in self.__components] +execute = [(c, c._executor()) for c in self.__components +if not c.done()] while True: new_execute = [] for child, executor in execute: diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py index 3eb7279d200ffd6ab33d8d914c8d4f13e567a171..948aac842951dc3cef9dace87b28a92d3f98dea3 100644 --- a/ipaserver/install/server/common.py +++ b/ipaserver/install/server/common.py @@ -409,34 +409,6 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): # Automatically disable pkinit w/ dogtag until that is supported self.no_pkinit = True -self.external_ca = self.ca.external_ca -self.external_ca_type = self.ca.external_ca_type -self.external_cert_files = self.ca.external_cert_files -self.dirsrv_cert_files = self.ca.dirsrv_cert_files -self.http_cert_files = self.ca.http_cert_files -self.pkinit_cert_files = self.ca.pkinit_cert_files -self.dirsrv_pin = self.ca.dirsrv_pin -self.http_pin = self.ca.http_pin -self.pkinit_pin = self.ca.pkinit_pin -self.dirsrv_cert_name = self.ca.dirsrv_cert_name -self.http_cert_name = self.ca.http_cert_name -self.pkinit_cert_name = self.ca.pkinit_cert_name -self.ca_cert_files = self.ca.ca_cert_files -self.subject = self.ca.subject -self.ca_signing_algorithm = self.ca.ca_signing_algorithm -self.skip_schema_check = self.ca.skip_schema_check - -self.forwarders = self.dns.forwarders -self.no_forwarders = self.dns.no_forwarders -self.reverse_zones = self.dns.reverse_zones -self.no_reverse = self.dns.no_reverse -self.no_dnssec_validation = self.dns.no_dnssec_validation -self.dnssec_master = self.dns.dnssec_master -self.disable_dnssec_master = self.dns.disable_dnssec_master -self.kasp_db_file = self.dns.kasp_db_file -self.force = self.dns.force -self.zonemgr = self.dns.zonemgr - self.unattended = not self.interactive ca = core.Component(BaseServerCA) -- 2.5.0 From cae86eb374c2342c8714b5917422a783d54fef34 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 16 Dec 2015 12:43:13 + Subject
Re: [Freeipa-devel] [PATCH 0075-0076] Fix installer regression
On 18/12/15 13:57, David Kupka wrote: On 17/12/15 13:44, Jan Cholasta wrote: On 17.12.2015 13:26, David Kupka wrote: On 17/12/15 12:14, Petr Vobornik wrote: On 12/16/2015 02:31 PM, David Kupka wrote: https://www.redhat.com/archives/freeipa-users/2015-December/msg00203.html please link the patch to https://fedorahosted.org/freeipa/ticket/5556 Updated patches attached. NACK, this is the correct procedure for the __getattr__(), sorry for confusing you earlier: def __getattr__(self, name): for owner_cls, knob_name in self.knobs(): if knob_name == name: break else: raise AttributeError(name) for component in self.__components: if isinstance(component, owner_cls): break else: raise AttributeError(name) return getattr(component, name) Honza Updated patches attached. Completing the update. -- David Kupka From f0c07859e5aaf5283418122ba0d492807d8ab92a Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 16 Dec 2015 12:43:13 + Subject: [PATCH] installer: Propagate option values from components instead of copying them. https://fedorahosted.org/freeipa/ticket/5556 --- ipapython/install/core.py | 21 ++--- ipaserver/install/server/common.py | 28 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/ipapython/install/core.py b/ipapython/install/core.py index 6b2da05d31d26bd3b7222e237c8646fa80ab5290..06acca6782f556b5880e7f6d023ab222f1025062 100644 --- a/ipapython/install/core.py +++ b/ipapython/install/core.py @@ -528,6 +528,21 @@ class Composite(Configurable): for order, owner_cls, name in result: yield owner_cls, name +def __getattr__(self, name): +for owner_cls, knob_name in self.knobs(): +if knob_name == name: +break +else: +raise AttributeError(name) + +for component in self.__components: +if isinstance(component, owner_cls): +break +else: +raise AttributeError(name) + +return getattr(component, name) + def _reset(self): self.__components = list(self._get_components()) @@ -545,8 +560,7 @@ class Composite(Configurable): try: validator.next() except StopIteration: -if child.done(): -self.__components.remove(child) +pass else: new_validate.append((child, validator)) if not new_validate: @@ -560,7 +574,8 @@ class Composite(Configurable): yield from_(super(Composite, self)._configure()) -execute = [(c, c._executor()) for c in self.__components] +execute = [(c, c._executor()) for c in self.__components +if not c.done()] while True: new_execute = [] for child, executor in execute: diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py index 3eb7279d200ffd6ab33d8d914c8d4f13e567a171..948aac842951dc3cef9dace87b28a92d3f98dea3 100644 --- a/ipaserver/install/server/common.py +++ b/ipaserver/install/server/common.py @@ -409,34 +409,6 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): # Automatically disable pkinit w/ dogtag until that is supported self.no_pkinit = True -self.external_ca = self.ca.external_ca -self.external_ca_type = self.ca.external_ca_type -self.external_cert_files = self.ca.external_cert_files -self.dirsrv_cert_files = self.ca.dirsrv_cert_files -self.http_cert_files = self.ca.http_cert_files -self.pkinit_cert_files = self.ca.pkinit_cert_files -self.dirsrv_pin = self.ca.dirsrv_pin -self.http_pin = self.ca.http_pin -self.pkinit_pin = self.ca.pkinit_pin -self.dirsrv_cert_name = self.ca.dirsrv_cert_name -self.http_cert_name = self.ca.http_cert_name -self.pkinit_cert_name = self.ca.pkinit_cert_name -self.ca_cert_files = self.ca.ca_cert_files -self.subject = self.ca.subject -self.ca_signing_algorithm = self.ca.ca_signing_algorithm -self.skip_schema_check = self.ca.skip_schema_check - -self.forwarders = self.dns.forwarders -self.no_forwarders = self.dns.no_forwarders -self.reverse_zones = self.dns.reverse_zones -self.no_reverse = self.dns.no_reverse -self.no_dnssec_validation = self.dns.no_dnssec_validation -self.dnssec_master = self.dns.dnssec_master -self.disable_dnssec_master = self.dns.disable_dnssec_master -self.kasp_db_file = self.dns.kasp_db_file -self.force = self.dns.force -self.zonemgr = self.dns.zonemgr - self.unattended = not self.interactive
[Freeipa-devel] [PATCH 0075-0076] Fix installer regression
https://www.redhat.com/archives/freeipa-users/2015-December/msg00203.html -- David Kupka From 114b4e2c1ffaa5c09dbfed54bb1f90cfa41f4678 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 16 Dec 2015 12:43:13 + Subject: [PATCH 1/2] installer: Propagate option values from components instead of copying them. --- ipapython/install/core.py | 7 +++ ipaserver/install/server/common.py | 28 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/ipapython/install/core.py b/ipapython/install/core.py index 6b2da05..62756da 100644 --- a/ipapython/install/core.py +++ b/ipapython/install/core.py @@ -528,6 +528,13 @@ class Composite(Configurable): for order, owner_cls, name in result: yield owner_cls, name +def __getattr__(self, name): +for owner_cls, knob_name in self.knobs(): +for component in self.__components: +if isinstance(component, owner_cls): +return getattr(component, knob_name) +raise AttributeError(name) + def _reset(self): self.__components = list(self._get_components()) diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py index 3eb7279..948aac8 100644 --- a/ipaserver/install/server/common.py +++ b/ipaserver/install/server/common.py @@ -409,34 +409,6 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): # Automatically disable pkinit w/ dogtag until that is supported self.no_pkinit = True -self.external_ca = self.ca.external_ca -self.external_ca_type = self.ca.external_ca_type -self.external_cert_files = self.ca.external_cert_files -self.dirsrv_cert_files = self.ca.dirsrv_cert_files -self.http_cert_files = self.ca.http_cert_files -self.pkinit_cert_files = self.ca.pkinit_cert_files -self.dirsrv_pin = self.ca.dirsrv_pin -self.http_pin = self.ca.http_pin -self.pkinit_pin = self.ca.pkinit_pin -self.dirsrv_cert_name = self.ca.dirsrv_cert_name -self.http_cert_name = self.ca.http_cert_name -self.pkinit_cert_name = self.ca.pkinit_cert_name -self.ca_cert_files = self.ca.ca_cert_files -self.subject = self.ca.subject -self.ca_signing_algorithm = self.ca.ca_signing_algorithm -self.skip_schema_check = self.ca.skip_schema_check - -self.forwarders = self.dns.forwarders -self.no_forwarders = self.dns.no_forwarders -self.reverse_zones = self.dns.reverse_zones -self.no_reverse = self.dns.no_reverse -self.no_dnssec_validation = self.dns.no_dnssec_validation -self.dnssec_master = self.dns.dnssec_master -self.disable_dnssec_master = self.dns.disable_dnssec_master -self.kasp_db_file = self.dns.kasp_db_file -self.force = self.dns.force -self.zonemgr = self.dns.zonemgr - self.unattended = not self.interactive ca = core.Component(BaseServerCA) -- 2.5.0 From b726b613d0f5afcbe7665368b9aaba336d6e2974 Mon Sep 17 00:00:00 2001 From: David Kupka <dku...@redhat.com> Date: Wed, 16 Dec 2015 12:43:13 + Subject: [PATCH 1/2] installer: Propagate option values from components instead of copying them. --- ipapython/install/core.py | 7 +++ ipaserver/install/server/common.py | 31 --- 2 files changed, 7 insertions(+), 31 deletions(-) diff --git a/ipapython/install/core.py b/ipapython/install/core.py index 2f62b85..3eb38d3 100644 --- a/ipapython/install/core.py +++ b/ipapython/install/core.py @@ -531,6 +531,13 @@ class Composite(Configurable): for order, owner_cls, name in result: yield owner_cls, name +def __getattr__(self, name): +for owner_cls, knob_name in self.knobs(): +for component in self.__components: +if isinstance(component, owner_cls): +return getattr(component, knob_name) +raise AttributeError(name) + def _reset(self): self.__components = list(self._get_components()) diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py index 19a1cc8..00b05c9 100644 --- a/ipaserver/install/server/common.py +++ b/ipaserver/install/server/common.py @@ -461,37 +461,6 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): # Automatically disable pkinit w/ dogtag until that is supported self.no_pkinit = True -self.external_ca = self.ca.external_ca -self.external_ca_type = self.ca.external_ca_type -self.external_cert_files = self.ca.external_cert_files -self.dirsrv_cert_files = self.ca.dirsrv_cert_files -self.http_cert_files = self.ca.http_cert_files -self.pkinit_cert_files = self.ca.pkinit_cert_files -self.dirsrv_pin = self.ca.dirsrv_pin -s
Re: [Freeipa-devel] [PATCH 0071] dns: Handle SERVFAIL in check if domain already exists
On 16/12/15 14:23, Petr Spacek wrote: Hello, dns: Handle SERVFAIL in check if domain already exists. In cases where domain is already delegated to IPA prior installation we might get timeout or SERVFAIL. The answer depends on the recursive server we are using for the check. Works for me, ACK. -- David Kupka -- 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] ca-less tests updated - POC
On 06/11/15 14:04, Oleg Fayans wrote:
Hi Jan,
On 11/06/2015 09:01 AM, Jan Cholasta wrote:
Actually it might be better to keep them, but fix them to expect
ipa-server-certinstall to success.
Done. Updated patch attached.
Also in the patch 0013 I removed a trailing whitespace which caused lint
to complain
Now with domain level 0 the test output looks like this:
[11:40:51]ofayans@vm-076:~]$ ipa-run-tests test_integration/test_caless.py
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..ss...xxssxx..ss...
= 76
passed, 6 skipped, 6 xfailed in 7871.10 seconds
=
On 6.11.2015 08:47, Jan Cholasta wrote:
Hi Oleg,
I think you can just remove
TestCertinstall.test_{http,ds}_intermediate_ca, the certificates are
imported correctly in this case and I didn't see anything break.
Honza
On 5.11.2015 20:20, Oleg Fayans wrote:
Patch 0014 updated and passes lint
On 11/05/2015 03:41 PM, Oleg Fayans wrote:
Wait a bit, the patch has problems with pylint: it does not build :)
The updated version (without the setupmaster nonsense) is being tested
now.
On 11/05/2015 08:45 AM, Oleg Fayans wrote:
Hi Jan,
Could you take a look at these, whenever you are free?
On 10/30/2015 02:57 PM, Oleg Fayans wrote:
Hi,
The following patches contain updates to ca-less integration tests.
It's still a proof of concept: 2 tests still fail seemingly due to
the
change in target system logic (marked as xfail with "ask jcholast
comment")
The test output looks like this:
$ ipa-run-tests test_integration/test_caless.py --pdb
test session starts
=
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.6.4
plugins: multihost, sourceorder
collected 88 items
test_integration/test_caless.py
..xx..sssss.ss.xx..ssxx.
53
passed, 29 skipped, 6 xfailed in 5620.17 seconds
=
Numerous skips correspond to the tests related to
ipa-replica-prepare
(unsupported under domain level 1)
This body part will be downloaded on demand.
Hello, thanks for updated patches. I'm really sorry it took so long
before I got to them.
There was change in ipapython.ipautil.run that happened after you sent
the patches. Feel free to squash attached patch that fixes it.
Unfortunately I see a lot of test failing with domain-level 0:
http://fpaste.org/301657/50275682/
domain-level 1 (domain-level 1: http://fpaste.org/301658/02757191/)
seems better. There are 2 failing test that you're probably mentioning
in commit message plus one that I think is bug in code rather than bug
in tests.
Do you have any proposal for fixing the two failing tests?
One nitpick: Please use mail for notes like "need further consulting
..." rather that commit message. When the patch gets accepted it will
still need modification before push just because inappropriate commit
message.
Thank you!
--
David Kupka
From 2a6e8f02ecd00da2b86d2f3f9847a86caa35e74d Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 16 Dec 2015 09:12:56 +0100
Subject: [PATCH] Addapt CA less test to new ipapython.ipautil.run
---
ipatests/test_integration/test_caless.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py
index 4b88ee9da1d5a476f13604f9a833e748a093..6cb55a708517062edb1bb950a72d6a66f717432e 100644
--- a/ipatests/test_integration/test_caless.py
+++ b/ipatests/test_integration/test_caless.py
@@ -300,10 +300,10 @@ class CALessBase(IntegrationTest):
@classmethod
def get_pem(cls, nickname):
-pem_cert, _stderr, _returncode = ipautil.run(
+result = ipautil.run(
['certutil', '-L', '-d', 'nssdb', '-n', nickname, '-a'],
-cwd=cls.cert_dir)
-return pem_cert
+cwd=cls.cert_dir, capture_output=True)
+return result.output
def verify_installation(self):
"""Verify CA cert PEM file and LDAP entry created by install
--
2.5.0
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribut
Re: [Freeipa-devel] [PATCH 0376] KRA: add RA cert during replica promotion
On 14/12/15 11:00, David Kupka wrote: On 10/12/15 19:40, Martin Basti wrote: https://fedorahosted.org/freeipa/ticket/5512 patch attached. Hi, thanks for the patch. It works but only when WAIT_AFTER_ARCHIVE is raised. Patch attached. IOW, your patch works for me, ACK. To let tests pass (and eventually fall on other issue when it appears) please include my patch. -- David Kupka -- 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 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG 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/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372, in __runner self._handle_exception(exc_info) File "/usr/lib/python2.7/site-packages/ipapython/install/core.py&q
