Re: [Freeipa-devel] [PATCH] Remove (un)wrap_binary_data cruft from */ipautil.py

2010-02-03 Thread Jason Gerard DeRose
On Thu, 2010-01-28 at 12:35 -0500, John Dennis wrote:
 Remove SAFE_STRING_PATTERN, safe_string_re, needs_base64(),
 wrap_binary_data(), unwrap_binary_data() from both instances
 of ipautil.py. This code is no longer in use and the
 SAFE_STRING_PATTERN regular expression string was causing xgettext
 to abort because it wasn't a valid ASCII string.
 ---
  ipapython/ipautil.py |   62 
 --
  ipaserver/ipautil.py |   62 
 --
  2 files changed, 0 insertions(+), 124 deletions(-)

Patch looks good, but I get an error when trying to apply with `git am`:

   Patch does not have a valid e-mail address.

Did you figure out your attachment problem?  For what it's worth, I
prepare patches with `git format-patch -1` and then manually attach the
patch to an email (I'm using Evolution).

Could you submit this again?  Or if someone with more git experience
could instruct me as to a work-around.




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


[Freeipa-devel] [PATCH] jderose 038 Fix ipalib doctest

2010-02-03 Thread Jason Gerard DeRose
This patch fixes doctests in ipalib/__init__.py that were broken by
Rob's 364 base64-encode binary data... patch.

This patch also removes the unneeded use of textui.encode_binary() in
the textui.print_keyval() method.  repr('cannot print me') will escape
non-ascii characters using the Python \xHH hexadecimal literal
notation... so the output will be terminal safe even without base64
encoding.

textui.print_keyval() isn't being used at the moment, AFAIK, but it's
indented for developer-centric debugging type commands where printing
the repr() is helpful.


P.S.: I think it might have got lost in the shuffle, but could someone
ack my 037 patch?  With 037 and this patch, all the unit tests should be
working again.
From 0a6d49498c59337e66685102bfd03a822f037910 Mon Sep 17 00:00:00 2001
From: Jason Gerard DeRose jder...@redhat.com
Date: Wed, 3 Feb 2010 04:03:58 -0700
Subject: [PATCH] Fixed doctests for ipalib package docstring; fixed unneeded use of textui.encode_binary() in textui.print_keyval()

---
 ipalib/__init__.py |   20 ++--
 ipalib/cli.py  |4 +++-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 83956e1..beaf0ab 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -584,9 +584,9 @@ For example, say we setup a command like this:
 ...
 ... def execute(self, key, **options):
 ... items = dict(
-... fruit='apple',
-... pet='dog',
-... city='Berlin',
+... fruit=u'apple',
+... pet=u'dog',
+... city=u'Berlin',
 ... )
 ... if key in items:
 ... return dict(result=items[key])
@@ -627,9 +627,9 @@ through the ``ipa`` script basically will do the following:
 ---
 show-items:
 ---
-  city = 'Berlin'
-  fruit = 'apple'
-  pet = 'dog'
+  city = u'Berlin'
+  fruit = u'apple'
+  pet = u'dog'
 ---
 3 items
 ---
@@ -641,9 +641,9 @@ Similarly, calling it with ``reverse=True``  would result in the following:
 ---
 show-items:
 ---
-  pet = 'dog'
-  fruit = 'apple'
-  city = 'Berlin'
+  pet = u'dog'
+  fruit = u'apple'
+  city = u'Berlin'
 --
 3 items (in reverse order)
 --
@@ -652,7 +652,7 @@ Lastly, providing a ``key`` would result in the following:
 
  result = api.Command.show_items(u'city')
  api.Command.show_items.output_for_cli(textui, result, 'city', reverse=False)
-city = 'Berlin'
+city = u'Berlin'
 
 See the `ipalib.cli.textui` plugin for a description of its methods.
 
diff --git a/ipalib/cli.py b/ipalib/cli.py
index b398094..124b625 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -244,7 +244,9 @@ class textui(backend.Backend):
 Also see `textui.print_indented`.
 
 for (key, value) in rows:
-self.print_indented('%s = %r' % (key, self.encode_binary(value)), indent)
+# Note that self.encode_binary(value) isn't needed as repr(value)
+# will escape an `str` using \xHH hexidicimal:
+self.print_indented('%s = %r' % (key, value), indent)
 
 def print_attribute(self, attr, value, indent=1, one_value_per_line=True):
 
-- 
1.6.3.3

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

Re: [Freeipa-devel] [PATCH] Remove (un)wrap_binary_data cruft from */ipautil.py

2010-02-03 Thread Rob Crittenden

John Dennis wrote:

On 02/03/2010 05:07 AM, Jason Gerard DeRose wrote:


Could you submit this again?


Done, attached.


This patch has already been committed.

Jason, btw what I did was to copy the entire message, headers and all, 
and give that to git-am. It was able to grok things ok. The update 
attachment is much easier to handle though.


rob

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


Re: [Freeipa-devel] [PATCH] jderose 037 Fix broken unit tests

2010-02-03 Thread Rob Crittenden

Jason Gerard DeRose wrote:

This patch gets (almost) all the XML-RPC tests working again under
Fedora12.  Some may not pass under Fedora11 due to 389 schema changes,
but Fedora12 should be our primary test target at this point, IHMO.
Does anyone disagree?

3 cert tests still fail, but I'm not familiar enough with the cert
plugins to confidently decide whether the tests need to be updated or
whether something is broken.  Rob or John, could you take a look at
these when you get a chance?

We really need to get strict about patches with regard to tests.  If a
patch breaks a test, the test needs to be updated in that same patch (or
if the test is correct, the code needs to be updated).  If a patch
introduces new functionality, it must be accompanied by tests.

Rob and Pavel, I'm looking at you.  If tests no passy, no acky-acky.  ;)

I know I've been at fault too, but I've already scolded myself off-list.


I'd rather you simply remove the extra attributes rather than commenting 
them out. If we aren't going to be providing them to the end-user we 
should eliminate them from the tests as well.


But there seems to be a deeper problem here, the question is: why aren't 
these attributes appearing?


In the case of 'l' or 'localityname' it seems to be a bug. It is in the 
default_attributes list in the host plugin but it isn't being returned 
by host_show().


I also wonder if we really should be returning uidnumber and gidnumber, 
they seem kinda handy. IIRC I based the output of the v1 ipa-finduser on 
finger which doesn't include these values.


So nack for now until we can figure out what is going on under the hood.

rob

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


Re: [Freeipa-devel] [PATCH] jderose 038 Fix ipalib doctest

2010-02-03 Thread Rob Crittenden

Jason Gerard DeRose wrote:

This patch fixes doctests in ipalib/__init__.py that were broken by
Rob's 364 base64-encode binary data... patch.

This patch also removes the unneeded use of textui.encode_binary() in
the textui.print_keyval() method.  repr('cannot print me') will escape
non-ascii characters using the Python \xHH hexadecimal literal
notation... so the output will be terminal safe even without base64
encoding.

textui.print_keyval() isn't being used at the moment, AFAIK, but it's
indented for developer-centric debugging type commands where printing
the repr() is helpful.


Ok, this is essentially identical to my patch 367 fix some encoding 
issues.


I had 2 other changes not included:

1. Remove the assert in encode_binary()
2. I had a fix for the docstrings in the textui class related to the 
value of production. Did you not run into this one?


rob




P.S.: I think it might have got lost in the shuffle, but could someone
ack my 037 patch?  With 037 and this patch, all the unit tests should be
working again.


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


Re: [Freeipa-devel] [PATCH] 366 don't reset log level

2010-02-03 Thread Rob Crittenden

John Dennis wrote:

On 02/02/2010 05:22 PM, Rob Crittenden wrote:

Don't let the framework reset the log level of an existing log handler.
In the installer we initialize logging and set the log level of the
install/uninstall log to DEBUG. When we initialize the framework later
it resets the log level to INFO which means we lose a lot of information
in the logs, particularly a problem when there are failures.

rob


ACK



pushed to master

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


Re: [Freeipa-devel] [PATCH] 359 allow cert renewal

2010-02-03 Thread Rob Crittenden

John Dennis wrote:

On 01/28/2010 04:16 PM, Rob Crittenden wrote:

Rob Crittenden wrote:

Be a bit smarter about decoding certificates that might be base64
encoded. First see if it only contains those characters allowed before
trying to decode it. This reduces the number of false positives.

rob



Er, duh, I got this description goofed up.


ACK to the correct patch



I pushed a slightly modified patch. I've done away with the --revoke 
flag for now. I wanted a way to warn the user that they are about to 
overwrite an existing cert. This raises other issues though and would 
require some changes to the certmonger program so for now I'll just let 
it overwrite.


rob

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


Re: [Freeipa-devel] [PATCH 363 find all group pwd policy

2010-02-03 Thread Rob Crittenden

Pavel Zuna wrote:

Rob Crittenden wrote:
Provide pwpolicy-find command to display all group-specific password 
policies.


find is a bit of a misnomer since you can't provide any terms to limit 
the search scope, but it's a start. I'm not sure this is the kind of 
thing we need/want to be able to query things like give me all the 
policies where the max lifetime is  20 days. But I could be wrong.


rob



entries = tuple(e for (dn, e) in entries)
for entry in xrange(len(entries)):
_convert_time_for_output(entries[entry])

Should be:

for e in entries:
_convert_time_for_output(e[1])
e[1]['dn'] = e[0]
entries = tuple(e for (dn, e) in entries)

Otherwise DNs is lost in the conversion from (dn, entry_attrs) to 
entry_with_dn_as_dict. Also, there's no need for xrange - I'm using it 
myself in LDAPSearch.execute(), but that's because I overwrite elements 
in the entries list. (And as I think about it now, overwriting elements 
there is also unnecessary, so it will be removed in a future patch.)


Everything else about the patch is fine.

Pavel


Good point about the dn, fixed and pushed to master.

rob

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


Re: [Freeipa-devel] [PATCH] fix error message to be i18n translator friendly

2010-02-03 Thread Rob Crittenden

John Dennis wrote:

This error message was producing a warning from xgettext
because there were multiple substations in the string.
In some languages it may be necessary to reorder the
substitutions for a proper translation, this is only
possible if the substitutions use named values.
---
 ipaserver/plugins/selfsign.py |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)



pushed to master

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


[Freeipa-devel] [PATCH] 369 fix word usage in installer

2010-02-03 Thread Rob Crittenden

Proper use of set up vs setup.

rob


freeipa-369-setup.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 353 enable sssd and certmonger

2010-02-03 Thread Rob Crittenden

Martin Nagy wrote:

On Wed, 2010-01-20 at 17:01 -0500, Rob Crittenden wrote:

Configure sssd and certmonger in ipa-client-install

This does a number of things under the hood:

- Use authconfig to enable sssd in nss and pam
- Configure /etc/sssd/sssd.conf to use our IPA provider
- Enable the certmonger process and request a server cert
- join the IPA domain and retrieve a principal. The clinet machine 
*must* exist in IPA to be able to do a join.

- And then undo all this on uninstall
rob


Heh, joining FreeIPA and SSSD at last, cool :-)

ACK

Martin



Pushed to master

certmonger is in updates-testing right now so to install you'll need to 
enable the updates-testing repo.


rob

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


[Freeipa-devel] [PATCH] 370 set default log level

2010-02-03 Thread Rob Crittenden
The ipa-*-manage programs had the default log level set to NOTSET which 
was causing copious logging to occur. Setting it to ERROR fixes this.


rob


freeipa-370-log.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 371 add status to ipactl

2010-02-03 Thread Rob Crittenden
We had an RFE for adding status to ipactl, seemed like low-hanging fruit 
(bug 503437)


rob


freeipa-371-ipactl.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 373 fix ipa-getkeytab man page

2010-02-03 Thread Rob Crittenden
The usage on the man page didn't follow standard conventions. Optional 
arguments shouldn't be enclosed in brackets [].


rob


freeipa-373-man.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 374 don't make assumptions about cwd

2010-02-03 Thread Rob Crittenden
Don't assume that cwd exists or is writable. I had worked on this 
previously so that we change to a known writable directory when issuing 
server certs. Enhance that so we change to the NSS db dir when issuing 
the self-signed CA. certutil wants to write a file to the cwd when 
generating a key so we need to be some place writable.


Also handle the case where cwd is an invalid directory. I tested this with:

term 1: mkdir foo
cd foo

term 2: rmdir foo

term 1: ipa-server-install ...

Probably not a major issue but not hard to more carefully handle it 
either. The installation will still fail but at least we have a good 
message when it does.


rob


freeipa-374-chdir.patch
Description: application/mbox
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 369 fix word usage in installer

2010-02-03 Thread David O'Brien

Rob Crittenden wrote:

Proper use of set up vs setup.

rob

I don't know how pedantic you want to get in these messages, but:
1. setup is a noun
2. set up is a verb
3. set-up is a compound adjective

So, for
   print on the computer (i.e. a non-root user).  The set up procedure
If you're describing the procedure, this would appear as The set-up 
procedure in our documentation, same as you did for server-specific 
operations. The other variants are correct.


/nit-pick

--

David O'Brien
Senior Technical Writer, Engineering Content Services
Red Hat Asia Pacific Pty Ltd
193 North Quay, Brisbane
+61 7 3514 8189

http://freeipa.org/page/DocumentationPortal
http://git.fedorahosted.org/git/ipadocs.git
http://www.opensource.com

He who asks is a fool for five minutes, but he who does not ask remains 
a fool forever.

 ~ Chinese proverb

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