zendframework / zend-ldap

Ldap component from Zend Framework
BSD 3-Clause "New" or "Revised" License
29 stars 29 forks source link

Enables LDAP-Testing on Travis #8

Closed heiglandreas closed 9 years ago

heiglandreas commented 9 years ago

This PR adds the possibility to run LDAP-tests on travis.

For that a new script is added that sets up an LDAP-server on travis and the phpunit.xml.dist has been adapted accordingly

gianarb commented 9 years ago

Good! :+1:

Maks3w commented 9 years ago

@gianarb No it's not good. I don't have time now for to put all the review notes.

Maks3w commented 9 years ago

These are the instructions for to setup the LDAP server https://github.com/zendframework/zf2/blob/eb6275e9227e9ed1995d326087488d3ea25811b5/tests/docs/README-LDAP.txt

gianarb commented 9 years ago

@Maks3w you are into the review team :D IMO this is a good PR because increase library's value.. :) You know syntax roles :) Thanks for your review! :+1:

Maks3w commented 9 years ago

This is close. I'll try to fork your PR and adapt it for to not use sudo and put ldif files outside of the script.

heiglandreas commented 9 years ago

Am 14.07.15 um 18:50 schrieb Maks3w:

This is close. I'll try to fork your PR and adapt it for to not use |sudo| and put ldif files outside of the script.

I tried to do that (not use |sudo| but I didn't get it to work, which is somewhat understandable. In the first step of the setup the LDAP-Admin-user has to be created and that's only possible for the root-user. As no user exists in LDAP up to that point a privileged user from outside the LDAP can log into the LDAP-Server and adapt it. That's where sudo is needed.

But I might be wrong on that and I'd be very glad to learn that it works without the root-access.

Regarding the ldif-files it's in my eyes far more convenient to be able to set some of the defaults via environment-variables than to have those informations hard-coded scattered over different files. Not being able to use the env-variables setting the password needs to use hardcoded values where no one is really sure whether they are the actual password that is documented elsewhere.

— Reply to this email directly or view it on GitHub https://github.com/zendframework/zend-ldap/pull/8#issuecomment-121305887.

                                                          ,,,
                                                         (o o)

+---------------------------------------------------------ooO-(_)-Ooo-+ | Andreas Heigl | | mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" | | http://andreas.heigl.org http://hei.gl/wiFKy7 | +---------------------------------------------------------------------+ | http://hei.gl/root-ca | +---------------------------------------------------------------------+

Maks3w commented 9 years ago

I have found this guy solved the sudo issue by launching a ldap instance in a non privilege port

https://github.com/FreeRADIUS/freeradius-server/pull/1107/files

heiglandreas commented 9 years ago

That's great!

But (you knew there was one coming (-; ) he uses the slapd.conf-version of ldap-configuration instead of the slapd-config one. The slapd.conf-version is deprecated as http://www.openldap.org/doc/admin24/slapdconfig.html states. I'm not sure whether the "old" version will be removed any time soon, but...

I can of course use the older slapd.conf-version to configure the server if you think we need to get rid of the sudo-stuff.

Oh, and the port isn't the issue as it's installed by travis itself.

Maks3w commented 9 years ago

Between use the legacy Travis infrastructure and the legacy LDAP Config I prefer sacrifice slapd.conf and use the deprecated version.

heiglandreas commented 9 years ago

Then I'll change that accordingly. But that might take some time as I'm busy until the weekend

Maks3w commented 9 years ago

@heiglandreas Don't worry I'm working in my PR adding a Vagrantfile too for easy local testing

heiglandreas commented 9 years ago

@Maks3w Ah! Good to know as I was thinking about that as well! You might then use the ldap-script too so we can reduce the amount of files needed!

Maks3w commented 9 years ago

My plan is reuse the same commands for Travis and Vagrant

heiglandreas commented 9 years ago

:+1:

Maks3w commented 9 years ago

Done. Please review #14

heiglandreas commented 9 years ago

Doing that tonight!

heiglandreas commented 9 years ago

I'll fix this as soon as your PR #14 is merged so I can use the LDAP-scripts in the travis.yml

heiglandreas commented 9 years ago

This PR is obsoleted by PR #14