<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 12.05.2016 11:01, Martin Basti wrote: </div> <blockquote cite="mid:a4f0ca8c-4099-9356-aa2b-6417e5897d8b@redhat.com" type="cite"> On 11.05.2016 09:41, Martin Basti wrote: <blockquote type="cite"> On 10.05.2016 18:56, Petr Spacek wrote: <blockquote type="cite">On 10.5.2016 15:38, Petr Spacek wrote: <blockquote type="cite">On 10.5.2016 15:26, Martin Basti wrote: <blockquote type="cite"> On 10.05.2016 15:23, Petr Spacek wrote: <blockquote type="cite">On 10.5.2016 14:44, Martin Basti wrote: <blockquote type="cite">On 10.05.2016 14:33, Petr Spacek wrote: <blockquote type="cite">On 6.5.2016 10:20, Martin Basti wrote: <blockquote type="cite"><a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/2008">https://fedorahosted.org/freeipa/ticket/2008</a> 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 <a class="moz-txt-link-rfc2396E" href="mailto:mbasti@redhat.com"><mbasti@redhat.com></a> 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' </blockquote> Seems reasonable, ACK. <blockquote type="cite">freeipa-mbasti-0474-DNS-Locations-add-new-attributes-and-objectclasses.patch From a7766da5fd1a72884308d4206c9cde262f5c8d35 Mon Sep 17 00:00:00 2001 From: Martin Basti <a class="moz-txt-link-rfc2396E" href="mailto:mbasti@redhat.com"><mbasti@redhat.com></a> Date: Thu, 5 May 2016 11:12:00 +0200 Subject: [PATCH 2/4] DNS Locations: add new attributes and objectclasses <a class="moz-txt-link-freetext" href="http://www.freeipa.org/page/V4/DNS_Location_Mechanism">http://www.freeipa.org/page/V4/DNS_Location_Mechanism</a> <a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/2008">https://fedorahosted.org/freeipa/ticket/2008</a> --- 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' ) </blockquote> 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). <blockquote type="cite">+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' ) </blockquote> Conditional ACK if you fix ipaLocationObject. <blockquote type="cite">freeipa-mbasti-0475-DNS-Locations-Add-location-commands.patch From 407b935ecd6df0ed98c6df6d45a575229ef3cd09 Mon Sep 17 00:00:00 2001 From: Martin Basti <a class="moz-txt-link-rfc2396E" href="mailto:mbasti@redhat.com"><mbasti@redhat.com></a> 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) <a class="moz-txt-link-freetext" href="http://www.freeipa.org/page/V4/DNS_Location_Mechanism">http://www.freeipa.org/page/V4/DNS_Location_Mechanism</a> <a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/2008">https://fedorahosted.org/freeipa/ticket/2008</a> --- 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(-) </blockquote> [...] <blockquote type="cite">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 </blockquote> Needs rebase. <blockquote type="cite">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 </blockquote> Ok. [...] <blockquote type="cite">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 </blockquote> [...] <blockquote type="cite">+__doc__ = _(""" +IPA locations +""") + _(""" +Manipulate with DNS locations </blockquote> IMHO "with" should be omited. [...] <blockquote type="cite">+@register() +class location(LDAPObject): + """ + IPA locations + """ </blockquote> [...] <blockquote type="cite">+ 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'}, + }, + } </blockquote> Sounds reasonable. ACI does not allow renaming location but IMHO this is okay. Less renames we support the better. <blockquote type="cite">+ + 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, + ), </blockquote> 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). <blockquote type="cite">+ Str( + 'description?', + label=_('Description'), + doc=_('IPA Location description'), + ), </blockquote> 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. [...] <blockquote type="cite">+@register() +class location_mod(LDAPUpdate): + __doc__ = _('Modify information about an IPA location .') </blockquote> This should say 'Modify description' because nothing else can be modified. More specific text would hopefully stop some people from looking for rename options. </blockquote> 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. </blockquote> 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!" :-) </blockquote> 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 </blockquote> I do not agree but push whatever variant you like, it costed too much already. </blockquote> NACK anyway. ipa-dns-install screams if you install a server without DNS and run ipa-dns-install later on: The log contains this: add objectClass: top groupofnames nestedgroup add cn: DNS Administrators add description: DNS Administrators adding new entry "cn=DNS Administrators,cn=privileges,cn=pbac,dc=dom-033,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com" 2016-05-10T16:53:05Z DEBUG stderr=ldap_initialize( ldapi://%2Fvar%2Frun%2Fslapd-DOM-033-ABC-IDM-LAB-ENG-BRQ-REDHAT-COM.socket/??base ) SASL/EXTERNAL authentication started SASL username: gidNumber=0+uidNumber=0,cn=peercred,cn=external,cn=auth SASL SSF: 0 ldap_add: Already exists (68) 2016-05-10T16:53:05Z CRITICAL Failed to load dns.ldif: Command '/usr/bin/ldapmodify -v -f /tmp/tmpMvWMaT -H ldapi://%2fvar%2frun%2fslapd-DOM-033-ABC-IDM-LAB-ENG-BRQ-REDHAT-COM.socket -Y EXTERNAL' returned non-zero exit status 68 </blockquote> Well I cannot reproduce it, this should be resolved by patch 473 </blockquote> Updated patches attached I found that IDNA did not work with previous version, fixed + IDNA tests added <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> Fixed here, I sent wrong patches before </body> </html>