Bug 230498

Created attachment 148982 [details]
diffs

Created attachment 148983 [details]
diffs

Looks good to me.

The C code has ENABLE_LDAPI macro, but the perl frontend does not.
1) If the inf file happens to have ldapifilepath value and the ds_newinst-bin is
built with LDAPI disabled, the attribute-value is just ignored.
2) If the port 0 is given to the LDAPI disabled server, checkport returns the
error and the installation fails.
I'm assuming these observations are correct... :)

The changes look good, but I have one question.

In ds_newinst.pl.in, the following code section looks like it would allow you to
explicitly specify "ServerPort = 0" in the inf file without specifying a ldapi
filename.  Should we check if ServerPort is set to 0 before setting "$canlisten
= 1"?

+# either servport or ldapifilepath must be specified - the server must
+# listen to something . . .
+my $canlisten = 0;
+if (defined($table{"slapd"}->{"ServerPort"})) {
+    $canlisten = 1;
+    $cgiargs{"servport"} = $table{"slapd"}->{"ServerPort"};
+} else {
+    $cgiargs{"servport"} = "0"; # 0 means do not listen
+}
+if (defined($table{"slapd"}->{"ldapifilepath"})) {
+    $canlisten = 1;
+    $cgiargs{"ldapifilepath"} = $table{"slapd"}->{"ldapifilepath"};
+}
+if (! $canlisten) {
+    usage("Either ServerPort or ldapifilepath must be specified in the slapd
section of $filename");
+}

(In reply to comment #3)
> Looks good to me.
> 
> The C code has ENABLE_LDAPI macro, but the perl frontend does not.
> 1) If the inf file happens to have ldapifilepath value and the ds_newinst-bin is
> built with LDAPI disabled, the attribute-value is just ignored.

Right.  That should make it easier to share .inf files between fedora-ds and
redhat-ds.

> 2) If the port 0 is given to the LDAPI disabled server, checkport returns the
> error and the installation fails.
> I'm assuming these observations are correct... :)

Right.

Created attachment 148985 [details]
new diffs

Diffs to address nkinder's concerns

Looks good!

Created attachment 148986 [details]
cvs commit log

Reviewed by: nkinder, nhosoi (Thanks!)
Files: see diff
Branch: HEAD
Fix Description: Two new fields have been added to the ds_newinst .inf files:
ldapifilepath - the full path and file name of the server ldapi file
start_server - if present and has a value of 0, this tells ds_newinst not to
start the server - default is 1
The ds_newinst code has been changed to allow an empty or "0" value servport if
an ldapifilepath is given (and ENABLE_LDAPI is defined).  Either a valid server
port or an ldapifilepath must be provided, or both.
In addition, I changed ds_newinst.pl to accept a .inf file given on stdin.
Platforms tested: RHEL4, FC6
Flag Day: no
Doc impact: We will have to document ldapi support on the wiki.

Using only ldapi:// doesn't seem to work.  The ds_newinst binary is stuck trying
to talk to localhost:389

Created attachment 148987 [details]
strace of ds_newinst binary

It looks like you are specifying ServerPort= 389 in your .inf file.  ds_newinst
will attempt to verify that the port is free by trying to connect to it.  Do you
need to specify a port number?  If I use an .inf file like this:[General]
FullMachineName=   localhost.localdomain
SuiteSpotUserID=   nobody
ServerRoot=    /usr/lib64/fedora-ds
[slapd]
#ServerPort=   1100
start_server= 0
ldapifilepath= /var/run/ldapi
ServerIdentifier=   localhost
Suffix=   dc=example,dc=com
RootDN=   cn=Directory Manager
RootDNPwd=  Secret123

It works just fine, and I don't see any connect() calls in strace (except for
other system daemons).

Created attachment 149056 [details]
Inf file generated by samba4 build system

This is what I'm currently using

Created attachment 149057 [details]
new inf file

The Config* and AdminDomain settings are not needed for fedora-ds-base - those
are admin server/console specific things.  In fact, not only do they not accept
ldapi LDAP URLs, they also cause the server to try to start even if
start_server=0.  ServerPort does not need to be specified if not using it.