[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

David Kupka dkupka at redhat.com
Fri Sep 26 12:47:42 UTC 2014


On 09/26/2014 10:30 AM, David Kupka wrote:
> On 09/26/2014 09:34 AM, Jan Cholasta wrote:
>> Dne 26.9.2014 v 08:28 David Kupka napsal(a):
>>> On 09/25/2014 04:17 PM, David Kupka wrote:
>>>> On 09/24/2014 08:54 PM, Martin Basti wrote:
>>>>> On 24/09/14 15:44, David Kupka wrote:
>>>>>> On 09/23/2014 08:25 PM, Martin Basti wrote:
>>>>>>> On 23/09/14 13:23, David Kupka wrote:
>>>>>>>> On 09/18/2014 06:34 PM, Martin Basti wrote:
>>>>>>>>> ...
>>>>>>>>> 1)
>>>>>>>>> +        if options.unattended:
>>>>>>>>> +            for ip in ip_addresses:
>>>>>>>>> +                if search_reverse_zones and
>>>>>>>>> find_reverse_zone(str(ip)):
>>>>>>>>> +                    # reverse zone is already in LDAP
>>>>>>>>> +                    continue
>>>>>>>>> +                for rz in ret_reverse_zones:
>>>>>>>>> +                    if verify_reverse_zone(rz, ip):
>>>>>>>>> +                        # reverse zone was entered by user
>>>>>>>>> +                        break
>>>>>>>>> +                else:
>>>>>>>>> +                    rz = get_reverse_zone_default(str(ip))
>>>>>>>>> +                    ret_reverse_zones.append(rz)
>>>>>>>>> +        elif options.reverse_zones or create_reverse():
>>>>>>>>> +            for ip in ip_addresses:
>>>>>>>>> +                if search_reverse_zones and
>>>>>>>>> find_reverse_zone(str(ip)):
>>>>>>>>> +                    # reverse zone is already in LDAP
>>>>>>>>> +                    continue
>>>>>>>>> +                for rz in ret_reverse_zones:
>>>>>>>>> +                    if verify_reverse_zone(rz, ip):
>>>>>>>>> +                        # reverse zone was entered by user
>>>>>>>>> +                        break
>>>>>>>>> +                else:
>>>>>>>>> +                    rz = get_reverse_zone_default(str(ip))
>>>>>>>>> +                    rz = read_reverse_zone(rz, str(ip))
>>>>>>>>> +                    ret_reverse_zones.append(rz)
>>>>>>>>> +        else:
>>>>>>>>> +            options.no_reverse = True
>>>>>>>>> +            ret_reverse_zones = []
>>>>>>>>>
>>>>>>>>> You can make it shorter without duplications:
>>>>>>>>>
>>>>>>>>> # this ifs can be in one line
>>>>>>>>> if not options.unatended:
>>>>>>>>>      if not options.reverse_zones
>>>>>>>>>          if not create_reverse():
>>>>>>>>>              options.no_reverse=True
>>>>>>>>>              return []
>>>>>>>>>
>>>>>>>>> for ip in ip_addresses:
>>>>>>>>>      if search_reverse_zones and find_reverse_zone(str(ip)):
>>>>>>>>>          # reverse zone is already in LDAP
>>>>>>>>>          continue
>>>>>>>>>      for rz in ret_reverse_zones:
>>>>>>>>>          if verify_reverse_zone(rz, ip):
>>>>>>>>>              # reverse zone was entered by user
>>>>>>>>>              break
>>>>>>>>>          else:
>>>>>>>>>              rz = get_reverse_zone_default(str(ip))
>>>>>>>>>              if not options.unattended:
>>>>>>>>>                  rz = read_reverse_zone(rz, str(ip))
>>>>>>>>>              ret_reverse_zones.append(rz)
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks, I modified it bit different way to alse address
>>>>>>>> recommendation
>>>>>>>> 3).
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2)
>>>>>>>>> Typo?     There is no IP address matching reverze_zone %s."
>>>>>>>>> ---------------------------------------------^^
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks, fixed.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 3)
>>>>>>>>> Would be nice to ask user to create new zones only if new zones
>>>>>>>>> are
>>>>>>>>> required. (If all required zones exist in LDAP, you ask user
>>>>>>>>> anyway)
>>>>>>>>>
>>>>>>>>
>>>>>>>> I added one more variable and ask only once.
>>>>>>>>
>>>>>>>>> 4)
>>>>>>>>> Ask framework gurus, if installutils module is better place for
>>>>>>>>> function
>>>>>>>>> above
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Petr^3 said that it's ok to have it in bindinstance.py.
>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>> NACK, most important point is 7
>>>>>>>
>>>>>>> 1)
>>>>>>> I'm not sure if this is bug, but interactively is allowed to add
>>>>>>> only
>>>>>>> one ip address
>>>>>>>
>>>>>>> Unable to resolve IP address for host name
>>>>>>> Please provide the IP address to be used for this host name:
>>>>>>> 2001:db8::2
>>>>>>> The kerberos protocol requires a Realm name to be defined.
>>>>>>>
>>>>>>
>>>>>> For the sake of infinite usability and UX I rewrote it to ask for
>>>>>> multiple addresses the same way as for DNS forwarders. Also I really
>>>>>> simplified IP address checking code when I was in it. I tested it but
>>>>>> please look at it carefully.
>>>>>> Also I found that ipa-dns-install and ipa-adtrust-install also accept
>>>>>> --ip-address param. So I modified ipa-dns-install in the same way as
>>>>>> ipa-server-install and ipa-replica-install. After discussion with
>>>>>> tbabej I decided to dont touch ipa-adtrust-install now as it do not
>>>>>> use specified value at all. I will remove the processing code and
>>>>>> mark
>>>>>> the param as deprecated in separate patch.
>>>>>>
>>>>>>> 2)
>>>>>>> I'm getting error message
>>>>>>>
>>>>>>> Invalid reverse zone 10.in-addr.arpa. for IP address
>>>>>>> 2001:db8::dead:beef
>>>>>>> Invalid reverse zone 10.in-addr.arpa. for IP address
>>>>>>> fed0:babe:baab:0:21a:4aff:fe10:4e18
>>>>>>>
>>>>>>>   - or -
>>>>>>>
>>>>>>> Do you want to configure the reverse zone? [yes]:
>>>>>>> Please specify the reverse zone name
>>>>>>> [0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.]:
>>>>>>> Invalid reverse zone 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.
>>>>>>> for IP
>>>>>>> address fed0:babe:baab:0:21a:4aff:fe10:4e18
>>>>>>> Please specify the reverse zone name
>>>>>>> [0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.]:
>>>>>>> Using reverse zone(s) 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.,
>>>>>>> 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.
>>>>>>>
>>>>>>> This shouldn't be there
>>>>>>
>>>>>> Moved the message to function used when installation is attended by
>>>>>> user.
>>>>>>
>>>>>>>
>>>>>>> Could be better to ask user to specific zone for ip address a.b.c.d.
>>>>>>
>>>>>> Probably, but lets leave some work for future.
>>>>>>
>>>>>>>
>>>>>>> 4) just nitpick
>>>>>>> The IPA Master Server will be configured with:
>>>>>>> ...
>>>>>>> IP address(es): 2001:db8::dead:beef,
>>>>>>> fed0:babe:baab:0:21a:4aff:fe10:4e18
>>>>>>> ...
>>>>>>> Reverse zone:  0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.,
>>>>>>> 0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.
>>>>>>>
>>>>>>> You have label "IP address(es)", so you should use label "Reverse
>>>>>>> zone(s)"
>>>>>>>
>>>>>>
>>>>>> Fixed.
>>>>>>
>>>>>>> 5)
>>>>>>> ipa-server-install --ip-address=10.16.78.105
>>>>>>> --reverse-zone=10.in-addr.arpa. --reverse-zone=16.10.in-addr.arpa.
>>>>>>> --setup-dns
>>>>>>>
>>>>>>> Creates both reverse zones, but 10.in-addr.arpa. is empty. I'm not
>>>>>>> sure
>>>>>>> if this is wrong, but we prevents user to add zone without
>>>>>>> address in
>>>>>>> it, so we should prevents, to add empty zone.
>>>>>>>
>>>>>>
>>>>>> It would be nice but not in this patch.
>>>>>>
>>>>>>> 6)
>>>>>>> ipa-replica-prepare --ip-address 10.16.78.105 --ip-address
>>>>>>> 2001:db8::dead:beef --reverse-zone 1.0.0.2.ip6.arpa.
>>>>>>> vm-105.example.com
>>>>>>> Directory Manager (existing master) password:
>>>>>>>
>>>>>>> Invalid reverse zone 1.0.0.2.ip6.arpa. for IP address 10.16.78.105
>>>>>>> Invalid reverse zone 1.0.0.2.ip6.arpa.
>>>>>>>
>>>>>>> IMO This should work, right?
>>>>>>>
>>>>>>> +                sys.exit("There is no IP address matching reverse
>>>>>>> zone
>>>>>>> %s." % rz)
>>>>>>> I expected at least this error to be shown.
>>>>>>
>>>>>> Fixed, thanks.
>>>>>>
>>>>>>>
>>>>>>> 7)
>>>>>>> ipa-replica-prepare --ip-address 10.16.78.105 --ip-address
>>>>>>> 2001:db8::dead:beef vm-105.example.com
>>>>>>> Directory Manager (existing master) password:
>>>>>>>
>>>>>>> .......
>>>>>>> Adding DNS records for vm-105.example.com
>>>>>>> Values instance has no attribute 'ip_address'
>>>>>>>
>>>>>>> Command returns the attribute error.
>>>>>>> It fails with one --ip-address too.
>>>>>>>
>>>>>>
>>>>>> Sorry, fixed.
>>>>>>
>>>>>>>
>>>>>>> *) Not related with your patch
>>>>>>> Problem with installation:
>>>>>>> I'm getting message:
>>>>>>> IPA server is already configured on this system.
>>>>>>> Even if I validation wasn't successful and installation was aborted.
>>>>>>>
>>>>>>> IPA install detects previous installations by checking state file
>>>>>>> and
>>>>>>> restore files. Function get_server_ip_address() stores some data and
>>>>>>> hosts file and modify the host file, before user agreed
>>>>>>> installation.
>>>>>>> This error was there before your patch.
>>>>>>> https://fedorahosted.org/freeipa/ticket/4561
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>> NACK
>>>>>
>>>>> 0)
>>>>> # ipa-dns-install --ip-address 2001:db8::feed
>>>>>
>>>>> 2014-09-24T06:02:13Z DEBUG stderr=
>>>>> 2014-09-24T06:02:13Z DEBUG   File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",
>>>>> line 645, in run_script
>>>>>      return_value = main_function()
>>>>>
>>>>>    File "/sbin/ipa-dns-install", line 135, in main
>>>>>      ip_addresses = get_server_ip_address(api.env.host, fstore,
>>>>> options.unattended, options)
>>>>>
>>>>>    File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",
>>>>> line 473, in get_server_ip_address
>>>>>      if options.setup_dns:
>>>>>
>>>>> 2014-09-24T06:02:13Z DEBUG The ipa-dns-install command failed,
>>>>> exception: AttributeError: Values instance has no attribute
>>>>> 'setup_dns'
>>>>
>>>> Obviously there is no option --setup-dns in ipa-dns-install.
>>>> Fixed, modified get_server_ip_address function.
>>>>
>>>>>
>>>>> 1)
>>>>> # ipa-replica-prepare vm-073.example.com --ip-address
>>>>> 2620:52:0::fe10:4e18 --ip-address 10.16.78.73
>>>>> Directory Manager (existing master) password:
>>>>>
>>>>> Preparing replica for vm-073.example.com from vm-105.example.com
>>>>> Creating SSL certificate for the Directory Server
>>>>> Creating SSL certificate for the dogtag Directory Server
>>>>> Saving dogtag Directory Server port
>>>>> Creating SSL certificate for the Web Server
>>>>> Exporting RA certificate
>>>>> Copying additional files
>>>>> Finalizing configuration
>>>>> Packaging replica information into
>>>>> /var/lib/ipa/replica-info-vm-073.example.com.gpg
>>>>> Adding DNS records for vm-073.example.com
>>>>> Values instance has no attribute 'unattended'
>>>>>
>>>>> It should be unatended automatically, or we need add the --unattended
>>>>> option to ipa-replica-prepare
>>>>
>>>> ipa-replica-install is missing --unattended option. I fixed it in my
>>>> code for now but we should add it there.
>>>>
>>>>>
>>>>> 2) This is nto user friendly, could be IP address check before
>>>>> installation?
>>>>> [root at vm-073 ~]# ipa-replica-install replica.file.gpg --ip-address
>>>>> 2620:52::fe10:4e18 --reverse-zone 10.in-addr.arpa.  --setup-dns
>>>>> --no-forwarders
>>>>> Directory Manager (existing master) password:
>>>>>
>>>>> Run connection check to master
>>>>> ...
>>>>> <long long list of succesfully configured services />
>>>>> ...
>>>>> Restarting the KDC
>>>>> There is no IP address matching reverse zone 10.in-addr.arpa..
>>>>>
>>>>> LOG:
>>>>>      return_value = main_function()
>>>>>
>>>>>    File "/sbin/ipa-replica-install", line 721, in main
>>>>>      install_bind(config, options)
>>>>>
>>>>>    File "/sbin/ipa-replica-install", line 265, in install_bind
>>>>>      reverse_zones =
>>>>> bindinstance.check_reverse_zones(config.ip_addresses,
>>>>> options.reverse_zones, options, True)
>>>>>
>>>>>    File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>>> line 426, in check_reverse_zones
>>>>>      sys.exit("There is no IP address matching reverse zone %s." % rz)
>>>>>
>>>>> 2014-09-24T06:39:23Z DEBUG The ipa-replica-install command failed,
>>>>> exception: SystemExit: There is no IP address matching reverse zone
>>>>> 10.in-addr.arpa..
>>>>
>>>> Fixed. Asking everything before actual installation.
>>>>
>>>>>
>>>>> 3)
>>>>> I'm not sure if sys.exit() is good, replica-install shoudl wrote
>>>>> something about partially configured system
>>>>
>>>> This is quite common in installation scripts. I moved this parts before
>>>> actual installation.
>>>>
>>>>>
>>>>> 4) I'm not sure if this is the best place to ask about reverse zones
>>>>> ....
>>>>> Done configuring the web interface (httpd).
>>>>> Configuring ipa-otpd
>>>>>    [1/2]: starting ipa-otpd
>>>>>    [2/2]: configuring ipa-otpd to start on boot
>>>>> Done configuring ipa-otpd.
>>>>> Applying LDAP updates
>>>>> Restarting the directory server
>>>>> Restarting the KDC
>>>>> Do you want to configure the reverse zone? [yes]:
>>>>
>>>> Moved.
>>>>
>>>>>
>>>>> 5) And error
>>>>> # ipa-replica-install replica.file.gpg --ip-address 2620:52:xxxx
>>>>> --setup-dns
>>>>> ....
>>>>> Do you want to configure the reverse zone? [yes]:
>>>>> Please specify the reverse zone name
>>>>> [c.4.0.1.0.0.0.0.2.5.0.0.0.2.6.2.ip6.arpa.]:
>>>>> Using reverse zone(s) c.4.0.1.0.0.0.0.2.5.0.0.0.2.6.2.ip6.arpa.
>>>>>
>>>>> Your system may be partly configured.
>>>>> Run /usr/sbin/ipa-server-install --uninstall to clean up.
>>>>>
>>>>> Unexpected error - see /var/log/ipareplica-install.log for details:
>>>>> AttributeError: 'str' object has no attribute 'version'
>>>>>
>>>>> LOG:
>>>>> 2014-09-24T06:50:44Z DEBUG retrieving schema for SchemaCache
>>>>> url=ldapi://%2fvar%2frun%2fslapd-IDM-LAB-BOS-REDHAT-COM.socket
>>>>> conn=<ldap.ldapobject.SimpleLDAPObject instance at 0x46ba950>
>>>>> 2014-09-24T06:50:45Z DEBUG   File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/installutils.py",
>>>>> line 645, in run_script
>>>>>      return_value = main_function()
>>>>>
>>>>>    File "/sbin/ipa-replica-install", line 721, in main
>>>>>      install_bind(config, options)
>>>>>
>>>>>    File "/sbin/ipa-replica-install", line 272, in install_bind
>>>>>      ca_configured=options.setup_ca)
>>>>>
>>>>>    File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>>> line 550, in setup
>>>>>      self.__setup_sub_dict()
>>>>>
>>>>>    File
>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
>>>>> line 651, in __setup_sub_dict
>>>>>      if addr.version in (4, 6):
>>>>>
>>>>> 2014-09-24T06:50:45Z DEBUG The ipa-replica-install command failed,
>>>>> exception: AttributeError: 'str' object has no attribute 'version'
>>>>
>>>> Fixed. We are using IP addresses as a strings and as a
>>>> CheckedIPAddress.
>>>> I swapped them here.
>>>>>
>>>>>
>>>>> *) I don't like this asking to specify zone without IP
>>>>> Do you want to configure the reverse zone? [yes]:
>>>>> Please specify the reverse zone name
>>>>> [0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.]:
>>>>> Please specify the reverse zone name [78.16.10.in-addr.arpa.]:
>>>>>
>>>>
>>>> Would be nice. Prefer to do it as a part of more powerfull reverse zone
>>>> validation logic.
>>>>>
>>>>>
>>>>> **) I'm not sure how often this case can happen:
>>>>> master and replica without DNS, you run --ipa-dns-install on master
>>>>> then
>>>>> on replica, then replica DNS installation will not try to find
>>>>> existent
>>>>> reverse zones in ldap, due configuration in ipa-dns-install.
>>>>> Maybe you should detect if DNS container exists and then set flag
>>>>> "search_reverse_zones=True"
>>>>>
>>>>> ipa-dns-install:
>>>>> +    reverse_zones = bindinstance.check_reverse_zones(ip_addresses,
>>>>> options.reverse_zones, options)
>>>>
>>>> Same as above.
>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Freeipa-devel mailing list
>>>> Freeipa-devel at redhat.com
>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>
>>> Rebased to current master and ipa-4-1. Removed unintentional
>>> modification of ipa-adtrust-install.
>>
>> +            for ip, ip_address in zip(config.ips, config.ip_addresses):
>> +                reverse_zone = bindinstance.find_reverse_zone(ip)
>>
>> Is it always 100% guaranteed that the values in config.ips and
>> config.ip_addresses are the same length and otherwise match each other
>> in a way that will not cause things to break? IMO it would be better to
>> store a single list of 2-tuples somewhere from the start instead of
>> zipping things later. (Note that zip([1], ['yes', 'oh noes!']) == [(1,
>> 'yes')].)
>>
>
> The chances that this will break are really small but I cannot guarantee
> that it won't eventually.
> One list (config.ip_addresses) is generated from the other (config.ips)
> and right now there is no need to modify them. But if someone in the
> future modify only one of them it will fail really badly. So I removed
> zip() and convert the ip address to string on demand. It will cost a few
> more cycles but also will be less error prone.
>
>
>
>
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel
>

Another few flaws eradicated.

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-13-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 43434 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140926/576d0b62/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-13-ipa41-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 43276 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140926/576d0b62/attachment-0001.bin>


More information about the Freeipa-devel mailing list