[Freeipa-devel] [PATCH 0473-0476]DNS Locations: Prologue
Petr Spacek
pspacek at redhat.com
Tue May 10 13:23:46 UTC 2016
On 10.5.2016 14:44, Martin Basti wrote:
>
>
> On 10.05.2016 14:33, Petr Spacek wrote:
>> On 6.5.2016 10:20, Martin Basti wrote:
>>> https://fedorahosted.org/freeipa/ticket/2008
>>>
>>> Patches attached.
>>>
>>>
>>> freeipa-mbasti-0473-DNS-Locations-Always-create-DNS-related-privileges.patch
>>>
>>>
>>> From 9a936740da7cdacec150acc92a45041a98ce7cb3 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti <mbasti at redhat.com>
>>> Date: Wed, 4 May 2016 17:33:52 +0200
>>> Subject: [PATCH 1/4] DNS Locations: Always create DNS related privileges
>>>
>>> DNS privileges are important for handling DNS locations which can be
>>> created without DNS servers in IPA topology. We will also need this
>>> privileges presented for future feature 'External DNS support'
>> Seems reasonable, ACK.
>>
>>
>>> freeipa-mbasti-0474-DNS-Locations-add-new-attributes-and-objectclasses.patch
>>>
>>>
>>> From a7766da5fd1a72884308d4206c9cde262f5c8d35 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti <mbasti at redhat.com>
>>> Date: Thu, 5 May 2016 11:12:00 +0200
>>> Subject: [PATCH 2/4] DNS Locations: add new attributes and objectclasses
>>>
>>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>> ---
>>> install/share/60ipadns.ldif | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
>>> index
>>> e0ed0ab869cea0478d9640bb509c6267abed1a01..31c2f71f8566d04a05709f1359b20e6fa51921ce
>>> 100644
>>> --- a/install/share/60ipadns.ldif
>>> +++ b/install/share/60ipadns.ldif
>>> @@ -70,9 +70,13 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME
>>> 'idnsSecKeyRevoke' DESC 'DNSKE
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME 'idnsSecKeySep' DESC
>>> 'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY booleanMatch
>>> SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME 'idnsSecAlgorithm' DESC
>>> 'DNSKEY algorithm: string used as mnemonic' EQUALITY caseIgnoreIA5Match
>>> SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26
>>> SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
>>> attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME 'idnsSecKeyRef' DESC
>>> 'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
>>> +attributeTypes: ( 2.16.840.1.113730.3.8.5.32 NAME 'ipaLocation' DESC
>>> 'Reference to IPA location' EQUALITY distinguishedNameMatch SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
>>> +attributeTypes: ( 2.16.840.1.113730.3.8.5.33 NAME 'ipaLocationWeight' DESC
>>> 'Weight for the server in IPA location' EQUALITY integerMatch SYNTAX
>>> 1.3.6.1.4.1.1466.115.121.1.27 SINGLE-VALUE X-ORIGIN 'IPA v4.4' )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord' DESC 'dns
>>> Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $
>>> idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $ a6Record $
>>> nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $ mXRecord $
>>> mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $ KeyRecord
>>> $ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $ dNameRecord
>>> $ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $
>>> TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $
>>> DHCIDRecord $ HIPRecord $ SPFRecord ) )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone
>>> class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $ idnsSOAmName $
>>> idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $
>>> idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowQuery $
>>> idnsAllowTransfer $ idnsAllowSyncPTR $ idnsForwardPolicy $ idnsForwarders $
>>> idnsSecInlineSigning $ nSEC3PARAMRecord ) )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.2 NAME 'idnsConfigObject' DESC
>>> 'DNS global config options' STRUCTURAL MAY ( idnsForwardPolicy $
>>> idnsForwarders $ idnsAllowSyncPTR $ idnsZoneRefresh $ idnsPersistentSearch ) )
>>> objectClasses: ( 2.16.840.1.113730.3.8.12.18 NAME 'ipaDNSZone' SUP top
>>> AUXILIARY MUST idnsName MAY managedBy X-ORIGIN 'IPA v3' )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.3 NAME 'idnsForwardZone' DESC
>>> 'Forward Zone class' SUP top STRUCTURAL MUST ( idnsName $ idnsZoneActive )
>>> MAY ( idnsForwarders $ idnsForwardPolicy ) )
>>> objectClasses: ( 2.16.840.1.113730.3.8.6.4 NAME 'idnsSecKey' DESC 'DNSSEC
>>> key metadata' STRUCTURAL MUST ( idnsSecKeyRef $ idnsSecKeyCreated $
>>> idnsSecAlgorithm ) MAY ( idnsSecKeyPublish $ idnsSecKeyActivate $
>>> idnsSecKeyInactive $ idnsSecKeyDelete $ idnsSecKeyZone $ idnsSecKeyRevoke $
>>> idnsSecKeySep $ cn ) X-ORIGIN 'IPA v4.1' )
>>> +objectClasses: ( 2.16.840.1.113730.3.8.6.7 NAME 'ipaLocationObject' DESC
>>> 'Object for storing IPA server location' AUXILIARY MUST ( idnsName ) MAY (
>>> description ) X-ORIGIN 'IPA v4.4' )
>> Why is it AUXILIARY? AFAIK it should be STRUCTURAL because there will not be
>> any other object class on the location object (at least not in the beginning).
>>
>>> +objectClasses: ( 2.16.840.1.113730.3.8.6.8 NAME 'ipaLocationMember' DESC
>>> 'Member object of IPA location' AUXILIARY MAY ( ipaLocation $
>>> ipaLocationWeight ) X-ORIGIN 'IPA v4.4' )
>> Conditional ACK if you fix ipaLocationObject.
>>
>>
>>> freeipa-mbasti-0475-DNS-Locations-Add-location-commands.patch
>>>
>>>
>>> From 407b935ecd6df0ed98c6df6d45a575229ef3cd09 Mon Sep 17 00:00:00 2001
>>> From: Martin Basti <mbasti at redhat.com>
>>> Date: Thu, 5 May 2016 11:13:07 +0200
>>> Subject: [PATCH 3/4] DNS Locations: Add location-* commands
>>>
>>> Added location-{add,mod,del,find,show} commands. Command are just
>>> prototypes and does not provide any information about server (will be
>>> done later)
>>>
>>> http://www.freeipa.org/page/V4/DNS_Location_Mechanism
>>>
>>> https://fedorahosted.org/freeipa/ticket/2008
>>> ---
>>> ACI.txt | 8 ++
>>> API.txt | 59 ++++++++++++++
>>> VERSION | 4 +-
>>> install/share/bootstrap-template.ldif | 6 ++
>>> install/updates/37-locations.update | 4 +
>>> install/updates/Makefile.am | 1 +
>>> ipalib/constants.py | 1 +
>>> ipalib/plugins/location.py | 142
>>> +++++++++++++++++++++++++++++++++-
>>> 8 files changed, 222 insertions(+), 3 deletions(-)
>> [...]
>>
>>> diff --git a/VERSION b/VERSION
>>> index
>>> aedebd185821d42fa48608f4c5fdf9ff510ace3f..7e3def151e9986454509a580515b9d34dc220a60
>>> 100644
>>> --- a/VERSION
>>> +++ b/VERSION
>>> @@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
>>> # #
>>> ########################################################
>>> IPA_API_VERSION_MAJOR=2
>>> -IPA_API_VERSION_MINOR=165
>>> -# Last change: mbasti - limit ipamaxusernamelength value to 255
>>> +IPA_API_VERSION_MINOR=166
>>> +# Last change: mbasti - location-* commands
>> Needs rebase.
>>
>>
>>> diff --git a/install/share/bootstrap-template.ldif
>>> b/install/share/bootstrap-template.ldif
>>> index
>>> 628a8e2e0f5483b9f6f565b0c7d11eb000a5912d..83be4399508a905f8eae7e2f59140a6b4051b661
>>> 100644
>>> --- a/install/share/bootstrap-template.ldif
>>> +++ b/install/share/bootstrap-template.ldif
>>> @@ -119,6 +119,12 @@ objectClass: nsContainer
>>> objectClass: top
>>> cn: etc
>>> +dn: cn=locations,cn=etc,$SUFFIX
>>> +changetype: add
>>> +objectClass: nsContainer
>>> +objectClass: top
>>> +cn: locations
>>> +
>>> dn: cn=sysaccounts,cn=etc,$SUFFIX
>>> changetype: add
>>> objectClass: nsContainer
>>> diff --git a/install/updates/37-locations.update
>>> b/install/updates/37-locations.update
>>> index
>>> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..cf47e6d6296af830a76aad2c9b9f5a6ea5d9f3a1
>>> 100644
>>> --- a/install/updates/37-locations.update
>>> +++ b/install/updates/37-locations.update
>>> @@ -0,0 +1,4 @@
>>> +dn: cn=locations,cn=etc,$SUFFIX
>>> +default: objectClass: nsContainer
>>> +default: objectClass: top
>>> +default: cn: locations
>> Ok.
>>
>> [...]
>>
>>> diff --git a/ipalib/plugins/location.py b/ipalib/plugins/location.py
>>> index
>>> 8090bb1637c4d826b9a746a82b98ece903e321cc..d52d2baeb8bfb2fddeac40b281268622d47c6aeb
>>> 100644
>>> --- a/ipalib/plugins/location.py
>>> +++ b/ipalib/plugins/location.py
>> [...]
>>> +__doc__ = _("""
>>> +IPA locations
>>> +""") + _("""
>>> +Manipulate with DNS locations
>> IMHO "with" should be omited. [...]
>>
>>
>>> + at register()
>>> +class location(LDAPObject):
>>> + """
>>> + IPA locations
>>> + """
>> [...]
>>
>>> + permission_filter_objectclasses = ['ipaLocationObject']
>>> + managed_permissions = {
>>> + 'System: Read IPA Locations': {
>>> + 'ipapermright': {'read', 'search', 'compare'},
>>> + 'ipapermdefaultattr': {
>>> + 'objectclass', 'idnsname', 'description',
>>> + },
>>> + 'default_privileges': {'DNS Administrators'},
>>> + },
>>> + 'System: Add IPA Locations': {
>>> + 'ipapermright': {'add'},
>>> + 'default_privileges': {'DNS Administrators'},
>>> + },
>>> + 'System: Remove IPA Locations': {
>>> + 'ipapermright': {'delete'},
>>> + 'default_privileges': {'DNS Administrators'},
>>> + },
>>> + 'System: Modify IPA Locations': {
>>> + 'ipapermright': {'write'},
>>> + 'ipapermdefaultattr': {
>>> + 'description',
>>> + },
>>> + 'default_privileges': {'DNS Administrators'},
>>> + },
>>> + }
>> Sounds reasonable. ACI does not allow renaming location but IMHO this is okay.
>> Less renames we support the better.
>>
>>
>>> +
>>> + takes_params = (
>>> + DNSNameParam(
>>> + 'idnsname',
>>> + cli_name='name',
>>> + primary_key=True,
>>> + label=_('Location name'),
>>> + doc=_('IPA location name'),
>>> + # dns name must be relative, we will put it into middle of
>>> + # location domain name for location records
>>> + only_relative=True,
>>> + ),
>> Okay. We need to make sure that relative names with multiple labels work - but
>> this should automagically work as long as we are handling DNS names using
>> proper data types (not as strings).
>>
>>
>>> + Str(
>>> + 'description?',
>>> + label=_('Description'),
>>> + doc=_('IPA Location description'),
>>> + ),
>> After discussion with Honza we will keep description as single-value in the
>> IPA framework and ignore that description attribute is multi-value in LDAP.
>> This is done for consitency with mistakes from the past.
>>
>> [...]
>>
>>> + at register()
>>> +class location_mod(LDAPUpdate):
>>> + __doc__ = _('Modify information about an IPA location .')
>> This should say 'Modify description' because nothing else can be modified.
>> More specific text would hopefully stop some people from looking for rename
>> options.
>
> I disagree, this is general description about the modify command, see
> privilege-add it is the same as I made. I can see in future that we will
> forgot to update description of command if we add something new there.
This is really an invalid argument.
"We must not touch XYZ because its documentation might become obsolete in
future if we forget to update it!" :-)
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list