Re: [Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user

[Martin Babinsky]

On 08/19/2015 02:54 PM, Martin Babinsky wrote:

this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from
happening.



Actually, we (myself, mbasti, jcholast) found out that `user-del 
--preserve` could use some more usability improvements.


This quick patch should fix both 
https://fedorahosted.org/freeipa/ticket/5234 and 
https://fedorahosted.org/freeipa/ticket/5236 and make user preservation 
operate on multiple arguments in a same way as plain deletion.


--
Martin^3 Babinsky
From e39668c86015c14ed13325db23470f0c66d13392 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 19 Aug 2015 14:43:14 +0200
Subject: [PATCH] improve the usability of `ipa user-del --preserve` command

`ipa user-del` with `--preserve` option will now process multiple entries and
handle `--continue` option in a manner analogous to `ipa user-del` in normal
mode.

In addition, it is now no longer possible to permanently delete a user by
accidentally running `ipa user-del --preserve` twice.

https://fedorahosted.org/freeipa/ticket/5234
https://fedorahosted.org/freeipa/ticket/5236
---
 ipalib/plugins/user.py | 123 ++---
 1 file changed, 66 insertions(+), 57 deletions(-)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 1d6073b4240d963e2b047c20fe5b8be702ef3184..6acea611127ee7b5af1f66c99c10dfe7b93ad47c 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -593,6 +593,60 @@ class user_del(baseuser_del):
 ),
 )
 
+def preserve_user(self, pkey, delete_container, **options):
+assert isinstance(delete_container, DN)
+
+dn = self.obj.get_either_dn(pkey, **options)
+delete_dn = DN(dn[0], delete_container)
+ldap = self.obj.backend
+self.log.debug("preserve move %s -> %s" % (dn, delete_dn))
+
+if dn.endswith(delete_container):
+raise errors.ExecutionError(
+_('User "%s" is already preserved' % pkey)
+)
+# Check that this value is a Active user
+try:
+original_entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(dn, ['dn'])
+except errors.NotFound:
+self.obj.handle_not_found(pkey)
+
+# start to move the entry to Delete container
+self._exc_wrapper(pkey, options, ldap.move_entry)(dn, delete_dn,
+  del_old=True)
+
+# Then clear the credential attributes
+attrs_to_clear = ['krbPrincipalKey', 'krbLastPwdChange',
+  'krbPasswordExpiration', 'userPassword']
+
+entry_attrs = self._exc_wrapper(pkey, options, ldap.get_entry)(
+delete_dn, attrs_to_clear)
+
+clearedCredential = False
+for attr in attrs_to_clear:
+if attr.lower() in entry_attrs:
+del entry_attrs[attr]
+clearedCredential = True
+if clearedCredential:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
+# Then restore some original entry attributes
+attrs_to_restore = ['secretary', 'managedby', 'manager', 'ipauniqueid',
+'uidnumber', 'gidnumber', 'passwordHistory']
+
+entry_attrs = self._exc_wrapper(
+pkey, options, ldap.get_entry)(delete_dn, attrs_to_restore)
+
+restoreAttr = False
+for attr in attrs_to_restore:
+if ((attr.lower() in original_entry_attrs) and
+not (attr.lower() in entry_attrs)):
+restoreAttr = True
+entry_attrs[attr.lower()] = original_entry_attrs[attr.lower()]
+if restoreAttr:
+self._exc_wrapper(pkey, options, ldap.update_entry)(entry_attrs)
+
 def forward(self, *keys, **options):
 if self.api.env.context == 'cli':
 if options['no_preserve'] and options['preserve']:
@@ -633,68 +687,23 @@ class user_del(baseuser_del):
 
 def execute(self, *keys, **options):
 
-dn = self.obj.get_either_dn(*keys, **options)
-
 # We are going to permanent delete or the user is already in the delete container.
 delete_container = DN(self.obj.delete_container_dn, self.api.env.basedn)
-user_from_delete_container = dn.endswith(delete_container)
-
-if not options.get('preserve', True) or user_from_delete_container:
-# Remove any ID overrides tied with this user
-remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
-
-# Issue a true DEL on that entry
-return super(user_del, self).execute(*keys, **options)
 
 # The user to delete is active and there is no 'no_preserve' option
 if options.get('preserve', False):
-
-ldap = self.obj.backend
-
-# need to handle multiple keys (e.g. keys[-1]=(u'tb8', u'tb9')..
-active_dn = self.obj.get_either_dn(*keys, **options)
-s

Re: [Freeipa-devel] [PATCH] First part of integration tests for Topology Plugin

[Martin Basti]

I got this:

https://paste.fedoraproject.org/256746/43999380/

On 08/19/2015 09:00 AM, Oleg Fayans wrote:

Hi Martin,

As discussed, here is a new version with pep8-related fixes


On 08/14/2015 10:44 AM, Oleg Fayans wrote:

Hi Martin,

Already noticed that. Implemented the named groups as Tomas advised.
Added the third test for
http://www.freeipa.org/page/V4/Manage_replication_topology/Test_plan#Test_case:_Removal_of_a_topology_segment_is_allowed_only_if_there_is_at_least_one_more_segment_connecting_the_given_replica 






On 08/13/2015 05:06 PM, Martin Basti wrote:



On 08/11/2015 03:36 PM, Oleg Fayans wrote:

Hi Martin,

On 08/11/2015 02:02 PM, Martin Basti wrote:

NACK, comments inline.

On 11/08/15 13:25, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 08/10/2015 07:08 PM, Martin Basti wrote:

Thank you for patch, I have a few nitpicks:

1)
On 10/08/15 13:05, Oleg Fayans wrote:

+def create_segment(master, leftnode, rightnode):
+"""create_segment(master, leftnode, rightnode)

Why do you add the name of method in docstring?

My bad, fixed.


still there

+tokenize_topologies(command_output)
+takes an output of `ipa topologysegment-find` and returns an
array of


Fixed, sorry.





2)

+def create_segment(master, leftnode, rightnode):
+"""create_segment(master, leftnode, rightnode)
+creates a topology segment. The first argument is a node to
run the
command on
+The first 3 arguments should be objects of class Host
+Returns a hash object containing segment's name, leftnode,
rightnode information
+"""

I would prefer to add assert there instead of just document that a
Host
object is needed
assert(isinstance(master, Host))
...


Fixed. Created a decorator that checks the type of arguments


This does not scale well.
If we will want to add new argument that is not host object, you need
change it again.


Agreed. Modified the decorator so that you can specify a slice of
arguments to be checked and a class to compare to. This does scale :)


This might be used as function with specified variables that have 
to be

host objects




3)
+def destroy_segment(master, segment_name):
+"""
+destroy_segment(master, segment_name)
+Destroys topology segment. First argument should be object of
class
Host

Instead of description of params as first, second etc., you may use
following:

+def destroy_segment(master, segment_name):
+"""
+Destroys topology segment.
+:param master: reference to master object of class Host
+:param segment: name fo segment
and eventually this in other methods
+:returns: Lorem ipsum sit dolor mit amet
+:raises NotFound: if segment is not found


Fixed


4)

cls.replicas[:len(cls.replicas) - 1],

I suggest cls.replicas[:-1]

In [2]: a = [1, 2, 3, 4, 5]

In [3]: a[:-1]
Out[3]: [1, 2, 3, 4]


Fixed


5)
Why re.findall() and then you just use the first result?
'leftnode': self.leftnode_re.findall(i)[0]

Isn't just re.search() enough?
leftnode_re.search(value).group(1)


in fact
leftnode_re.findall(string)[0]
and
leftnode_re.search(string).group(1),
Are equally bad from the readability point of view. The first one is
even shorter a bit, so why change? :)


It depends on point of view,  because when I reviewed it yesterday my
brain raises exception that you are trying to add multiple values to
single value attribute, because you used findall, I expected that you
need multiple values, and then I saw that index [0] at the end, and I
was pretty confused what are you trying to achieve.

And because findall is not effective in case when you need to find 
just

one occurrence.


I got it. Fixed.









Python 3 nitpick:
I'm not sure if time when we should enforce python 2/3 compability
already comes, but just for record:
instead of open(file, 'r'), please use io.open(file, 'r') 
(import io

before)


Done.








1)

+#

empty comment here (several times)


Removed






NACK

you changed it wrong

group() returns everything, you need use group(1) to return content in
braces.










-- 
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] Added try/except for error handling ipautil

[Martin Basti]

On 08/19/2015 01:49 PM, Abhijeet Kasurde wrote:

Hi All,


Please find the latest patch with review comments included.

Thanks Martin for your help and review comments.

Thanks,
Abhijeet Kasurde

On 08/19/2015 05:08 PM, Martin Basti wrote:



On 08/17/2015 02:08 PM, Abhijeet Kasurde wrote:

Hi All,

I have included all review comments. Please let me know your views.

On 08/17/2015 04:09 PM, Martin Basti wrote:



On 08/17/2015 11:11 AM, Abhijeet Kasurde wrote:

Hi All,

Please find the update patch with review comments,


On 08/14/2015 05:19 PM, Martin Basti wrote:



On 08/14/2015 06:57 AM, Abhijeet Kasurde wrote:


On 08/13/2015 07:08 PM, Martin Basti wrote:



On 08/10/2015 01:47 PM, Abhijeet Kasurde wrote:

Hi All,

This patch fixes bug - 
https://fedorahosted.org/freeipa/ticket/3406


Thanks,
Abhijeet Kasurde




Hello,

thank you for the patch

1)
-except ValueError:
+except EOFError, ValueError:

Please use
except (EOFError, ValueError):
https://docs.python.org/2/tutorial/errors.html#handling-exceptions

OK, I will include this.

2)
I'm not sure if this code will work (I did not test it)

I expect when stdin is closed, this will result into infinite 
loop, because raw_input will always return EOFError.


while True:
try:
ret = raw_input("%s: " % prompt)
if allow_empty or ret.strip():
return ret
except EOFError:
pass

Could you please elaborate more on, so that I can include fix in 
this section of code?
If you receive EOF you cannot continue in while cycle because, it 
will return EOF every iteration forever.


If EOF is received the while cycle must end, and appropriate 
action must be take.
It depends on situation, if default value is present then default 
value should be used, or in case if empty value is allowed, empty 
string should be returned.


In case there is no default value and empty value is not allowed, 
then an exception should be raised.


Martin^2



NACK

There is still infinity loop.
1)
+except EOFError:
+if allow_empty:
+return ''

This will continue in while cycle if allow_empty=False, you need to raise 
exception there.

2)
Why so complicated? just return default looks enough to me.
+except EOFError:
+if choice.lower()[0] == "y":
+return True
+elif choice.lower()[0] == "n":
+return False
3)
Remove this change please
-
  def get_gsserror(e):






NACK

1)
Your commit message does not reflect the changes you made in code. 
The patch just solves the EOFError not Value error or "various errors"


2)
You removed if statement which should be there. I expected something 
like this in first case.

  while True:
-ret = raw_input("%s: " % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input("%s: " % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+   return ''
+raise RuntimeError("Failed to get user input")
Thanks
Martin




ACK

Pushed to master: 7c48621bb8e9efc47c68bb7b4af936da93325050
-- 
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 0003] TEST: Stageuser plugin

[Lenka Doudova]

Hi,

I'm sending additional patch for stageuser plugin tests. It applies on 
top of 0002.3 patch sent earlier and reflects the change of 
'stageuser-add --from-delete' command to 'user-stage'.


Lenka
From 03521e0683eb8a9a81d3acd97b08e2402affd270 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 19 Aug 2015 15:24:23 +0200
Subject: [PATCH] Updated automated test for stageuser plugin

Updated test - this version reflects the change of command 'stageuser-add --from-delete' to 'user-stage'.
Ticket: https://fedorahosted.org/freeipa/ticket/3813
Test plan: http://www.freeipa.org/page/V4/User_Life-Cycle_Management/Test_Plan
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py | 42 ---
 ipatests/test_xmlrpc/test_user_plugin.py  | 32 +++-
 2 files changed, 62 insertions(+), 12 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index f3024a8f98a7fb4cf40e63b112a0f3946715633f..f0ca5669043daffe3392e3cfc035eadc069596ef 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -119,14 +119,13 @@ class StageUserTracker(Tracker):
 
 self.kwargs = kwargs
 
-def make_create_command(self, options=None, force=None, from_delete=False):
+def make_create_command(self, options=None, force=None):
 """ Make function that creates a staged user using stageuser-add """
 if options is not None:
 self.kwargs = options
 return self.make_command('stageuser_add', self.uid,
  givenname=self.givenname,
- sn=self.sn, from_delete=from_delete,
- **self.kwargs)
+ sn=self.sn, **self.kwargs)
 
 def make_delete_command(self):
 """ Make function that deletes a staged user using stageuser-del """
@@ -276,6 +275,13 @@ class StageUserTracker(Tracker):
 result=self.filter_attrs(self.update_keys | set(extra_keys))
 ), result)
 
+def check_restore_preserved(self, result):
+assert_deepequal(dict(
+value=self.uid,
+summary=u'Staged user account "%s"' % self.uid,
+result=True,
+), result)
+
 def make_fixture_activate(self, request):
 """Make a pytest fixture for a staged user that is to be activated
 
@@ -307,6 +313,9 @@ class StageUserTracker(Tracker):
 self.uid = user.uid
 self.givenname = user.givenname
 self.sn = user.sn
+self.dn = DN(
+('uid', self.uid), api.env.container_stageuser, api.env.basedn)
+self.attrs[u'dn'] = self.dn
 
 
 @pytest.fixture(scope='class')
@@ -369,6 +378,12 @@ def user6(request):
 return tracker.make_fixture(request)
 
 
+@pytest.fixture(scope='class')
+def user7(request):
+tracker = UserTracker(u'puser1', u'preserved', u'user')
+return tracker.make_fixture_restore(request)
+
+
 class TestNonexistentStagedUser(XMLRPC_test):
 def test_retrieve_nonexistent(self, stageduser):
 stageduser.ensure_missing()
@@ -709,22 +724,27 @@ class TestPreserved(XMLRPC_test):
 
 user.delete()
 
-def test_staged_from_preserved(self, user, stageduser):
-user.make_preserved_user()
+def test_staged_from_preserved(self, user7, stageduser):
+user7.make_preserved_user()
 
 stageduser.ensure_missing()
-stageduser = StageUserTracker(user.uid, user.givenname, user.sn)
-stageduser.create_from_preserved(user)
-command = user.make_create_command(from_delete=True)
+stageduser = StageUserTracker(user7.uid, user7.givenname, user7.sn)
+stageduser.create_from_preserved(user7)
+command = user7.make_stage_command()
 result = command()
-stageduser.check_create(result)
+stageduser.check_restore_preserved(result)
+stageduser.exists = True
 
-command = user.make_retrieve_command()
+command = user7.make_retrieve_command()
 with raises_exact(errors.NotFound(
 reason=u'%s: user not found' % stageduser.uid)):
 command()
+
+command = stageduser.make_retrieve_command()
+result = command()
+stageduser.check_retrieve(result)
+
 stageduser.delete()
-user.delete()
 
 
 class TestManagers(XMLRPC_test):
diff --git a/ipatests/test_xmlrpc/test_user_plugin.py b/ipatests/test_xmlrpc/test_user_plugin.py
index ec72c3241ea9c075e32d36132d9869d23fd57f22..08f9d1667185d5477cbf49f293196714fd3a0864 100644
--- a/ipatests/test_xmlrpc/test_user_plugin.py
+++ b/ipatests/test_xmlrpc/test_user_plugin.py
@@ -34,7 +34,7 @@ from ipatests.util import (
 assert_equal, assert_not_equal, raises, assert_deepequal)
 from xmlrpc_test import (
 XMLRPC_test, Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_password,
-fuzzy_string, fuzzy_dergeneralizedtime, add_sid, add_oc)
+fu

[Freeipa-devel] [PATCH 0060] raise an error when trying to preserve an already preserved user

[Martin Babinsky]

this patch prevents https://fedorahosted.org/freeipa/ticket/5234 from 
happening.


--
Martin^3 Babinsky
From a01ceb4906cb35141c664ba94c088f4e29209b68 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 19 Aug 2015 14:43:14 +0200
Subject: [PATCH] raise an error when trying to preserve an already preserved
 user

this also fixes a case when a user is permanently deleted when `ipa user-del
--preserve` is accidentally called multiple times on the same uid.

https://fedorahosted.org/freeipa/ticket/5234
---
 ipalib/plugins/user.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index 1d6073b4240d963e2b047c20fe5b8be702ef3184..1659830d77c822887ea7e6d0f293db02bffb3250 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -639,12 +639,16 @@ class user_del(baseuser_del):
 delete_container = DN(self.obj.delete_container_dn, self.api.env.basedn)
 user_from_delete_container = dn.endswith(delete_container)
 
-if not options.get('preserve', True) or user_from_delete_container:
+if not options.get('preserve', False):
 # Remove any ID overrides tied with this user
 remove_ipaobject_overrides(self.obj.backend, self.obj.api, dn)
 
 # Issue a true DEL on that entry
 return super(user_del, self).execute(*keys, **options)
+elif user_from_delete_container:
+raise errors.ExecutionError(
+_('One or more users are already preserved')
+)
 
 # The user to delete is active and there is no 'no_preserve' option
 if options.get('preserve', False):
-- 
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 023] Add flag to list all service and user vaults

[Jan Cholasta]

On 19.8.2015 14:39, Christian Heimes wrote:

On 2015-08-19 14:12, Jan Cholasta wrote:

The new flags should be handled in vault_find's pre_callback instead of
vault's get_dn, as they are exclusive to vault_find and worse yet,
conflict with vault_{add,remove}_{owner,member}'s flags, leading to
unwanted behavior:

$ ipa vault-add-member --service testsvc/example.com testvault
--services testsvc/example.com
ipa: ERROR: Service(s), shared, and user(s) options cannot be specified
simultaneously


Here is an updated patch. The new flags are now handled by the
pre_callback method. I have regenerated API.txt, too.

Christian



Thanks, ACK.

Bumped VERSION and pushed to:
master: 0abaf195dc3b0920d2439dd4ec6df61e0aadc4f9
ipa-4-2: 89c9feaf93299c96bb227b3705246193a1de1d82

Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 023] Add flag to list all service and user vaults

[Christian Heimes]

On 2015-08-19 14:12, Jan Cholasta wrote:
> The new flags should be handled in vault_find's pre_callback instead of
> vault's get_dn, as they are exclusive to vault_find and worse yet,
> conflict with vault_{add,remove}_{owner,member}'s flags, leading to
> unwanted behavior:
> 
> $ ipa vault-add-member --service testsvc/example.com testvault
> --services testsvc/example.com
> ipa: ERROR: Service(s), shared, and user(s) options cannot be specified
> simultaneously

Here is an updated patch. The new flags are now handled by the
pre_callback method. I have regenerated API.txt, too.

Christian

From a6eb87a73c1462a4de516f19b219b51e415852e5 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Wed, 19 Aug 2015 13:32:01 +0200
Subject: [PATCH] Add flag to list all service and user vaults

The vault-find plugin has two additional arguments to list all
service vaults or user vaults. Since the name of a vault is only unique
for a particular user or service, the commands also print the vault user
or vault service. The virtual attributes were added in rev
01dd951ddc0181b559eb3dd5ff0336c81e245628.

Example:

$ ipa vault-find --users

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault user: admin

  Vault name: UserVault
  Type: standard
  Vault user: admin

Number of entries returned 2


$ ipa vault-find --services

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault service: HTTP/ipatest.freeipa.local@FREEIPA.LOCAL

  Vault name: myvault
  Type: standard
  Vault service: ldap/ipatest.freeipa.local@FREEIPA.LOCAL

Number of entries returned 2


https://fedorahosted.org/freeipa/ticket/5150
---
 API.txt |  4 +++-
 ipalib/plugins/vault.py | 48 +---
 2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/API.txt b/API.txt
index 4d8d9dc3d3c38d4740bda3574396ecd85877b805..dd6bcc3c39895e6af213fcece85505fa0bd6d2f2 100644
--- a/API.txt
+++ b/API.txt
@@ -5508,7 +5508,7 @@ output: Output('result', , None)
 output: Output('summary', (, ), None)
 output: ListOfPrimaryKeys('value', None, None)
 command: vault_find
-args: 1,13,4
+args: 1,15,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('cn', attribute=True, autofill=False, cli_name='name', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.-]+$', primary_key=True, query=True, required=False)
@@ -5518,10 +5518,12 @@ option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('service?')
+option: Flag('services?', autofill=True, default=False)
 option: Flag('shared?', autofill=True, default=False)
 option: Int('sizelimit?', autofill=False, minvalue=0)
 option: Int('timelimit?', autofill=False, minvalue=0)
 option: Str('username?', cli_name='user')
+option: Flag('users?', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Output('count', , None)
 output: ListOfEntries('result', (, ), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 712e2d5ddfa723eb84b80a261289a7cf1c75674f..83dc085b5aadb4e2878e29d17449f0808cc7a9c2 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -343,21 +343,11 @@ class vault(LDAPObject):
 """
 Generates vault DN from parameters.
 """
-
 service = options.get('service')
 shared = options.get('shared')
 user = options.get('username')
 
-count = 0
-if service:
-count += 1
-
-if shared:
-count += 1
-
-if user:
-count += 1
-
+count = (bool(service) + bool(shared) + bool(user))
 if count > 1:
 raise errors.MutuallyExclusiveError(
 reason=_('Service, shared, and user options ' +
@@ -387,8 +377,10 @@ class vault(LDAPObject):
 parent_dn = DN(('cn', service), ('cn', 'services'), container_dn)
 elif shared:
 parent_dn = DN(('cn', 'shared'), container_dn)
-else:
+elif user:
 parent_dn = DN(('cn', user), ('cn', 'users'), container_dn)
+else:
+raise RuntimeError
 
 return DN(rdns, parent_dn)
 
@@ -814,7 +806,16 @@ class vault_del(LDAPDelete):
 class vault_find(LDAPSearch):
 __doc__ = _('Search for vaults.')
 
-takes_options = LDAPSearch.takes_options + vault_options
+takes_options = LDAPSearch.takes_options + vault_options + (
+Flag(
+'services?',
+doc=_('List all service vaults'),
+),
+Flag(
+'users?'

Re: [Freeipa-devel] [PATCH 023] Add flag to list all service and user vaults

[Petr Vobornik]

On 08/19/2015 02:12 PM, Jan Cholasta wrote:

Hi,

On 19.8.2015 13:39, Christian Heimes wrote:

The vault-find plugin has two additional arguments to list all
service vaults or user vaults. Since the name of a vault is only unique
for a particular user or service, the commands also print the vault user
or vault service. The virtual attributes were added in rev
01dd951ddc0181b559eb3dd5ff0336c81e245628.

Example:

$ ipa vault-find --users

2 vaults matched

   Vault name: myvault
   Type: standard
   Vault user: admin

   Vault name: UserVault
   Type: standard
   Vault user: admin

Number of entries returned 2


$ ipa vault-find --services

2 vaults matched

   Vault name: myvault
   Type: standard
   Vault service: HTTP/ipatest.freeipa.local@FREEIPA.LOCAL

   Vault name: myvault
   Type: standard
   Vault service: ldap/ipatest.freeipa.local@FREEIPA.LOCAL

Number of entries returned 2


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


The new flags should be handled in vault_find's pre_callback instead of
vault's get_dn, as they are exclusive to vault_find and worse yet,
conflict with vault_{add,remove}_{owner,member}'s flags, leading to
unwanted behavior:

$ ipa vault-add-member --service testsvc/example.com testvault
--services testsvc/example.com
ipa: ERROR: Service(s), shared, and user(s) options cannot be specified
simultaneously

Honza



Also you forgot to update API.txt and VERSION
 ./makeapi

--
Petr Vobornik

--
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] Subject: [PATCH 0061-2] Fix backup/restore (#5071)

[Jan Cholasta]

On 19.8.2015 13:52, Martin Babinsky wrote:

On 08/19/2015 12:00 PM, David Kupka wrote:

On 19/08/15 10:44, David Kupka wrote:

On 19/08/15 09:21, David Kupka wrote:

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


Updated patches attached.


Removed copy-pasted returns. Updated patch attached.




ACK



Pushed to:
master: db88985c0d4920191b840b5d04d133015293dbe0
ipa-4-2: 4fe994b11f7e5978c969626dedc593b7357b7fd2

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 023] Add flag to list all service and user vaults

[Jan Cholasta]

Hi,

On 19.8.2015 13:39, Christian Heimes wrote:

The vault-find plugin has two additional arguments to list all
service vaults or user vaults. Since the name of a vault is only unique
for a particular user or service, the commands also print the vault user
or vault service. The virtual attributes were added in rev
01dd951ddc0181b559eb3dd5ff0336c81e245628.

Example:

$ ipa vault-find --users

2 vaults matched

   Vault name: myvault
   Type: standard
   Vault user: admin

   Vault name: UserVault
   Type: standard
   Vault user: admin

Number of entries returned 2


$ ipa vault-find --services

2 vaults matched

   Vault name: myvault
   Type: standard
   Vault service: HTTP/ipatest.freeipa.local@FREEIPA.LOCAL

   Vault name: myvault
   Type: standard
   Vault service: ldap/ipatest.freeipa.local@FREEIPA.LOCAL

Number of entries returned 2


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


The new flags should be handled in vault_find's pre_callback instead of 
vault's get_dn, as they are exclusive to vault_find and worse yet, 
conflict with vault_{add,remove}_{owner,member}'s flags, leading to 
unwanted behavior:


$ ipa vault-add-member --service testsvc/example.com testvault 
--services testsvc/example.com
ipa: ERROR: Service(s), shared, and user(s) options cannot be specified 
simultaneously


Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)

[Martin Babinsky]

On 08/19/2015 12:00 PM, David Kupka wrote:

On 19/08/15 10:44, David Kupka wrote:

On 19/08/15 09:21, David Kupka wrote:

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


Updated patches attached.


Removed copy-pasted returns. Updated patch attached.




ACK

--
Martin^3 Babinsky

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

Re: [Freeipa-devel] [PATCH] Added try/except for error handling ipautil

[Abhijeet Kasurde]

Hi All,


Please find the latest patch with review comments included.

Thanks Martin for your help and review comments.

Thanks,
Abhijeet Kasurde

On 08/19/2015 05:08 PM, Martin Basti wrote:



On 08/17/2015 02:08 PM, Abhijeet Kasurde wrote:

Hi All,

I have included all review comments. Please let me know your views.

On 08/17/2015 04:09 PM, Martin Basti wrote:



On 08/17/2015 11:11 AM, Abhijeet Kasurde wrote:

Hi All,

Please find the update patch with review comments,


On 08/14/2015 05:19 PM, Martin Basti wrote:



On 08/14/2015 06:57 AM, Abhijeet Kasurde wrote:


On 08/13/2015 07:08 PM, Martin Basti wrote:



On 08/10/2015 01:47 PM, Abhijeet Kasurde wrote:

Hi All,

This patch fixes bug - 
https://fedorahosted.org/freeipa/ticket/3406


Thanks,
Abhijeet Kasurde




Hello,

thank you for the patch

1)
-except ValueError:
+except EOFError, ValueError:

Please use
except (EOFError, ValueError):
https://docs.python.org/2/tutorial/errors.html#handling-exceptions

OK, I will include this.

2)
I'm not sure if this code will work (I did not test it)

I expect when stdin is closed, this will result into infinite 
loop, because raw_input will always return EOFError.


while True:
try:
ret = raw_input("%s: " % prompt)
if allow_empty or ret.strip():
return ret
except EOFError:
pass

Could you please elaborate more on, so that I can include fix in 
this section of code?
If you receive EOF you cannot continue in while cycle because, it 
will return EOF every iteration forever.


If EOF is received the while cycle must end, and appropriate 
action must be take.
It depends on situation, if default value is present then default 
value should be used, or in case if empty value is allowed, empty 
string should be returned.


In case there is no default value and empty value is not allowed, 
then an exception should be raised.


Martin^2



NACK

There is still infinity loop.
1)
+except EOFError:
+if allow_empty:
+return ''

This will continue in while cycle if allow_empty=False, you need to raise 
exception there.

2)
Why so complicated? just return default looks enough to me.
+except EOFError:
+if choice.lower()[0] == "y":
+return True
+elif choice.lower()[0] == "n":
+return False
3)
Remove this change please
-
  def get_gsserror(e):






NACK

1)
Your commit message does not reflect the changes you made in code. The 
patch just solves the EOFError not Value error or "various errors"


2)
You removed if statement which should be there. I expected something 
like this in first case.

  while True:
-ret = raw_input("%s: " % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input("%s: " % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+   return ''
+raise RuntimeError("Failed to get user input")
Thanks
Martin


From 1fd37e950afaa05b7e582e82bcd7dbe992f5f19d Mon Sep 17 00:00:00 2001
From: Abhijeet Kasurde 
Date: Wed, 19 Aug 2015 17:13:43 +0530
Subject: [PATCH] Added try/except block for user_input in ipautil

Added error handling for function user_input in order to
handle EOFError in ipautil.py

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

Signed-off-by: Abhijeet Kasurde 
---
 ipapython/ipautil.py | 46 +-
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 88e89706b8e2aa6dea80809510d88bceaa836e85..b05a1159ab2a89fda917dd6b48afba706204579f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -746,30 +746,40 @@ def ipa_generate_password(characters=None,pwd_len=None):
 def user_input(prompt, default = None, allow_empty = True):
 if default == None:
 while True:
-ret = raw_input("%s: " % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input("%s: " % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+return ''
+raise RuntimeError("Failed to get user input")
 
 if isinstance(default, basestring):
 while True:
-ret = raw_input("%s [%s]: " % (prompt, default))
-if not ret and (allow_empty or default):
+try:
+ret = raw_input("%s [%s]: " % (prompt, default))
+if not ret and (allow_empty or default):
+return default
+elif ret.strip():
+return ret
+except EOFErr

Re: [Freeipa-devel] [PATCH] Added try/except for error handling ipautil

[Martin Basti]

On 08/17/2015 02:08 PM, Abhijeet Kasurde wrote:

Hi All,

I have included all review comments. Please let me know your views.

On 08/17/2015 04:09 PM, Martin Basti wrote:



On 08/17/2015 11:11 AM, Abhijeet Kasurde wrote:

Hi All,

Please find the update patch with review comments,


On 08/14/2015 05:19 PM, Martin Basti wrote:



On 08/14/2015 06:57 AM, Abhijeet Kasurde wrote:


On 08/13/2015 07:08 PM, Martin Basti wrote:



On 08/10/2015 01:47 PM, Abhijeet Kasurde wrote:

Hi All,

This patch fixes bug - https://fedorahosted.org/freeipa/ticket/3406

Thanks,
Abhijeet Kasurde




Hello,

thank you for the patch

1)
-except ValueError:
+except EOFError, ValueError:

Please use
except (EOFError, ValueError):
https://docs.python.org/2/tutorial/errors.html#handling-exceptions

OK, I will include this.

2)
I'm not sure if this code will work (I did not test it)

I expect when stdin is closed, this will result into infinite 
loop, because raw_input will always return EOFError.


while True:
try:
ret = raw_input("%s: " % prompt)
if allow_empty or ret.strip():
return ret
except EOFError:
pass

Could you please elaborate more on, so that I can include fix in 
this section of code?
If you receive EOF you cannot continue in while cycle because, it 
will return EOF every iteration forever.


If EOF is received the while cycle must end, and appropriate action 
must be take.
It depends on situation, if default value is present then default 
value should be used, or in case if empty value is allowed, empty 
string should be returned.


In case there is no default value and empty value is not allowed, 
then an exception should be raised.


Martin^2



NACK

There is still infinity loop.
1)
+except EOFError:
+if allow_empty:
+return ''

This will continue in while cycle if allow_empty=False, you need to raise 
exception there.

2)
Why so complicated? just return default looks enough to me.
+except EOFError:
+if choice.lower()[0] == "y":
+return True
+elif choice.lower()[0] == "n":
+return False
3)
Remove this change please
-
  def get_gsserror(e):






NACK

1)
Your commit message does not reflect the changes you made in code. The 
patch just solves the EOFError not Value error or "various errors"


2)
You removed if statement which should be there. I expected something 
like this in first case.


 while True:
-ret = raw_input("%s: " % prompt)
-if allow_empty or ret.strip():
-return ret
+try:
+ret = raw_input("%s: " % prompt)
+if allow_empty or ret.strip():
+return ret
+except EOFError:
+if allow_empty:
+   return ''
+raise RuntimeError("Failed to get user input")

Thanks
Martin
-- 
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 023] Add flag to list all service and user vaults

[Christian Heimes]

The vault-find plugin has two additional arguments to list all
service vaults or user vaults. Since the name of a vault is only unique
for a particular user or service, the commands also print the vault user
or vault service. The virtual attributes were added in rev
01dd951ddc0181b559eb3dd5ff0336c81e245628.

Example:

$ ipa vault-find --users

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault user: admin

  Vault name: UserVault
  Type: standard
  Vault user: admin

Number of entries returned 2


$ ipa vault-find --services

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault service: HTTP/ipatest.freeipa.local@FREEIPA.LOCAL

  Vault name: myvault
  Type: standard
  Vault service: ldap/ipatest.freeipa.local@FREEIPA.LOCAL

Number of entries returned 2


https://fedorahosted.org/freeipa/ticket/5150
From 513e4ab2e02e3b5f72b5a83a176b74ee0acba631 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Wed, 19 Aug 2015 13:32:01 +0200
Subject: [PATCH] Add flag to list all service and user vaults

The vault-find plugin has two additional arguments to list all
service vaults or user vaults. Since the name of a vault is only unique
for a particular user or service, the commands also print the vault user
or vault service. The virtual attributes were added in rev
01dd951ddc0181b559eb3dd5ff0336c81e245628.

Example:

$ ipa vault-find --users

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault user: admin

  Vault name: UserVault
  Type: standard
  Vault user: admin

Number of entries returned 2


$ ipa vault-find --services

2 vaults matched

  Vault name: myvault
  Type: standard
  Vault service: HTTP/ipatest.freeipa.local@FREEIPA.LOCAL

  Vault name: myvault
  Type: standard
  Vault service: ldap/ipatest.freeipa.local@FREEIPA.LOCAL

Number of entries returned 2


https://fedorahosted.org/freeipa/ticket/5150
---
 ipalib/plugins/vault.py | 39 +--
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/ipalib/plugins/vault.py b/ipalib/plugins/vault.py
index 712e2d5ddfa723eb84b80a261289a7cf1c75674f..0b22a5375b71dbdcb374d6b284c12a5f49a7638e 100644
--- a/ipalib/plugins/vault.py
+++ b/ipalib/plugins/vault.py
@@ -343,24 +343,17 @@ class vault(LDAPObject):
 """
 Generates vault DN from parameters.
 """
-
 service = options.get('service')
+services = options.get('services')
 shared = options.get('shared')
 user = options.get('username')
+users = options.get('users')
 
-count = 0
-if service:
-count += 1
-
-if shared:
-count += 1
-
-if user:
-count += 1
-
+count = (bool(service) + bool(services) + bool(shared)
+ + bool(user) + bool(users))
 if count > 1:
 raise errors.MutuallyExclusiveError(
-reason=_('Service, shared, and user options ' +
+reason=_('Service(s), shared, and user(s) options ' +
  'cannot be specified simultaneously'))
 
 # TODO: create container_dn after object initialization then reuse it
@@ -385,10 +378,16 @@ class vault(LDAPObject):
 
 if service:
 parent_dn = DN(('cn', service), ('cn', 'services'), container_dn)
+elif services:
+parent_dn = DN(('cn', 'services'), container_dn)
 elif shared:
 parent_dn = DN(('cn', 'shared'), container_dn)
-else:
+elif user:
 parent_dn = DN(('cn', user), ('cn', 'users'), container_dn)
+elif users:
+parent_dn = DN(('cn', 'users'), container_dn)
+else:
+raise RuntimeError
 
 return DN(rdns, parent_dn)
 
@@ -814,7 +813,16 @@ class vault_del(LDAPDelete):
 class vault_find(LDAPSearch):
 __doc__ = _('Search for vaults.')
 
-takes_options = LDAPSearch.takes_options + vault_options
+takes_options = LDAPSearch.takes_options + vault_options + (
+Flag(
+'services?',
+doc=_('List all service vaults'),
+),
+Flag(
+'users?',
+doc=_('List all user vaults'),
+),
+)
 
 has_output_params = LDAPSearch.has_output_params
 
@@ -832,6 +840,9 @@ class vault_find(LDAPSearch):
 raise errors.InvocationError(
 format=_('KRA service is not enabled'))
 
+if options.get('users') or options.get('services'):
+scope = ldap.SCOPE_SUBTREE
+
 base_dn = self.obj.get_dn(None, **options)
 
 return (filter, base_dn, scope)
-- 
2.4.3



signature.asc
Descripti

Re: [Freeipa-devel] [PATCH 0063] client: Update DNS with all available local IP addresses.

[Martin Basti]

On 08/19/2015 12:46 PM, David Kupka wrote:

On 19/08/15 11:06, Jan Cholasta wrote:

On 19.8.2015 10:36, Martin Basti wrote:



On 08/18/2015 10:53 PM, Martin Basti wrote:



On 08/18/2015 08:02 PM, David Kupka wrote:

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

Re: [Freeipa-devel] [PATCH 0063] client: Update DNS with all available local IP addresses.

[David Kupka]

On 19/08/15 11:06, Jan Cholasta wrote:

On 19.8.2015 10:36, Martin Basti wrote:



On 08/18/2015 10:53 PM, Martin Basti wrote:



On 08/18/2015 08:02 PM, David Kupka wrote:

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 choo

Re: [Freeipa-devel] [PATCH] 0038 cert-request: remove allowed extensions check

[Jan Cholasta]

On 13.8.2015 07:54, Fraser Tweedale wrote:

The attached patch fixes
https://fedorahosted.org/freeipa/ticket/5205

Thanks,
Fraser


ACK.

Pushed to:
master: 02969d09d868907c6d2d5b2aee3089f2f5540dda
ipa-4-2: 7723b3a677b7198bb59957c749d20053611bf32c

Honza

--
Jan Cholasta

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

Re: [Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)

[David Kupka]

On 19/08/15 10:44, David Kupka wrote:

On 19/08/15 09:21, David Kupka wrote:

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


Updated patches attached.


Removed copy-pasted returns. Updated patch attached.

--
David Kupka
From 04644ea40b5bcf6385f5e68a8407bc6b8858b93c Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Wed, 19 Aug 2015 08:10:03 +0200
Subject: [PATCH] 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  |  8 
 ipaserver/install/ipa_backup.py  |  4 
 ipaserver/install/ipa_restore.py |  4 
 5 files changed, 37 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 b26604aa736eb472c88bc0dcbc3a4b515712ce9d..1af99d318c6745b1e5285c7829c2b292f86c8390 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -161,6 +161,14 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 auth_config.add_option("nostart")
 auth_config.execute()
 
+def backup_auth_configuration(self, path):
+auth_config = RedHatAuthConfig()
+auth_config.backup(path)
+
+def restore_auth_configuration(self, path):
+auth_config = RedHatAuthConfig()
+auth_config.restore(path)
+
 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 b655be87dfb90ca3cd0df8bce5e4693ab4f136fd..6ce467f1e819a2ea8693e5ae9d1ee631ecc31bd2 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:
@@ -302,6 +303,9 @@ class Backup(admintool.AdminTool):
 self.db2ldif(instance, 'userRoot', online=options.online)
 self.db2bak(instance, online=options.online)
 if not options.data_only:
+# create backup of auth configuration
+auth_backup_path = os.path.join(paths.VAR_LIB_IPA, 'auth_backup')
+tasks.backup_auth_configuration(auth_backup_path)
 self.file_backup(options)
 self.finalize_backup(options.data_only, options.gpg, options.gpg_keyring)
 
diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index cb2aa781a1331171aa3a31f889bff9736f22ef80..ea9f8228f3cff233abb160a28c9af8b2435b65ec 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -386,6 +386,10 @@ class Restore(admintool.AdminTool):
 self.log.info('Starting Directory Server')
 dirsrv.start(capture_output=False)
 else:
+# restore access controll configuration
+auth_backup_path = os.path.join(paths.VAR_LIB_IPA, 'auth_backup')
+if os.path.exists(auth_backup_path):
+tasks.restore_auth_configuration(auth_backup_path)
 # explicitly enable then disable the pki tomcatd service to
 # re-register its instance. FIXME, this is really wierd.
 services.knownservices.pki_tomcatd.enable()
-- 
2.4.3

-- 

Re: [Freeipa-devel] [PATCH] 371 Added support for changing vault encryption.

[Martin Basti]

On 08/13/2015 07:11 PM, Endi Sukma Dewata wrote:

On 8/13/2015 8:06 AM, Martin Basti wrote:

The vault-mod command has been modified to support changing vault
encryption attributes (i.e. type, password, public/private keys)
in addition to normal attributes (i.e. description). Changing the
encryption requires retrieving the stored secret with the old
attributes and rearchieving it with the new attributes.

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


Hello, does this patch require any additional patches?

I have current master branch and I cannot apply it.

git am
freeipa-edewata-0371-Added-support-for-changing-vault-encryption.patch -3 


Applying: Added support for changing vault encryption.
error: invalid object 100644 3b62822366a62c90f843a6293589c28383e782ef
for 'ipalib/plugins/vault.py'
fatal: git-write-tree: error building trees
Repository lacks necessary blobs to fall back on 3-way merge.


Martin^2


New patch attached. It requires patch #0369-3.


I cannot apply patch (and 369-3 was pushed)

git am 
freeipa-edewata-0371-1-Added-support-for-changing-vault-encryption.patch -3

Applying: Added support for changing vault encryption.
error: invalid object 100644 5d367b376ef41427ed983f3eafe120ed477018d2 
for 'ipalib/plugins/vault.py'

fatal: git-write-tree: error building trees
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

--
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 0298] Server Upgrade: start DS before CA is started

[Jan Cholasta]

On 19.8.2015 11:04, Jan Cholasta wrote:

On 18.8.2015 19:22, Martin Basti wrote:



On 08/18/2015 07:14 PM, Martin Basti wrote:



On 08/18/2015 07:05 PM, Martin Basti wrote:

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

Patch attached.



Self-NACK, I sent wrong patch




The correct patch attached.


Thanks, ACK.

Pushed to:
master: 9fe67dcf2b6c10ca4eebab1c573d101316f481cd
ipa-4-2: 7924007a83a82674a495afe0e63a4bc85ab2a5ab


Sorry, wrong information.

Pushed to:
master: 556e97bf23657cb11d93c7d8a37b5ed4840fdb7a
ipa-4-2: 9cb6018367d958cdef03bef9780349b9651744a9

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0002] Port from python-krbV to python-gssapi

[Alexander Bokovoy]

On Tue, 18 Aug 2015, Michael Šimáček wrote:

On 2015-08-17 21:10, Robbie Harwood wrote:

Michael Šimáček  writes:


Attaching new revision of the patch. Changes from the previous:
- ldap2's connect now chooses the bind type same way as in ipaldap
- get_default_realm usages replaced by api.env.realm
- fixed missing third kinit attempt in trust-fetch-domains
- removed rewrapping gssapi errors to ccache errors in krb_utils
- updated some parts of exception handling


This patch doesn't seem to apply to master.  Can you update it or
indicate what you're patching against?  Thanks!



Attaching patch rebased on top of current master.

My refactoring of com.redhat.idm.trust-fetch-domains went in yesterday
so this patch will not apply anymore. Could you please rebase it?


--
/ Alexander Bokovoy

--
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] 375 Added mechanism to copy vault secrets.

[Martin Basti]

On 08/16/2015 05:29 PM, Endi Sukma Dewata wrote:

The vault-add and vault-archive commands have been modified to
optionally retrieve a secret from a source vault, then re-archive
the secret into the new/existing target vault.

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




I cannot apply this patch.
-- 
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] 0299 client: Update DNS with all available local IP addresses.

[Jan Cholasta]

On 19.8.2015 10:36, Martin Basti wrote:



On 08/18/2015 10:53 PM, Martin Basti wrote:



On 08/18/2015 08:02 PM, David Kupka wrote:

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

Re: [Freeipa-devel] [PATCH 0298] Server Upgrade: start DS before CA is started

[Jan Cholasta]

On 18.8.2015 19:22, Martin Basti wrote:



On 08/18/2015 07:14 PM, Martin Basti wrote:



On 08/18/2015 07:05 PM, Martin Basti wrote:

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

Patch attached.



Self-NACK, I sent wrong patch




The correct patch attached.


Thanks, ACK.

Pushed to:
master: 9fe67dcf2b6c10ca4eebab1c573d101316f481cd
ipa-4-2: 7924007a83a82674a495afe0e63a4bc85ab2a5ab

--
Jan Cholasta

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

Re: [Freeipa-devel] [PATCH 0297] ULC: add user-stage command

[Jan Cholasta]

On 19.8.2015 10:47, thierry bordaz wrote:

On 08/19/2015 10:34 AM, Jan Cholasta wrote:

On 19.8.2015 09:39, thierry bordaz wrote:

Hi,

It worked like a charm.
I had a problem to commit it because of the VERSION stuff that changed.

Except that (changing VERSION), the fix looks good to me

thanks
thierry
On 08/18/2015 07:21 PM, Martin Basti wrote:

Thank you for the patch, I checked it, I just changed permission name
to have all first letters in uppercase as others.
Updated merged patch attached.

On 08/18/2015 05:34 PM, thierry bordaz wrote:

On 08/18/2015 04:13 PM, thierry bordaz wrote:

On 08/18/2015 04:04 PM, Martin Basti wrote:



On 08/18/2015 03:49 PM, thierry bordaz wrote:

On 08/18/2015 03:06 PM, Martin Basti wrote:



On 08/18/2015 11:32 AM, thierry bordaz wrote:

On 08/18/2015 10:02 AM, Martin Basti wrote:



On 08/18/2015 09:59 AM, thierry bordaz wrote:

On 08/18/2015 09:55 AM, Martin Basti wrote:



On 08/18/2015 09:50 AM, thierry bordaz wrote:

On 08/17/2015 08:33 PM, Martin Basti wrote:

Hello,

the 'user-stage' command replaces 'stageuser-add
--from-delete' command.
https://fedorahosted.org/freeipa/ticket/5041

Thierry can you check If I don't break everything, it works
for me, but the one never knows.

Honza can you please check the framework side? I use
self.api.Object.stageuser.add.* in user command, I'm not
sure if this is right way, but it works.

Patch attached. I created it in hurry, I'm expecting NACK :D


Just question at the end: should I implement way Active
user -> stageuser? IMHO it would be implemented internally
by calling 'user-del --preserve' inside 'user-stage'.




Hi Martin,

There is a small failure with VERSION (edewata pushed his
patch first ;-) )

git apply -v
/tmp/freeipa-mbasti-0297-Add-user-stage-command.patch
Checking patch API.txt...
Checking patch VERSION...
error: while searching for:
# #

IPA_API_VERSION_MAJOR=2
IPA_API_VERSION_MINOR=148
# Last change: ftweedal - add --out option to user-show

error: patch failed: VERSION:90
error: VERSION: patch does not apply
Checking patch ipalib/plugins/stageuser.py...
Checking patch ipalib/plugins/user.py...



There is many pending patches that may change VERSION number,
I will change it to right one before push.

Does code looks good for you?

Hi Martin,

Just a question, there is no additional permission. Did you
test being 'admin' ?

thanks
theirry

No I didn't,.

I preserver all permission, the original permissions should
work.

Martin

Hi Martin,

Running a test script, I have an issue with

ipa stageuser-add --first=t --last=b tb1
ipa: ERROR: an internal error has occurred


[Tue Aug 18 11:16:56.440658 2015] [wsgi:error] [pid 10486]
ipa: INFO: [jsonserver_kerb]
stage...@abc.idm.lab.eng.brq.redhat.com:
stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
displayname=u't b', initials=u'tb', gecos=u't b',
krbprincipalname=u't...@abc.idm.lab.eng.brq.redhat.com',
random=False, all=False, raw=False, version=u'2.149',
no_members=False): AttributeError
[Tue Aug 18 11:21:25.198021 2015] [wsgi:error] [pid 10485]
ipa: ERROR: non-public: AttributeError: 'DN' object has no
attribute 'setdefault'
[Tue Aug 18 11:21:25.198053 2015] [wsgi:error] [pid 10485]
Traceback (most recent call last):
[Tue Aug 18 11:21:25.198058 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
line 347, in wsgi_execute
[Tue Aug 18 11:21:25.198062 2015] [wsgi:error] [pid
10485] result = self.Command[name](*args, **options)
[Tue Aug 18 11:21:25.198066 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 443, in __call__
[Tue Aug 18 11:21:25.198070 2015] [wsgi:error] [pid
10485] ret = self.run(*args, **options)
[Tue Aug 18 11:21:25.198081 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 760, in run
[Tue Aug 18 11:21:25.198133 2015] [wsgi:error] [pid
10485] return self.execute(*args, **options)
[Tue Aug 18 11:21:25.198139 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py",
line 1227, in execute
[Tue Aug 18 11:21:25.198144 2015] [wsgi:error] [pid
10485] *keys, **options)
[Tue Aug 18 11:21:25.198147 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 373, in pre_callback
[Tue Aug 18 11:21:25.198151 2015] [wsgi:error] [pid
10485] attrs_list, *keys, **options)
[Tue Aug 18 11:21:25.198155 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 277, in set_default_values_pre_callback
[Tue Aug 18 11:21:25.198159 2015] [wsgi:error] [pid 10485]
entry_attrs.setdefault('description', [])

Re: [Freeipa-devel] [PATCH 0297] ULC: add user-stage command

[thierry bordaz]

On 08/19/2015 10:34 AM, Jan Cholasta wrote:

On 19.8.2015 09:39, thierry bordaz wrote:

Hi,

It worked like a charm.
I had a problem to commit it because of the VERSION stuff that changed.

Except that (changing VERSION), the fix looks good to me

thanks
thierry
On 08/18/2015 07:21 PM, Martin Basti wrote:

Thank you for the patch, I checked it, I just changed permission name
to have all first letters in uppercase as others.
Updated merged patch attached.

On 08/18/2015 05:34 PM, thierry bordaz wrote:

On 08/18/2015 04:13 PM, thierry bordaz wrote:

On 08/18/2015 04:04 PM, Martin Basti wrote:



On 08/18/2015 03:49 PM, thierry bordaz wrote:

On 08/18/2015 03:06 PM, Martin Basti wrote:



On 08/18/2015 11:32 AM, thierry bordaz wrote:

On 08/18/2015 10:02 AM, Martin Basti wrote:



On 08/18/2015 09:59 AM, thierry bordaz wrote:

On 08/18/2015 09:55 AM, Martin Basti wrote:



On 08/18/2015 09:50 AM, thierry bordaz wrote:

On 08/17/2015 08:33 PM, Martin Basti wrote:

Hello,

the 'user-stage' command replaces 'stageuser-add
--from-delete' command.
https://fedorahosted.org/freeipa/ticket/5041

Thierry can you check If I don't break everything, it works
for me, but the one never knows.

Honza can you please check the framework side? I use
self.api.Object.stageuser.add.* in user command, I'm not
sure if this is right way, but it works.

Patch attached. I created it in hurry, I'm expecting NACK :D


Just question at the end: should I implement way Active
user -> stageuser? IMHO it would be implemented internally
by calling 'user-del --preserve' inside 'user-stage'.




Hi Martin,

There is a small failure with VERSION (edewata pushed his
patch first ;-) )

git apply -v
/tmp/freeipa-mbasti-0297-Add-user-stage-command.patch
Checking patch API.txt...
Checking patch VERSION...
error: while searching for:
# #

IPA_API_VERSION_MAJOR=2
IPA_API_VERSION_MINOR=148
# Last change: ftweedal - add --out option to user-show

error: patch failed: VERSION:90
error: VERSION: patch does not apply
Checking patch ipalib/plugins/stageuser.py...
Checking patch ipalib/plugins/user.py...



There is many pending patches that may change VERSION number,
I will change it to right one before push.

Does code looks good for you?

Hi Martin,

Just a question, there is no additional permission. Did you
test being 'admin' ?

thanks
theirry

No I didn't,.

I preserver all permission, the original permissions should 
work.


Martin

Hi Martin,

Running a test script, I have an issue with

ipa stageuser-add --first=t --last=b tb1
ipa: ERROR: an internal error has occurred


[Tue Aug 18 11:16:56.440658 2015] [wsgi:error] [pid 10486]
ipa: INFO: [jsonserver_kerb]
stage...@abc.idm.lab.eng.brq.redhat.com:
stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
displayname=u't b', initials=u'tb', gecos=u't b',
krbprincipalname=u't...@abc.idm.lab.eng.brq.redhat.com',
random=False, all=False, raw=False, version=u'2.149',
no_members=False): AttributeError
[Tue Aug 18 11:21:25.198021 2015] [wsgi:error] [pid 10485]
ipa: ERROR: non-public: AttributeError: 'DN' object has no
attribute 'setdefault'
[Tue Aug 18 11:21:25.198053 2015] [wsgi:error] [pid 10485]
Traceback (most recent call last):
[Tue Aug 18 11:21:25.198058 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
line 347, in wsgi_execute
[Tue Aug 18 11:21:25.198062 2015] [wsgi:error] [pid
10485] result = self.Command[name](*args, **options)
[Tue Aug 18 11:21:25.198066 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 443, in __call__
[Tue Aug 18 11:21:25.198070 2015] [wsgi:error] [pid
10485] ret = self.run(*args, **options)
[Tue Aug 18 11:21:25.198081 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 760, in run
[Tue Aug 18 11:21:25.198133 2015] [wsgi:error] [pid
10485] return self.execute(*args, **options)
[Tue Aug 18 11:21:25.198139 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py",
line 1227, in execute
[Tue Aug 18 11:21:25.198144 2015] [wsgi:error] [pid
10485] *keys, **options)
[Tue Aug 18 11:21:25.198147 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 373, in pre_callback
[Tue Aug 18 11:21:25.198151 2015] [wsgi:error] [pid
10485] attrs_list, *keys, **options)
[Tue Aug 18 11:21:25.198155 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 277, in set_default_values_pre_callback
[Tue Aug 18 11:21:25.198159 2015] [wsgi:error] [pid 10485]
entry_attrs.setdefault('description', [])
[Tue Aug 18 11:21:25.198163 2015] [wsgi:e

Re: [Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)

[David Kupka]

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 
Date: Thu, 13 Aug 2015 16:41:23 +0200
Subject: [PATCH 1/2] Add /etc/tmpfiles.d/dirsrv-.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 
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):
 self.db2ldi

Re: [Freeipa-devel] [PATCH 476] vault: Add container information to vault command results

[Petr Vobornik]

On 08/18/2015 09:48 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes part of
.

Christian is working on a fix for the other part of the ticket.

Honza




ACK

Pushed to:
master: 01dd951ddc0181b559eb3dd5ff0336c81e245628
ipa-4-2: cb575e6a16c3f1a001b13b96279ecd758c8783a1
--
Petr Vobornik

--
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] 0299 client: Update DNS with all available local IP addresses.

[Martin Basti]

On 08/18/2015 10:53 PM, Martin Basti wrote:



On 08/18/2015 08:02 PM, David Kupka wrote:

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 specifie

Re: [Freeipa-devel] [PATCH 0297] ULC: add user-stage command

[Jan Cholasta]

On 19.8.2015 09:39, thierry bordaz wrote:

Hi,

It worked like a charm.
I had a problem to commit it because of the VERSION stuff that changed.

Except that (changing VERSION), the fix looks good to me

thanks
thierry
On 08/18/2015 07:21 PM, Martin Basti wrote:

Thank you for the patch, I checked it, I just changed permission name
to have all first letters in uppercase as others.
Updated merged patch attached.

On 08/18/2015 05:34 PM, thierry bordaz wrote:

On 08/18/2015 04:13 PM, thierry bordaz wrote:

On 08/18/2015 04:04 PM, Martin Basti wrote:



On 08/18/2015 03:49 PM, thierry bordaz wrote:

On 08/18/2015 03:06 PM, Martin Basti wrote:



On 08/18/2015 11:32 AM, thierry bordaz wrote:

On 08/18/2015 10:02 AM, Martin Basti wrote:



On 08/18/2015 09:59 AM, thierry bordaz wrote:

On 08/18/2015 09:55 AM, Martin Basti wrote:



On 08/18/2015 09:50 AM, thierry bordaz wrote:

On 08/17/2015 08:33 PM, Martin Basti wrote:

Hello,

the 'user-stage' command replaces 'stageuser-add
--from-delete' command.
https://fedorahosted.org/freeipa/ticket/5041

Thierry can you check If I don't break everything, it works
for me, but the one never knows.

Honza can you please check the framework side? I use
self.api.Object.stageuser.add.* in user command, I'm not
sure if this is right way, but it works.

Patch attached. I created it in hurry, I'm expecting NACK :D


Just question at the end: should I implement way Active
user -> stageuser? IMHO it would be implemented internally
by calling 'user-del --preserve' inside 'user-stage'.




Hi Martin,

There is a small failure with VERSION (edewata pushed his
patch first ;-) )

git apply -v
/tmp/freeipa-mbasti-0297-Add-user-stage-command.patch
Checking patch API.txt...
Checking patch VERSION...
error: while searching for:
# #

IPA_API_VERSION_MAJOR=2
IPA_API_VERSION_MINOR=148
# Last change: ftweedal - add --out option to user-show

error: patch failed: VERSION:90
error: VERSION: patch does not apply
Checking patch ipalib/plugins/stageuser.py...
Checking patch ipalib/plugins/user.py...



There is many pending patches that may change VERSION number,
I will change it to right one before push.

Does code looks good for you?

Hi Martin,

Just a question, there is no additional permission. Did you
test being 'admin' ?

thanks
theirry

No I didn't,.

I preserver all permission, the original permissions should work.

Martin

Hi Martin,

Running a test script, I have an issue with

ipa stageuser-add --first=t --last=b tb1
ipa: ERROR: an internal error has occurred


[Tue Aug 18 11:16:56.440658 2015] [wsgi:error] [pid 10486]
ipa: INFO: [jsonserver_kerb]
stage...@abc.idm.lab.eng.brq.redhat.com:
stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
displayname=u't b', initials=u'tb', gecos=u't b',
krbprincipalname=u't...@abc.idm.lab.eng.brq.redhat.com',
random=False, all=False, raw=False, version=u'2.149',
no_members=False): AttributeError
[Tue Aug 18 11:21:25.198021 2015] [wsgi:error] [pid 10485]
ipa: ERROR: non-public: AttributeError: 'DN' object has no
attribute 'setdefault'
[Tue Aug 18 11:21:25.198053 2015] [wsgi:error] [pid 10485]
Traceback (most recent call last):
[Tue Aug 18 11:21:25.198058 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
line 347, in wsgi_execute
[Tue Aug 18 11:21:25.198062 2015] [wsgi:error] [pid
10485] result = self.Command[name](*args, **options)
[Tue Aug 18 11:21:25.198066 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 443, in __call__
[Tue Aug 18 11:21:25.198070 2015] [wsgi:error] [pid
10485] ret = self.run(*args, **options)
[Tue Aug 18 11:21:25.198081 2015] [wsgi:error] [pid 10485]
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",
line 760, in run
[Tue Aug 18 11:21:25.198133 2015] [wsgi:error] [pid
10485] return self.execute(*args, **options)
[Tue Aug 18 11:21:25.198139 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py",
line 1227, in execute
[Tue Aug 18 11:21:25.198144 2015] [wsgi:error] [pid
10485] *keys, **options)
[Tue Aug 18 11:21:25.198147 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 373, in pre_callback
[Tue Aug 18 11:21:25.198151 2015] [wsgi:error] [pid
10485] attrs_list, *keys, **options)
[Tue Aug 18 11:21:25.198155 2015] [wsgi:error] [pid 10485]
File
"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 277, in set_default_values_pre_callback
[Tue Aug 18 11:21:25.198159 2015] [wsgi:error] [pid 10485]
entry_attrs.setdefault('description', [])
[Tue Aug 18 11:21:25.198163 2015] [wsgi:error] [pid 10485]
 

Re: [Freeipa-devel] [PATCH 475] vault: Fix vault-find with criteria

[Petr Vobornik]

On 08/18/2015 09:23 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




ACK

Pushed to:
master: 29cee7a4bc5a6d2506e7937c982339274fa0edb4
ipa-4-2: 9d32bcafab0f7fa35ae089bff0a5001e2406767d
--
Petr Vobornik

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

[Pavel Reichl]

On 08/18/2015 10:53 PM, Martin Basti wrote:



On 08/18/2015 08:02 PM, David Kupka wrote:

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 specifie

Re: [Freeipa-devel] [PATCH 0297] ULC: add user-stage command

[thierry bordaz]

Hi,

It worked like a charm.
I had a problem to commit it because of the VERSION stuff that changed.

Except that (changing VERSION), the fix looks good to me

thanks
thierry
On 08/18/2015 07:21 PM, Martin Basti wrote:
Thank you for the patch, I checked it, I just changed permission name 
to have all first letters in uppercase as others.

Updated merged patch attached.

On 08/18/2015 05:34 PM, thierry bordaz wrote:

On 08/18/2015 04:13 PM, thierry bordaz wrote:

On 08/18/2015 04:04 PM, Martin Basti wrote:



On 08/18/2015 03:49 PM, thierry bordaz wrote:

On 08/18/2015 03:06 PM, Martin Basti wrote:



On 08/18/2015 11:32 AM, thierry bordaz wrote:

On 08/18/2015 10:02 AM, Martin Basti wrote:



On 08/18/2015 09:59 AM, thierry bordaz wrote:

On 08/18/2015 09:55 AM, Martin Basti wrote:



On 08/18/2015 09:50 AM, thierry bordaz wrote:

On 08/17/2015 08:33 PM, Martin Basti wrote:

Hello,

the 'user-stage' command replaces 'stageuser-add 
--from-delete' command.

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

Thierry can you check If I don't break everything, it works 
for me, but the one never knows.


Honza can you please check the framework side? I use 
self.api.Object.stageuser.add.* in user command, I'm not 
sure if this is right way, but it works.


Patch attached. I created it in hurry, I'm expecting NACK :D


Just question at the end: should I implement way Active 
user -> stageuser? IMHO it would be implemented internally 
by calling 'user-del --preserve' inside 'user-stage'.





Hi Martin,

There is a small failure with VERSION (edewata pushed his 
patch first ;-) )


git apply -v
/tmp/freeipa-mbasti-0297-Add-user-stage-command.patch
Checking patch API.txt...
Checking patch VERSION...
error: while searching for:
# #

IPA_API_VERSION_MAJOR=2
IPA_API_VERSION_MINOR=148
# Last change: ftweedal - add --out option to user-show

error: patch failed: VERSION:90
error: VERSION: patch does not apply
Checking patch ipalib/plugins/stageuser.py...
Checking patch ipalib/plugins/user.py...


There is many pending patches that may change VERSION number, 
I will change it to right one before push.


Does code looks good for you?

Hi Martin,

Just a question, there is no additional permission. Did you 
test being 'admin' ?


thanks
theirry

No I didn't,.

I preserver all permission, the original permissions should work.

Martin

Hi Martin,

Running a test script, I have an issue with

ipa stageuser-add --first=t --last=b tb1
ipa: ERROR: an internal error has occurred


[Tue Aug 18 11:16:56.440658 2015] [wsgi:error] [pid 10486]
ipa: INFO: [jsonserver_kerb]
stage...@abc.idm.lab.eng.brq.redhat.com:
stageuser_add(u'tb1', givenname=u't', sn=u'b', cn=u't b',
displayname=u't b', initials=u'tb', gecos=u't b',
krbprincipalname=u't...@abc.idm.lab.eng.brq.redhat.com',
random=False, all=False, raw=False, version=u'2.149',
no_members=False): AttributeError
[Tue Aug 18 11:21:25.198021 2015] [wsgi:error] [pid 10485]
ipa: ERROR: non-public: AttributeError: 'DN' object has no
attribute 'setdefault'
[Tue Aug 18 11:21:25.198053 2015] [wsgi:error] [pid 10485]
Traceback (most recent call last):
[Tue Aug 18 11:21:25.198058 2015] [wsgi:error] [pid 10485]  
File

"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py",
line 347, in wsgi_execute
[Tue Aug 18 11:21:25.198062 2015] [wsgi:error] [pid
10485] result = self.Command[name](*args, **options)
[Tue Aug 18 11:21:25.198066 2015] [wsgi:error] [pid 10485]  
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",

line 443, in __call__
[Tue Aug 18 11:21:25.198070 2015] [wsgi:error] [pid
10485] ret = self.run(*args, **options)
[Tue Aug 18 11:21:25.198081 2015] [wsgi:error] [pid 10485]  
File "/usr/lib/python2.7/site-packages/ipalib/frontend.py",

line 760, in run
[Tue Aug 18 11:21:25.198133 2015] [wsgi:error] [pid
10485] return self.execute(*args, **options)
[Tue Aug 18 11:21:25.198139 2015] [wsgi:error] [pid 10485]  
File

"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py",
line 1227, in execute
[Tue Aug 18 11:21:25.198144 2015] [wsgi:error] [pid
10485] *keys, **options)
[Tue Aug 18 11:21:25.198147 2015] [wsgi:error] [pid 10485]  
File

"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 373, in pre_callback
[Tue Aug 18 11:21:25.198151 2015] [wsgi:error] [pid
10485] attrs_list, *keys, **options)
[Tue Aug 18 11:21:25.198155 2015] [wsgi:error] [pid 10485]  
File

"/usr/lib/python2.7/site-packages/ipalib/plugins/stageuser.py",
line 277, in set_default_values_pre_callback
[Tue Aug 18 11:21:25.198159 2015] [wsgi:error] [pid 10485]
entry_attrs.setdefault('description', [])
[Tue Aug 18 11:21:25.198163 2015] [wsgi:error] [pid 10485]
Attrib

[Freeipa-devel] Subject: [PATCH 0061-2] Fix backup/restore (#5071)

[David Kupka]

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

--
David Kupka
From c4a72b64aab5abfde15f06b037da1c3ab2cfa220 Mon Sep 17 00:00:00 2001
From: David Kupka 
Date: Thu, 13 Aug 2015 16:41:23 +0200
Subject: [PATCH 1/2] Add /etc/tmpfiles.d/dirsrv-.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 
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)
 self.db2bak(insta

Re: [Freeipa-devel] [PATCH] First part of integration tests for Topology Plugin

[Oleg Fayans]

Hi Martin,

As discussed, here is a new version with pep8-related fixes


On 08/14/2015 10:44 AM, Oleg Fayans wrote:

Hi Martin,

Already noticed that. Implemented the named groups as Tomas advised.
Added the third test for
http://www.freeipa.org/page/V4/Manage_replication_topology/Test_plan#Test_case:_Removal_of_a_topology_segment_is_allowed_only_if_there_is_at_least_one_more_segment_connecting_the_given_replica




On 08/13/2015 05:06 PM, Martin Basti wrote:



On 08/11/2015 03:36 PM, Oleg Fayans wrote:

Hi Martin,

On 08/11/2015 02:02 PM, Martin Basti wrote:

NACK, comments inline.

On 11/08/15 13:25, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 08/10/2015 07:08 PM, Martin Basti wrote:

Thank you for patch, I have a few nitpicks:

1)
On 10/08/15 13:05, Oleg Fayans wrote:

+def create_segment(master, leftnode, rightnode):
+"""create_segment(master, leftnode, rightnode)

Why do you add the name of method in docstring?

My bad, fixed.


still there

+tokenize_topologies(command_output)
+takes an output of `ipa topologysegment-find` and returns an
array of


Fixed, sorry.





2)

+def create_segment(master, leftnode, rightnode):
+"""create_segment(master, leftnode, rightnode)
+creates a topology segment. The first argument is a node to
run the
command on
+The first 3 arguments should be objects of class Host
+Returns a hash object containing segment's name, leftnode,
rightnode information
+"""

I would prefer to add assert there instead of just document that a
Host
object is needed
assert(isinstance(master, Host))
...


Fixed. Created a decorator that checks the type of arguments


This does not scale well.
If we will want to add new argument that is not host object, you need
change it again.


Agreed. Modified the decorator so that you can specify a slice of
arguments to be checked and a class to compare to. This does scale :)


This might be used as function with specified variables that have to be
host objects




3)
+def destroy_segment(master, segment_name):
+"""
+destroy_segment(master, segment_name)
+Destroys topology segment. First argument should be object of
class
Host

Instead of description of params as first, second etc., you may use
following:

+def destroy_segment(master, segment_name):
+"""
+Destroys topology segment.
+:param master: reference to master object of class Host
+:param segment: name fo segment
and eventually this in other methods
+:returns: Lorem ipsum sit dolor mit amet
+:raises NotFound: if segment is not found


Fixed


4)

cls.replicas[:len(cls.replicas) - 1],

I suggest cls.replicas[:-1]

In [2]: a = [1, 2, 3, 4, 5]

In [3]: a[:-1]
Out[3]: [1, 2, 3, 4]


Fixed


5)
Why re.findall() and then you just use the first result?
'leftnode': self.leftnode_re.findall(i)[0]

Isn't just re.search() enough?
leftnode_re.search(value).group(1)


in fact
leftnode_re.findall(string)[0]
and
leftnode_re.search(string).group(1),
Are equally bad from the readability point of view. The first one is
even shorter a bit, so why change? :)


It depends on point of view,  because when I reviewed it yesterday my
brain raises exception that you are trying to add multiple values to
single value attribute, because you used findall, I expected that you
need multiple values, and then I saw that index [0] at the end, and I
was pretty confused what are you trying to achieve.

And because findall is not effective in case when you need to find just
one occurrence.


I got it. Fixed.









Python 3 nitpick:
I'm not sure if time when we should enforce python 2/3 compability
already comes, but just for record:
instead of open(file, 'r'), please use io.open(file, 'r') (import io
before)


Done.








1)

+#

empty comment here (several times)


Removed






NACK

you changed it wrong

group() returns everything, you need use group(1) to return content in
braces.






--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From c7aecff2f7d09b33a5d806f43598373df79b4751 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Wed, 19 Aug 2015 08:57:29 +0200
Subject: [PATCH] Added integration tests for Topology plugin

---
 ipatests/test_integration/tasks.py  | 101 ++---
 ipatests/test_integration/test_topology.py  | 143 
 ipatests/test_ipaserver/test_topology_plugin.py |  14 ++-
 3 files changed, 236 insertions(+), 22 deletions(-)
 create mode 100644 ipatests/test_integration/test_topology.py

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 2b83f274619fcb18bcb87118d7df35a85ce52c1a..2544502a692b3a818e489bd94d8651d46972fa60 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -40,6 +40,24 @@ from ipatests.test_integration.host import Host
 log = log_mgr.get_logger(__name__)
 
 
+def check_arguments_are(slice, instanceof):
+"""
+:param: slice - tuple of integers denoting the beginning and the