[Freeipa-devel] [PATCH] 225 Remove leading zero from IPA_NUM_VERSION
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
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
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
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
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
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
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
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
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
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
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.
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
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
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