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

Martin Basti mbasti at redhat.com
Tue May 24 10:10:45 UTC 2016



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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0473.4-DNS-Locations-Always-create-DNS-related-privileges.patch
Type: text/x-patch
Size: 4774 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0474.4-DNS-Locations-add-new-attributes-and-objectclasses.patch
Type: text/x-patch
Size: 4065 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0475.4-DNS-Locations-location-commands.patch
Type: text/x-patch
Size: 14464 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0476.4-DNS-Locations-API-tests.patch
Type: text/x-patch
Size: 9331 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0478.4-Allow-to-use-non-Str-attributes-as-keys-for-members.patch
Type: text/x-patch
Size: 29561 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0479.4-DNS-Locations-extend-server-command-with-locations.patch
Type: text/x-patch
Size: 10592 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0480.4-DNS-Location-location-show-return-list-of-servers-in.patch
Type: text/x-patch
Size: 5186 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0481.4-DNS-Locations-prevent-to-remove-used-locations.patch
Type: text/x-patch
Size: 4056 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0482.4-DNS-Locations-extend-tests-with-server-commands.patch
Type: text/x-patch
Size: 11837 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160524/34ae6ed1/attachment-0008.bin>


More information about the Freeipa-devel mailing list