[SSSD] sss_cache flush ssh hosts list.

Jan Cholasta jcholast at redhat.com
Fri Aug 29 08:41:58 UTC 2014


Dne 28.8.2014 v 18:11 Pavel Reichl napsal(a):
>
> On 08/25/2014 02:05 PM, Jan Cholasta wrote:
>> Dne 4.8.2014 v 19:34 Pavel Reichl napsal(a):
>>>
>>> On 07/21/2014 02:08 PM, Jakub Hrozek wrote:
>>>> On Mon, Jul 21, 2014 at 01:55:20PM +0200, Jan Cholasta wrote:
>>>>> On 18.7.2014 06:41, William wrote:
>>>>>>>> Ignore that last patch, I messed up and didn't include a .h file.
>>>>>>>> Here
>>>>>>>> is the fixed patch.
>>>>>>> This new patch seems to be based on some older version of the
>>>>>>> patch, it
>>>>>>> uses wrong option name, sysdb_update_ssh_host still has the confdb
>>>>>>> argument, etc.
>>>>>>>
>>>>>> Fixed, and fixed the documentation into the xml file, removed the
>>>>>> references to the .pot file.
>>>>> I don't see any of the requested fixes in the patch.
>>>>>
>>>>> See attachment for fixed patch. I also did a few additional tweaks to
>>>>> make
>>>>> the patch work when SSSD is updated from a previous version and when
>>>>> building without SSH bits.
>>>> Awesome thank you, I guess some of us need to review the patch now
>>>> you're also an author.
>>> While Jan was so kind to give me some hints on how to test patch
>>> properly, we found out a little bug. So Jan is going to  prepare an
>>> updated patch.
>>>
>>> I'll do the review then.
>>
>> I can't reproduce the issue, maybe something else was wrong in your
>> setup?
>>
>> For the record, I installed IPA in a VM and then ran these commands on
>> it:
>>
>> $ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
>> [no errors]
>> $ cat /var/lib/sss/pubconf/known_hosts
>> [not empty]
>> # systemctl stop ipa
>> $ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
>> [no errors]
>> $ cat /var/lib/sss/pubconf/known_hosts
>> [not empty]
>> # sss_cache -H
>> $ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
>> [no errors]
>> $ cat /var/lib/sss/pubconf/known_hosts
>> [empty]
>> # systemctl start ipa
>> $ sss_ssh_knownhostsproxy --debug 10 $HOSTNAME
>> [no errors]
>> $ cat /var/lib/sss/pubconf/known_hosts
>> [not empty]
>>
>> Pavel had an empty /var/lib/sss/pubconf/known_hosts in the last step,
>> I have it non-empty as expected.
>>
>> (sorry for the delay, I was on leave)
>>
> This test ran OK this time, so I think you are right.
>
> I have some comments to the patch itself.
>
> I'm not personally fan of big patches like this one. Could you at least
> separate the man page changes to separate patch - so we can ask some
> native speaker to review it.
>
> Also new options in sss_cache -h and -H are not documented in man pages,
> could you document them?
>
> Please see some minor comments inline:

Fixed everything, except:

>> +    ret = sysdb_set_entry_attr(domain->sysdb, dn, attrs, mod_op);
>
> We should check return value here, right?

We want to return this value directly, there's no need to check it 
(other sysdb_set_*_attr functions do this as well).

Updated patch attached.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-sss_cache-tool-to-flush-SSH-hosts-cache.patch
Type: text/x-patch
Size: 20496 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140829/aef3af15/attachment.bin>


More information about the sssd-devel mailing list