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

[Martin Basti]

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.

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

Could be better to ask user to specific zone for ip address a.b.c.d.

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

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.

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.

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.


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



-- 
Martin Basti