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