[Freeipa-devel] [PATCH 0048] Add nfs:NONE to default PAC types only when needed
Petr Viktorin
pviktori at redhat.com
Mon Apr 15 12:02:31 UTC 2013
On 04/15/2013 12:32 PM, Tomas Babej wrote:
> On 04/12/2013 04:52 PM, Petr Viktorin wrote:
>> On 04/12/2013 04:10 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> We need to add nfs:NONE as a default PAC type only if there's no
>>> other default PAC type for nfs. Adds a update plugin which
>>> determines whether default PAC type for nfs is set and adds
>>> nfs:NONE PAC type accordingly.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3555
>>>
>>> Tomas
>>
>> Looks good, works well, see a few nitpicks below.
>>
>> [...]
>>> diff --git a/ipaserver/install/plugins/update_pacs.py
>>> b/ipaserver/install/plugins/update_pacs.py
>> [...]
>>> +from ipapython.ipa_log_manager import *
>>
>> Please don't use star imports in new code. Only import what you need
>> (that is, remove this import entirely).
>>
>>> +class update_pacs(PostUpdate):
>>> + """
>>> + Includes default nfs:None only if no nfs: entry present in
>>> ipakrbauthzdata.
>>> + """
>>> +
>>> + order = MIDDLE
>>> +
>>> + def execute(self, **options):
>>> + ldap = self.obj.backend
>>> +
>>> + try:
>>> + dn = DN('cn=ipaConfig', 'cn=etc', api.env.basedn)
>>> + entry = ldap.get_entry(dn, ['ipakrbauthzdata'])
>>> + pacs = entry.get('ipakrbauthzdata')
>>> +
>>> + if pacs is None:
>>
>> This shouldn't happen (ipaKrbAuthzData gets a default in previous
>> updates). It's not necessary to complicate the code, `pacs =
>> entry.get('ipakrbauthzdata', [])` would do.
> Yes, this was probably overcautious.
>>
>>> + self.log.warning('No ipakrbauthzdata attribute found.')
>>> + return (False, False, [])
>>>
>>> + except errors.NotFound:
>>> + self.log.warning('Error retrieving: %s' % str(dn))
>>> + return (False, False, [])
>>> +
>>> + nfs_pac_set = any(pac.startswith('nfs:') for pac in pacs)
>>> +
>>> + if not nfs_pac_set:
>>> + self.log.debug('Adding nfs:None to default PAC types')
>>> + updated_pacs = pacs + [u'nfs:NONE']
>>> + update = dict(ipakrbauthzdata=updated_pacs)
>>> + ldap.update_entry(dn, update)
>>
>> This will work, but the preferred form for updates is now:
>> entry['ipakrbauthzdata'] = updated_pacs
>> ldap.update_entry(entry)
>>
>>> + else:
>>> + self.log.debug('PAC for nfs is already set, not adding
>>> nfs:NONE.')
>>> +
>>> + return (False, False, [])
>>> +
>>> +api.register(update_pacs)
>>> -- 1.8.1.4
>>
>> Please also add the new update file to Makefile.am.
>>
>
> Thank you for the review. Updated patch attached.
>
> Tomas
ACK
--
Petr³
More information about the Freeipa-devel
mailing list