Re: [Freeipa-devel] [PATCH][DOC] 432 Add direct bug reporting links to Feedback section

2013-10-16 Thread Petr Spacek

On 17.10.2013 03:06, Simo Sorce wrote:

On Wed, 2013-10-16 at 21:59 +0200, Petr Spacek wrote:

On 16.10.2013 15:31, Martin Kosek wrote:

This change should enable faster and easier filing of new bugs. Patch
also simplifies the section for both redhat and fedora variants.

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


Hmm, is there a way to add the "Report a bug" link to each page footer (in
HTML output)? I think that people should see this option all the time.


This recalls me another thing:
Could we add TICKET_CREATE privilege to anonymous 'subject' in the Trac? This
should allow anyone to create ticket even without registration/logging in,
which lowers the barrier.


Bad idea, you'll soon be submerge by the worst looking spam, seriously,
don't do it.

Besides you wouldn't be able to notify the reporter that you need more
info and so on, its not worth to have fire-and-forget reports.


There is an input box for reporter's e-mail...


TICKET_CREATE is not enough for any modification to existing tickets/wiki, so
we should be safe. The worst case is that we will have to remove few tickets
with gibberish and revoke the permission.

I did that for https://fedorahosted.org/bind-dyndb-ldap/ , feel free to create
testing tickets there (don't forget to log-out :-).


--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH][DOC] 432 Add direct bug reporting links to Feedback section

2013-10-16 Thread Simo Sorce
On Wed, 2013-10-16 at 21:59 +0200, Petr Spacek wrote:
> On 16.10.2013 15:31, Martin Kosek wrote:
> > This change should enable faster and easier filing of new bugs. Patch
> > also simplifies the section for both redhat and fedora variants.
> >
> > https://fedorahosted.org/freeipa/ticket/3754
> 
> Hmm, is there a way to add the "Report a bug" link to each page footer (in 
> HTML output)? I think that people should see this option all the time.
> 
> 
> This recalls me another thing:
> Could we add TICKET_CREATE privilege to anonymous 'subject' in the Trac? This 
> should allow anyone to create ticket even without registration/logging in, 
> which lowers the barrier.

Bad idea, you'll soon be submerge by the worst looking spam, seriously,
don't do it.

Besides you wouldn't be able to notify the reporter that you need more
info and so on, its not worth to have fire-and-forget reports.

> TICKET_CREATE is not enough for any modification to existing tickets/wiki, so 
> we should be safe. The worst case is that we will have to remove few tickets 
> with gibberish and revoke the permission.
> 
> I did that for https://fedorahosted.org/bind-dyndb-ldap/ , feel free to 
> create 
> testing tickets there (don't forget to log-out :-).



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

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


Re: [Freeipa-devel] [PATCHES] 0072-0074 Add automember rebuild membership to the web UI

2013-10-16 Thread Ana Krivokapic

On 09/27/2013 04:38 PM, Petr Vobornik wrote:

On 09/25/2013 11:51 AM, Ana Krivokapic wrote:

Hello,

This patch set addresses ticket 
https://fedorahosted.org/freeipa/ticket/3928.


Patch 0072 hooks the new automember-rebuild command to the web UI 
(user and host

pages).
Patch 0073 adds some fixes to the web UI test driver, which are 
necessary for

patch 0074.
Patch 0074 adds web UI integration tests for the new feature.

The patch set applies on top of my patches 0068-0071



patch 72: Me, Ana and Kyle discussed position of the actions and the 
decision was to move them to action list.


`ipa automember-rebuild --type=hostgroup` can't be run from UI 
(already discussed with Ana as well)


I'm thinking about adding/creating refresh facet policies for this 
action so that appropriate facet would be marked as dirty and 
therefore refreshed so user would not have click association facet 
refresh button.


patch 73: Looks OK but some changes might not be needed.

patch 74: I would use different methods for certain actions to be 
consistent with other tests and to make the test more robust against 
Web UI refactoring:


1.

self.click_on_link('Refresh')


For buttons in .facet-controls use rather 
`self.facet_button_click('refresh')` where 'refresh' is the button 
name, not text.


2.

self.add_record(
'automember',
{'pkey': 'webservers', 'add': [
('selectbox', 'key', 'fqdn'),
('textbox', 'automemberinclusiveregex', '^web[1-9]+')
]},
facet='hostgrouprule',
facet_btn_css_class='widget',
navigate=False
)


use `add_table_record('automemberinclusiveregex', data, parent)`. 
Example in test_dns.py:94.


3. A nitpick(or not even that): When working on one entity, I would 
rather use `navigate_by_breadcrumb('link text') instead of direct 
`navigate_to_record` call which changes url. It resembles user's 
behavior more. But it depends on situation. Ie. when I'm on hostgroup 
page and want to go to host search page, but the last host page was 
some details or association I may just use `navigate_to_entity('host',
'search'). Anyway, the important thing is to avoid navigating to the 
same url twice in a row - IE10 does not like that.


4. Do not use `wait(1)` for AJAX calls. The test will fail if there is 
bigger delay. User rather `wait_for_request(n=X)` where X is number of 
AJAX calls you want to wait for.


5. Instead of `click_on_link('Rebuild auto membership')` you should 
use `action_panel_action(panel_name, action)` There is also similar 
call for

executing action list action: `action_list_action(action_name)`.

6. Nitpick: you can reduce the cleanup code:

self.navigate_by_menu('identity/host')
self.delete('host', [{'pkey': host1}])
self.delete('host', [{'pkey': host2}])


Can be written as:

self.delete('host', [{'pkey': host1}, {'pkey': host2}])

or
self.navigate_by_menu('identity/host')
self.delete_record('host', [host1, host2])

`delete` is basically a shortcut for `navigate_to_entity` and 
`delete_record` with CRUD data format.


All fixed, new patches attached.

--
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

>From 7559aeda6632e39c0bd912c567d7eccc5c7710f2 Mon Sep 17 00:00:00 2001
From: Ana Krivokapic 
Date: Wed, 25 Sep 2013 11:29:31 +0200
Subject: [PATCH] Add automember rebuild command to the web UI

Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership
https://fedorahosted.org/freeipa/ticket/3928
---
 install/ui/src/freeipa/automember.js | 44 
 install/ui/src/freeipa/host.js   | 18 ++-
 install/ui/src/freeipa/user.js   | 16 +++--
 install/ui/test/data/ipa_init.json   | 10 
 ipalib/plugins/internal.py   | 10 
 5 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/install/ui/src/freeipa/automember.js b/install/ui/src/freeipa/automember.js
index f8083b89e49ec270ed6170f9826ef4cd45e08865..2dff34227351c582d028a43bc71bc2fe6a7d3142 100644
--- a/install/ui/src/freeipa/automember.js
+++ b/install/ui/src/freeipa/automember.js
@@ -699,12 +699,55 @@ IPA.automember.default_group_widget = function(spec) {
 return that;
 };
 
+IPA.automember.rebuild_action = function(spec) {
+
+spec = spec || {};
+spec.name = spec.name || 'automember_rebuild';
+spec.label = spec.label || '@i18n:actions.automember_rebuild';
+
+var that = IPA.action(spec);
+
+that.execute_action = function(facet) {
+var entity = facet.entity.name;
+if (facet.name == 'search') {
+var entries = facet.get_selected_values();
+} else {
+entries = facet.get_pkeys();
+}
+
+var options = {};
+if (entries.length > 0) {
+options[entity + 's'] = entries;
+} else if (entity == 'user') {
+options['type'] = 'group';
+} else {
+options['type'] = 'hostgroup

Re: [Freeipa-devel] [PATCH][DOC] 432 Add direct bug reporting links to Feedback section

2013-10-16 Thread Petr Spacek

On 16.10.2013 15:31, Martin Kosek wrote:

This change should enable faster and easier filing of new bugs. Patch
also simplifies the section for both redhat and fedora variants.

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


Hmm, is there a way to add the "Report a bug" link to each page footer (in 
HTML output)? I think that people should see this option all the time.



This recalls me another thing:
Could we add TICKET_CREATE privilege to anonymous 'subject' in the Trac? This 
should allow anyone to create ticket even without registration/logging in, 
which lowers the barrier.


TICKET_CREATE is not enough for any modification to existing tickets/wiki, so 
we should be safe. The worst case is that we will have to remove few tickets 
with gibberish and revoke the permission.


I did that for https://fedorahosted.org/bind-dyndb-ldap/ , feel free to create 
testing tickets there (don't forget to log-out :-).


--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES] 106-113 Access raw LDAP values directly from LDAPEntry

2013-10-16 Thread Petr Viktorin

On 10/14/2013 10:59 AM, Jan Cholasta wrote:

On 10.10.2013 09:45, Jan Cholasta wrote:

On 9.10.2013 13:57, Petr Viktorin wrote:

On 09/26/2013 02:22 PM, Jan Cholasta wrote:

On 24.9.2013 15:35, Jan Cholasta wrote:

On 27.2.2013 16:31, Jan Cholasta wrote:

Hi,

these patches add the ability to access and manipulate raw attribute
values as they are returned from python-ldap to the LDAPEntry class.
This is useful for comparing entries, computing modlists for the
modify
operation, deleting values that don't match the syntax of an
attribute,
etc., because we don't need to care what syntax does a particular
attribute use, or what Python type is used for it in the framework
(raw
values are always list of str). It should also make interaction with
non-389 DS LDAP servers easier in the framework.

(It might be too late for this kind of changes to get into 3.2 now,
I'm
posting these patches mainly so that you are aware that they exist.)

Honza



This is now planned for 3.4:


I fixed some issues in these patches and refined the API. Updated
patches attached.

Also added a patch to use raw values when adding new entries and a
patch
which refines LDAPEntry.single_value, so that it is consistent with
the
rest of the changes introduced in the patches.

Patch 110 will probably be dropped in favor of Petr Viktorin's schema
update patches, but I included it anyway.

Incidentally, this also fixes
 and possibly also
.

Honza



Noticed a couple more issues and fixed them. Updated patches attached.

Honza


Thanks for the patches!


106. Make LDAPEntry a wrapper around dict rather than a dict subclass.


Git rants about one more whitespace error.

[...]

+def __eq__(self, other):
+if not isinstance(other, LDAPEntry):
+return NotImplemented
+if self._dn != other._dn:
+return False
+return super(LDAPEntry, self).__eq__(other)

I don't think equality checking makes sense on a LDAPEntry, where you
might have different capitalizations/variants of attribute names,
different _orig, or a different set of attributes loaded on the same
entry. It's not obvious which of those differences should make the
entries inequal.
I'd just base it on identity (`self is other`).


Right, I'm not sure why I even did it this way. But I remember seeing
some code that did comparison of entries with ==...


Thanks.
Please also implement __ne__() when reimplementing __eq__().





  def __iter__(self):
  yield self._dn
  yield self

This makes the whole thing sort of hackish -- we need to reimplement
everything in MutableMapping that uses iter() internally :(
Hopefully we can get rid of it soon.


Yes, it's a shame MutableMapping uses iter() instead of iterkeys().


I'd welcome FIXME comments on whatever is reimplemented for this reason.


I thought the comment above __iter__ would be enough. Apparently I was
wrong.


Right, IMO it's not immediately obvious that these are reimplemented 
because they depend on __iter__.


[...]

109. Decode and encode attribute values in LDAPEntry on demand.

The syncing looks rather over-engineered to me.
Did you consider a custom MutableSequence for the values?
I think it would be much cleaner in the end than merging two sets of
changes together.


I'm not entirely happy about it either, but it works. I did consider a
custom sequence type, but I didn't feel like it was the right time to
this kind of change in this patchset.

Unlike the (DN, dict) -> LDAPEntry
transition, this change won't be backward compatible and there is a lot
of isinstance(value, list) and entry[attr] = list() kind of things in
the framework code.


That's what I was afraid of.

We could live with `isinstance(value, list)`; hopefully we could get rid 
of `type(value) == list` that is the real problem.

With `entry[attr] = list()` we could convert automatically.

But OK, let's settle for a worse solution in the meantime.


To be frank I don't particularly like the LDAPEntryView.
While the dict-like interface is great, there isn't a case for storing a 
Raw view long-term, i.e. you'd always want to do something like

values = entry.raw[x]
...
entry.raw[x] = new_values
rather than
raw = entry.raw
values = raw[x]
...
raw[x] = new_values
The latter is confusing because LDAPEntryView and RawLDAPEntryView are 
two classes that have exactly the same interface, but do something 
different. In a duck-typed world that's a recipe for disaster.
I think it would be better if the view implemented just the dict 
protocol, and not `conn`, `dn`, `nice`, `raw`, etc.
The code would also be much simpler without the elaborate view class 
hierarchy.


If you don't agree then at least don't make `raw` available on raw views 
and `nice` on nice views; the programmer *always* needs to know which 
version they're working with so these aren't 

Re: [Freeipa-devel] [PATCH] 448 Load updated Web UI files after server upgrade

2013-10-16 Thread Petr Vobornik

On 10/15/2013 01:51 PM, Ana Krivokapic wrote:

On 10/02/2013 12:45 PM, Petr Vobornik wrote:

On 09/27/2013 09:16 AM, Ana Krivokapic wrote:

On 08/30/2013 05:21 PM, Petr Vobornik wrote:


snip



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



I tested the patch and it seems to work fine. Code-wise it looks good as well.

Nitpick: There is an unused function 'updated()' in the new loader.js.in file.



Unused function removed. Updated patch attached.


ACK


Pushed to master
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 451-458 Web UI devel and source code documentation

2013-10-16 Thread Petr Vobornik

On 10/15/2013 03:30 PM, Ana Krivokapic wrote:

On 10/02/2013 02:20 PM, Petr Vobornik wrote:

On 09/16/2013 05:24 PM, Ana Krivokapic wrote:

On 09/11/2013 12:44 PM, Petr Vobornik wrote:

Hello,



snip





I looked into the documentation effort and (ruby dependency discussion aside) I
don't have any major objections. I like how the generated pages look, and they
are intuitive and easy to navigate.

A couple of nitpicks:

1) There are some spelling mistakes (e.g. Apllication_controller)

Fixed


2) Bulleted lists are not rendered nicely in the html output (see for example
the doc string for _base.Builder property 'string_mode'. I think a list needs to
look like this in the source code:

  /**
   * This is a list:
   *
   * - 'element1'
   * - 'element2'
   *
   */

as opposed to this:

  /**
   * This is a list:
   * - 'element1'
   * - 'element2'
   */



Fixed on many places. Also fixed the same issue in some code examples.

- _base.Builder doc was heavily revised
- added doc comments to ./plugin_loader

All patches are rebased but just patches 452 and 453 are changed.


Looks good, ACK.



Pushed to master

--
Petr Vobornik

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


[Freeipa-devel] [PATCH] 0125 Trusts documentation update

2013-10-16 Thread Alexander Bokovoy

Hi!

Attached is first update to AD trusts documentation for FreeIPA user
guide. I've fixed number of outdated statements and added some more
material.

More patches will follow to cover functionality up to FreeIPA 3.3.2.

--
/ Alexander Bokovoy
>From cb5af31974186d7f3c2acc67bdb2448deecbe02c Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Wed, 16 Oct 2013 18:25:55 +0300
Subject: [PATCH] Update Trust.xml to be on par with FreeIPA 3.3

---
 src/user_guide/en-US/Trust.xml | 240 -
 1 file changed, 165 insertions(+), 75 deletions(-)

diff --git a/src/user_guide/en-US/Trust.xml b/src/user_guide/en-US/Trust.xml
index 0f89c79..0d8cba7 100644
--- a/src/user_guide/en-US/Trust.xml
+++ b/src/user_guide/en-US/Trust.xml
@@ -2,12 +2,19 @@
 http://www.oasis-open.org/docbook/xml/4.5/docbookx.dtd"; [
 ]>
 
-   Identity: Integrating with Active Directory Through Cross-Realm 
Kerberos Trusts &TITLE_TPREVIEW;
-   
-   Kerberos allows the configuration of trusted 
realms. Each realm has its own
-   resources and users, yet the trust relationship allows users of 
any trusted realm to obtain tickets
-   and connect to machines or services in a peer realm as if they 
were members of that peer realm.
-   
+Identity: Integrating with Active Directory Through Cross-Realm 
Kerberos Trusts &TITLE_TPREVIEW;
+
+Active Directory is a Microsoft's implementation of LDAP, Kerberos,
+SMB, and few other protocol families.  While there are many differences
+in the way these protocol families are implemented, in its core
+trusting one &AD; domain to another means establishing relationships
+between the two domains on the Kerberos protocol level.  Kerberos
+allows the configuration of trusted realms. Each
+realm has its own resources and users, yet the trust relationship
+allows users of any trusted realm to obtain tickets and connect to
+machines or services in a peer realm as if they were members of that
+peer realm.
+

Because of differences in the way that Windows and Linux 
domains implement LDAP services, DNS
management, and even Kerberos realms, it is difficult to 
establish a direct trust between &AD;
@@ -15,6 +22,18 @@
the Kerberos trust and DNS mappings so that &AD; users can 
access Linux hosts and services
completely transparently, using one set of credentials.

+
+&AD; was implemented on top of existing domain membership as provided 
by SMB
+protocol. In order to give short transition path for existing
+deployments, many of the concepts from SMB protocol are used
+internally by &AD;. Trust relationship is one of these; establishing
+trust between two domains, in fact, requires execution of an SMB
+command sequence that leads to creation of specialized accounts in LDAP
+storage of domain controllers in both domains. When this step is
+performed, resulting accounts can be used to perform Kerberos
+authentication against the other domain and represent shared ticket and
+key for Kerberos cross-realm trust.
+
 
The Meaning of "Trust"

@@ -45,13 +64,19 @@



-   Samba, to perform identity 
lookups on &AD; and retrieve user and group security identifiers (SIDs) for 
authorization information
-   
+Samba, to perform SMB protocol operations against
+domain controllers in &AD; and represent points of
+communication that &AD; domain controllers expect to
+exist in another &AD; domain
+



-   SSSD, to cache user, group, and 
ticket information for users and to map Kerberos and DNS domains
-   
+SSSD, to query and cache user, group, and Kerberos
+ticket information for users from &AD;. SSSD also maps
+Security Identifiers of user and group objects on &AD;
+side to user and group identifiers on &IPA; side
+



@@ -86,17 +111,36 @@



-   Trusts, then, are essentially unidirectional. 
&AD; users can access &IPA; resources and services, but &IPA; users
- 

Re: [Freeipa-devel] [PATCH 0119] ipatests: Extend the order plugin to properly handle inheritance

2013-10-16 Thread Tomas Babej

On 10/16/2013 01:57 PM, Petr Viktorin wrote:

On 10/14/2013 04:28 PM, Tomas Babej wrote:

Hi,

When trying to create a new ordered test case by inheriting
from already defined test case, by overriding few of its methods,
the execution order of the tests is as follows:
 - first all non-overriden test methods from the parent test class
 - then all overriden tests methods


That is not strictly true: if the base class is defined in a different 
file, with larger line numbers, with this patch its tests would run last.



This patch makes sure that methods are executed in the logical order,
that is, the order defined in the parent class.


We'll also need to support longer inheritance chains than just one 
parent.
The sort should look at the first @ordered superclass where the method 
was defined, and run the methods defined at the beginning of the 
inheritance chain first.


I've modified your patch to accomplish this, does it look OK?



Thanks! This will even work with cross-module imports.

I added a section to the Integration testing design page: 
http://www.freeipa.org/page/V3/Integration_testing#Ordering_of_the_tests


Also, I changed the commit message and comments to reflect the new 
behaviour.


Updated patch attached.

Tomas

--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

From 122a66eab57c18d60bd408ccfa9d8c9860e93873 Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Mon, 14 Oct 2013 14:28:24 +0200
Subject: [PATCH] ipatests: Extend the order plugin to properly handle
 inheritance

When trying to create a new ordered test case by inheriting
from already defined test case, by overriding few of its methods,
the execution order of the tests is as follows:
- first all non-overriden test methods from the parent test class
- then all overriden tests methods

This patch makes sure that methods are executed in the logical order,
that is, the order defined in the parent class.
---
 ipatests/order_plugin.py | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/ipatests/order_plugin.py b/ipatests/order_plugin.py
index 9ecff32adb23249bb152b031654e2f0883826e57..7b114a56783a6b3f2e34498567a61c2668ab5c08 100644
--- a/ipatests/order_plugin.py
+++ b/ipatests/order_plugin.py
@@ -69,8 +69,31 @@ class OrderTests(Plugin):
 return False
 return loader.selector.wantMethod(item)
 
+def sort_with_respect_to_overriding(func):
+"""
+Sorts the methods in respect with the parent classes.
+
+The methods are sorted with respect to the inheritance chain,
+methods that were defined in the same class are sorted by the line
+number on which they were defined.
+"""
+
+# Check each *ordered* class in MRO for definition of func method
+for i, parent_class in enumerate(reversed(cls.mro())):
+if getattr(parent_class, '_order_plugin__ordered', False):
+method = getattr(parent_class, func.__name__, None)
+if method:
+# This sorts methods as tuples  (position of the class
+# in the inheritance chain, position of the method
+# within that class)
+return 0, i, method.func_code.co_firstlineno
+
+# Weird case fallback
+# Method name not in any of the classes in MRO, run it last
+return 1, func.func_code.co_firstlineno
+
 methods = [getattr(cls, case) for case in dir(cls) if wanted(case)]
-methods.sort(key=lambda m: m.func_code.co_firstlineno)
+methods.sort(key=sort_with_respect_to_overriding)
 cases = [loader.makeTest(m, cls) for m in methods]
 return cases
 
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCHES 100-106] Initial implementation of AD integration tests

2013-10-16 Thread Petr Viktorin

On 10/14/2013 05:47 PM, Tomas Babej wrote:

Hi,

Sending the updated version of the patchset. It includes many fixes,
plus incorporations of the comments by the QE. Tests are now broken down
into multiple test cases.

This requires my patch 119.

Tomas


Patches 0100 & 0101:

I still think it would be simpler if IPA and AD domains shared the 
numbering namespace (users would need to define $AD_env2; if they had 
$MASTER_env1 and $AD_env1 they would be in the same Domain).


But if we use _env1 for both the AD and the IPA domain, they need to be 
separated in Domain.from_env. With patch 0101 both MASTER_env1 and 
AD_env1 will be in both domains[0] and ad_domains[0].
Also ipa-test-config should have an --ad-domain option to get info about 
the AD domain.



Patch 0102: Looks good


Patch 0103:
Looks like run_repeatedly should always be called with an assert in 
front. We may want it to raise an exception directly if it times out, so 
forgetting the assert won't hide errors.



Patch 0104: Looks good


Patch 0105:
In /ipatests/man/ipa-test-task.1:
- Typo in  - s/generatee/generate/
- Subcommand names have unescaped hyphens (e.g. configure\-dns-for-trust 
should be configure\-dns\-for\-trust)

- The last subcommand should be sync-time

Wrappers for the tasks need to be added to ipatests/ipa-test-task.

Nitpicks:
- In configure_dns_for_trust:is_subdomain, lockstep iteration over two 
lists at once is better done with zip() than `for i in range(len(...))`.
- In estabilish_trust_with_ad, don't use mutable values for argument 
defaults; do `extra_args=()` and `+ list(extra_args)`
- I can't say I'm a fan of configure_auth_to_local_rule, but I guess 
it's OK for tests.



Patch 0106: Looks OK; unfortunately I don't have an AD machine to test 
functionality




--
Petr³

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


[Freeipa-devel] [PATCH][DOC] 432 Add direct bug reporting links to Feedback section

2013-10-16 Thread Martin Kosek
This change should enable faster and easier filing of new bugs. Patch
also simplifies the section for both redhat and fedora variants.

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

-- 
Martin Kosek 
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.
From 5970f2591734edba76c2d66a96016f32858e00fc Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Wed, 16 Oct 2013 15:30:04 +0200
Subject: [PATCH] Add direct bug reporting links to Feedback section

This change should enable faster and easier filing of new bugs. Patch
also simplifies the section for both redhat and fedora variants.

https://fedorahosted.org/freeipa/ticket/3754
---
 src/user_guide/en-US/Feedback.xml | 111 ++
 1 file changed, 29 insertions(+), 82 deletions(-)

diff --git a/src/user_guide/en-US/Feedback.xml b/src/user_guide/en-US/Feedback.xml
index b79ec0ca3ec023f55728255d34e2514e6fefb5d1..4d36b3fd8554ddae9df6b0d5b2a8a817223494ac 100644
--- a/src/user_guide/en-US/Feedback.xml
+++ b/src/user_guide/en-US/Feedback.xml
@@ -3,86 +3,33 @@
 ]>
 
 
-	Giving Feedback
-	
-		If there is any error in this book or there is any way to improve the
-		documentation, please let us know. Bugs can be filed against the documentation for &IPA; through
-		Bugzilla, http://bugzilla.redhat.com/bugzilla";>http://bugzilla.redhat.com/bugzilla. Make the bug report as specific as
-		possible, so we can be more effective in correcting any issues:
-	
-
-
-	
-		
-			
-Select the &RH; group and the &RHEL; &RVER; product.
-			
-		
-		
-			
-Set the component to &BOOKID;.
-			
-		
-		
-			
-For errors, give the page number (for the PDF) or URL (for the HTML), and give a
-succinct description of the problem, such as incorrect procedure or typo.
-			
-			
-For enhancements, put in what information needs to be added and why.
-			
-		
-		
-			
-Give a clear title for the bug. For example, "Incorrect command example for
-setup script options" is better than "Bad example".
-			
-		
-	
-	
-		We appreciate receiving any feedback — requests for new sections, corrections, improvements, enhancements, even
-		new ways of delivering the documentation or new styles of docs. You are welcome to contact &RH; Content Services
-		directly at d...@redhat.com.
-	
-
-
-
-
-	
-		
-			
-Select the Community group and the freeIPA product.
-			
-		
-		
-			
-Set the component to Documentation.
-			
-		
-		
-			
-Set the version number to &VER;.
-			
-		
-		
-			
-For errors, give the page number (for the PDF) or URL (for the HTML), and give a
-succinct description of the problem, such as incorrect procedure or typo.
-			
-			
-For enhancements, put in what information needs to be added and why.
-			
-		
-		
-			
-Give a clear title for the bug. For example, "Incorrect command example for
-setup script options" is better than "Bad example".
-			
-		
-	
-	
-		We appreciate receiving any feedback — requests for new sections, corrections, improvements, enhancements, even
-		new ways of delivering the documentation or new styles of docs. You are welcome to contact the Fedora docs team at d...@lists.fedoraproject.org.
-	
-
+Giving Feedback
+
+If there is any error in this book or there is any way to improve the
+documentation, please let us know. Make the bug report as specific as
+possible, so we can be more effective in correcting any issues.
+
+
+Bugs can be filed both to upstream ∏
+https://fedorahosted.org/freeipa/";>Trac instance,
+by selecting a Documentation component
+(https://fedorahosted.org/freeipa/newticket?component=Documentation";>direct
+link, requires logging in first), or to
+http://bugzilla.redhat.com/";>Red Hat Bugzilla,
+by selecting a freeipa component
+(https://bugzilla.redhat.com/enter_bug.cgi?bug_status=NEW&component=freeipa&form_name=enter_bug&product=Fedora";>direct link,
+requires logging in first).
+
+
+Bugs can be filed to
+http://bugzilla.redhat.com/";>Red Hat Bugzilla,
+by selecting a ipa component
+(https://bugzilla.redhat.com/enter_bug.cgi?bug_status=NEW&component=ipa&form_name=enter_bug&product=Red%20Hat%20Enterprise%20Linux%206";>direct link,
+requires logging in first).
+
+
+We appreciate receiving any feedback — requests for new sections,
+corrections, improvements, enhancements, even new ways of delivering
+the documentation or new styles of docs.
+
 
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH 0119] ipatests: Extend the order plugin to properly handle inheritance

2013-10-16 Thread Petr Viktorin

On 10/14/2013 04:28 PM, Tomas Babej wrote:

Hi,

When trying to create a new ordered test case by inheriting
from already defined test case, by overriding few of its methods,
the execution order of the tests is as follows:
 - first all non-overriden test methods from the parent test class
 - then all overriden tests methods


That is not strictly true: if the base class is defined in a different 
file, with larger line numbers, with this patch its tests would run last.



This patch makes sure that methods are executed in the logical order,
that is, the order defined in the parent class.


We'll also need to support longer inheritance chains than just one parent.
The sort should look at the first @ordered superclass where the method 
was defined, and run the methods defined at the beginning of the 
inheritance chain first.


I've modified your patch to accomplish this, does it look OK?

--
Petr³
From de0bcf5a3ba1e715bac44e4c3007fa8c6a4bdbbf Mon Sep 17 00:00:00 2001
From: Tomas Babej 
Date: Mon, 14 Oct 2013 14:28:24 +0200
Subject: [PATCH] ipatests: Extend the order plugin to properly handle
 inheritance

When trying to create a new ordered test case by inheriting
from already defined test case, by overriding few of its methods,
the execution order of the tests is as follows:
- first all non-overriden test methods from the parent test class
- then all overriden tests methods

This patch makes sure that methods are executed in the logical order,
that is, the order defined in the parent class.
---
 ipatests/order_plugin.py | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ipatests/order_plugin.py b/ipatests/order_plugin.py
index 9ecff32adb23249bb152b031654e2f0883826e57..c09d4db2d185b6750e3c35cce86d98fd80f7e342 100644
--- a/ipatests/order_plugin.py
+++ b/ipatests/order_plugin.py
@@ -69,8 +69,27 @@ def wanted(attr):
 return False
 return loader.selector.wantMethod(item)
 
+def sort_with_respect_to_overriding(func):
+"""
+Sorts the methods in respect with the parent class. If the method
+is overriding a method from a parent ordered class, returns the
+position of the method from the parent class.
+"""
+
+# Check each *ordered* parent class for definition of func method
+for i, parent_class in enumerate(reversed(cls.mro())):
+if getattr(parent_class, '_order_plugin__ordered', False):
+method = getattr(parent_class, func.__name__, None)
+if method:
+# If it was defined, return the position of the parent
+# function
+return 0, i, method.func_code.co_firstlineno
+
+# Method not found in superclasses, run it last
+return 1, func.func_code.co_firstlineno
+
 methods = [getattr(cls, case) for case in dir(cls) if wanted(case)]
-methods.sort(key=lambda m: m.func_code.co_firstlineno)
+methods.sort(key=sort_with_respect_to_overriding)
 cases = [loader.makeTest(m, cls) for m in methods]
 return cases
 
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH] 0010 ipa-client-install does not clean up /etc/ipa/ca.crt after a failed attempt

2013-10-16 Thread Martin Kosek
On 10/16/2013 10:52 AM, Martin Basti wrote:
> On Wed, 2013-10-16 at 10:36 +0200, Martin Kosek wrote:
>> On 10/16/2013 10:10 AM, Martin Basti wrote:
>>> On Tue, 2013-10-15 at 11:37 +0200, Martin Basti wrote:
 Added warning if cert. exists (client)

 https://fedorahosted.org/freeipa/ticket/3944
>>
>> 1) Patch subject uses wrong path to cert
>>
> Fixed
>> 2) The warning seems to chatty to me:
>>
>> # ipa-client-install -p admin -w kokos123
>> Certificate '/etc/ipa/ca.crt' exists and will be used. Make sure that
>> certificate is valid (or remove it), otherwise client will not be able to 
>> join.
>> Discovery was successful!
>> ...
>>
>> We just want to notify user that we are using the cert and what is the path 
>> (as
>> this is something new to FreeIPA newbies), this seems easier to read to me:
>>
>> # ipa-client-install -p admin -w kokos123
>> Using existing certificate /etc/ipa/ca.crt
>> Discovery was successful!
>> ...
>>
> Fixed
> 

ACK! Pushed to master.

Thanks,
Martin

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


Re: [Freeipa-devel] [PATCH] 0010 ipa-client-install does not clean up /etc/ipa/ca.crt after a failed attempt

2013-10-16 Thread Martin Basti
On Wed, 2013-10-16 at 10:36 +0200, Martin Kosek wrote:
> On 10/16/2013 10:10 AM, Martin Basti wrote:
> > On Tue, 2013-10-15 at 11:37 +0200, Martin Basti wrote:
> >> Added warning if cert. exists (client)
> >>
> >> https://fedorahosted.org/freeipa/ticket/3944
> 
> 1) Patch subject uses wrong path to cert
> 
Fixed
> 2) The warning seems to chatty to me:
> 
> # ipa-client-install -p admin -w kokos123
> Certificate '/etc/ipa/ca.crt' exists and will be used. Make sure that
> certificate is valid (or remove it), otherwise client will not be able to 
> join.
> Discovery was successful!
> ...
> 
> We just want to notify user that we are using the cert and what is the path 
> (as
> this is something new to FreeIPA newbies), this seems easier to read to me:
> 
> # ipa-client-install -p admin -w kokos123
> Using existing certificate /etc/ipa/ca.crt
> Discovery was successful!
> ...
> 
Fixed

> Martin

-- 
Martin Basti
>From 1ba0ef838dab971f51005cae0bd8f9e39f398ee3 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 15 Oct 2013 11:31:49 +0200
Subject: [PATCH] Added warning if cert '/etc/ipa/ca.crt' exists

https://fedorahosted.org/freeipa/ticket/3944
---
 ipa-client/ipa-install/ipa-client-install | 4 
 1 file changed, 4 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 3c78c844b17468f347ef04198d58a12b11e4b4cb..cf27788f8c189721a1f644fa5841466abfbca54e 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1889,6 +1889,10 @@ def install(options, env, fstore, statestore):
 root_logger.warning("Option 'force-join' has no additional effect "
 "when used with together with option 'keytab'.")
 
+# Check if old certificate exist and show warning
+if not options.ca_cert_file and get_cert_path(options.ca_cert_file) == CACERT:
+root_logger.warning("Using existing certificate '%s'.", CACERT)
+
 # Create the discovery instance
 ds = ipadiscovery.IPADiscovery()
 
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH] 0010 ipa-client-install does not clean up /etc/ipa/ca.crt after a failed attempt

2013-10-16 Thread Martin Kosek
On 10/16/2013 10:10 AM, Martin Basti wrote:
> On Tue, 2013-10-15 at 11:37 +0200, Martin Basti wrote:
>> Added warning if cert. exists (client)
>>
>> https://fedorahosted.org/freeipa/ticket/3944

1) Patch subject uses wrong path to cert

2) The warning seems to chatty to me:

# ipa-client-install -p admin -w kokos123
Certificate '/etc/ipa/ca.crt' exists and will be used. Make sure that
certificate is valid (or remove it), otherwise client will not be able to join.
Discovery was successful!
...

We just want to notify user that we are using the cert and what is the path (as
this is something new to FreeIPA newbies), this seems easier to read to me:

# ipa-client-install -p admin -w kokos123
Using existing certificate /etc/ipa/ca.crt
Discovery was successful!
...

Martin

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


[Freeipa-devel] [PATCH] 431 Installer should always wait until CA starts up

2013-10-16 Thread Martin Kosek
Patch for ticket 3964 changed the installer so that it does not
always wait for CA if the proxy is not configured. However,
it was found out that it may freeze an installation when
a step subsequent after CA restart call the CA and receives no
reply.

Change the wait so that it always waits for CA to start up. If
HTTP proxy is already configured, it should wait on port 443.
If not, it should wait on local PKI port 8443.

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

-- 
Martin Kosek 
Supervisor, Software Engineering - Identity Management Team
Red Hat Inc.
From d1ee8e86c50ff35f3a22e8377e969ceb7c7d19ec Mon Sep 17 00:00:00 2001
From: Martin Kosek 
Date: Wed, 16 Oct 2013 09:58:23 +0200
Subject: [PATCH] Installer should always wait until CA starts up

Patch for ticket 3964 changed the installer so that it does not
always wait for CA if the proxy is not configured. However,
it was found out that it may freeze an installation when
a step subsequent after CA restart call the CA and receives no
reply.

Change the wait so that it always waits for CA to start up. If
HTTP proxy is already configured, it should wait on port 443.
If not, it should wait on local PKI port 8443.

https://fedorahosted.org/freeipa/ticket/3973
---
 ipapython/dogtag.py| 10 +++---
 ipapython/platform/fedora16/service.py |  7 ---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index ec3f2beb8d4feb283570b8cc21be8ee08f89c983..ea769b0275c4642d5da457996165e5a348cb7299 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -184,7 +184,7 @@ def get_ca_certchain(ca_host=None, dogtag_constants=None):
 return chain
 
 
-def ca_status(ca_host=None):
+def ca_status(ca_host=None, use_proxy=True):
 """Return the status of the CA, and the httpd proxy in front of it
 
 The returned status can be:
@@ -194,9 +194,13 @@ def ca_status(ca_host=None):
 """
 if ca_host is None:
 ca_host = api.env.ca_host
-# Use port 443 to test the proxy as well
+if use_proxy:
+# Use port 443 to test the proxy as well
+ca_port = 443
+else:
+ca_port = 8443
 status, reason, headers, body = unauthenticated_https_request(
-ca_host, 443, '/ca/admin/ca/getStatus')
+ca_host, ca_port, '/ca/admin/ca/getStatus')
 if status == 503:
 # Service temporarily unavailable
 return reason
diff --git a/ipapython/platform/fedora16/service.py b/ipapython/platform/fedora16/service.py
index 36e7a31c41307e16178a9bb5ec9491063d3d7213..edf2d7ff824399171f59a72a9b8fb49b1c4b08df 100644
--- a/ipapython/platform/fedora16/service.py
+++ b/ipapython/platform/fedora16/service.py
@@ -143,17 +143,18 @@ def __wait_until_running(self):
 # Unfortunately, knownservices.httpd.is_installed() can return
 # false positives, so check for existence of our configuration file.
 # TODO: Use a cleaner solution
+use_proxy = True
 if not (os.path.exists('/etc/httpd/conf.d/ipa.conf') and
 os.path.exists('/etc/httpd/conf.d/ipa-pki-proxy.conf')):
 root_logger.debug(
-'The httpd proxy is not installed, skipping wait for CA')
-return
+'The httpd proxy is not installed, wait on local port')
+use_proxy = False
 root_logger.debug('Waiting until the CA is running')
 timeout = api.env.startup_timeout
 op_timeout = time.time() + timeout
 while time.time() < op_timeout:
 try:
-status = dogtag.ca_status()
+status = dogtag.ca_status(use_proxy=use_proxy)
 except Exception:
 status = 'check interrupted'
 root_logger.debug('The CA status is: %s' % status)
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH] 0010 ipa-client-install does not clean up /etc/ipa/ca.crt after a failed attempt

2013-10-16 Thread Martin Basti
On Tue, 2013-10-15 at 11:37 +0200, Martin Basti wrote:
> Added warning if cert. exists (client)
> 
> https://fedorahosted.org/freeipa/ticket/3944
> ___
> Freeipa-devel mailing list
> Freeipa-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

Supress warning if user select /etc/ipa/ca.crt manually with
--ca-cert-file option

-- 
Martin Basti
>From 129183b1d8237eca61bd9219d7c44a84e1747cce Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 15 Oct 2013 11:31:49 +0200
Subject: [PATCH] Added warning if cert '/etc/ipa/ca.cert' exists

https://fedorahosted.org/freeipa/ticket/3944
---
 ipa-client/ipa-install/ipa-client-install | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 3c78c844b17468f347ef04198d58a12b11e4b4cb..00a45a8929783d3e0f71171a6a149f5633396f53 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -1889,6 +1889,12 @@ def install(options, env, fstore, statestore):
 root_logger.warning("Option 'force-join' has no additional effect "
 "when used with together with option 'keytab'.")
 
+# Check if old certificate exist and show warning
+if not options.ca_cert_file and get_cert_path(options.ca_cert_file) == CACERT:
+root_logger.warning("Certificate '%s' exists and will be used. "
+"Make sure that certificate is valid (or remove it), "
+"otherwise client will not be able to join.", CACERT)
+
 # Create the discovery instance
 ds = ipadiscovery.IPADiscovery()
 
-- 
1.8.3.1

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