[Freeipa-devel] [PATCHES 202-222] Ipaplatform refactoring

[Tomas Babej]

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.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0228-ipaplatform-Move-paths-from-installers-to-paths-modu.patch
Type: text/x-patch
Size: 61142 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140617/c79df79b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0226-2-ipaplatform-Document-the-platform-tasks-API.patch
Type: text/x-patch
Size: 4756 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140617/c79df79b/attachment-0001.bin>