Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add py3-ldap3 to Docker image #162

Open
unmacaque opened this issue Oct 18, 2024 · 10 comments · May be fixed by #169
Open

Add py3-ldap3 to Docker image #162

unmacaque opened this issue Oct 18, 2024 · 10 comments · May be fixed by #169

Comments

@unmacaque
Copy link

Since Radicale 3.3.0.0, authentication with LDAP is supported - again.

References:

This does not yet work with this image yet, as Radicale will fail to start up, because Python module ldap3 is missing.

Would it be possible to pre-bundle py3-ldap3 with this image, so LDAP support works out of the box?

@tomsquest
Copy link
Owner

Hi @unmacaque ,

I am unsure about adding a dependency in this image. It can make it brittle and a bit heavier.
The more is added, the more change need to be tracked and... fixed.

Can you consider extending the image using https://github.com/tomsquest/docker-radicale?tab=readme-ov-file#extending-the-image ?

@tomsquest
Copy link
Owner

Closing for now. Re-open if needed, or better upvote the issue.

@alceasan
Copy link

alceasan commented Nov 5, 2024

I'd ask to reopen this again, maybe the extended option is no necessary anymore for LDAP auth, but then I'm not sure how to configure it. I used the radicale-extended option including the radicale3-auth-ldap library for LDAP auth, but builiding the extended image after 3.3 update results in app not working anymore:

[2024-11-05 07:44:43 +0000] [7] [INFO] Logging of backtrace is disabled in this loglevel
[2024-11-05 07:44:43 +0000] [7] [DEBUG] Logging of backtrace is disabled by option in this loglevel
[2024-11-05 07:44:43 +0000] [7] [INFO] Loaded default config
[2024-11-05 07:44:43 +0000] [7] [INFO] Loaded config file '/config/config'
[2024-11-05 07:44:43 +0000] [7] [INFO] Starting Radicale
[2024-11-05 07:44:43 +0000] [7] [INFO] auth type is 'radicale3_auth_ldap'
[2024-11-05 07:44:43 +0000] [7] [CRITICAL] An exception occurred during server startup: option already exists in 'auth': 'ldap_base'

It stills works building with the 3.2 version.

@tomsquest
Copy link
Owner

Hi @alceasan ,

Hard to diagnose without a LDAP system.

Could it be related to the 'ldap_base in your config?

@TilBlechschmidt
Copy link

TilBlechschmidt commented Nov 20, 2024

Hey there 👋

Just chiming in since I stumbled over the same problem.

I can totally understand that adding LDAP as a dependency might not be needed in 90% of cases and adds dead weight. However, the option of manually building a docker image requires users to housekeep it which might work fine with Docker (-compose) but is a nightmare when running Kubernetes which quite a few homelabs, including mine, do.

A possible solution — if you would be willing to take in the LDAP dependency from a maintenance standpoint that is — could be to publish a secondary image either tagged or suffixed with -ldap. This allows most users to still use the bare-bones image while those that do need it can rely on the slightly bulkier pre-built one.


Oh and a big thank you for maintaining this project, it made getting started with Radicale trivial <3

@TilBlechschmidt
Copy link

TilBlechschmidt commented Nov 20, 2024

Created a proof-of-concept over here which adds another layer to the Dockerfile containing the LDAP dependency. It also adapts the pipeline to run for both targets and pushes them to Docker Hub independently as two tags of the same image:

image

just-a-test is the name of the tag I created for testing. I have the image running in a test setup and it appears to work (login fails with wrong password, login succeeds with valid user, filter applies as expected):

image

Let me know if this is something you would be interested in 🙂

One open question is whether the tests should run on the base image, the LDAP one, or both. At the moment they just use the entire Dockerfile and thus include LDAP. This could also cause confusion for users that are extending the image. Might be an argument for a separate Dockerfile?

@tomsquest
Copy link
Owner

@TilBlechschmidt ,

Thanks for your suggestions.

You totally made the point: maintenance, what to test, what about security, release tag, how to document, what if there is another variant with oauth (should it include ldap or not?)...

Still, for now, we are talking about adding 1 or 2 dependencies for LDAP.

Questions:

  • Does py3-ldap3 is needed along installing ldap3? (one in the system, one in the venv)?
  • How much of increase of the size of the image?

@TilBlechschmidt
Copy link

  1. It is not
  2. ~5 MB uncompressed and ~1 MB compressed so <4%

I added py3-ldap initially based on the original post but it seems that the pip package inside the venv suffices.

@tomsquest
Copy link
Owner

Hi @TilBlechschmidt ,

I created a PR at #169 (just adding ldap3).

Can you build it to validate it's working in your use case?

I will then merge and release it.

Thanks

@tomsquest tomsquest reopened this Nov 27, 2024
@tomsquest tomsquest linked a pull request Nov 27, 2024 that will close this issue
@TilBlechschmidt
Copy link

TilBlechschmidt commented Nov 30, 2024

Just tested it, works like a charm 🙂

Thank you @tomsquest !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants