[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.
David Kupka
dkupka at redhat.com
Fri Sep 26 08:30:01 UTC 2014
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.
--
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-12-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 43392 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140926/7314849f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0009-12-ipa41-Detect-and-configure-all-usable-IP-addresses.patch
Type: text/x-patch
Size: 43234 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140926/7314849f/attachment-0001.bin>
More information about the Freeipa-devel
mailing list