Add required libraries for sentry-ldap-auth to base Sentry image

[JHegarty14]

Summary

Since the Dockerfile is being removed and we can no longer build with these libraries locally, we're hoping they can be added to the base image. We're referencing a comment left by another user on the PR that took out the Sentry Dockerfile: #834 (comment)

Motivation

It allows the use of the ldap plugin for authentication.

Additional Context

N/A

[BYK]

I think we just need to add libldap2-dev, right? I see mentions of gcc but that is not visible on any documentation I check regarding the sentry-ldap-auth package and its dependencies.

[bobnautic]

I think we just need to add libldap2-dev, right? I see mentions of gcc but that is not visible on any documentation I check regarding the sentry-ldap-auth package and its dependencies.

After extending the Docker Image with those Dependencies the installation works on my side.
===> [libtool libsasl2-dev libldap2-dev libssl-dev].

I also would love to see this change coming

[BYK]

Okay tried this locally and it requires the whole build chain (gcc and friends) which adds over 100 MB to the final image size. I'm wondering if we can contribute to the upstream project so they upload pre-build wheels. This way we'd only need libldap2 to link against.

[JHegarty14]

We've gotten it working locally with gcc, libsasl2-dev, and libldap2-dev.

[rrauenza]

Is it the ldap python module itself that needs to compile? What if it were preinstalled during the Dockerfile creation, and then gcc, etc removed after? (That could be a problem though if the plugin later requires a newer version of the python module...)

Would it be reasonable to just absorb the cost of the gcc / dev space in order to also support other plugins in the future?

....Could we add a plugin file that does the apt install? Pinning the version is a requirement, too, though... and I think it would have to do the apt install every run?

What if the install.sh process went back to a Dockerfile that just starts with the appropriate FROM and nothing else? Then additional local modifications can be made in place?

I've already modified my .env to

SENTRY_IMAGE=sentry-with-ldap:21.11.0

And the docker-compose to:

$ more docker-compose.override.yml 
services:
    sentry-image:
        build: sentry/.
        image: sentry-with-ldap:21.11.0

but this gives errors (which can be ignored, but could be confusing) during the install.sh because it tries to fetch my custom image from global docker registry.

My dockerfile:

$ cat sentry/Dockerfile 
FROM getsentry/sentry:21.11.0
# https://www.broadcastify.com/listen/ctid/225
# https://github.com/Banno/getsentry-ldap-auth/issues/55
RUN apt-get update
RUN apt-get install -y libsasl2-dev python-dev libldap2-dev libssl-dev build-essential
RUN apt-get install -y postgresql-client

[huixisheng]

Banno/getsentry-ldap-auth#55 (comment)

[PMExtra]

My solution is:

ARG SENTRY_IMAGE
FROM ${SENTRY_IMAGE}

COPY requirements.txt /usr/src/sentry/

# Hook for installing additional plugins
RUN set -ex; \
  apt-get update; \
  apt-get install -y --no-install-recommends build-essential libldap2-dev libsasl2-dev; \
  pip install -r /usr/src/sentry/requirements.txt; \
  apt-get purge -y --auto-remove build-essential; \
  rm -rf /var/lib/apt/lists/*

It won't inflate the image size too much.

By the way, I made a new ldap plugin that based on Banno/getsentry-ldap-auth.

It upgraded the dependency django-auth-ldap from 1.2.* to 4.0.0 which support django 2.2+ (Sentry 21.9.0 and later).

Please ref: PMExtra/sentry-auth-ldap and https://pypi.org/project/sentry-auth-ldap/

[chadwhitacre]

Nice, thanks for sharing @PMExtra!

[PMExtra]

@BYK I just reviewed #834 .

Because of the ldap dependencies problem, I have manually rollbacked the docker-compose.yml to build my local image in months. Refer to my above comment.

Actually, I'm a supporter of building the local image.

But now I'm facing a new problem. In the old docker-compose.yml, sentry-cleanup-local-onpremise is depends on another local image sentry-local-onpremise. But the new docker compose 2.0 will build images parallelly. So building may be failed or incorrectly refer the old base image.

refer docker/compose#5228 and docker/compose#6332

So, I think we have just one solution if we don't want a 3rd party tool. Just combine the cron/Dockerfile and sentry/Dockerfile and use multi-stage builds.

I will do it anyway, I just hope you'll consider to adopt this solution, this will help me a lot to avoid manually solving conflicts again and again when I pull the updates from official.

Please let me know if you approve this, I can make a pull request for it.

[spawnia]

I think this issue is asking for a very specific solution without clearly stating the problem it is trying to solve.

Here is what I think is problematic with the current approach of installing dependencies and plugins:

• plugins are not easily added to both web and sentry-cleanup (see Install plugins in web and sentry-cleanup #1477)
• installation is run for every service using sentry/entrypoint.sh
• installation happens whenever a service using sentry/entrypoint.sh restarts
• the relatively complex sentry/entrypoint.sh needs to be modified and kept up to date with new versions

The proposed solution might work for popular or commonly used plugins, but I reckon it is not feasible to include every possible plugin and their dependencies within the base image. Thus, I propose that the solution should allow installation of plugins and dependencies:

• through an optionally added configuration, not requiring the modification of setup files
• run exactly once during install.sh
• work for all services that are running the sentry core or cli

[BYK]

I strongly disagree with defaulting to building local images but it is clear that we need to find a solution for a bit more customization.

Can one not use docker-compose.override.yml to force Sentry services to use a local Dockerfile and go from there?

[spawnia]

I strongly disagree with defaulting to building local images

Can you eloborate why? The default Dockerfile could just be empty and thus cause little to no overhead, but allow extensibility on that level.

[BYK]

@spawnia that's mostly what I'm suggesting but without including that Dockerfile in .git so people can modify it?

[spawnia]

I don't think there is a way to define docker-compose.yml in such a way that the presence of a Dockerfile is optional, build and image seem mutually exclusive to me.

What precisely is the issue with building local images? It seems like the most natural way to solve exactly the issue at hand: enhancing the base image with additional functionality and using it for multiple services.

[BYK]

I don't think there is a way to define docker-compose.yml in such a way that the presence of a Dockerfile is optional, build and image seem mutually exclusive to me.

They are not: https://docs.docker.com/compose/compose-file/build/#consistency-with-image -- That said you would need to override the image property for something local so it fails to fetch from a registry.

What precisely is the issue with building local images? It seems like the most natural way to solve exactly the issue at hand: enhancing the base image with additional functionality and using it for multiple services.

We moved away from that approach because it was a maintenance nightmare: #834 (comment)

[spawnia]

Yeah, it looks like image is preferred when using both - and build alone requires a Dockerfile to be present. Setting image to something that is not on the registry would force a local build, but might be a bit of a hacky solution.

I read through #834 and the comments in there. I totally agree that the bulk of the image should come pre-built from the registry.

Forgive my stubborness, but how is it problematic for maintenance or administration to have minimal modifications persisted in a locally built image? I fail to come up with a series of steps or a scenario where it would be an issue. It seems to work fine with cron/Dockerfile too. How is it different for the Sentry base image?

[BYK]

Forgive my stubborness, but how is it problematic for maintenance or administration to have minimal modifications persisted in a locally built image? I fail to come up with a series of steps or a scenario where it would be an issue. It seems to work fine with cron/Dockerfile too. How is it different for the Sentry base image?

I dislike cron/Dockerfile system a lot too as it is the most fragile part of the whole system that kept breaking. The only reason we have that monstrosity is the insistence on Docker Compose here and that lacking a proper task scheduler.

Anyhow, I think I have some past trauma clouding my judgement at this point and I cannot find rational reasons to refute your points ergo, would you be willing to submit a PR demonstrating your idea and let's see how that works out, and whether the CI likes it? :)

[spawnia]

@BYK done, please review #1479