[Freeipa-devel] [PATCH 0473-0476]DNS Locations: Prologue

Jan Cholasta jcholast at redhat.com
Thu May 26 05:25:47 UTC 2016


On 25.5.2016 18:28, Martin Basti wrote:
>
>
> On 24.05.2016 12:10, Martin Basti wrote:
>>
>>
>> On 23.05.2016 07:44, Jan Cholasta wrote:
>>> On 20.5.2016 15:32, Martin Basti wrote:
>>>>
>>>>
>>>> On 20.05.2016 12:30, Petr Spacek wrote:
>>>>> On 18.5.2016 12:43, Martin Basti wrote:
>>>>>>
>>>>>> On 12.05.2016 16:16, Martin Basti wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 12.05.2016 11:01, Martin Basti wrote:
>>>>>>>>
>>>>>>>> On 11.05.2016 09:41, Martin Basti wrote:
>>>>>>>>>
>>>>>>>>> On 10.05.2016 18:56, Petr Spacek wrote:
>>>>>>>>>> On 10.5.2016 15:38, Petr Spacek wrote:
>>>>>>>>>>> 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 <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!" :-)
>>>>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>> Well I cannot reproduce it, this should be resolved by patch 473
>>>>>>>>>
>>>>>>>> Updated patches attached
>>>>>>>>
>>>>>>>> I found that IDNA did not work with previous version, fixed + IDNA
>>>>>>>> tests added
>>>>>>>>
>>>>>>>>
>>>>>>> Fixed here, I sent wrong patches before
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> More patches added, all patches are available here:
>>>>>> https://github.com/bastiak/freeipa/tree/DNS-locations
>>>>> Functional NACK
>>>>>
>>>>> location-show returns incorrect location weight for servers
>>>>>
>>>>> # ipa server-show vm-046.abc.idm.lab.eng.brq.redhat.com --all
>>>>>    dn:
>>>>> cn=vm-046.abc.idm.lab.eng.brq.redhat.com,cn=masters,cn=ipa,cn=etc,dc=dom-058-082,dc=abc,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
>>>>>
>>>>>
>>>>>    Server name: vm-046.abc.idm.lab.eng.brq.redhat.com
>>>>>    Managed suffixes: domain
>>>>>    Min domain level: 0
>>>>>    Max domain level: 1
>>>>>    Location: l2
>>>>>    Location weight: 200
>>>>>    objectclass: ipalocationmember, ipaReplTopoManagedServer, top,
>>>>> ipaConfigObject, nsContainer, ipaSupportedDomainLevelConfig
>>>>>
>>>>> # ipa location-show l2
>>>>>    Location name: l2
>>>>>    Description: Loc 2
>>>>>    Servers: vm-046.abc.idm.lab.eng.brq.redhat.com (weight: 100)
>>>>>
>>>>> - Did you forget --all somewhere?
>>>> Nope, I forgot to put it to default attributes, nice catch, I will add
>>>> test for it.
>>>>>
>>>>> Other than that I have only nitpick regarding error message:
>>>>> ipa: ERROR: l2 cannot be deleted because IPA Server(s)
>>>>> vm-046.abc.idm.lab.eng.brq.redhat.com requires it
>>>>>
>>>>> - Please unify singular/plural.
>>>> This is how the exception is generated, so you can choose just
>>>> label, so
>>>> which label is the right? {'IPA server', 'IPA servers', 'IPA
>>>> server(s)'}
>>>> (I think server should be with lower cased 's')
>>>>                 raise DependentEntry(
>>>>                     label=_('IPA Server(s)'),
>>>>                     key=keys[-1],
>>>>                     dependent=location_members
>>>>                 )
>>>
>>> Use ngettext here to handle singular/plural correctly.
>> It will fix only half of sentence, I put there just singular.
>>
>>>
>>>>>
>>>>> I did not require the code because Honza will surely look into
>>>>> details.
>>>
>>> "Allow to use non-Str attributes as keys for members":
>>>
>>> There is no actual API change, so don't bump VERSION.
>> Bump version removed
>>
>>>
>>>
>>> "Remove CSV from from member params":
>>>
>>> Again, no actual API change, don't bump VERSION.
>>>
>>> CSV params should be removed completely. I have a patch in my drawer
>>> which does exactly that, should I post it?
>>>
>> Removed
>>
>> Please post it.
>>
>>>
>>> "DNS Locations: extend server-* command with locations":
>>>
>>> I don't think ipalocationweight should be in server-find. Searching
>>> for servers by exact weight does not strike me as something useful.
>>> Or is it?
>> I agree, it will even not work with default location weight
>>
>>>
>>> In server.normalize_location(), don't call location.get_dn() with
>>> **options, as it contains server options, not location options. Also
>>> you are calling kw.pop('ipalocation_location') twice, which is most
>>> likely a bug.
>> Fixed.
>> It is not twice, it was called in different if-else branches, but I
>> rewrote the code it should looks better now.
>>
>>>
>>> In server_mod.pre_callback(), don't raise NotFound manually, but
>>> rather use self.api.Object.location.handle_not_found().
>>>
>> Changed
>>>
>>> Honza
>>>
>>
>> Updated patches attached.
>>
>>
> Rebased patches attached

"DNS Location: location-show: return list of servers in location":

The list of servers should be returned only without --raw.


Otherwise LGTM.

-- 
Jan Cholasta




More information about the Freeipa-devel mailing list