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@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@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')].)

--
Jan Cholasta

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to