Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 09:48 PM, Tomas Babej wrote: On 06/25/2014 09:35 PM, Petr Viktorin wrote: On 06/25/2014 07:16 PM, Tomas Babej wrote: On 06/25/2014 04:59 PM, Tomas Babej wrote: On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451: host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. OK, not a reason to hold the patch back. But, the fact that the tool can't handle them doesn't make them less important. Let's keep the ticket open, or open a new one. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. I've opened an easyfix ticket for the rest: https://fedorahosted.org/freeipa/ticket/4399 Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Attaching a correct patchset this time. I hate to break it to you, but... you sent the wrong patch 228 :( No way! That's a official fail combo on my part. Here's the correct patch. ACK, pushed to master: e5e42fc83ae74f0e0c68e68417a39fe6f2f2ae63 -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 2fda5e386b9fdf75b6c02fbeedafaeb001d80a74 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 25 Jun 2014 16:12:19 +0200 Subject: [PATCH] ipaplatform: Fix misspelled path constant --- ipatests/test_integration/tasks.py | 2 +- ipatests/test_integration/test_caless.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index ccb0d8693a1e89d95bbeb4c75fc263d0f689cb36..cd8f98306030f46c099a08ca1a558fd10807bfa9 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -219,7 +219,7 @@ def install_replica(master, replica, setup_ca=True): '--ip-address', replica.ip, replica.hostname]) replica_bundle = master.get_file_contents( -paths.REPLICA_INFO_TEMPLATE_GPG % replica.hostname) +paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname) replica_filename = os.path.join(replica.config.test_dir, 'replica-info.gpg') replica.put_file_contents(replica_filename, replica_bundle) diff --git a/ipatests/test_integration/test_caless.py b/ipatests/test_integration/test_caless.py index
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 5c1cc30a4100ab11fa9a31d478ecb4677edf78dc Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Wed, 25 Jun 2014 16:12:19 +0200 Subject: [PATCH] ipaplatform: Fix misspelled path constant --- ipatests/test_integration/tasks.py | 2 +- ipatests/test_integration/test_caless.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py index ccb0d8693a1e89d95bbeb4c75fc263d0f689cb36..cd8f98306030f46c099a08ca1a558fd10807bfa9 100644 --- a/ipatests/test_integration/tasks.py +++ b/ipatests/test_integration/tasks.py @@ -219,7 +219,7 @@ def install_replica(master, replica, setup_ca=True): '--ip-address', replica.ip,
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 04:59 PM, Tomas Babej wrote: On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451:host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org Attaching a correct patchset this time. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 9c402cb6d6e8abf59284e6a524114253024059b3 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Thu, 19 Jun 2014 15:09:37 +0200 Subject: [PATCH] ipaplatform: Fix build warnings The newly created ipaplatform subdirectories base and fedora were mentioned multiple times in the specfile, which produced build warnings. Part of: https://fedorahosted.org/freeipa/ticket/4052 --- freeipa.spec.in | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/freeipa.spec.in b/freeipa.spec.in index
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/25/2014 07:16 PM, Tomas Babej wrote: On 06/25/2014 04:59 PM, Tomas Babej wrote: On 06/25/2014 04:13 PM, Tomas Babej wrote: On 06/25/2014 04:01 PM, Tomas Babej wrote: On 06/25/2014 10:48 AM, Petr Viktorin wrote: On 06/19/2014 03:52 PM, Tomas Babej wrote: On 06/19/2014 12:52 PM, Tomas Babej wrote: On 06/18/2014 10:52 AM, Petr Viktorin wrote: On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. Well, since you asked... install/tools/ipa-upgradeconfig:236: ipautil.run([paths.PKI_SETUP_PROXY, '-pki_instance_root=/var/lib' ipaserver/install/cainstance.py:1330: -pki_instance_root=/var/lib, ipaserver/install/dsinstance.py:209:InstallLdifFile= /var/lib/dirsrv/boot.ldif ipaserver/install/dsinstance.py:210:inst_dir= /var/lib/dirsrv/scripts-$SERVERID ipaserver/install/ipa_backup.py:464: '--exclude=/var/lib/ipa/backup', ipatests/test_integration/tasks.py:451: host.run_command(find /var/lib/sss/db -name '*.ldb' | install/tools/ipa-replica-conncheck:403: /usr/sbin/ipa-replica-conncheck + install/tools/ipa-replica-conncheck:414: print_info(/usr/sbin/ipa-replica-conncheck + .join(remote_check_opts)) ipapython/ipautil.py:296:env[PATH] = /bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin ipaserver/install/cainstance.py:88:ConfigFile = /usr/share/pki/ca/conf/database.ldif ipaserver/install/bindinstance.py:829: ipautil.run(['/usr/libexec/generate-rndc-key.sh']) /me will think twice about teasing nex time. This are paths requiring manual changes in one way or the other and as such cannot be handled by my tool. Let's not stall the patcheset on this. We can fix these (and surely there are other) as we go along. OK, not a reason to hold the patch back. But, the fact that the tool can't handle them doesn't make them less important. Let's keep the ticket open, or open a new one. I guess it'll be a while before we catch them all, but now it's at least clear where these paths should be, so anyone porting to another distro can send patches (or tickets) upstream. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Could you just pick one instead? Would ipa_backup.py break if it had a trailing slash here? Yes. I verified it produces the same result with or without trailing slash, fixed. In ipa-client-install, if you set: NSSWITCH_CONF = paths.NSSWITCH_CONF then you should only use one of those later. (Preferably paths.*, to get rid of the redundant constants.) Perhaps this is for another patch that would clean up all the cases where these trivial module variables are used. I agree. Fixed this occurence. I've opened an easyfix ticket for the rest: https://fedorahosted.org/freeipa/ticket/4399 Fixed all mentioned issues. I also attached a patch 230, which removes the base Authconfig class. Attaching one additional patch, which removes unnecessary build warnings. 226, 230, 231 look good Attaching whole updated patchset. Attaching one more patch which should fix broken CI tests. Self-NACK - It seems I omitted one occurence of NSSWITCH_CONF in ipa-client-install, fixed now. Attaching the whole patchset for your convenience. -- Attaching a correct patchset this time. I hate to break it to you, but... you sent the wrong patch 228 :( They're pretty independent, and there are fixes for failing tests, so I went ahead with the other ones. 226, 230, 231, 235: ACK, pushed to master: c8511d3b3baa389069156bf9991a9f4c7d64cf4a -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/19/2014 12:52 PM, Tomas Babej wrote:
On 06/18/2014 10:52 AM, Petr Viktorin wrote:
On 06/17/2014 02:15 PM, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
Attached is a new version of patch 226, and a new patch 228, which moves
the paths from installers to the paths module.
In patch 226, there's another certificated typo in
remove_ca_cert_from_systemwide_ca_store
I greped the repository, and I do not see many paths lurking around any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
If you see any forgotten paths, which should be added to the module, let
me know.
I see another duplicate:
SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d
SSSD_PUBCONF_KRB5_INCLUDE_D_DIR =
/var/lib/sss/pubconf/krb5.include.d/
Rather than using e.g.
filename = paths.VAR_KERBEROS_KRB5KDC_DIR + file
It would be cleaner to use
filename = os.path.join(paths.VAR_KERBEROS_KRB5KDC_DIR, file)
Fixed all mentioned issues. I also attached a patch 230, which removes
the base Authconfig class.
Attaching one additional patch, which removes unnecessary build warnings.
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
From 687e44b6ab9eab2bd0349c3ddbe7457c20f40bf1 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 19 Jun 2014 15:09:37 +0200
Subject: [PATCH] ipaplatform: Fix build warnings
The newly created ipaplatform subdirectories base and fedora were
mentioned multiple times in the specfile, which produced build
warnings.
Part of: https://fedorahosted.org/freeipa/ticket/4052
---
freeipa.spec.in | 4
1 file changed, 4 deletions(-)
diff --git a/freeipa.spec.in b/freeipa.spec.in
index b719b4a21fd2263a6fb58b3dffd4784868e630e9..823c34354e4a42ae548f0d969bbf67d6c2471a9b 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -832,11 +832,7 @@ fi
%dir %{python_sitelib}/ipalib
%{python_sitelib}/ipalib/*
%dir %{python_sitelib}/ipaplatform
-%dir %{python_sitelib}/ipaplatform/base
-%dir %{python_sitelib}/ipaplatform/fedora
%{python_sitelib}/ipaplatform/*
-%{python_sitelib}/ipaplatform/base/*.py*
-%{python_sitelib}/ipaplatform/fedora/*.py*
%attr(0644,root,root) %{python_sitearch}/default_encoding_utf8.so
%{python_sitelib}/ipapython-*.egg-info
%{python_sitelib}/freeipa-*.egg-info
--
1.9.3
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/17/2014 02:15 PM, Tomas Babej wrote: On 06/17/2014 12:03 PM, Timo Aaltonen wrote: On 17.06.2014 11:16, Martin Kosek wrote: Attached is a new version of patch 226, and a new patch 228, which moves the paths from installers to the paths module. In patch 226, there's another certificated typo in remove_ca_cert_from_systemwide_ca_store I greped the repository, and I do not see many paths lurking around any more, there are only some in the error messages (as these can't be reliably replaced automatically, and will need some manual love). If you see any forgotten paths, which should be added to the module, let me know. I see another duplicate: SSS_KRB5_INCLUDE_D = /var/lib/sss/pubconf/krb5.include.d SSSD_PUBCONF_KRB5_INCLUDE_D_DIR = /var/lib/sss/pubconf/krb5.include.d/ Rather than using e.g. filename = paths.VAR_KERBEROS_KRB5KDC_DIR + file It would be cleaner to use filename = os.path.join(paths.VAR_KERBEROS_KRB5KDC_DIR, file) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing the `self`
argument.
Rebasing this all the time must be painful, so to avoid another review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure around? I
thought it is replaced by the tasks concept that are easier to customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install:(sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n, nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install:run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install:run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most platformized
module (more than the server, IMO).
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing the `self`
argument.
Rebasing this all the time must be painful, so to avoid another review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure around? I
thought it is replaced by the tasks concept that are easier to customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install:(sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n, nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install:
run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install:
run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
--
t
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 17.6.2014 14:15, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing the `self`
argument.
Rebasing this all the time must be painful, so to avoid another review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure around? I
thought it is replaced by the tasks concept that are easier to customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install:(sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n, nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install:run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install:run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
Attached is a new version of patch 226, and a new patch 228, which moves
the paths from installers to the paths module.
I greped the repository, and I do not see many paths lurking around any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
I don't know the code but why it is not possible to use
%s % (paths.blahblah) ?
IMHO paths should never be hardcoded in strings/messages shown to user.
Petr^2 Spacek
If you see any forgotten paths, which should be added to the module, let
me know.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/17/2014 02:44 PM, Petr Spacek wrote:
On 17.6.2014 14:15, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing
the `self`
argument.
Rebasing this all the time must be painful, so to avoid another
review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure
around? I
thought it is replaced by the tasks concept that are easier to
customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in
AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install: (sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n,
nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most
platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
Attached is a new version of patch 226, and a new patch 228, which moves
the paths from installers to the paths module.
I greped the repository, and I do not see many paths lurking around any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
I don't know the code but why it is not possible to use
%s % (paths.blahblah) ?
IMHO paths should never be hardcoded in strings/messages shown to user.
Petr^2 Spacek
Of course they can, they just cannot be replaced by the tool I developed
for this refactoring, and need to be replaced manually, as the tool
lacks the capability.
They need to be replaced manually.
If you see any forgotten paths, which should be added to the module, let
me know.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 17.06.2014 15:15, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing the `self`
argument.
Rebasing this all the time must be painful, so to avoid another review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure around? I
thought it is replaced by the tasks concept that are easier to customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install:(sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n, nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install:
run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install:
run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
Attached is a new version of patch 226, and a new patch 228, which moves
the paths from installers to the paths module.
I greped the repository, and I do not see many paths lurking around any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
If you see any forgotten paths, which should be added to the module, let
me know.
Sure thing! Looks more complete now, and at least the paths I was
patching before (in ipa-client-automount) are fixed.
--
t
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 17.6.2014 14:50, Tomas Babej wrote:
On 06/17/2014 02:44 PM, Petr Spacek wrote:
On 17.6.2014 14:15, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing
the `self`
argument.
Rebasing this all the time must be painful, so to avoid another
review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure
around? I
thought it is replaced by the tasks concept that are easier to
customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in
AuthConfig base
class refers to nonexistent paths.
2) There are still many non-converted paths in ipa-client installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND =
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install: (sout, serr, returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n,
nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most
platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
Attached is a new version of patch 226, and a new patch 228, which moves
the paths from installers to the paths module.
I greped the repository, and I do not see many paths lurking around any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
I don't know the code but why it is not possible to use
%s % (paths.blahblah) ?
IMHO paths should never be hardcoded in strings/messages shown to user.
Petr^2 Spacek
Of course they can, they just cannot be replaced by the tool I developed
for this refactoring, and need to be replaced manually, as the tool
lacks the capability.
They need to be replaced manually.
Ah, sure. I didn't notice you were speaking about automatic replacement. I'm
sorry!
Petr^2 Spacek
If you see any forgotten paths, which should be added to the module, let
me know.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring
On 06/17/2014 03:12 PM, Petr Spacek wrote:
On 17.6.2014 14:50, Tomas Babej wrote:
On 06/17/2014 02:44 PM, Petr Spacek wrote:
On 17.6.2014 14:15, Tomas Babej wrote:
On 06/17/2014 12:03 PM, Timo Aaltonen wrote:
On 17.06.2014 11:16, Martin Kosek wrote:
On 06/16/2014 07:50 PM, Petr Viktorin wrote:
On 06/16/2014 02:53 PM, Tomas Babej wrote:
On 06/10/2014 05:07 PM, Petr Viktorin wrote:
On 06/10/2014 10:13 AM, Tomas Babej wrote:
On 06/06/2014 01:04 PM, Petr Viktorin wrote:
On 06/05/2014 03:14 PM, Petr Viktorin wrote:
On 06/04/2014 11:42 AM, Tomas Babej wrote:
Hi,
the following set of patches implements the ticket:
https://fedorahosted.org/freeipa/ticket/4052
[...]
0202: OK
0203: OK
0204: OK
0205: OK
0206: OK
0207: OK
(sorry for the conflict!)
0208: OK
0209: OK
0210: OK
0211: OK
0212: OK
0213: OK
0214: OK
0215: OK
0216: OK
0217: OK
0218: OK
0219: OK
0220: OK
0221: OK
0222: OK
modify_nsswitch_pam_stack and modify_pam_to_use_krb5 are missing
the `self`
argument.
Rebasing this all the time must be painful, so to avoid another
review
round-trip I've had Tomáš ACK the attached four-liner on IRC.
Thanks guys!
I looked at the changes and have couple questions:
1) What is the motivation for keeping AuthConfig infrastructure
around? I
thought it is replaced by the tasks concept that are easier to
customize in
other platforms. IMO, it just obfuscates the process.
How is
def modify_pam_to_use_krb5(self, statestore):
auth_config = FedoraAuthConfig()
statestore.backup_state('authconfig', 'krb5', True)
auth_config.enable(krb5)
auth_config.add_option(nostart)
auth_config.execute()
more readable than
def modify_pam_to_use_krb5(self, statestore):
statestore.backup_state('authconfig', 'krb5', True)
ipautil.run(authconfig --enablekrb5 --nostart)
? And this was just the easy example. Also, documentation in
AuthConfig base
class refers to nonexistent paths.
Also, regarding the Authconfig class, attached is a diff that refactors
a more complex method (than the one Martin used) to not use Authconfig
class. I can see some value in keeping it, as it makes the code somewhat
clearer (enable/disable methods as opposed to long-winded options).
However, at the very least we should remove the Authconfig class from
the base platform module.
Please look at the diff and speak out your opinions.
2) There are still many non-converted paths in ipa-client
installers:
$ git grep bin/ ipa-client/
...
ipa-client/ipa-install/ipa-client-install:SSH_AUTHORIZEDKEYSCOMMAND
=
'/usr/bin/sss_ssh_authorizedkeys'
ipa-client/ipa-install/ipa-client-install:SSH_PROXYCOMMAND =
'/usr/bin/sss_ssh_knownhostsproxy'
ipa-client/ipa-install/ipa-client-install: (sout, serr,
returncode) =
run([/usr/bin/certutil, -L, -d, /etc/pki/nssdb, -n,
nickname],
raiseonerr=False)
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, IPA CA])
ipa-client/ipa-install/ipa-client-install: run([/usr/bin/certutil,
-D, -d, /etc/pki/nssdb, -n, client_nss_nickname])
...
We should convert at least those as ipa-client will be the most
platformized
module (more than the server, IMO).
and many others all over the place, just 'git grep /etc/'
working on the debian client patches now..
Attached is a new version of patch 226, and a new patch 228, which
moves
the paths from installers to the paths module.
I greped the repository, and I do not see many paths lurking around
any
more, there are only some in the error messages (as these can't be
reliably replaced automatically, and will need some manual love).
I don't know the code but why it is not possible to use
%s % (paths.blahblah) ?
IMHO paths should never be hardcoded in strings/messages shown to user.
Petr^2 Spacek
Of course they can, they just cannot be replaced by the tool I developed
for this refactoring, and need to be replaced manually, as the tool
lacks the capability.
They need to be replaced manually.
Ah, sure. I didn't notice you were speaking about automatic
replacement. I'm sorry!
Petr^2 Spacek
If you see any forgotten paths, which should be added to the
module, let
me know.
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
diff --git a/ipaplatform/fedora/tasks.py b/ipaplatform/fedora/tasks.py
index c20ecd3..54d97f1 100644
--- a/ipaplatform/fedora/tasks.py
+++ b/ipaplatform/fedora/tasks.py
@@ -92,7 +92,8 @@ class FedoraTaskNamespace(BaseTaskNamespace):
was_sssd_installed,
was_sssd_configured):
-auth_config = FedoraAuthConfig()
+args = []
+
if statestore.has_state('authconfig'):
# disable only those configurations that we enabled during install
for conf in ('ldap', 'krb5', 'sssd', 'sssdauth', 'mkhomedir'):
@@ -101,20 +102,20 @@ class
