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 [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
