Re: [Freeipa-devel] Locations design v2: LDAP schema & user interface

[Simo Sorce]

On Fri, 2016-02-19 at 08:58 +0100, Petr Spacek wrote:
> On 4.2.2016 18:21, Petr Spacek wrote:
> > On 3.2.2016 18:41, Petr Spacek wrote:
> >> Hello,
> >>
> >> I've updated the design page
> >> http://www.freeipa.org/page/V4/DNS_Location_Mechanism
> >>
> >> Namely it now contains 'Version 2'.
> > 
> > Okay, here is the idea how we can make it flexible:
> > http://www.freeipa.org/page/V4/DNS_Location_Mechanism#Implementation
> 
> Hello,
> 
> I'm thinking about LDAP schema for DNS locations.
> 
> Purpose
> ===
> * Allow admins to define any number of locations.
> * 1 DNS server advertises at most 1 location.
> * 1 location generally contains set of services with different priorities and
> weights (in DNS SRV terms).
> * Express server & service priority for each defined location in a way which
> is granular and flexible and ad the same time easy to manage.
> 
> 
> Proposal
> 
> a) Container for locations
> --
> cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> 
> 
> b) 1 location
> -
> Attributes:
> 2.16.840.1.113730.3.8.5.32 idnsLocationMember
> Server/service assigned to a DNS Location. Usually used to define 'main'
> servers for that location. Should it point to service DNs to be sure we have
> smooth upgrade to containers?
> 
> 2.16.840.1.113730.3.8.5.33 idnsBackupLocation
> Pointer to another location. Sucks in all servers from that location as one
> group with the same priority. Easy to use with _default location where all
> 'other' servers are used as backup.
> 
> These two attributes use sub-type priority and relativeweight.
> This is the only way I could express all the information without need for
> separate objects.
> 
> 
> Object classes:
> 2.16.840.1.113730.3.8.6.7  idnsLocation
> MAY ( idnsLocationMember $ idnsBackupLocation )
> 
> 
> 1st example:
> Location CZ:
> - servers czserver1, czserver2
> - priority=1
> - relative weight = 50 % each
> - if both CZ servers fail, use servers in location UK as backup (priority 2)
> - if all CZ and UK servers fail, use servers in location US as backup
> (priority 3) - servers on the other continent are used only as option of last
> resort
> DN: cn=cz,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> objectClass: idnsLocation
> idnsLocationMember;priority1;relativeweight50:
> cn=czserver1,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsLocationMember;priority1;relativeweight50:
> cn=czserver2,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority2: 
> cn=uk,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority3: 
> cn=us,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> 
> Location UK:
> - servers ukserver1, ukserver2
> - priority=1
> - server ukserver1 is a new beefy machine so it can handle 3 times more load
> than ukserver2, thus relative weights 75 % and 25 %
> - if both UK servers fail, use servers in location CZ as backup (priority 2)
> - if all CZ and UK servers fail, use servers in location US as backup
> (priority 3) - servers on the other continent are used only as option of last
> resort
> DN: cn=uk,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> objectClass: idnsLocation
> idnsLocationMember;priority1;relativeweight3:
> cn=ukserver1,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsLocationMember;priority1;relativeweight1:
> cn=ukserver2,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority2: 
> cn=uk,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority3: 
> cn=us,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> 
> Location US:
> - servers usserver1, usserver2
> - priority=1
> - relative weight = 50 % each
> - if both US servers fail, use servers in location CZ and UK as backup
> (priority 2) - it is over ocean anyway, so US clients will not make any
> difference between CZ and UK locations
> DN: cn=uk,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> objectClass: idnsLocation
> idnsLocationMember;priority1;relativeweight50:
> cn=ukserver1,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsLocationMember;priority1;relativeweight50:
> cn=ukserver2,cn=masters,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority2: 
> cn=cz,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> idnsBackupLocation;priority2: 
> cn=uk,cn=locations,cn=ipa,cn=etc,dc=example,dc=com
> 
> 
> Resulting DNS SRV records (generated by FreeIPA framework). Please note that
> only numbers in SRV records matter only relatively. Priorities work as group,
> weights are relative only inside the group. Absolute values above are used
> only in algorithm which generates SRV records:
> Location CZ:
> _kerberos._udp SRV 1 50 czserver1
> _kerberos._udp SRV 1 50 czserver2
> _kerberos._udp SRV 2 75 ukserver1
> _kerberos._udp SRV 2 25 ukserver1
> _kerberos._udp SRV 3 50 usserver1
> _kerberos._udp SRV 3 50 usserver2
> 
> Location UK:
> _kerberos._udp SRV 1 75 ukserver1
> _kerberos._udp SRV 1 25 ukserver1
> _kerberos._udp SRV 2 50 czserver1
> _kerberos._udp SRV 2 50 

Re: [Freeipa-devel] [PATCH 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Alexander Bokovoy]

On Fri, 19 Feb 2016, Martin Kosek wrote:

On 02/19/2016 03:14 PM, Alexander Bokovoy wrote:

On Fri, 19 Feb 2016, Martin Kosek wrote:

Why trust-add?

I'm not a big fan of cluttering existing commands(find, show, mod) with logic
to fix one upgrade bug. But I understand a need to communicate it somehow.

Would it make sense to move such logic to a separate command, e.g.
trust-check/trust-verify?  The command can do additional check in a future.

No. What is the value of trust-add if it says you 'Trust established and
verified' while in reality it didn't? This specific issue is very easy
to catch.


I think the point was that we are proposing and doing a non-trivial engineering
effort to do warning for one-off bug that will be fixed in next bugfix release.
I would agree if the check is more systematic, but doing a special LDAP search
(for example) from now on because of this one-off bug does not seem to me as
ideal path.

I don't want to have a separate LDAP search. We do LDAP search *already*
in trust_add because the object is actually created by Samba code as
result of our interactions with local smbd, so we anyway are reading it.


Ok.


The problem here is in the fact that somebody may restore a backup done
before the package upgrade which means whole data in LDAP will be back
to the old state even though we ran the upgrade before.


I think that is a highly unlikely situation to warrant any non-trivial
development effort. After such restore, when the admin notice that trust is not
working, they would start redoing trust-add anyway.

May be. I still think adding check for ipaNTSecurityIdentifier missing
for normal operation of trust-add is beneficial because it actually
allows us to detect problem immediately.
--
/ 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 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Alexander Bokovoy]

On Fri, 19 Feb 2016, Martin Kosek wrote:

Why trust-add?

I'm not a big fan of cluttering existing commands(find, show, mod) with logic
to fix one upgrade bug. But I understand a need to communicate it somehow.

Would it make sense to move such logic to a separate command, e.g.
trust-check/trust-verify?  The command can do additional check in a future.

No. What is the value of trust-add if it says you 'Trust established and
verified' while in reality it didn't? This specific issue is very easy
to catch.


I think the point was that we are proposing and doing a non-trivial engineering
effort to do warning for one-off bug that will be fixed in next bugfix release.
I would agree if the check is more systematic, but doing a special LDAP search
(for example) from now on because of this one-off bug does not seem to me as
ideal path.

I don't want to have a separate LDAP search. We do LDAP search *already*
in trust_add because the object is actually created by Samba code as
result of our interactions with local smbd, so we anyway are reading it.

The problem here is in the fact that somebody may restore a backup done
before the package upgrade which means whole data in LDAP will be back
to the old state even though we ran the upgrade before.
--
/ 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 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Martin Kosek]

On 02/19/2016 03:02 PM, Alexander Bokovoy wrote:
> On Fri, 19 Feb 2016, Petr Vobornik wrote:
>> On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:
>>> On Fri, 19 Feb 2016, Martin Basti wrote:
 WIP patch attached

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

>>> Comments inline.
>>>
 +# we need to run sidgen task
 +sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
 +"cn=config")
 +sidgen_tasks_attr = {
 +"objectclass": ["top", "extensibleObject"],
 +"cn": ["sidgen"],
 +"delay": [0],
 +"nsslapd-basedn": [self.api.env.basedn],
 +}
>>> May be you are better to name this task more uniquely?
>>> Something like 'cn=generate domain sid,cn=...'?
>>>
 +
 +task_entry = ldap.make_entry(sidgen_task_dn,
 + **sidgen_tasks_attr)
 +try:
 +ldap.add_entry(task_entry)
 +except errors.DuplicateEntry:
 +self.log.debug("sidgen task already created")
 +else:
 +self.log.debug("sidgen task has been created")
>>> There could be multiple tasks running in parallel, that's why it could
>>> be good to use a different and unique name.
>>>
 +# we have to check all trusts domains which have been added
 after the
 +# upgrade that caused bug was done.
 +
 +base_dn = DN(self.api.env.container_adtrusts,
 self.api.env.basedn)
 +trust_domain_entries, truncated = ldap.find_entries(
 +base_dn=base_dn,
 +scope=ldap.SCOPE_ONELEVEL,
 +attrs_list=[attr_name, "cn"],
 +)
 +
 +if truncated:
 +self.log.warning("update_sids: Search results were
 truncated")
 +
 +for entry in trust_domain_entries:
 +if entry.single_value[attr_name] is None:
 +domain = entry.single_value["cn"]
 +self.log.error(
 +"Your trust to %s is broken. Please re-create it
 by "
 +"running 'ipa trust-add' again", domain)
 +
 +sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
 +return False, ()
>>> This part looks fine. Basically, a similar check needs to be added to
>>> trust_find, trust_show, and may be trust_add.
>>>
>>
>> Why trust-add?
>>
>> I'm not a big fan of cluttering existing commands(find, show, mod) with logic
>> to fix one upgrade bug. But I understand a need to communicate it somehow.
>>
>> Would it make sense to move such logic to a separate command, e.g.
>> trust-check/trust-verify?  The command can do additional check in a future.
> No. What is the value of trust-add if it says you 'Trust established and
> verified' while in reality it didn't? This specific issue is very easy
> to catch.

I think the point was that we are proposing and doing a non-trivial engineering
effort to do warning for one-off bug that will be fixed in next bugfix release.
I would agree if the check is more systematic, but doing a special LDAP search
(for example) from now on because of this one-off bug does not seem to me as
ideal path.

-- 
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 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Alexander Bokovoy]

On Fri, 19 Feb 2016, Petr Vobornik wrote:

On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:

On Fri, 19 Feb 2016, Martin Basti wrote:

WIP patch attached

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


Comments inline.


+# we need to run sidgen task
+sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
+"cn=config")
+sidgen_tasks_attr = {
+"objectclass": ["top", "extensibleObject"],
+"cn": ["sidgen"],
+"delay": [0],
+"nsslapd-basedn": [self.api.env.basedn],
+}

May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?


+
+task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+try:
+ldap.add_entry(task_entry)
+except errors.DuplicateEntry:
+self.log.debug("sidgen task already created")
+else:
+self.log.debug("sidgen task has been created")

There could be multiple tasks running in parallel, that's why it could
be good to use a different and unique name.


+# we have to check all trusts domains which have been added
after the
+# upgrade that caused bug was done.
+
+base_dn = DN(self.api.env.container_adtrusts,
self.api.env.basedn)
+trust_domain_entries, truncated = ldap.find_entries(
+base_dn=base_dn,
+scope=ldap.SCOPE_ONELEVEL,
+attrs_list=[attr_name, "cn"],
+)
+
+if truncated:
+self.log.warning("update_sids: Search results were
truncated")
+
+for entry in trust_domain_entries:
+if entry.single_value[attr_name] is None:
+domain = entry.single_value["cn"]
+self.log.error(
+"Your trust to %s is broken. Please re-create it
by "
+"running 'ipa trust-add' again", domain)
+
+sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+return False, ()

This part looks fine. Basically, a similar check needs to be added to
trust_find, trust_show, and may be trust_add.



Why trust-add?

I'm not a big fan of cluttering existing commands(find, show, mod) 
with logic to fix one upgrade bug. But I understand a need to 
communicate it somehow.


Would it make sense to move such logic to a separate command, e.g. 
trust-check/trust-verify?  The command can do additional check in a 
future.

No. What is the value of trust-add if it says you 'Trust established and
verified' while in reality it didn't? This specific issue is very easy
to catch.

--
/ 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 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Petr Vobornik]

On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:

On Fri, 19 Feb 2016, Martin Basti wrote:

WIP patch attached

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


Comments inline.


+# we need to run sidgen task
+sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
+"cn=config")
+sidgen_tasks_attr = {
+"objectclass": ["top", "extensibleObject"],
+"cn": ["sidgen"],
+"delay": [0],
+"nsslapd-basedn": [self.api.env.basedn],
+}

May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?


+
+task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+try:
+ldap.add_entry(task_entry)
+except errors.DuplicateEntry:
+self.log.debug("sidgen task already created")
+else:
+self.log.debug("sidgen task has been created")

There could be multiple tasks running in parallel, that's why it could
be good to use a different and unique name.


+# we have to check all trusts domains which have been added
after the
+# upgrade that caused bug was done.
+
+base_dn = DN(self.api.env.container_adtrusts,
self.api.env.basedn)
+trust_domain_entries, truncated = ldap.find_entries(
+base_dn=base_dn,
+scope=ldap.SCOPE_ONELEVEL,
+attrs_list=[attr_name, "cn"],
+)
+
+if truncated:
+self.log.warning("update_sids: Search results were
truncated")
+
+for entry in trust_domain_entries:
+if entry.single_value[attr_name] is None:
+domain = entry.single_value["cn"]
+self.log.error(
+"Your trust to %s is broken. Please re-create it
by "
+"running 'ipa trust-add' again", domain)
+
+sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+return False, ()

This part looks fine. Basically, a similar check needs to be added to
trust_find, trust_show, and may be trust_add.



Why trust-add?

I'm not a big fan of cluttering existing commands(find, show, mod) with 
logic to fix one upgrade bug. But I understand a need to communicate it 
somehow.


Would it make sense to move such logic to a separate command, e.g. 
trust-check/trust-verify?  The command can do additional check in a future.

--
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] [TESTS][PATCH 0009] WebUI tests fix

[Lenka Doudova]

On 02/19/2016 10:51 AM, Petr Vobornik wrote:

On 02/16/2016 10:10 AM, Lenka Doudova wrote:



On 02/11/2016 11:13 AM, Lenka Doudova wrote:

Hi all,

most of webUI tests fail with

AssertionError: Can't click on checkbox label: table.table
Message: Element is not clickable at point (37, 340.338964844).
Other element would receive the click:



The problem seems to be that the tests attempt to click on a checkbox
label that is no longer there. Since the checkbox is clickable
directly, I changed the code accordingly. Most of the tests should now
proceed successfully.

Lenka




Attaching updated patch with minor fix.

Lenka




would ACK  but the commit message is so generic that it doesn't say 
anything.


Also the description in the mail should be in commit message to be 
usable in a future.


Fix attached, hope this is better.

Lenka
From df577030af9e29f41a18838d2d8c83b5cca95154 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 10 Feb 2016 16:16:22 +0100
Subject: [PATCH] WebUI tests: fix failing of tests due to unclicable label

Checkbox label is no longer clickable, most tests fail with error like this:

AssertionError: Can't click on checkbox label: table.table
Message: Element is not clickable at point (37, 340.338964844). Other element would receive the click:


The checkbox is clickable directly without the label, this patch provides according fix.
---
 ipatests/test_webui/ui_driver.py | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/ipatests/test_webui/ui_driver.py b/ipatests/test_webui/ui_driver.py
index fc22f8612d49e300437eb91cb58add1a0376eb2c..ad3a9100f65f8ce64aaaf707df04a2bd411c3299 100644
--- a/ipatests/test_webui/ui_driver.py
+++ b/ipatests/test_webui/ui_driver.py
@@ -933,12 +933,8 @@ class UI_driver(object):
 s = self.get_table_selector(table_name)
 input_s = s + " tbody td input[value='%s']" % pkey
 checkbox = self.find(input_s, By.CSS_SELECTOR, parent, strict=True)
-checkbox_id = checkbox.get_attribute('id')
-label_s = s + " tbody td label[for='%s']" % checkbox_id
-print(label_s)
-label = self.find(label_s, By.CSS_SELECTOR, parent, strict=True)
 try:
-ActionChains(self.driver).move_to_element(label).click().perform()
+ActionChains(self.driver).move_to_element(checkbox).click().perform()
 except WebDriverException as e:
 assert False, 'Can\'t click on checkbox label: %s \n%s' % (s, e)
 self.wait()
-- 
2.5.0

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

[Freeipa-devel] [PATCH 0390] Fix build with GCC 4.9+

[Petr Spacek]

Hello,

Fix build with GCC 4.9+.

GCC 4.9+ is too aggressive when optimizing functions with nonnull
attributes. This removes most of asserts() in the plugin.
GCC 6 adds warnings for these cases.

We are disabling the unwanted condition pruning by adding
-fno-delete-null-pointer-checks argument.
BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.

Additionally we silence warnings to prevent build failures when -Werror
is used.

https://bugzilla.redhat.com/show_bug.cgi?id=1307346

-- 
Petr^2 Spacek
From bc379e56336abae93d593b878dac6f4da4b497de Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Fri, 19 Feb 2016 13:39:27 +0100
Subject: [PATCH] Fix build with GCC 4.9+.

GCC 4.9+ is too aggressive when optimizing functions with nonnull
attributes. This removes most of asserts() in the plugin.
GCC 6 adds warnings for these cases.

We are disabling the unwanted condition pruning by adding
-fno-delete-null-pointer-checks argument.
BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a.

Additionally we silence warnings to prevent build failures when -Werror
is used.

https://bugzilla.redhat.com/show_bug.cgi?id=1307346
---
 configure.ac | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/configure.ac b/configure.ac
index a06708b1a5ee64bb64c80272c10ed1a35670c8d0..486c9a37b081c8d60111694a5815ac10e4e5559a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -39,6 +39,52 @@ AC_TRY_COMPILE([
 [CFLAGS="$SAVED_CFLAGS"
  AC_MSG_RESULT([no])])
 
+# Check if build chain supports -fno-delete-null-pointer-checks
+# this flag avoids too agressive optimizations which would remove some asserts
+# BIND 9 did the same in its commit 603a78708343f063b44affb882ef93bb19a5142a
+AC_MSG_CHECKING([for -fno-delete-null-pointer-checks compiler flag])
+SAVED_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -fno-delete-null-pointer-checks"
+AC_TRY_COMPILE([
+	extern int fdef(void);
+],[],
+[AC_MSG_RESULT([yes])],
+[CFLAGS="$SAVED_CFLAGS"
+ AC_MSG_RESULT([no])])
+
+# detect if __attribute__(nonnull) is causing warnings we do not care about
+# this is typically caused by REQUIRE(ptr != NULL);
+AC_MSG_CHECKING([for too agressive __attribute__(nonnull) handling])
+NONNULL_COMPARE_CODE="
+	__attribute__((nonnull))
+	void f(void *ptr) {
+		if (ptr == 0)
+			;
+	};
+"
+SAVED_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -Wall -Werror"
+AC_TRY_COMPILE([$NONNULL_COMPARE_CODE
+],[],
+[CFLAGS="$SAVED_CFLAGS"
+ AC_MSG_RESULT([no problem detected])],
+[AC_MSG_RESULT([it is too aggresive])
+ AC_MSG_CHECKING([if -Wno-nonnull-compare is sufficient])
+ CFLAGS="$SAVED_CFLAGS -Wno-nonnull-compare -Wall -Werror"
+ AC_TRY_COMPILE([$NONNULL_COMPARE_CODE],[],
+ [AC_MSG_RESULT([yes])
+  CFLAGS="$SAVED_CFLAGS -Wno-nonnull-compare"],
+ [AC_MSG_RESULT([no])
+  AC_MSG_CHECKING([for -Wno-nonnull support])
+  CFLAGS="$SAVED_CFLAGS -Wno-nonnull -Wall -Werror"
+  AC_TRY_COMPILE([$NONNULL_COMPARE_CODE],[],
+  [AC_MSG_RESULT([yes])
+   CFLAGS="$SAVED_CFLAGS -Wno-nonnull"],
+  [AC_MSG_RESULT([no])
+   CFLAGS="$SAVED_CFLAGS"])
+ ])
+])
+
 # Get CFLAGS from isc-config.sh
 AC_ARG_VAR([BIND9_CFLAGS],
[C compiler flags for bind9, overriding isc-config.sh])
-- 
2.5.0

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

[Freeipa-devel] [PATCH] 0770 Switch /usr/bin/ipa to Python 3

[Petr Viktorin]

Is it time yet?

This patch switches /usr/bin/ipa to Python 3 for
- the in-tree ./ipa command
- RPMs, when built with_python3



-- 
Petr Viktorin
From 1fafb210c9d83be84696af0179a0dccd6839478d Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Thu, 18 Feb 2016 16:02:46 +0100
Subject: [PATCH] Switch /usr/bin/ipa to Python 3

When building RPMs with Python 3 support, /usr/bin/ipa will now
use Python 3.
The in-tree ipa command will also run on Python 3.

When building with make install, $(PYTHON) is honored and it will
still default to Python 2.

Part of the work for https://fedorahosted.org/freeipa/ticket/5638
---
 freeipa.spec.in | 11 +++
 ipa |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 54a11bfc8cced643c19c29c5ada70bacf7540395..8a580b17b9ec399d97d3b796677160f42b67453f 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -394,7 +394,11 @@ Summary: IPA administrative tools
 Group: System Environment/Base
 BuildArch: noarch
 Requires: %{name}-client-common = %{version}-%{release}
+%if 0%{?with_python3}
+Requires: python3-ipalib = %{version}-%{release}
+%else
 Requires: python2-ipalib = %{version}-%{release}
+%endif
 Requires: python-ldap
 
 Provides: %{alt_name}-admintools = %{version}
@@ -691,6 +695,13 @@ make client-install DESTDIR=%{buildroot}
 (cd ipalib && make PYTHON=%{__python3} IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} DESTDIR=%{buildroot} install)
 (cd ipapython && make PYTHON=%{__python3} IPA_VERSION_IS_GIT_SNAPSHOT=no %{?_smp_mflags} DESTDIR=%{buildroot} install)
 (cd ipaplatform && %{__python3} setup.py install --root %{buildroot})
+
+# Switch shebang of /usr/bin/ipa
+# XXX: This script is installed with ipaserver. When all of ipaserver is
+# built with Python 3, this will no longer be necessary (as long as the py3
+# version is installed after the py2 version, so it overwrites /usr/bin/ipa)
+sed -i -e'1s/python\(2\|$\)/python3/' %{buildroot}%{_bindir}/ipa
+
 %endif # with_python3
 
 %find_lang %{gettext_domain}
diff --git a/ipa b/ipa
index 9ef356868a444555172d35e5f44a86f6f9914d71..342c5414792cbc2b6a2a393c5a3b8d9a54dbac80 100755
--- a/ipa
+++ b/ipa
@@ -1,4 +1,4 @@
-#!/usr/bin/python2
+#!/usr/bin/python3
 
 # Authors:
 #   Jason Gerard DeRose 
-- 
2.5.0

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

Re: [Freeipa-devel] [PATCH 0415] Pylint: disable new pylint checks

[David Kupka]

On 17/02/16 18:02, Martin Basti wrote:

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

Disable unwanted or false positive checks.

New checks cannot be disabled inline because pylint < 1.5 will report
them as errors, thus must be disable globally until we decide to move
completely to pytlint 1.5 (Fedora 25?)

Patches attached.



Works for me, ACK.

All tree branches tested on F23 with pylint 1.4.3-3

--
David Kupka

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

Re: [Freeipa-devel] [PATCH 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Alexander Bokovoy]

On Fri, 19 Feb 2016, Martin Basti wrote:

WIP patch attached

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


Comments inline.


+# we need to run sidgen task
+sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
+"cn=config")
+sidgen_tasks_attr = {
+"objectclass": ["top", "extensibleObject"],
+"cn": ["sidgen"],
+"delay": [0],
+"nsslapd-basedn": [self.api.env.basedn],
+}

May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?


+
+task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+try:
+ldap.add_entry(task_entry)
+except errors.DuplicateEntry:
+self.log.debug("sidgen task already created")
+else:
+self.log.debug("sidgen task has been created")

There could be multiple tasks running in parallel, that's why it could
be good to use a different and unique name.


+# we have to check all trusts domains which have been added after the
+# upgrade that caused bug was done.
+
+base_dn = DN(self.api.env.container_adtrusts, self.api.env.basedn)
+trust_domain_entries, truncated = ldap.find_entries(
+base_dn=base_dn,
+scope=ldap.SCOPE_ONELEVEL,
+attrs_list=[attr_name, "cn"],
+)
+
+if truncated:
+self.log.warning("update_sids: Search results were truncated")
+
+for entry in trust_domain_entries:
+if entry.single_value[attr_name] is None:
+domain = entry.single_value["cn"]
+self.log.error(
+"Your trust to %s is broken. Please re-create it by "
+"running 'ipa trust-add' again", domain)
+
+sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+return False, ()

This part looks fine. Basically, a similar check needs to be added to
trust_find, trust_show, and may be trust_add.


--
/ 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] [TESTS][PATCH 0009] WebUI tests fix

[Petr Vobornik]

On 02/16/2016 10:10 AM, Lenka Doudova wrote:



On 02/11/2016 11:13 AM, Lenka Doudova wrote:

Hi all,

most of webUI tests fail with

AssertionError: Can't click on checkbox label: table.table
Message: Element is not clickable at point (37, 340.338964844).
Other element would receive the click:



The problem seems to be that the tests attempt to click on a checkbox
label that is no longer there. Since the checkbox is clickable
directly, I changed the code accordingly. Most of the tests should now
proceed successfully.

Lenka




Attaching updated patch with minor fix.

Lenka




would ACK  but the commit message is so generic that it doesn't say 
anything.


Also the description in the mail should be in commit message to be 
usable in a future.

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

[Freeipa-devel] [PATCH 0416][WIP] fix broken configuration of sidgen and extdom plugins

[Martin Basti]

WIP patch attached

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

From e576e73acdee6c5db15017b9c73bdeb3aaede534 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Thu, 18 Feb 2016 19:59:50 +0100
Subject: [PATCH] WIP: fix upgrade sidgen and extdom

---
 install/updates/90-post_upgrade_plugins.update |   2 +
 ipaserver/install/dsinstance.py|  12 +-
 ipaserver/install/plugins/adtrust.py   | 151 +
 ipaserver/install/server/upgrade.py|   4 +-
 4 files changed, 161 insertions(+), 8 deletions(-)

diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 5642021ad93cf336db2d872bf3ef6db99b5ffa46..9c9ee160fffedbc8e8d59705169e6cf08ddc9779 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -5,6 +5,8 @@
 plugin: update_ca_topology
 plugin: update_dnszones
 plugin: update_dns_limits
+plugin: update_sigden_extdom_broken_config
+plugin: update_sids
 plugin: update_default_range
 plugin: update_default_trust_view
 plugin: update_ca_renewal_master
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 2905c31c64c4fa8bd034d2cdf6ce5d90ecfea091..f474e189a47f945b7c91cba4ccc17266b9c7e430 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -1054,9 +1054,9 @@ class DsInstance(service.Service):
 """
 Add sidgen directory server plugin configuration if it does not already exist.
 """
-self._ldap_mod('ipa-sidgen-conf.ldif', self.sub_dict)
+self.add_sidgen_plugin(self.sub_dict['SUFFIX'])
 
-def add_sidgen_plugin(self):
+def add_sidgen_plugin(self, suffix):
 """
 Add sidgen plugin configuration only if it does not already exist.
 """
@@ -1064,7 +1064,7 @@ class DsInstance(service.Service):
 try:
 self.admin_conn.get_entry(dn)
 except errors.NotFound:
-self._add_sidgen_plugin()
+self._ldap_mod('ipa-sidgen-conf.ldif', dict(SUFFIX=suffix))
 else:
 root_logger.debug("sidgen plugin is already configured")
 
@@ -1072,9 +1072,9 @@ class DsInstance(service.Service):
 """
 Add directory server configuration for the extdom extended operation.
 """
-self._ldap_mod('ipa-extdom-extop-conf.ldif', self.sub_dict)
+self.add_extdom_plugin(self.sub_dict['SUFFIX'])
 
-def add_extdom_plugin(self):
+def add_extdom_plugin(self, suffix):
 """
 Add extdom configuration if it does not already exist.
 """
@@ -1082,7 +1082,7 @@ class DsInstance(service.Service):
 try:
 self.admin_conn.get_entry(dn)
 except errors.NotFound:
-self._add_extdom_plugin()
+self._ldap_mod('ipa-extdom-extop-conf.ldif', dict(SUFFIX=suffix))
 else:
 root_logger.debug("extdom plugin is already configured")
 
diff --git a/ipaserver/install/plugins/adtrust.py b/ipaserver/install/plugins/adtrust.py
index df9412adba833251cc1c70d7d72ebebbdc77c2a4..7836645050b0d24b14e5e261b2faa41816e5095c 100644
--- a/ipaserver/install/plugins/adtrust.py
+++ b/ipaserver/install/plugins/adtrust.py
@@ -21,6 +21,8 @@ from ipalib import api, errors
 from ipalib import Updater
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
+from ipaserver.install import sysupgrade
+
 
 DEFAULT_ID_RANGE_SIZE = 20
 
@@ -161,5 +163,154 @@ class update_default_trust_view(Updater):
 
 return False, [update]
 
+
+class update_sigden_extdom_broken_config(Updater):
+"""Fix configuration of sidgen and extdom plugins
+
+Upgrade to IPA 4.2+ cause that sidgen and extdom plugins have improperly
+configured basedn.
+
+All trusts which have been added when config was broken must to be
+re-added manually.
+
+https://fedorahosted.org/freeipa/ticket/5665
+"""
+
+sidgen_config_dn = DN("cn=IPA SIDGEN,cn=plugins,cn=config")
+extdom_config_dn = DN("cn=ipa_extdom_extop,cn=plugins,cn=config")
+
+def _fix_config(self):
+"""Due upgrade error configuration of sidgen and extdom plugins may
+contain literally "$SUFFIX" value instead of real DN in nsslapd-basedn
+attribute
+
+:return: True if config was fixed, False if fix is not needed
+"""
+ldap = self.api.Backend.ldap2
+basedn_attr = 'nsslapd-basedn'
+modified = False
+
+for dn in (self.sidgen_config_dn, self.extdom_config_dn):
+try:
+entry = ldap.get_entry(dn, attrs_list=[basedn_attr])
+except errors.NotFound:
+self.log.debug("configuration for %s not found, skipping", dn)
+else:
+configured_suffix = entry.single_value.get(basedn_attr)
+if configured_suffix is None:
+raise RuntimeError(
+   

Re: [Freeipa-devel] Design: Automatic Empty Zone handling in bind-dyndb-ldap

[Petr Spacek]

On 12.1.2016 15:10, Martin Basti wrote:
> 
> 
> On 12.01.2016 15:06, Petr Spacek wrote:
>> On 8.1.2016 18:14, Martin Basti wrote:
>>>
>>> On 08.01.2016 16:57, Petr Spacek wrote:
 Hello,

 recent improvements in FreeIPA 4.3.0 (finally) prevent FreeIPA installer 
 from
 creating made-up DNS reverse zones, which already exist on some other DNS
 server.

 This change uncovered a well-hidden automatic empty zones in BIND 9.9+, 
 which
 is now causing problem to users.

 It seems that this can be fixed by change to the code which handles forward
 DNS zones. Short design document with necessary background is available on:
 https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/AutomaticEmptyZones


 Please be so kind and review it ASAP, so I can write the patch quickly and
 make life of our QE guys easier.

 Have a nice Friday.

>>> Hello,
>>>
>>> IIUC, the differences between default bind behaviour and bind-dyndb-ldap
>>> behaviour are:
>>>
>>> * disable automatic empty zone when policy is 'first' or 'only', instead of
>>> just 'only'
>>> I liked it more than default behaviour of named, but could be this somehow
>>> unexpected by users, or they will be happy that it works better (?) than in
>>> named?
>> I hope users will appreciate it :-)
>>
>>> * bind-dyndb-ldap will not recreate automate empty zone
>>> IMO this should not harm at all
>>>
>>> so design LGTM, I will thinking about it over this weekend
>> Did you find any problem?
>>
>>
>> Petr^2 Spacek
> 
> My mind did not pop out any issue during weekend.
> It should work :)

I was discussing this further with BIND upstream and Mark Andrews do not like
it. IMHO we should respect his opinion and do that same what BIND 9.11 is
going to do.

For this reason I've updated design page
https://fedorahosted.org/bind-dyndb-ldap/wiki/BIND9/Design/AutomaticEmptyZones
with the new approach.

Please review it again. It contains new sections Configuration and Upgrade.

Thank you!

-- 
Petr^2 Spacek

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