[Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION

2013-05-13 Thread Petr Viktorin
Due to my mistake in commit 9125285, the IPA_NUM_VERSION variable 
contained a leading zero, so it was treated as octal in Python code.
Bumping the development version to 3.2.99 resulted in 030299, an invalid 
value.

Luckily, 320  030200  30200 so the ordering is not broken.

Sorry for the mistake.

--
Petr³
From 9b926f8fdc60f9d814ded8dc41929357ea73894c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Mon, 13 May 2013 10:39:55 +0200
Subject: [PATCH] Remove leading zero from IPA_NUM_VERSION

The numeric IPA_NUM_VERSION contained a leading zero, so it was treated
as octal value in Python code instead of decimal.
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8f4053b5db08bafb062ab4c08f2b7382c1d100f7..b6f4fa20c3d4a106c7dc7e69a3bd791134d59565 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ TARGET ?= master
 
 SUPPORTED_PLATFORM ?= redhat
 
-IPA_NUM_VERSION ?= $(shell printf %02d%02d%02d $(IPA_VERSION_MAJOR) $(IPA_VERSION_MINOR) $(IPA_VERSION_RELEASE))
+IPA_NUM_VERSION ?= $(shell printf %d%02d%02d $(IPA_VERSION_MAJOR) $(IPA_VERSION_MINOR) $(IPA_VERSION_RELEASE))
 
 # After updating the version in VERSION you should run the version-update
 # target.
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add

2013-05-13 Thread Petr Viktorin

On 05/12/2013 03:10 PM, Ana Krivokapic wrote:

On 05/10/2013 10:37 PM, Endi Sukma Dewata wrote:

On 5/10/2013 9:38 AM, Petr Viktorin wrote:

On 05/10/2013 03:57 PM, Ana Krivokapic wrote:
[...]

Thanks for catching the bugs. Updated patches are attached.


Thanks! It works nicely.
Endi is doing a quick check of the Javascript, if he doesn't find an
issue then ACK.

If this still makes it into 3.2.0, please only push the first patch there.


I tried this in the UI:

   Zone name: test.com
   Authoritative nameserver: ns.sometest.com.

The 'Nameserver IP address' field is still enabled. This is because the name
server is considered in the zone although it's actually not.

The CLI seems to work fine, it didn't ask for IP address.

The UI probably could be fixed using endsWith(ns, '.' + zone). Everything else
looks fine. ACK with the fix.



Fixed, updated patch attached.


ACK for both patches

--
Petr³

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


Re: [Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION

2013-05-13 Thread Tomas Babej

On 05/13/2013 11:47 AM, Petr Viktorin wrote:
Due to my mistake in commit 9125285, the IPA_NUM_VERSION variable 
contained a leading zero, so it was treated as octal in Python code.
Bumping the development version to 3.2.99 resulted in 030299, an 
invalid value.

Luckily, 320  030200  30200 so the ordering is not broken.

Sorry for the mistake.



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


ACK, solves the issue, thanks.

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

Re: [Freeipa-devel] [PATCH] 0027 Prompt for nameserver IP address in dnszone-add

2013-05-13 Thread Petr Vobornik

On 05/12/2013 03:10 PM, Ana Krivokapic wrote:

On 05/10/2013 10:37 PM, Endi Sukma Dewata wrote:

On 5/10/2013 9:38 AM, Petr Viktorin wrote:

On 05/10/2013 03:57 PM, Ana Krivokapic wrote:
[...]

Thanks for catching the bugs. Updated patches are attached.


Thanks! It works nicely.
Endi is doing a quick check of the Javascript, if he doesn't find an
issue then ACK.

If this still makes it into 3.2.0, please only push the first patch there.


I tried this in the UI:

   Zone name: test.com
   Authoritative nameserver: ns.sometest.com.

The 'Nameserver IP address' field is still enabled. This is because the name
server is considered in the zone although it's actually not.

The CLI seems to work fine, it didn't ask for IP address.

The UI probably could be fixed using endsWith(ns, '.' + zone). Everything else
looks fine. ACK with the fix.



Fixed, updated patch attached.



A nitpick for UI part which is not a blocker(nack) because we don't have 
any strict rules for following topic:


We should avoid depending on widget's html output outside of the widget 
code.


So we should use:
   zone_w.save()[0]
instead of:
  $('input', zone_w.container).val();

same for `ns`.

Unfortunately there is no text_widget.is_enabled() method  so 
`zone_w.input.prop('disabled')` can't be replaced.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION

2013-05-13 Thread Rob Crittenden

Petr Viktorin wrote:

Due to my mistake in commit 9125285, the IPA_NUM_VERSION variable
contained a leading zero, so it was treated as octal in Python code.
Bumping the development version to 3.2.99 resulted in 030299, an invalid
value.
Luckily, 320  030200  30200 so the ordering is not broken.

Sorry for the mistake.



I think at least a comment somewhere is required too, probably in 
version.h, describing which versions were affected by this octal problem.


rob

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


Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust

2013-05-13 Thread Lukas Slebodnik
On (06/05/13 14:03), Petr Spacek wrote:
On 18.4.2013 11:04, Petr Spacek wrote:
Hello,

Clean up PTR record synchronization code and make it more robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.

Tbabej's GCC cries about uninitialized variable 'ptr_a_equal', but we
weren't able to find any real error.

This version of the patch contains a workaround for the GCC oddities.

-- 
Petr^2 Spacek

From 5e6abb29df58ce00ecf7045254dfc7fb09fc4650 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
 robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 507 --
 1 file changed, 342 insertions(+), 165 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 
8448412b7a1a9150bd24d9ca46575c0402be0c9f..6c5cf2e79d762251954e3bb099dbef98a0b2d805
 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,360 @@ cleanup:
 #undef SET_LDAP_MOD
 }
 
+
+#define SYNCPTR_PREFPTR record synchronization 
+#define SYNCPTR_FMTPRE  SYNCPTR_PREF (%s) for A/ '%s' 
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+  static const char add[] = addition;
+  static const char del[] = deletion;
   ^
Declaration(definition) of string should be consistent everywhere in the source 
code.
Please change char _name[] to char * _name
Sorry for nitpicking, I know that semantically is it the same,
but in the other places in source code, strings are
defined like (const)? char *.
If you don't believe me, you can run next command :-)

grep -Rn -E 'char.*=.*[^]*' path_to_bind-dyndb-ldap

[snip]

+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+dns_name_t *ptr_name, ld_string_t *ptr_dn,
+dns_name_t *zone_name) {
+  isc_result_t result;
+  isc_mem_t *mctx = ldap_inst-mctx;
+
+  in_addr_t ip;
+
+  /* Get string with IP address from change request
+   * and convert it to in_addr structure. */
+  if ((ip = inet_addr(ip_str)) t) == 0) {

If the input is invalid, INADDR_NONE (usually -1) is returned.
For details: man inet_addr

+  log_bug(SYNCPTR_PREF  could not convert IP address 
+  from string '%s', ip_str);
+  CLEANUP_WITH(ISC_R_UNEXPECTED);
+  }

LS

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


Re: [Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION

2013-05-13 Thread Petr Viktorin

On 05/13/2013 02:58 PM, Rob Crittenden wrote:

Petr Viktorin wrote:

Due to my mistake in commit 9125285, the IPA_NUM_VERSION variable
contained a leading zero, so it was treated as octal in Python code.
Bumping the development version to 3.2.99 resulted in 030299, an invalid
value.
Luckily, 320  030200  30200 so the ordering is not broken.

Sorry for the mistake.



I think at least a comment somewhere is required too, probably in
version.h, describing which versions were affected by this octal problem.

rob


Added.

--
Petr³
From 1ff6e677f6575539b34dd329802774a10f49da2c Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Mon, 13 May 2013 10:39:55 +0200
Subject: [PATCH] Remove leading zero from IPA_NUM_VERSION

The numeric IPA_NUM_VERSION contained a leading zero, so it was treated
as octal value in Python code instead of decimal.

https://fedorahosted.org/freeipa/ticket/3622
---
 Makefile|  2 +-
 ipapython/version.py.in | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8f4053b5db08bafb062ab4c08f2b7382c1d100f7..b6f4fa20c3d4a106c7dc7e69a3bd791134d59565 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ TARGET ?= master
 
 SUPPORTED_PLATFORM ?= redhat
 
-IPA_NUM_VERSION ?= $(shell printf %02d%02d%02d $(IPA_VERSION_MAJOR) $(IPA_VERSION_MINOR) $(IPA_VERSION_RELEASE))
+IPA_NUM_VERSION ?= $(shell printf %d%02d%02d $(IPA_VERSION_MAJOR) $(IPA_VERSION_MINOR) $(IPA_VERSION_RELEASE))
 
 # After updating the version in VERSION you should run the version-update
 # target.
diff --git a/ipapython/version.py.in b/ipapython/version.py.in
index 9cf8ddbe8bc755b955c4e2d357f68e52220a7446..04cf5f81f4fa32168ba208dd9c872464e62003d7 100644
--- a/ipapython/version.py.in
+++ b/ipapython/version.py.in
@@ -20,9 +20,26 @@
 # The full version including strings
 VERSION=__VERSION__
 
+
 # Just the numeric portion of the version so one can do direct numeric
 # comparisons to see if the API is compatible.
+#
+# How NUM_VERSION was generated changed over time:
+# Before IPA 3.1.3, it was simply concatenated decimal numbers:
+#   IPA 2.2.2:  NUM_VERSION=222
+#   IPA 2.2.99: NUM_VERSION=2299 (development version)
+#   IPA 3.1.0:  NUM_VERSION=310
+#   IPA 3.1.3:  NUM_VERSION=313
+# In IPA 3.1.4 and 3.2.0, the version was taken as an octal number due to a bug
+# (https://fedorahosted.org/freeipa/ticket/3622):
+#   IPA 3.1.4:  NUM_VERSION=12356 (octal 030104)
+#   IPA 3.2.0:  NUM_VERSION=12416 (octal 030200)
+# After IPA 3.2.0, it is decimal number where each part has two digits:
+#   IPA 3.2.1:  NUM_VERSION=30201
+#   IPA 3.2.99: NUM_VERSION=30299 (development version)
+#   IPA 3.3.0:  NUM_VERSION=30300
 NUM_VERSION=__NUM_VERSION__
 
+
 # The version of the API.
 API_VERSION=u'__API_VERSION__'
-- 
1.8.1.4

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

Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust

2013-05-13 Thread Petr Spacek

On 13.5.2013 15:23, Lukas Slebodnik wrote:

On (06/05/13 14:03), Petr Spacek wrote:

On 18.4.2013 11:04, Petr Spacek wrote:

Hello,

Clean up PTR record synchronization code and make it more robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.


Tbabej's GCC cries about uninitialized variable 'ptr_a_equal', but we
weren't able to find any real error.

This version of the patch contains a workaround for the GCC oddities.

--
Petr^2 Spacek



From 5e6abb29df58ce00ecf7045254dfc7fb09fc4650 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 507 --
1 file changed, 342 insertions(+), 165 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 
8448412b7a1a9150bd24d9ca46575c0402be0c9f..6c5cf2e79d762251954e3bb099dbef98a0b2d805
 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,360 @@ cleanup:
#undef SET_LDAP_MOD
}

+
+#define SYNCPTR_PREFPTR record synchronization 
+#define SYNCPTR_FMTPRE  SYNCPTR_PREF (%s) for A/ '%s' 
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+   static const char add[] = addition;
+   static const char del[] = deletion;

^
Declaration(definition) of string should be consistent everywhere in the source 
code.
Please change char _name[] to char * _name
Sorry for nitpicking, I know that semantically is it the same,
but in the other places in source code, strings are
defined like (const)? char *.
If you don't believe me, you can run next command :-)

 grep -Rn -E 'char.*=.*[^]*' path_to_bind-dyndb-ldap

[snip]


+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+ dns_name_t *ptr_name, ld_string_t *ptr_dn,
+ dns_name_t *zone_name) {
+   isc_result_t result;
+   isc_mem_t *mctx = ldap_inst-mctx;
+
+   in_addr_t ip;
+
+   /* Get string with IP address from change request
+* and convert it to in_addr structure. */
+   if ((ip = inet_addr(ip_str)) t) == 0) {

 
If the input is invalid, INADDR_NONE (usually -1) is returned.
For details: man inet_addr


+   log_bug(SYNCPTR_PREF  could not convert IP address 
+   from string '%s', ip_str);
+   CLEANUP_WITH(ISC_R_UNEXPECTED);
+   }


Thank you for catching this! Nobody noticed it for one and half year :-)

In any case, this code can't handle IPv6 addresses. We will triage it 
tomorrow: https://fedorahosted.org/bind-dyndb-ldap/ticket/118


Fixed patch is attached. The new version includes also typo fix:  could = 
could.


--
Petr Spacek
From 9642cc19136ae69d0ac161f97d01f0e26fe6c772 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
 robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 507 --
 1 file changed, 342 insertions(+), 165 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index af630e53dde36c3eec4d1a286cb096bcd8f3b0ca..b8d79de15d2f4ba5784bb314fc52cc8eae74a700 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,360 @@ cleanup:
 #undef SET_LDAP_MOD
 }
 
+
+#define SYNCPTR_PREFPTR record synchronization 
+#define SYNCPTR_FMTPRE  SYNCPTR_PREF (%s) for A/ '%s' 
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+	static const char *add = addition;
+	static const char *del = deletion;
+
+	switch (mod_op) {
+	case LDAP_MOD_ADD:
+		return add;
+
+	case LDAP_MOD_DELETE:
+		return del;
+
+	default:
+		INSIST(unsupported LDAP mod_op == NULL);
+		return NULL;
+	}
+}
+
+static void
+append_trailing_dot(char *str, unsigned int size) {
+	unsigned int length = strlen(str);
+	if (str[length] != '.') {
+		REQUIRE(length + 1  size);
+		str[length] = '.';
+		str[length+1] = '\0';
+	}
+}
+
+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+	  dns_name_t *ptr_name, ld_string_t *ptr_dn,
+	  dns_name_t *zone_name) {
+	isc_result_t result;
+	isc_mem_t *mctx = ldap_inst-mctx;
+
+	in_addr_t ip;
+
+	/* Get string with IP address from change request
+	 * and convert it to in_addr structure. */

Re: [Freeipa-devel] [PATCH] 0026 Do not display success message on failure in web UI

2013-05-13 Thread Petr Vobornik

On 05/07/2013 05:16 PM, Ana Krivokapic wrote:

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



1) The change from on_success to on_error is causing problems when some 
command in a batch doesn't fail. Ie.: disable multiple users on user 
search facet. Disabling already disabled user causes an error. The 
dialog is shown but the page is not refreshed so the newly disabled 
records are still displayed as enabled. We might even call this case a 
success.


IMO we shouldn't change the method because the batch itself succeeded. 
The problem should be fixed on caller side (users of batch command).


2) Also `ajax` context should be left there instead of `this`, otherwise 
it would get the context of on_ok handler:


3) (not an actual issue) Some of my old code doesn't contain space 
between for/if and opening curly bracet, opposite to the rest of the Web 
UI. Spaces should be added when touching these parts of code.

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH 0149] Clean up PTR record synchronization code and make it more robust

2013-05-13 Thread Petr Spacek

On 13.5.2013 16:49, Simo Sorce wrote:

On Mon, 2013-05-13 at 16:32 +0200, Petr Spacek wrote:

+   if ((ip = inet_addr(ip_str)) == INADDR_NONE) {



This kind of construct is hard to read and debug in gdb
I would suggest you do:
ip = inet_addr(ip_str);
if (ip == INADDER_NONE) {


I agree, done.

--
Petr^2 Spacek
From 8d12512d0eb4445f4966fd0e326cde9823f6a0bb Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 16 Apr 2013 16:10:09 +0200
Subject: [PATCH] Clean up PTR record synchronization code and make it more
 robust.

PTR record synchronization was split to smaller functions.
Input validation, error handling and logging was improved
significantly.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/ldap_helper.c | 508 --
 1 file changed, 343 insertions(+), 165 deletions(-)

diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index af630e53dde36c3eec4d1a286cb096bcd8f3b0ca..4baf7437d1666915729562b5465f0553a32c45a0 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -2830,35 +2830,361 @@ cleanup:
 #undef SET_LDAP_MOD
 }
 
+
+#define SYNCPTR_PREFPTR record synchronization 
+#define SYNCPTR_FMTPRE  SYNCPTR_PREF (%s) for A/ '%s' 
+#define SYNCPTR_FMTPOST ldap_modop_str(mod_op), a_name_str
+
+static const char *
+ldap_modop_str(unsigned int mod_op) {
+	static const char *add = addition;
+	static const char *del = deletion;
+
+	switch (mod_op) {
+	case LDAP_MOD_ADD:
+		return add;
+
+	case LDAP_MOD_DELETE:
+		return del;
+
+	default:
+		INSIST(unsupported LDAP mod_op == NULL);
+		return NULL;
+	}
+}
+
+static void
+append_trailing_dot(char *str, unsigned int size) {
+	unsigned int length = strlen(str);
+	if (str[length] != '.') {
+		REQUIRE(length + 1  size);
+		str[length] = '.';
+		str[length+1] = '\0';
+	}
+}
+
+static isc_result_t
+ldap_find_ptr(ldap_instance_t *ldap_inst, const char *ip_str,
+	  dns_name_t *ptr_name, ld_string_t *ptr_dn,
+	  dns_name_t *zone_name) {
+	isc_result_t result;
+	isc_mem_t *mctx = ldap_inst-mctx;
+
+	in_addr_t ip;
+
+	/* Get string with IP address from change request
+	 * and convert it to in_addr structure. */
+	ip = inet_addr(ip_str);
+	if (ip == INADDR_NONE) {
+		log_bug(SYNCPTR_PREF could not convert IP address 
+			from string '%s', ip_str);
+		CLEANUP_WITH(ISC_R_UNEXPECTED);
+	}
+
+	/* Use internal net address representation. */
+	isc_netaddr_t isc_ip;
+	/* Only copy data to isc_ip stucture. */
+	isc_netaddr_fromin(isc_ip,(struct in_addr *) ip);
+
+	/*
+	 * Convert IP address to PTR record.
+	 *
+	 * @example
+	 * 192.168.0.1 - 1.0.168.192.in-addr.arpa
+	 *
+	 * @todo Check if it works for IPv6 correctly.
+	 */
+	CHECK(dns_byaddr_createptrname2(isc_ip, 0, ptr_name));
+
+	/* Get LDAP entry indentifier. */
+	CHECK(dnsname_to_dn(ldap_inst-zone_register, ptr_name, ptr_dn));
+
+	/*
+	 * @example
+	 * owner_dn_ptr = idnsName=100.0.168, idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+	 * owner_zone_dn_ptr = idnsname=192.in-addr.arpa,cn=dns,$SUFFIX
+	 */
+	char *owner_zone_dn_ptr = strstr(str_buf(ptr_dn),, ) + 1;
+
+	/* Get attribute idnsAllowDynUpdate for reverse zone or use default. */
+	CHECK(dn_to_dnsname(mctx, owner_zone_dn_ptr, zone_name, NULL));
+
+cleanup:
+	return result;
+}
+
+/**
+ * Check if PTR record's value in LDAP == name of the modified A/ record.
+ * Update will be refused if the PTR name contains multiple PTR records or
+ * if the value in LDAP != expected name.
+ *
+ * @param[in] a_name Name of modified A/ record.
+ * @param[in] a_name_str Name of modified A/ record as NUL terminated string.
+ * @param[in] ptr_name   Name of PTR record generated from IP address in A/.
+ * @param[in] mod_op LDAP_MOD_DELETE if A/ record is being deleted
+ *   or LDAP_MOD_ADD if A/ record is being added.
+ *
+ * @retval ISC_R_IGNORE  A and PTR records match, no change is required.
+ * @retval ISC_R_SUCCESS Prerequisites fulfilled, update is allowed.
+ * @retval other Errors
+ *
+ * @code
+ * ** A record deletion **
+ * ; nsupdate command:
+ * update delete www.example.com. IN A	192.0.2.1
+ *
+ * ; PTR update will be allowed if the zone contains following data:
+ * www.example.com.		A	192.0.2.1
+ * 1.2.0.192.in-addr.arpa. 	PTR	www.example.com.
+
+ * ; PTR update will not be allowed if the zone contains following data:
+ * www.example.com.		A	192.0.2.1
+ * 1.2.0.192.in-addr.arpa. 	PTR	mail.example.com.
+ * @endcode
+ *
+ * @code
+ * ** A record addition **
+ * ; nsupdate command:
+ * update add www.example.com. 3600 IN A 192.0.2.1
+ *
+ * ; PTR update will be allowed if the zone does not contain A and PTR record.
+ *
+ * ; PTR update will not be allowed if the zone contains following data:
+ * 1.2.0.192.in-addr.arpa. 	PTR	mail.example.com.
+ * @endcode
+ */
+static isc_result_t
+ldap_sync_ptr_validate(ldap_instance_t *ldap_inst, dns_name_t *a_name,
+		   const char *a_name_str, dns_name_t *ptr_name,
+		   int mod_op) {
+	

Re: [Freeipa-devel] [PATCH 0146] Disallow all dynamic updates if update policy configuration failed

2013-05-13 Thread Petr Spacek

On 7.5.2013 16:53, Tomas Hozza wrote:

On 04/16/2013 10:40 AM, Petr Spacek wrote:

Hello,

Disallow all dynamic updates if update policy configuration failed.

Without this patch the old update policy stays in effect
when re-configuration failed.



ACK.

The patch looks good. (I didn't do functional test)


Pushed to master: b21eb8a84e2c02e7dd090d24d68f1e385b0604c3

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0147] Improve error logging for zones with idnsAllowDynUpdate == FALSE.

2013-05-13 Thread Petr Spacek

On 9.5.2013 10:35, Tomas Hozza wrote:

On 04/16/2013 12:44 PM, Petr Spacek wrote:

Hello,

Improve error logging for zones with idnsAllowDynUpdate == FALSE.

Zones with dynamic updates disabled are re-configured with empty
update policy string, so the update is refused by BIND and
an error is logged.



ACK.

The patch looks reasonable. (I didn't do functional test)


Pushed to master: 88a472349aec5216467aa1e30a35b8689b1cd439

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCHES 0053-0055] Prompt for RID base if trusted domain SID / name is specified and vice versa

2013-05-13 Thread Tomas Babej

On 05/10/2013 04:39 PM, Tomas Babej wrote:

Hi,

this patcheset deals with https://fedorahosted.org/freeipa/ticket/3602

See commit messages for details.

Tomas


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


I noticed during further development that logic in 
interactive_prompt_callback did not follow the pre_callback logic precisely.


Fixed patches attached.

Tomas
From e931d3f883f9b04a397b262ebf79ca6b637841df Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 9 May 2013 14:47:29 +0200
Subject: [PATCH 55/55] Incorporate interactive prompts in idrange-add

In idrange-add command, ensure that RID base is prompted for
in the interactive mode if domain SID or domain name was
specified.

If domain name nor SID was specified, make sure rid base is
prompted for if secondary rid base was specified and vice versa.

https://fedorahosted.org/freeipa/ticket/3602
---
 ipalib/plugins/idrange.py | 39 +--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py
index 54f6fbb3e19b9aa01dfde2a8d0c5da4498632386..f83ced32888bc984d02fdbd8ab5ff4c512c3ec06 100644
--- a/ipalib/plugins/idrange.py
+++ b/ipalib/plugins/idrange.py
@@ -361,6 +361,41 @@ class idrange_add(LDAPCreate):
 
 msg_summary = _('Added ID range %(value)s')
 
+def interactive_prompt_callback(self, kw):
+
+Ensure that rid-base is prompted for when dom-sid is specified.
+
+Also ensure that secondary-rid-base is prompted for when rid-base is
+specified and vice versa, in case that dom-sid was not specified.
+
+
+# dom-sid can be specified using dom-sid or dom-name options
+
+# it can be also set using --setattr or --addattr, in these cases
+# we will not prompt, but raise an ValidationError later
+
+dom_sid_set = any(dom_id in kw for dom_id in
+  ('ipanttrusteddomainname', 'ipanttrusteddomainsid'))
+
+rid_base_set = 'ipabaserid' in kw
+secondary_rid_base_set = 'ipasecondarybaserid' in kw
+
+# Prompt for RID base if domain SID / name was given
+if dom_sid_set and not rid_base_set:
+value = self.prompt_param(self.params['ipabaserid'])
+kw.update(dict(ipabaserid=value))
+
+if not dom_sid_set:
+# Prompt for secondary RID base if RID base was given
+if rid_base_set and not secondary_rid_base_set:
+value = self.prompt_param(self.params['ipasecondarybaserid'])
+kw.update(dict(ipasecondarybaserid=value))
+
+# Symetrically, prompt for RID base if secondary RID base was given
+if not rid_base_set and secondary_rid_base_set:
+value = self.prompt_param(self.params['ipabaserid'])
+kw.update(dict(ipabaserid=value))
+
 def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
 assert isinstance(dn, DN)
 
@@ -415,8 +450,8 @@ class idrange_add(LDAPCreate):
 entry_attrs['ipasecondarybaserid'],
 entry_attrs['ipaidrangesize']):
raise errors.ValidationError(name='ID Range setup',
-   error=_(Primary RID range and secondary RID range
-cannot overlap))
+error=_(Primary RID range and secondary RID range
+ cannot overlap))
 
 entry_attrs['objectclass'].append('ipadomainidrange')
 
-- 
1.8.1.4

From 90aaf1839cb731b0e495fd86f881f2f1014af711 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 9 May 2013 15:36:41 +0200
Subject: [PATCH 54/55] Add prompt_param method to avoid code duplication

Extracted common code from ipalib/plugins/cli.py and
ipalib/plugins/dns.py that provided way to prompt user
for the value of specific attribute.

Added prompt_param method to Command class in ipalib/frontend.py

Done as part of https://fedorahosted.org/freeipa/ticket/3602
---
 ipalib/cli.py | 25 -
 ipalib/frontend.py| 29 -
 ipalib/plugins/dns.py | 33 +++--
 3 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index c4b4492a59ff0a1a1b69ad045b253fdd30833eb9..5f02e929fe0df7051f4bb925a960678d780d4883 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -1178,11 +1178,13 @@ class cli(backend.Executioner):
 ``self.env.prompt_all`` is ``True``, this method will prompt for any
 params that have a missing values, even if the param is optional.
 
+
 honor_alwaysask = True
 for param in cmd.params():
 if param.alwaysask and param.name in kw:
 honor_alwaysask = False
 

[Freeipa-devel] [PATCH 0056] Support multiple local domain ranges with RID base set

2013-05-13 Thread Tomas Babej

Hi,

In ipa-adtrust-install, adding RID bases step would fail
if there was more than one local range defined. This can be a
common case if e.g. there are users that migrated from previous
IdM solution.

With this patch, we fail only if there are multiple local ranges
that do not have RID bases set.

Keep in mind that overlap checking is ensured by ipa-range-check
DS plugin.

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

Tomas
From fd62902846b9cb8d81d0eb0dd19f9f33fa60feca Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Mon, 13 May 2013 13:19:12 +0200
Subject: [PATCH] Support multiple local domain ranges with RID base set

In ip-adtrust-install, adding RID bases step would fail
if there was more than one local range defined. This can be a
common case if e.g. there are users that migrated from previous
IdM solution.

With this patch, we fail only if there are multiple local ranges
that do not have RID bases set.

Keep in mind that overlap checking is ensured by ipa-range-check
DS plugin.

https://fedorahosted.org/freeipa/ticket/3498
---
 ipaserver/install/adtrustinstance.py | 50 
 1 file changed, 34 insertions(+), 16 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index a47c80b3983f3086199353694ddb629e2c1c4492..d2929801b431625764e5b949349ab63d2caaf696 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -258,36 +258,54 @@ class ADTRUSTInstance(service.Service):
 
 
 try:
-res = self.admin_conn.get_entries(
+# Get the ranges
+ranges = self.admin_conn.get_entries(
 DN(api.env.container_ranges, self.suffix),
 ldap.SCOPE_ONELEVEL, (objectclass=ipaDomainIDRange))
-if len(res) != 1:
-root_logger.critical(Found more than one ID range for the  \
- local domain.)
-raise RuntimeError(Too many ID ranges\n)
 
-if res[0].single_value('ipaBaseRID', None) or \
-   res[0].single_value('ipaSecondaryBaseRID', None):
+# Filter out ranges where RID base is already set
+no_rid_base_set = lambda r: not any((
+  r.single_value('ipaBaseRID', None),
+  r.single_value('ipaSecondaryBaseRID', None)))
+
+ranges_with_no_rid_base = filter(no_rid_base_set, ranges)
+
+# Return if no range is without RID base
+if len(ranges_with_no_rid_base) == 0:
 self.print_msg(RID bases already set, nothing to do)
 return
 
-size = res[0].single_value('ipaIDRangeSize', None)
+# Abort if RID base needs to be added to more than one range
+if len(ranges_with_no_rid_base) != 1:
+root_logger.critical(Found more than one local domain ID 
+ range with no RID base set.)
+raise RuntimeError(Too many ID ranges\n)
+
+# Abort if RID bases are too close
+local_range = ranges_with_no_rid_base[0]
+size = local_range.single_value('ipaIDRangeSize', None)
+
 if abs(self.rid_base - self.secondary_rid_base)  size:
-self.print_msg(Primary and secondary RID base are too close.  \
+self.print_msg(Primary and secondary RID base are too close. 
   They have to differ at least by %d. % size)
 raise RuntimeError(RID bases too close.\n)
 
+# Modify the range
+# If the RID bases would cause overlap with some other range,
+# this will be detected by ipa-range-check DS plugin
 try:
-self.admin_conn.modify_s(res[0].dn,
- [(ldap.MOD_ADD, ipaBaseRID, \
- str(self.rid_base)), \
- (ldap.MOD_ADD, ipaSecondaryBaseRID, \
+self.admin_conn.modify_s(local_range.dn,
+ [(ldap.MOD_ADD, ipaBaseRID,
+ str(self.rid_base)),
+ (ldap.MOD_ADD, ipaSecondaryBaseRID,
  str(self.secondary_rid_base))])
-except:
-self.print_msg(Failed to add RID bases to the local range object)
+except ldap.CONSTRAINT_VIOLATION, e:
+self.print_msg(Failed to add RID bases to the local range 
+   object:\n  %s % e[0]['info'])
+raise RuntimeError(Constraint violation.\n)
 
 except errors.NotFound as e:
-root_logger.critical(ID range of the local domain not found,  \
+root_logger.critical(ID range of the local domain not