[Freeipa-devel] [PATCH 0473-0476]DNS Locations: Prologue
Martin Basti
mbasti at redhat.com
Tue May 10 12:44:23 UTC 2016
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.
Martin^2
>
>
>> freeipa-mbasti-0476-DNS-Locations-API-tests.patch
> ACK.
>
>
> Nice patch set. If you fix the nitpicks above feel free to push it right away.
>
More information about the Freeipa-devel
mailing list