[Freeipa-devel] [PATCH 0473-0476]DNS Locations: Prologue
Jan Cholasta
jcholast at redhat.com
Mon May 23 05:44:29 UTC 2016
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.
>>
>> 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.
"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?
"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?
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.
In server_mod.pre_callback(), don't raise NotFound manually, but rather
use self.api.Object.location.handle_not_found().
Honza
--
Jan Cholasta
More information about the Freeipa-devel
mailing list