On 10.5.2016 15:26, Martin Basti wrote: > > > On 10.05.2016 15:23, Petr Spacek wrote: >> 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 <mba...@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 <mba...@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 <mba...@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. [...] >>>> >>>> >>>>> +@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. >>>> >>>> [...] >>>> >>>>> +@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!" :-) >> > > How about inconsistency with description of older commands? I don't think that > command description should describe attributes that are allowed to change. > Allowed attributes are shown in --help output
I do not agree but push whatever variant you like, it costed too much already. -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code