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

Reply via email to