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 <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.

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

-- 
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

Reply via email to