Re: [Freeipa-devel] [PATCH] Add {user, host, sourcehost}Category to HBAC and make accessTime multivalue.
Rob Crittenden wrote: Pavel Zuna wrote: Rob Crittenden wrote: Pavel Zuna wrote: Due to the format of accessTime (it has commas and spaces in it), we can't use the List parameter type. I made it so that accessTime values have to be entered one by one using new commands. We also agreed, that we're going to rename GeneralizedTime parameter to AccessTime to prevent confusion with RFC 4517 standard. I attached a separate patch for clarity. Pavel A couple of questions: - Would it make sense to leave time in as an option that takes a singular value? If someone wants multiple times they can use the new add interface, right? It would and I think it's a good idea, updated patch attached. - What are these new enums for? If there is only one choice do you really have a choice? Well for now, we only have the 'all' in categories, but the list is expected to grow. At first I didn't include categories in the plugin, because of this, but Sumit wanted it to be complete. - We still need some tests for GeneralizedTime/AccessTime. Ok, added to my TODO list. The patch isn't applying for me: $ patch -p1 --dry-run 0003-Fix-takes_options-in-automount-plugin.patch patching file ipalib/plugins/hbac.py patching file tests/test_xmlrpc/test_hbac_plugin.py Hunk #1 FAILED at 52. Hunk #2 FAILED at 84. 2 out of 3 hunks FAILED -- saving rejects to file tests/test_xmlrpc/test_hbac_plugin.py.rej Since you have to mess with this anyway, can you: - add another test to also test adding the access time on the add. You added back the capability but the tests are still removed AFAICT. - add a FUTURE or FIXME comment indicating that the enumerators are future-proofing things by making them a 1-option enumerator for now? rob Fixed patch attached. Pavel 0001-Add-user-host-sourcehost-Category-to-HBAC-and-make.patch Description: application/mbox 0002-Rename-GeneralizedTime-to-AccessTime.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Remove unnecessary error: prefixes
Martin From 96c64ff2a1051c1e8bdcad9e8aef9488f0e26e87 Mon Sep 17 00:00:00 2001 From: Martin Nagy mn...@redhat.com Date: Mon, 23 Nov 2009 08:42:30 +0100 Subject: [PATCH] Remove unnecessary error: prefixes The parser.error() method prepends the error: prefix itself. Adding it to the error string is not necessary and doesn't look good. --- install/tools/ipa-replica-prepare |2 +- install/tools/ipa-server-install | 10 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare index 3dc0ccc..bc86a41 100755 --- a/install/tools/ipa-replica-prepare +++ b/install/tools/ipa-replica-prepare @@ -59,7 +59,7 @@ def parse_options(): pkcs12 = [options.dirsrv_pkcs12, options.http_pkcs12, options.dirsrv_pin, options.http_pin] cnt = pkcs12.count(None) if cnt 0 and cnt 4: -parser.error(error: All PKCS#12 options are required if any are used.) +parser.error(All PKCS#12 options are required if any are used.) if len(args) != 1: parser.error(must provide the fully-qualified name of the replica) diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install index be525f7..9b5946a 100755 --- a/install/tools/ipa-server-install +++ b/install/tools/ipa-server-install @@ -131,11 +131,11 @@ def parse_options(): if (options.ds_user or options.realm_name or options.dm_password or options.admin_password or options.master_password): -parser.error(error: In uninstall mode, -u, r, -p and -P options are not allowed) +parser.error(In uninstall mode, -u, r, -p and -P options are not allowed) elif options.unattended: if (not options.ds_user or not options.realm_name or not options.dm_password or not options.admin_password): -parser.error(error: In unattended mode you need to provide at least -u, -r, -p and -a options) +parser.error(In unattended mode you need to provide at least -u, -r, -p and -a options) if options.setup_dns: if not options.forwarders and not options.no_forwarders: parser.error(You must specify at least one --forwarder option or --no-forwarders option) @@ -146,14 +146,14 @@ def parse_options(): pkcs12 = [options.dirsrv_pkcs12, options.http_pkcs12, options.dirsrv_pin, options.http_pin] cnt = pkcs12.count(None) if cnt 0 and cnt 4: -parser.error(error: All PKCS#12 options are required if any are used.) +parser.error(All PKCS#12 options are required if any are used.) if (options.external_cert_file or options.external_ca_file) and not options.ca: -parser.error(error: --ca required to use the external CA options.) +parser.error(--ca required to use the external CA options.) if ((options.external_cert_file and not options.external_ca_file) or (not options.external_cert_file and options.external_ca_file)): -parser.error(error: if either external option is used, both are required.) +parser.error(if either external option is used, both are required.) if options.external_ca and not options.ca: # Go ahead and be nice and fix things up -- 1.6.2.5 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Remove unnecessary error: prefixes
Martin Nagy wrote: Martin ack ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Ask the user before overwriting /etc/named.conf
Martin Nagy wrote: Martin ack. As an aside, it might be nice if the actual package name(s) were used to make it easier for the user to know exactly what they are missing for BIND and the BIND LDAP plug-in. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add idnsUpdatePolicy into the dns plug-in
Martin Nagy wrote: Martin Should there be a validator on idnsUpdatePolicy to ensure that each policy is terminated by a ;? If one wants to have multiple policies is it set with idnspolicy=policy1;policy2;policy3;? Should the formatting be included in the doc message, or an example of usage be added? rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 320 remove /etc/ipa/ipa.conf
Jason Gerard DeRose wrote: On Wed, 2009-11-25 at 17:43 -0500, Rob Crittenden wrote: The configuration file /etc/ipa/ipa.conf was used by the v1 clients and servers to manually set realm, domain and server(s). This has been renamed to /etc/ipa/default.conf in v2. Some old utilities still referenced this old file and we still created it. This patch should completely remove it. rob This isn't applying to the current master: Applying: Replace /etc/ipa/ipa.conf with /etc/ipa/default.conf error: patch failed: ipa.spec.in:473 error: ipa.spec.in: patch does not apply Patch failed at 0001 Replace /etc/ipa/ipa.conf with /etc/ipa/default.conf Boy that spec file trips me up ever time. New patch attached. rob freeipa-320.2-conf.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add idnsUpdatePolicy into the dns plug-in
On Tue, 2009-12-01 at 10:17 -0500, Rob Crittenden wrote: Martin Nagy wrote: Martin Should there be a validator on idnsUpdatePolicy to ensure that each policy is terminated by a ;? If one wants to have multiple policies is it set with idnspolicy=policy1;policy2;policy3;? Should the formatting be included in the doc message, or an example of usage be added? That might not be that easy to do, we would probably need to do more than that, e.g. make sure bind can accept the policy string. For now, I'm only adding the idnsupdatepolicy into the dns plugin so that I can use it to create zones with it during installation (patch will follow soon). Might I add the other bits later after I'm done with this? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add {user, host, sourcehost}Category to HBAC and make accessTime multivalue.
Pavel Zůna wrote: Rob Crittenden wrote: Pavel Zuna wrote: Rob Crittenden wrote: Pavel Zuna wrote: Due to the format of accessTime (it has commas and spaces in it), we can't use the List parameter type. I made it so that accessTime values have to be entered one by one using new commands. We also agreed, that we're going to rename GeneralizedTime parameter to AccessTime to prevent confusion with RFC 4517 standard. I attached a separate patch for clarity. Pavel A couple of questions: - Would it make sense to leave time in as an option that takes a singular value? If someone wants multiple times they can use the new add interface, right? It would and I think it's a good idea, updated patch attached. - What are these new enums for? If there is only one choice do you really have a choice? Well for now, we only have the 'all' in categories, but the list is expected to grow. At first I didn't include categories in the plugin, because of this, but Sumit wanted it to be complete. - We still need some tests for GeneralizedTime/AccessTime. Ok, added to my TODO list. The patch isn't applying for me: $ patch -p1 --dry-run 0003-Fix-takes_options-in-automount-plugin.patch patching file ipalib/plugins/hbac.py patching file tests/test_xmlrpc/test_hbac_plugin.py Hunk #1 FAILED at 52. Hunk #2 FAILED at 84. 2 out of 3 hunks FAILED -- saving rejects to file tests/test_xmlrpc/test_hbac_plugin.py.rej Since you have to mess with this anyway, can you: - add another test to also test adding the access time on the add. You added back the capability but the tests are still removed AFAICT. - add a FUTURE or FIXME comment indicating that the enumerators are future-proofing things by making them a 1-option enumerator for now? rob Fixed patch attached. Pavel ack x2, push master x2 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Change object_class of group object.
Pavel Zůna wrote: Rob Crittenden wrote: Pavel Zuna wrote: Some groups created by default don't have ipaUserGroup and won't show up in searches. Pavel nack, isn't the better approach to fix up the groups that are created by default without the ipaUserGroup objectclass? It is. Fixed patch attached. rob Pavel ack, pushed to master. NOTE: we should probably revisit the editors group to see if it is needed/wanted in the new UI. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Ask the user before overwriting /etc/named.conf
On Tue, 2009-12-01 at 10:15 -0500, Rob Crittenden wrote:
Martin Nagy wrote:
Martin
ack.
As an aside, it might be nice if the actual package name(s) were used to
make it easier for the user to know exactly what they are missing for
BIND and the BIND LDAP plug-in.
Yeah, I guess you're right. New patch attached.
Martin
From 258092b18fcba45631202833975e71817b647450 Mon Sep 17 00:00:00 2001
From: Martin Nagy mn...@redhat.com
Date: Fri, 13 Nov 2009 16:57:51 +0100
Subject: [PATCH] Ask the user before overwriting /etc/named.conf
---
install/tools/ipa-replica-install |6 ++
install/tools/ipa-server-install |6 ++
ipaserver/install/bindinstance.py | 10 +-
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index e8fabd7..9827bef 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -251,10 +251,8 @@ def check_dirsrv():
sys.exit(1)
def check_bind():
-if not bindinstance.check_inst():
-print --setup-dns was specified but bind or the BIND LDAP plug-in
-print is not installed on the system
-print Please install bind and the LDAP plug-in and restart the setup program
+if not bindinstance.check_inst(unattended=True):
+print Aborting installation
sys.exit(1)
def main():
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 748101d..34ddb0f 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -541,10 +541,8 @@ def main():
# check bind packages are installed
if options.setup_dns:
-if not bindinstance.check_inst():
-print --setup-dns was specified but bind or the BIND LDAP plug-in
-print is not installed on the system
-print Please install bind and the LDAP plug-in and restart the setup program
+if not bindinstance.check_inst(options.unattended):
+print Aborting installation
return 1
if options.ca:
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 2a922a3..e2edcd3 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -30,17 +30,25 @@ from ipapython import sysrestore
from ipapython import ipautil
from ipalib import api, util
-def check_inst():
+def check_inst(unattended):
# So far this file is always present in both RHEL5 and Fedora if all the necessary
# bind packages are installed (RHEL5 requires also the pkg: caching-nameserver)
if not os.path.exists('/etc/named.rfc1912.zones'):
+print BIND was not found on this system
+print Please install the bind package and start the installation again
return False
# Also check for the LDAP BIND plug-in
if not os.path.exists('/usr/lib/bind/ldap.so') and \
not os.path.exists('/usr/lib64/bind/ldap.so'):
+print The BIND LDAP plug-in was not found on this system
+print Please install the bind-dyndb-ldap package and start the installation again
return False
+if not unattended and os.path.exists('/etc/named.conf'):
+msg = Existing BIND configuration detected, overwrite?
+return ipautil.user_input(msg, False)
+
return True
class BindInstance(service.Service):
--
1.6.2.5
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 320 remove /etc/ipa/ipa.conf
On Tue, 2009-12-01 at 10:36 -0500, Rob Crittenden wrote: Jason Gerard DeRose wrote: On Wed, 2009-11-25 at 17:43 -0500, Rob Crittenden wrote: The configuration file /etc/ipa/ipa.conf was used by the v1 clients and servers to manually set realm, domain and server(s). This has been renamed to /etc/ipa/default.conf in v2. Some old utilities still referenced this old file and we still created it. This patch should completely remove it. rob This isn't applying to the current master: Applying: Replace /etc/ipa/ipa.conf with /etc/ipa/default.conf error: patch failed: ipa.spec.in:473 error: ipa.spec.in: patch does not apply Patch failed at 0001 Replace /etc/ipa/ipa.conf with /etc/ipa/default.conf Boy that spec file trips me up ever time. New patch attached. rob ack. pushed to master. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Add idnsUpdatePolicy into the dns plug-in
Martin Nagy wrote: On Tue, 2009-12-01 at 10:17 -0500, Rob Crittenden wrote: Martin Nagy wrote: Martin Should there be a validator on idnsUpdatePolicy to ensure that each policy is terminated by a ;? If one wants to have multiple policies is it set with idnspolicy=policy1;policy2;policy3;? Should the formatting be included in the doc message, or an example of usage be added? That might not be that easy to do, we would probably need to do more than that, e.g. make sure bind can accept the policy string. For now, I'm only adding the idnsupdatepolicy into the dns plugin so that I can use it to create zones with it during installation (patch will follow soon). Might I add the other bits later after I'm done with this? Martin Sure, that makes sense. ack. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 323 type argument for x509.load_certificate()
Add a type argument (PEM or DER) for x509.load_certificate(). Certs are coming out of LDAP as binary so we need to be able to handle that too. Seems more sane to add an argument that to base64-encode it. rob freeipa-323-cert.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 323 type argument for x509.load_certificate()
On Tue, 2009-12-01 at 17:20 -0500, Rob Crittenden wrote: Add a type argument (PEM or DER) for x509.load_certificate(). Certs are coming out of LDAP as binary so we need to be able to handle that too. Seems more sane to add an argument that to base64-encode it. rob ack. pushed to master. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 324 add errors.NotImplementedError
On Tue, 2009-12-01 at 17:23 -0500, Rob Crittenden wrote: This deprecates a similar patch from John last month. The server-side baseclass rabase defines a framework for CA plugins. When I added this code I set it up to return errors.NotImplementedError but didn't actually include that error class in the commit. I'm adding that in now, favoring it over the python built-in exception of the same name because it is more friendly to the client (they get a command not implemented instead of an InternalError. Ideally we should not register commands that aren't implemented, I'll tackle that soon but for now this will fill in the gap. This also wraps the call to cert_revoke() in the service plugin to not blow up if using the selfsign CA which doesn't implement revocation. rob ack. pushed to master. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
