[Freeipa-devel] [PATCH] 117 Added attrs field to permission for target=subtree
Permission form was missing attrs field for target=subtree. All other
target types have it.
It uses multivalued text widget, same as filter, because we can't
predict the target type.
https://fedorahosted.org/freeipa/ticket/2592
--
Petr Vobornik
From 9da9d186b2abbd633dc9dd36a1ba653a8d6f9e0f Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Mon, 2 Apr 2012 15:22:07 +0200
Subject: [PATCH] Added attrs field to permission for target=subtree
Permission form was missing attrs field for target=subtree. All other target types have it.
It uses multivalued text widget, same as filter, because we can't predict the target type.
https://fedorahosted.org/freeipa/ticket/2592
---
install/ui/aci.js|3 +++
install/ui/test/aci_tests.js |2 +-
2 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/install/ui/aci.js b/install/ui/aci.js
index d65d3f88436358dcf96619488d260a5b48cf2bab..21ffa718e8a50edfc1fc78cea5799009adc768d4 100644
--- a/install/ui/aci.js
+++ b/install/ui/aci.js
@@ -836,6 +836,9 @@ IPA.permission_target_policy = function (widget_name) {
additional: [
{
name: 'memberof'
+},
+{
+name: 'attrs_multi'
}
]
},
diff --git a/install/ui/test/aci_tests.js b/install/ui/test/aci_tests.js
index c51107da45c04bee4de6b29bd452bce93a9b7d93..4055120f22b59fcadd63228536d56836edb81f78 100644
--- a/install/ui/test/aci_tests.js
+++ b/install/ui/test/aci_tests.js
@@ -282,7 +282,7 @@ test(Testing subtree target., function() {
same(record.subtree[0], data.result.result.subtree, 'subtree set correctly');
-same(get_visible_rows(target_widget), ['memberof', 'subtree'], 'subtree row visible');
+same(get_visible_rows(target_widget), ['memberof', 'subtree', 'attrs_multi'], 'subtree row visible');
});
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 0015 Don't try to remove auxiliary nodes from internal RBT
On Tue, Apr 03, 2012 at 03:06:31PM +0200, Petr Spacek wrote:
Hello,
this patch optimizes code for removing deleted zones from BIND
instance little bit.
In some cases there are auxiliary zones (= not really served zones)
in internal Red-Black tree. Current code tries to remove these
auxiliary zones on each zone_refresh attempt.
Everything works fine, because auxiliary zones are detected deeper
in zone deletion code.
Now plugin prints very confusing message Zone '%s' has been removed
from database. each 'zone_refresh' seconds, again and again. This
patch prevents this.
I think it's very very confusing. I spent a lot of time while
debugging before I realized where is the problem.
The patch is OK, please push it.
Regards, Adam
Petr^2 Spacek
From ce620e1e4bb888d784b8cdfac5ba75182d45b6c3 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 3 Apr 2012 14:50:12 +0200
Subject: [PATCH] Don't try to remove auxiliary nodes from internal RBT
Signed-off-by: Petr Spacek pspa...@redhat.com
---
src/ldap_helper.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index d0cde9d..df8b01e 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -1192,6 +1192,14 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst)
goto next;
}
+ /* Do not remove auxilitary (= non-zone) nodes. */
+ char buf[DNS_NAME_FORMATSIZE];
+ dns_name_format(aname, buf, DNS_NAME_FORMATSIZE);
+ if (!node-data) {
+ log_debug(5,auxilitary zone/node '%s' will not be
removed, buf);
+ goto next;
+ }
+
DECLARE_BUFFERED_NAME(foundname);
INIT_BUFFERED_NAME(foundname);
@@ -1201,8 +1209,6 @@ refresh_zones_from_ldap(ldap_instance_t *ldap_inst)
goto next;
}
/* Log zone removing. */
- char buf[255];
- dns_name_format(aname, buf, 255);
log_debug(1, Zone '%s' has been removed from database., buf);
delete = ISC_TRUE;
--
1.7.7.6
--
Adam Tkac, Red Hat, Inc.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0025-26 Test improvements
On 04/02/2012 05:05 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/26/2012 10:44 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Patch 25 fixes errors I found by running pylint on the testsuite. They
were in code that was unused, either by error or because it only
runs on
errors.
Patch 26 adds a test for the batch plugin.
In patch 25 the second test_internal_error should really be
test_unauthorized_error. I think that is a clearer name. Otherwise looks
good.
Patch 26 needs a very minor rebase to fix an error caused by improved
error code handling:
expected = Fuzzy(uinvalid 'gidnumber'.*, type 'unicode', None)
got = uinvalid 'gid': Gettext('must be an integer', domain='ipa',
localedir=None)
I tested this:
diff --git a/tests/test_xmlrpc/test_batch_plugin.py
b/tests/test_xmlrpc/test_bat
ch_plugin.py
index e4280ed..d69bfd9 100644
--- a/tests/test_xmlrpc/test_batch_plugin.py
+++ b/tests/test_xmlrpc/test_batch_plugin.py
@@ -186,7 +186,7 @@ class test_batch(Declarative):
dict(error=u'params' is required),
dict(error=u'givenname' is required),
dict(error=u'description' is required),
- dict(error=Fuzzy(uinvalid 'gidnumber'.*)),
+ dict(error=Fuzzy(uinvalid 'gid'.*)),
),
),
),
rob
Thank you! Fixed, attaching updated patches.
These look ok but it is baffling to me why tuple needs to be added to
the Output format in batch. Do you know when it is being converted into
a tuple?
In XML-RPC unmarshalling, specifically ipalib/rpc.py:
109 if type(value) in (list, tuple):
110 return tuple(xml_unwrap(v, encoding) for v in value)
Maybe we should relax the validation? That's out of scope for this patch
though.
The hbactest plugin has similar list/tuple Outputs.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 118-119 DNS forward policy: checkboxes changed to radio buttons
DNS forward policy fields were using mutually exclusive checkboxes. Such
behavior is unusual for users.
Checkboxes were changed to radios with new option 'none/default' to set
empty value ''.
https://fedorahosted.org/freeipa/ticket/2599
Second patch removes mutex option from checkboxes. Not sure if needed.
--
Petr Vobornik
From 99eec2431d4e98e16b339f6103318ab96af34843 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Wed, 4 Apr 2012 10:49:16 +0200
Subject: [PATCH] DNS forward policy: checkboxes changed to radio buttons
DNS forward policy fields were using mutually exclusive checkboxes. Such behavior is unusual for users.
Checkboxes were changed to radios with new option 'none/default' to set empty value ''.
https://fedorahosted.org/freeipa/ticket/2599
---
install/ui/dns.js | 22 --
install/ui/ipa.js | 21 +
install/ui/test/data/ipa_init.json |1 +
ipalib/plugins/internal.py |1 +
4 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/install/ui/dns.js b/install/ui/dns.js
index 6a684b9ea207feee2425c8f14dd398918fcaca6b..5a0f5194d9c91434608bfec904368dd3d59c4f9a 100644
--- a/install/ui/dns.js
+++ b/install/ui/dns.js
@@ -52,10 +52,15 @@ IPA.dns.config_entity = function(spec) {
validators: [IPA.dnsforwarder_validator()]
},
{
-type: 'checkboxes',
+type: 'radio',
name: 'idnsforwardpolicy',
-mutex: true,
-options: IPA.create_options(['only', 'first'])
+options: IPA.create_options([
+{
+value: '',
+label: IPA.messages.widget.none_default
+},
+'only', 'first'
+])
},
'idnszonerefresh'
]
@@ -170,10 +175,15 @@ IPA.dns.zone_entity = function(spec) {
validators: [IPA.dnsforwarder_validator()]
},
{
-type: 'checkboxes',
+type: 'radio',
name: 'idnsforwardpolicy',
-mutex: true,
-options: IPA.create_options(['only', 'first'])
+options: IPA.create_options([
+{
+value: '',
+label: IPA.messages.widget.none_default
+},
+'only', 'first'
+])
},
{
type: 'checkbox',
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index be87481018654c6803b9c610df3723566a4efac2..eeac030531302fffc0af79e70a835dca8120f674 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -1516,17 +1516,22 @@ IPA.limit_text = function(value, max_length) {
return limited_text;
};
-IPA.create_options = function(labels, values) {
-
-if(!values) values = labels;
+IPA.create_options = function(values) {
var options = [];
-for (var i=0; ilabels.length; i++) {
-options.push({
-label: labels[i],
-value: values[i]
-});
+for (var i=0; ivalues.length; i++) {
+var val = values[i];
+var option = val;
+
+if (typeof val === 'string') {
+option = {
+value: val,
+label: val
+};
+}
+
+options.push(option);
}
return options;
diff --git a/install/ui/test/data/ipa_init.json b/install/ui/test/data/ipa_init.json
index 6eed01e924abac09eaba843be7d2be8d5b75ce9c..e394a63ddc7bb9284b306049e934256175ec18ef 100644
--- a/install/ui/test/data/ipa_init.json
+++ b/install/ui/test/data/ipa_init.json
@@ -439,6 +439,7 @@
true: True,
widget: {
next: Next,
+none_default: none/default,
page: Page,
prev: Prev,
undo: undo,
diff --git a/ipalib/plugins/internal.py b/ipalib/plugins/internal.py
index 8ce3a00678fec4396cf5bbcdcd2b596bdb751820..fb409d96009272691c22e109547b74001e322657 100644
--- a/ipalib/plugins/internal.py
+++ b/ipalib/plugins/internal.py
@@ -578,6 +578,7 @@ class i18n_messages(Command):
true: _(True),
widget: {
next: _(Next),
+none_default: _(none/default),
page: _(Page),
prev: _(Prev),
undo: _(undo),
--
1.7.7.6
From 596ce427047f80dfd965204e13edab5ece00271e Mon Sep 17 00:00:00 2001
From: Petr
Re: [Freeipa-devel] [PATCH] 0016 Fixes for{add, set, del}attr with managed attributes
On 03/30/2012 03:22 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/16/2012 08:01 PM, Petr Viktorin wrote:
On 03/16/2012 06:35 PM, Petr Viktorin wrote:
On 03/16/2012 06:33 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/15/2012 09:24 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/29/2012 04:34 PM, Petr Viktorin wrote:
On 02/29/2012 03:50 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/27/2012 11:03 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Patch 16 defers validation conversion until after
{add,del,set}attr is
processed, so that we don't search for an integer in a
list of
strings
(this caused ticket #2405), and so that the end result of
these
operations is validated (#2407).
Patch 17 makes these options honor params marked no_create
and
no_update.
https://fedorahosted.org/freeipa/ticket/2405
https://fedorahosted.org/freeipa/ticket/2407
https://fedorahosted.org/freeipa/ticket/2408
NACK on Patch 17 which breaks patch 16.
How is patch 16 broken? It works for me.
My point is they rely on one another, IMHO, so without 17 the
reported
problem still exists.
*attr is specifically made to be powerful. We don't want to
arbitrarily
block updating certain values.
Noted
Not having patch 17 means that the problem reported in 2408
still
occurs. It should probably check both the schema and the
param to
determine if something can have multiple values and reject
that
way.
I see the problem now: the certificate subject base is defined
as a
multi-value attribute in the LDAP schema. If it's changed to
single-value the existing validation should catch it.
Also, most of the config attributes should probably be
required in
the
schema. Am I right?
I'm a newbie here; what do I need to do when changing the
schema? Is
there a patch that does something similar I could use as an
example?
The framework should be able to impose its own single-value
will as
well. If a Param is designated as single-value the *attr should
honor
it.
Here is the updated patch.
Since *attr is powerful enough to modify 'no_update' Params,
which
CRUDUpdate forgets about, I need to check the params of the
LDAPObject
itself.
Updating schema is a bit of a nasty business right now. See
10-selinuxusermap.update for an example.
I'll leave schema changes for after the release, then.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Attached patch includes tests. Note that the test depends on my
patches
12-13, which make ipasearchrecordslimit required.
I gather that this eliminates the need for patch 17? It seems to
work
as-is.
Yes. Patch 17 made *attr honor no_create and no_update, which you
said
is not desired behavior.
The patch doesn't apply because of an encoding change Martin made
recently.
It does seem to do the right thing though.
rob
Attaching rebased patch.
This deletes Martin's change, but unless I tested wrong, his bug
(https://fedorahosted.org/freeipa/ticket/2418) stays fixed. The
tests in
my patch should apply to that ticket as well.
In another fork of this thread, there's discussion if this
approach is
good at all. Maybe we're overengineering a corner case here.
Found another issue, a very subtle one.
When using *attr and an exception occurs where the param name would
appear we want the name passed in to be used.
For example:
$ ipa pwpolicy-mod --setattr=krbpwdmaxfailure=xyz
With this patch it will return:
ipa: ERROR: invalid 'maxfail': must be an integer
It should return:
ipa: ERROR: invalid 'krbpwdmaxfailure': must be an integer
On second thought, this may not be related...
This is https://fedorahosted.org/freeipa/ticket/1418, I'll see if it
makes sense to fix it here.
Okay, this is a different problem. A quick grep of ConversionError in
parameters.py reveals that all of the wrong datatype errors are raised
with the raw parameter name.
Other errors are raised with either that or the cli_name or they
auto-detect. I don't think they follow some rule in this.
Furthermore, our test suite doesn't check exception arguments. Sounds
like a major cleanup coming up here...
The encoding problem does still exist too:
$ ipa config-mod --setattr ipamigrationenabled=false
ipa: ERROR: ipaMigrationEnabled: value #0 invalid per syntax: Invalid
syntax.
Will fix.
Fixed in the attached update, which encodes the value.
I was surprised to find that
config_mod.params['ipamigrationenabled'].attribute is True, while
config_mod.obj.params['ipamigrationenabled'].attribute is False (and so
its encode() method does nothing). That's because 'attribute' is only
set when the params are cloned from the LDAPObject to the CRUD method.
Is there a reason behind this, or is it just that it was easier to do?
For this case it means that params marked no_update will not be encoded
properly -- getting to a working encode() would require either setting
Re: [Freeipa-devel] More types of replica in FreeIPA
On 04/03/2012 12:45 PM, Ondrej Hamada wrote: On 03/13/2012 01:13 AM, Dmitri Pal wrote: On 03/12/2012 06:10 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 17:40 -0400, Dmitri Pal wrote: On 03/12/2012 04:16 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 20:38 +0100, Ondrej Hamada wrote: USER'S operations when connection is OK: --- read data - local write data - forwarding to master authentication: -credentials cached -- authenticate against credentials in local cache -on failure: log failure locally, update data about failures only on lock-down of account -credentials not cached -- forward request to master, on success cache the credentials This scheme doesn't work with Kerberos. Either you have a copy of the user's keys locally or you don't, there is nothing you can really cache if you don't. Simo. Yes this is what we are talking about here - the cache would have to contain user Kerberos key but there should be some expiration on the cache so that fetched and stored keys periodically cleaned following the policy an admin has defined. We would need a mechanism to transfer Kerberos keys, but that would not be sufficient, you'd have to give read-only servers also the realm krbtgt in order to be able to do anything with those keys. The way MS solves hits (I think) is by giving a special RODC krbtgt to each RODC, and then replicating all RODC krbtgt's with full domain controllers. Full domain controllers have logic to use RODC's krbtgt keys instead of the normal krbtgt to perform operations when user's krbtgt are presented to a different server. This is a lot of work and changes in the KDC, not something we can implement easily. As a first implementation I would restrict read-only replicas to not do Kerberos at all, only LDAP for all the lookup stuff necessary. to add a RO KDC we will need to plan a lot of changes in the KDC. We will also need intelligent partial replication where the rules about which object (and which attributes in the object) need/can be replicated are established based on some grouping+filter mechanism. This also is a pretty important change to 389ds. Simo. I agree. I am just trying to structure the discussion a bit so that all what you are saying can be captured in the design document and then we can pick a subset of what Ondrej will actually implement. So let us capture all the complexity and then do a POC for just LDAP part. Sorry for inactivity, I was struggling with a lot of school stuff. I've summed up the main goals, do you agree on them or should I add/remove any? GOALS === Create Hub and Consumer types of replica with following features: * Hub is read-only * Hub interconnects Masters with Consumers or Masters with Hubs or Hubs with other Hubs * Hub is hidden in the network topology * Consumer is read-only * Consumer interconnects Masters/Hubs with clients * Write operations should be forwarded to Master * Consumer should be able to log users into system without communication with master See below. * Consumer should cache user's credentials * Caching of credentials should be configurable If caching of the creds is not configured consumer should forward authentication to master. You also need to think about password changes and kerberos key rotation. * CA server should not be allowed on Hubs and Consumers -- Thank you, Dmitri Pal Sr. Engineering Manager IPA project, Red Hat Inc. --- Looking to carve out IT costs? www.redhat.com/carveoutcosts/ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 69] Use indexed format specifiers in i18n strings
On 04/02/2012 03:15 PM, Rob Crittenden wrote: John Dennis wrote: Translators need to reorder messages to suit the needs of the target language. The conventional positional format specifiers (e.g. %s %d) do not permit reordering because their order is tied to the ordering of the arguments to the printf function. The fix is to use indexed format specifiers. I guess this looks ok but all of these errors are of the format: string error, error number (and inconsistently, sometimes the reverse). Not all of them, e.g. - fprintf(stderr, _(Search for %s on rootdse failed with error %d), + fprintf(stderr, _(Search for %1$s on rootdse failed with error %2$d\n), root_attrs[0], ret); - fprintf(stderr, _(Failed to open keytab '%s': %s\n), keytab, + fprintf(stderr, _(Failed to open keytab '%1$s': %2$s\n), keytab, error_message(krberr)); Do those really need to be re-orderable? You can never make too few assumptions about foreign languages. Most likely at least some will need reordering. Enforcing indexed specifiers everywhere means we don't have to worry about individual cases, or change our strings when a new language is added. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] More types of replica in FreeIPA
On Tue, 2012-04-03 at 18:45 +0200, Ondrej Hamada wrote: On 03/13/2012 01:13 AM, Dmitri Pal wrote: On 03/12/2012 06:10 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 17:40 -0400, Dmitri Pal wrote: On 03/12/2012 04:16 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 20:38 +0100, Ondrej Hamada wrote: USER'S operations when connection is OK: --- read data - local write data - forwarding to master authentication: -credentials cached -- authenticate against credentials in local cache -on failure: log failure locally, update data about failures only on lock-down of account -credentials not cached -- forward request to master, on success cache the credentials This scheme doesn't work with Kerberos. Either you have a copy of the user's keys locally or you don't, there is nothing you can really cache if you don't. Simo. Yes this is what we are talking about here - the cache would have to contain user Kerberos key but there should be some expiration on the cache so that fetched and stored keys periodically cleaned following the policy an admin has defined. We would need a mechanism to transfer Kerberos keys, but that would not be sufficient, you'd have to give read-only servers also the realm krbtgt in order to be able to do anything with those keys. The way MS solves hits (I think) is by giving a special RODC krbtgt to each RODC, and then replicating all RODC krbtgt's with full domain controllers. Full domain controllers have logic to use RODC's krbtgt keys instead of the normal krbtgt to perform operations when user's krbtgt are presented to a different server. This is a lot of work and changes in the KDC, not something we can implement easily. As a first implementation I would restrict read-only replicas to not do Kerberos at all, only LDAP for all the lookup stuff necessary. to add a RO KDC we will need to plan a lot of changes in the KDC. We will also need intelligent partial replication where the rules about which object (and which attributes in the object) need/can be replicated are established based on some grouping+filter mechanism. This also is a pretty important change to 389ds. Simo. I agree. I am just trying to structure the discussion a bit so that all what you are saying can be captured in the design document and then we can pick a subset of what Ondrej will actually implement. So let us capture all the complexity and then do a POC for just LDAP part. Sorry for inactivity, I was struggling with a lot of school stuff. I've summed up the main goals, do you agree on them or should I add/remove any? GOALS === Create Hub and Consumer types of replica with following features: * Hub is read-only * Hub interconnects Masters with Consumers or Masters with Hubs or Hubs with other Hubs * Hub is hidden in the network topology * Consumer is read-only * Consumer interconnects Masters/Hubs with clients * Write operations should be forwarded to Master * Consumer should be able to log users into system without communication with master We need to define how this can be done, it will almost certainly mean part of the consumer is writable, plus it also means you need additional access control and policies, on what the Consumer should be allowed to see. * Consumer should cache user's credentials Ok what credentials ? As I explained earlier Kerberos creds cannot really be cached. Either they are transferred with replication or the KDC needs to be change to do chaining. Neither I consider as 'caching'. A password obtained through an LDAP bind could be cached, but I am not sure it is worth it. * Caching of credentials should be configurable See above. * CA server should not be allowed on Hubs and Consumers Missing points: - Masters should not transfer KRB keys to HUBs/Consumers by default. - We need selective replication if you want to allow distributing a partial set of Kerberos credentials to consumers. With Hubs it becomes complicated to decide what to replicate about credentials. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 247 Fix installation when server hostname is not in a default domain
When IPA server is configured with DNS and its hostname is not
located in a default domain, SRV records are not valid.
Additionally, httpd does not serve XMLRPC interface because it
IPA server domain-realm mapping is missing in krb5.conf. All CLI
commands were then failing.
This patch amends this configuration. It fixes SRV records in
served domain to include full FQDN instead of relative hostname
when the IPA server hostname is not located in served domain.
IPA server forward record is also placed to correct zone.
When IPA server is not in a served domain a proper domain-realm
mapping is configured to krb5.conf. The template was improved
in order to be able to hold this information.
https://fedorahosted.org/freeipa/ticket/2602
From 1ca8d09b9552ec9a55921a29a37f8d0472e96e6b Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Wed, 4 Apr 2012 16:31:04 +0200
Subject: [PATCH] Fix installation when server hostname is not in a default
domain
When IPA server is configured with DNS and its hostname is not
located in a default domain, SRV records are not valid.
Additionally, httpd does not serve XMLRPC interface because it
IPA server domain-realm mapping is missing in krb5.conf. All CLI
commands were then failing.
This patch amends this configuration. It fixes SRV records in
served domain to include full FQDN instead of relative hostname
when the IPA server hostname is not located in served domain.
IPA server forward record is also placed to correct zone.
When IPA server is not in a served domain a proper domain-realm
mapping is configured to krb5.conf. The template was improved
in order to be able to hold this information.
https://fedorahosted.org/freeipa/ticket/2602
---
install/share/krb5.conf.template |2 +-
ipaserver/install/bindinstance.py | 38 +---
ipaserver/install/krbinstance.py | 13
3 files changed, 40 insertions(+), 13 deletions(-)
diff --git a/install/share/krb5.conf.template b/install/share/krb5.conf.template
index 3bdbc9995386b98e4b5b449c5f37ede0408cf775..eda8ba6fe647d54d5feef1acda41c482b0dbcefa 100644
--- a/install/share/krb5.conf.template
+++ b/install/share/krb5.conf.template
@@ -22,7 +22,7 @@
[domain_realm]
.$DOMAIN = $REALM
$DOMAIN = $REALM
-
+$OTHER_DOMAIN_REALM_MAPS
[dbmodules]
$REALM = {
db_library = ipadb.so
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index ba8b7b5cc3c7f327fb86b98a3cc6d11720ed1d47..ce316612251ae96021caa62fd785858f7b438703 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -395,7 +395,6 @@ class BindInstance(service.Service):
self.domain = domain_name
self.forwarders = forwarders
self.host = fqdn.split(.)[0]
-self.host_domain = '.'.join(fqdn.split(.)[1:])
self.suffix = util.realm_to_suffix(self.realm)
self.ntp = ntp
self.reverse_zone = reverse_zone
@@ -409,6 +408,21 @@ class BindInstance(service.Service):
self.__setup_sub_dict()
+@property
+def host_domain(self):
+return '.'.join(self.fqdn.split(.)[1:])
+
+@property
+def host_in_rr(self):
+# when a host is not in a default domain, it needs to be referred
+# with FQDN and not in a domain-relative host name
+if not self.host_in_default_domain():
+return normalize_zone(self.fqdn)
+return self.host
+
+def host_in_default_domain(self):
+return normalize_zone(self.host_domain) == normalize_zone(self.domain)
+
def create_sample_bind_zone(self):
bind_txt = ipautil.template_file(ipautil.SHARE_DIR + bind.zone.db.template, self.sub_dict)
[bind_fd, bind_name] = tempfile.mkstemp(.db,sample.zone.)
@@ -474,7 +488,7 @@ class BindInstance(service.Service):
if self.ntp:
optional_ntp = \n;ntp server\n
-optional_ntp += _ntp._udp\t\tIN SRV 0 100 123\t%s % self.host
+optional_ntp += _ntp._udp\t\tIN SRV 0 100 123\t%s % self.host_in_rr
else:
optional_ntp =
@@ -495,7 +509,7 @@ class BindInstance(service.Service):
self._ldap_mod(dns.ldif, self.sub_dict)
def __setup_zone(self):
-if self.host_domain != self.domain:
+if not self.host_in_default_domain():
# add DNS domain for host first
root_logger.debug(Host domain (%s) is different from DNS domain (%s)! \
% (self.host_domain, self.domain))
@@ -512,14 +526,14 @@ class BindInstance(service.Service):
def __add_self(self):
zone = self.domain
resource_records = (
-(_ldap._tcp, SRV, 0 100 389 %s % self.host),
+(_ldap._tcp, SRV, 0 100 389 %s % self.host_in_rr),
(_kerberos, TXT, self.realm),
-(_kerberos._tcp, SRV, 0 100 88 %s % self.host),
-(_kerberos._udp, SRV, 0 100 88 %s % self.host),
-(_kerberos-master._tcp, SRV, 0
Re: [Freeipa-devel] [PATCH] 0016 Fixes for{add, set, del}attr with managed attributes
Petr Viktorin wrote:
On 03/30/2012 03:22 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/16/2012 08:01 PM, Petr Viktorin wrote:
On 03/16/2012 06:35 PM, Petr Viktorin wrote:
On 03/16/2012 06:33 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/15/2012 09:24 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/29/2012 04:34 PM, Petr Viktorin wrote:
On 02/29/2012 03:50 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/27/2012 11:03 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
Patch 16 defers validation conversion until after
{add,del,set}attr is
processed, so that we don't search for an integer in a
list of
strings
(this caused ticket #2405), and so that the end result of
these
operations is validated (#2407).
Patch 17 makes these options honor params marked no_create
and
no_update.
https://fedorahosted.org/freeipa/ticket/2405
https://fedorahosted.org/freeipa/ticket/2407
https://fedorahosted.org/freeipa/ticket/2408
NACK on Patch 17 which breaks patch 16.
How is patch 16 broken? It works for me.
My point is they rely on one another, IMHO, so without 17 the
reported
problem still exists.
*attr is specifically made to be powerful. We don't want to
arbitrarily
block updating certain values.
Noted
Not having patch 17 means that the problem reported in 2408
still
occurs. It should probably check both the schema and the
param to
determine if something can have multiple values and reject
that
way.
I see the problem now: the certificate subject base is defined
as a
multi-value attribute in the LDAP schema. If it's changed to
single-value the existing validation should catch it.
Also, most of the config attributes should probably be
required in
the
schema. Am I right?
I'm a newbie here; what do I need to do when changing the
schema? Is
there a patch that does something similar I could use as an
example?
The framework should be able to impose its own single-value
will as
well. If a Param is designated as single-value the *attr should
honor
it.
Here is the updated patch.
Since *attr is powerful enough to modify 'no_update' Params,
which
CRUDUpdate forgets about, I need to check the params of the
LDAPObject
itself.
Updating schema is a bit of a nasty business right now. See
10-selinuxusermap.update for an example.
I'll leave schema changes for after the release, then.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Attached patch includes tests. Note that the test depends on my
patches
12-13, which make ipasearchrecordslimit required.
I gather that this eliminates the need for patch 17? It seems to
work
as-is.
Yes. Patch 17 made *attr honor no_create and no_update, which you
said
is not desired behavior.
The patch doesn't apply because of an encoding change Martin made
recently.
It does seem to do the right thing though.
rob
Attaching rebased patch.
This deletes Martin's change, but unless I tested wrong, his bug
(https://fedorahosted.org/freeipa/ticket/2418) stays fixed. The
tests in
my patch should apply to that ticket as well.
In another fork of this thread, there's discussion if this
approach is
good at all. Maybe we're overengineering a corner case here.
Found another issue, a very subtle one.
When using *attr and an exception occurs where the param name would
appear we want the name passed in to be used.
For example:
$ ipa pwpolicy-mod --setattr=krbpwdmaxfailure=xyz
With this patch it will return:
ipa: ERROR: invalid 'maxfail': must be an integer
It should return:
ipa: ERROR: invalid 'krbpwdmaxfailure': must be an integer
On second thought, this may not be related...
This is https://fedorahosted.org/freeipa/ticket/1418, I'll see if it
makes sense to fix it here.
Okay, this is a different problem. A quick grep of ConversionError in
parameters.py reveals that all of the wrong datatype errors are
raised
with the raw parameter name.
Other errors are raised with either that or the cli_name or they
auto-detect. I don't think they follow some rule in this.
Furthermore, our test suite doesn't check exception arguments. Sounds
like a major cleanup coming up here...
The encoding problem does still exist too:
$ ipa config-mod --setattr ipamigrationenabled=false
ipa: ERROR: ipaMigrationEnabled: value #0 invalid per syntax:
Invalid
syntax.
Will fix.
Fixed in the attached update, which encodes the value.
I was surprised to find that
config_mod.params['ipamigrationenabled'].attribute is True, while
config_mod.obj.params['ipamigrationenabled'].attribute is False (and so
its encode() method does nothing). That's because 'attribute' is only
set when the params are cloned from the LDAPObject to the CRUD method.
Is there a reason behind this, or is it just that it was easier to do?
For this case it means that params marked no_update will not be encoded
properly -- getting to a working encode() would
[Freeipa-devel] [PATCH] 0033 Pass make-test arguments through to Nose + Test coverage
Currently, our test script forwards a select few command line arguments
to nosetests.
This patch removes the filtering, passing all arguments through.
This allows things like disabling output redirection (--nocapture),
dropping into a debugger (--pdb, --pdb-failures), coverage reporting
(--with-cover, once installed), etc.
https://fedorahosted.org/freeipa/ticket/2135
I believe this is a better solution than adding individual options as
they're needed.
---
A coverage report can be generated by combining data from both the tests
and the server. I run this:
Setup:
yum install python-coverage
echo /.coverage* .git/info/exclude
echo /htmlcov/ .git/info/exclude
Terminal 1:
coverage erase
coverage run -p --source . lite-server.py
Terminal 2:
kinit
./make-test --with-coverage --cover-inclusive
Terminal 1 again:
^C
coverage combine
coverage html --omit=/usr/lib/*
Then view ./htmlcov/index.html in a browser.
--
Petr³
From 1200eb4efedc18ea1a12c67c0f754d84fc58bd9e Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Wed, 4 Apr 2012 09:42:22 -0400
Subject: [PATCH] Pass make-test arguments through to Nose
Currently, our test script forwards a select few command line arguments
to nosetests.
This patch removes the filtering, passing all arguments through.
This allows things like disabling output redirection (--nocapture),
dropping into a debugger (--pdb, --pdb-failures), coverage reporting
(--with-cover, if installed), etc.
https://fedorahosted.org/freeipa/ticket/2135
---
make-test | 34 ++
1 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/make-test b/make-test
index b429a7162f4f5c0121355f0fcfff8ba1039ba90a..02a17db9c039869dc11f48a3880841f1ad8a9cde 100755
--- a/make-test
+++ b/make-test
@@ -16,38 +16,14 @@ nose = '/usr/bin/nosetests'
ran = []
fail = []
-parser = optparse.OptionParser(
- usage='usage: %prog [MODULE...]',
-)
-parser.add_option('--stop',
- action='store_true',
- default=False,
- help='Stop running tests after the first error or failure',
-)
-parser.add_option('--pdb',
- action='store_true',
- default=False,
- help='Drop into debugger on errors',
-)
-parser.add_option('--pdb-failures',
- action='store_true',
- default=False,
- help='Drop into debugger on failures',
-)
-(options, args) = parser.parse_args()
-
-cmd = [nose] + args + [
+cmd = [
+nose,
'-v',
'--with-doctest',
'--doctest-tests',
'--exclude=plugins',
]
-if options.stop:
-cmd.append('--stop')
-if options.pdb:
-cmd.append('--pdb')
-if options.pdb_failures:
-cmd.append('--pdb-failures')
+cmd += sys.argv[1:]
# This must be set so ipalib.api gets initialized property for tests:
@@ -60,7 +36,9 @@ for v in versions:
pver = python + v
if not path.isfile(pver):
continue
-if 0 != call([pver] + cmd):
+command = [pver] + cmd
+print ' '.join(cmd)
+if 0 != call(cmd):
fail.append(pver)
ran.append(pver)
--
1.7.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 246 Configure SELinux for httpd during upgrades
Martin Kosek wrote: SELinux configuration for httpd instance was set for new installations only. Upgraded IPA servers (namely 2.1.x - 2.2.x upgrade) missed the configuration. This lead to AVCs when httpd tries to contact ipa_memcached and user not being able to log in. This patch updates ipa-upgradeconfig to configure SELinux in the same way as ipa-server-install does. https://fedorahosted.org/freeipa/ticket/2603 ACK, pushed to master and ipa-2-2 rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] More types of replica in FreeIPA
On 04/04/2012 03:02 PM, Simo Sorce wrote: On Tue, 2012-04-03 at 18:45 +0200, Ondrej Hamada wrote: On 03/13/2012 01:13 AM, Dmitri Pal wrote: On 03/12/2012 06:10 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 17:40 -0400, Dmitri Pal wrote: On 03/12/2012 04:16 PM, Simo Sorce wrote: On Mon, 2012-03-12 at 20:38 +0100, Ondrej Hamada wrote: USER'S operations when connection is OK: --- read data - local write data - forwarding to master authentication: -credentials cached -- authenticate against credentials in local cache -on failure: log failure locally, update data about failures only on lock-down of account -credentials not cached -- forward request to master, on success cache the credentials This scheme doesn't work with Kerberos. Either you have a copy of the user's keys locally or you don't, there is nothing you can really cache if you don't. Simo. Yes this is what we are talking about here - the cache would have to contain user Kerberos key but there should be some expiration on the cache so that fetched and stored keys periodically cleaned following the policy an admin has defined. We would need a mechanism to transfer Kerberos keys, but that would not be sufficient, you'd have to give read-only servers also the realm krbtgt in order to be able to do anything with those keys. The way MS solves hits (I think) is by giving a special RODC krbtgt to each RODC, and then replicating all RODC krbtgt's with full domain controllers. Full domain controllers have logic to use RODC's krbtgt keys instead of the normal krbtgt to perform operations when user's krbtgt are presented to a different server. This is a lot of work and changes in the KDC, not something we can implement easily. As a first implementation I would restrict read-only replicas to not do Kerberos at all, only LDAP for all the lookup stuff necessary. to add a RO KDC we will need to plan a lot of changes in the KDC. We will also need intelligent partial replication where the rules about which object (and which attributes in the object) need/can be replicated are established based on some grouping+filter mechanism. This also is a pretty important change to 389ds. Simo. I agree. I am just trying to structure the discussion a bit so that all what you are saying can be captured in the design document and then we can pick a subset of what Ondrej will actually implement. So let us capture all the complexity and then do a POC for just LDAP part. Sorry for inactivity, I was struggling with a lot of school stuff. I've summed up the main goals, do you agree on them or should I add/remove any? GOALS === Create Hub and Consumer types of replica with following features: * Hub is read-only * Hub interconnects Masters with Consumers or Masters with Hubs or Hubs with other Hubs * Hub is hidden in the network topology * Consumer is read-only * Consumer interconnects Masters/Hubs with clients * Write operations should be forwarded to Master * Consumer should be able to log users into system without communication with master We need to define how this can be done, it will almost certainly mean part of the consumer is writable, plus it also means you need additional access control and policies, on what the Consumer should be allowed to see. Right, in such case the Consumers and Hubs will have to be masters (from 389-DS's point of view). * Consumer should cache user's credentials Ok what credentials ? As I explained earlier Kerberos creds cannot really be cached. Either they are transferred with replication or the KDC needs to be change to do chaining. Neither I consider as 'caching'. A password obtained through an LDAP bind could be cached, but I am not sure it is worth it. * Caching of credentials should be configurable See above. * CA server should not be allowed on Hubs and Consumers Missing points: - Masters should not transfer KRB keys to HUBs/Consumers by default. Add point: - storing of the Krb creds must be configurable and disabled by default - We need selective replication if you want to allow distributing a partial set of Kerberos credentials to consumers. With Hubs it becomes complicated to decide what to replicate about credentials. Simo. Rich mentioned that they are planning support for LDAP filters in fractional replication in the future, but currently it is not supported. -- Regards, Ondrej Hamada FreeIPA team jabber:oh...@jabbim.cz IRC: ohamada ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 73 Check whether the default user group is POSIX when adding new user with --noprivate
On 3.4.2012 13:04, Martin Kosek wrote:
On Tue, 2012-04-03 at 13:02 +0200, Martin Kosek wrote:
On Tue, 2012-04-03 at 11:58 +0200, Jan Cholasta wrote:
https://fedorahosted.org/freeipa/ticket/2572
Honza
NACK.
This creates a regression:
# ipa group-show foogroup
Group name: foogroup
Description: foo
GID: 358800017
# ipa user-add --first=Foo --last=Bar fbar5 --gidnumber=358800017 --noprivate
--
Added user fbar5
--
User login: fbar5
First name: Foo
Last name: Bar
Full name: Foo Bar
Display name: Foo Bar
Initials: FB
Home directory: /home/fbar5
GECOS field: Foo Bar
Login shell: /bin/sh
Kerberos principal: fb...@idm.lab.bos.redhat.com
UID: 358800019
GID: 358800012
Password: False
Member of groups: ipausers
Kerberos keys available: False
# id fbar5
uid=358800019(fbar5) gid=358800012(ipausers) groups=358800012(ipausers)
Custom user group (GID) was overwritten.
I think we also want a test case for this situation.
Martin
... and we also want to have the new error message(s) i18n-able.
Martin
Updated patch attached.
Honza
--
Jan Cholasta
From 72258c8cf11ebe798db26782fa0d392972e556b6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Thu, 29 Mar 2012 09:12:36 -0400
Subject: [PATCH] Check whether the default user group is POSIX when adding
new user with --noprivate.
ticket 2572
---
API.txt|6 +-
ipalib/plugins/user.py | 12 ++-
tests/test_xmlrpc/test_group_plugin.py |4 +-
tests/test_xmlrpc/test_user_plugin.py | 163 +++-
tests/util.py |8 ++
5 files changed, 183 insertions(+), 10 deletions(-)
diff --git a/API.txt b/API.txt
index e9eb1e1..cc0d1eb 100644
--- a/API.txt
+++ b/API.txt
@@ -3082,7 +3082,7 @@ option: Str('mail', attribute=True, cli_name='email', multivalue=True, required=
option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=True)
-option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', minvalue=1, multivalue=False, required=True)
+option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, required=True)
option: Str('street', attribute=True, cli_name='street', multivalue=False, required=False)
option: Str('l', attribute=True, cli_name='city', multivalue=False, required=False)
option: Str('st', attribute=True, cli_name='state', multivalue=False, required=False)
@@ -3140,7 +3140,7 @@ option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='princi
option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue=True, query=True, required=False)
option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, query=True, required=False)
option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', default=999, minvalue=1, multivalue=False, query=True, required=False)
-option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', minvalue=1, multivalue=False, query=True, required=False)
+option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, query=True, required=False)
option: Str('street', attribute=True, autofill=False, cli_name='street', multivalue=False, query=True, required=False)
option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, query=True, required=False)
option: Str('st', attribute=True, autofill=False, cli_name='state', multivalue=False, query=True, required=False)
@@ -3189,7 +3189,7 @@ option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue
option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, required=False)
option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', default=999, minvalue=1, multivalue=False, required=False)
-option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', minvalue=1, multivalue=False, required=False)
+option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', default=999, minvalue=1, multivalue=False, required=False)
option: Str('street', attribute=True, autofill=False, cli_name='street', multivalue=False, required=False)
option: Str('l', attribute=True, autofill=False, cli_name='city', multivalue=False, required=False)
option: Str('st',
[Freeipa-devel] [PATCH] 1002 make revocation_reason required
When revoking a certificate passing in an empty revocation reason caused
an Internal Error. It already sets a default so making it required
prevents empty values and it still operates the same way.
rob
From 669879148bd0c98911327661007e18906dd5499d Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Wed, 4 Apr 2012 14:57:22 -0400
Subject: [PATCH] Make revocation_reason required when revoking a certificate.
This will prevent errors if an empty reason is provided and it is
set by default one doesn't have to always set it on the command-line.
https://fedorahosted.org/freeipa/ticket/2597
---
API.txt|2 +-
VERSION|2 +-
ipalib/plugins/cert.py |2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/API.txt b/API.txt
index 1464bc60f1bf08ed5b2f0ee5213a721ab1e95382..c2b17942f7aeaf64715343b51c95b25c6c4099d3 100644
--- a/API.txt
+++ b/API.txt
@@ -433,7 +433,7 @@ output: Output('result', type 'dict', None)
command: cert_revoke
args: 1,1,1
arg: Str('serial_number')
-option: Int('revocation_reason?', autofill=True, default=0, maxvalue=10, minvalue=0)
+option: Int('revocation_reason', autofill=True, default=0, maxvalue=10, minvalue=0)
output: Output('result', None, None)
command: cert_show
args: 1,1,1
diff --git a/VERSION b/VERSION
index e15d463ba5c8ad10e2fe7bc7b4fcaf83c00ac8a8..dd8cfbb4ae3cf0d238448a4e48d94c5724fba2c3 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=2010061412
# #
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=32
+IPA_API_VERSION_MINOR=33
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index 7a3888121b21567d7c3ada0bddb01187aa4bd775..75eace24651b39bac7af7091653afe995b8aff13 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -526,7 +526,7 @@ class cert_revoke(VirtualCommand):
# FIXME: The default is 0. Is this really an Int param?
takes_options = (
-Int('revocation_reason?',
+Int('revocation_reason',
label=_('Reason'),
doc=_('Reason for revoking the certificate (0-10)'),
minvalue=0,
--
1.7.6
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 71] improve handling of ds instances during uninstall
--
John Dennis jden...@redhat.com
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
From 032ebced279d32bb3e6d51ef23fecaca36774ec7 Mon Sep 17 00:00:00 2001
From: John Dennis jden...@redhat.com
Date: Wed, 4 Apr 2012 15:19:29 -0400
Subject: [PATCH 71] improve handling of ds instances during uninstall
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Ticket #2502
* remove the running flag from backup_state in cainstance.py and
dsinstance.py because it does not provide the correct
information. In cainstance the running flag was never referenced
because restarting dirsrv instances occurs later in dsinstance. In
dsinstance when the running flag is set it incorrectly identifed the
PKI ds instance configured earlier by cainstance. The intent was to
determine if there were any ds instances other than those owned by
IPA which will need to be restarted upon uninstall. Clearly the PKI
ds instance does not qualify. We were generating a traceback when at
the conclusion of dsinstance.uninstall we tried to start the
remaining ds instances as indicated by the running flag, but there
were none to restart (because the running flag had been set as a
consequence of the PKI ds instance).
* We only want to restart ds instances if there are other ds instances
besides those owned by IPA. We shouldn't be stopping all ds
instances either, but that's going to be covered by another
ticket. The fix for restarting other ds instances at the end of
uninstall is to check and see if there are other ds instances
remaining after we've removed ours, if so we restart them. Also it's
irrelevant if those ds instances were not present when we installed,
it only matters if they exist after we restore things during
uninstall. If they are present we have to start them back up because
we shut them down during uninstall.
* Add new function get_ds_instances() which returns a list of existing
ds instances.
* fixed error messages that incorrectly stated it failed to restart
a ds instance when it should be failed to create.
---
ipaserver/install/cainstance.py |7 +-
ipaserver/install/dsinstance.py | 44 +++---
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index f953100..64175e4 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -292,7 +292,6 @@ class CADSInstance(service.Service):
root_logger.critical(failed to add user %s % e)
def __create_instance(self):
-self.backup_state(running, dsinstance.is_ds_running())
self.backup_state(serverid, self.serverid)
inf_txt = ipautil.template_str(INF_TEMPLATE, self.sub_dict)
@@ -310,7 +309,7 @@ class CADSInstance(service.Service):
ipautil.run(args)
root_logger.debug(completed creating ds instance)
except ipautil.CalledProcessError, e:
-root_logger.critical(failed to restart ds instance %s % e)
+root_logger.critical(failed to create ds instance %s % e)
inf_fd.close()
def load_pkcs12(self):
@@ -383,13 +382,9 @@ class CADSInstance(service.Service):
if self.is_configured():
self.print_msg(Unconfiguring CA directory server)
-running = self.restore_state(running)
enabled = self.restore_state(enabled)
serverid = self.restore_state(serverid)
-if not running is None:
-ipaservices.knownservices.dirsrv.stop(self.serverid)
-
if not enabled is None and not enabled:
ipaservices.knownservices.dirsrv.disable()
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index e549e13..9dc82c3 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -90,6 +90,34 @@ def erase_ds_instance_data(serverid):
#except:
#pass
+def get_ds_instances():
+'''
+Return a sorted list of all 389ds instances.
+
+If the instance name ends with '.removed' it is ignored. This
+matches 389ds behavior.
+'''
+
+dirsrv_instance_dir='/etc/dirsrv'
+instance_prefix = 'slapd-'
+
+instances = []
+
+for basename in os.listdir(dirsrv_instance_dir):
+pathname = os.path.join(dirsrv_instance_dir, basename)
+# Must be a directory
+if os.path.isdir(pathname):
+# Must start with prefix and not end with .removed
+if basename.startswith(instance_prefix) and not basename.endswith('.removed'):
+# Strip off prefix
+instance = basename[len(instance_prefix):]
+# Must be non-empty
+if instance:
+instances.append(basename[len(instance_prefix):])
+
+instances.sort()
+return instances
+
def check_ports():
ds_unsecure = installutils.port_available(389)
ds_secure =
Re: [Freeipa-devel] [PATCH 71] improve handling of ds instances during uninstall
On Wed, 04 Apr 2012, John Dennis wrote:
Ticket #2502
* remove the running flag from backup_state in cainstance.py and
dsinstance.py because it does not provide the correct
information. In cainstance the running flag was never referenced
because restarting dirsrv instances occurs later in dsinstance. In
dsinstance when the running flag is set it incorrectly identifed the
PKI ds instance configured earlier by cainstance. The intent was to
determine if there were any ds instances other than those owned by
IPA which will need to be restarted upon uninstall. Clearly the PKI
ds instance does not qualify. We were generating a traceback when at
the conclusion of dsinstance.uninstall we tried to start the
remaining ds instances as indicated by the running flag, but there
were none to restart (because the running flag had been set as a
consequence of the PKI ds instance).
* We only want to restart ds instances if there are other ds instances
besides those owned by IPA. We shouldn't be stopping all ds
instances either, but that's going to be covered by another
ticket. The fix for restarting other ds instances at the end of
uninstall is to check and see if there are other ds instances
remaining after we've removed ours, if so we restart them. Also it's
irrelevant if those ds instances were not present when we installed,
it only matters if they exist after we restore things during
uninstall. If they are present we have to start them back up because
we shut them down during uninstall.
* Add new function get_ds_instances() which returns a list of existing
ds instances.
* fixed error messages that incorrectly stated it failed to restart
a ds instance when it should be failed to create.
---
+def get_ds_instances():
+'''
+Return a sorted list of all 389ds instances.
+
+If the instance name ends with '.removed' it is ignored. This
+matches 389ds behavior.
+'''
+
+dirsrv_instance_dir='/etc/dirsrv'
+instance_prefix = 'slapd-'
+
+instances = []
+
+for basename in os.listdir(dirsrv_instance_dir):
+pathname = os.path.join(dirsrv_instance_dir, basename)
+# Must be a directory
+if os.path.isdir(pathname):
+# Must start with prefix and not end with .removed
+if basename.startswith(instance_prefix) and not
basename.endswith('.removed'):
+# Strip off prefix
+instance = basename[len(instance_prefix):]
+# Must be non-empty
+if instance:
+instances.append(basename[len(instance_prefix):])
You have already generated basename[len(instance_prefix):], may be it
could be as simple as
instances.append(instance)
here?
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
