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

Enhancement to support Docker Compose #53

Closed
wants to merge 1 commit into from

Conversation

weyert
Copy link

@weyert weyert commented Apr 11, 2020

I have written a Dockerfile and a docker-compose.yml to allow running Plausible within Docker, the docker compose will create a postgres-database, and also will build the Plausible docker image you can try it out using;

docker-compose build
docker-compose up

After the latter command succeeds you should have access to the development build of Plausible at url http://localhost:8000. You can use the docker logs to dig up the verification url from the mail ;)

Of course, this is not the right way to run things in Production but I don't have enough experience with Elixer to make this work. I tried to make a release but that didn't work as it appears use the environment variables at build time when running the release. If anyone know how to do this that would be great :)

@alexanderadam
Copy link

alexanderadam commented Apr 14, 2020

Nice, thank you for your work! 🙏
This looks great. I have not enough knowledge about Elixir/Phoenix so I can't say much about those but I have some feedback anyway, if that's okay for you:

1. Docker images should not run as root if possible

…and it's nearly always possible 😉. Or how the Docker docs put it:

If a service can run without privileges, use USER to change to a non-root user.

Ideally UID and GID are also arguments, so that the ids of the unprivileged user can match some ids of the volume owner in case that's necessary.

So you could add something like this:

ARG UID
ARG GID

RUN apk add --no-cache shadow && \
    if [ -z "`getent group $GID`" ]; then \
      addgroup -S -g $GID plausibleuser; \
    else \
      groupmod -n plausibleuser `getent group $GID | cut -d: -f1`; \
    fi && \
    if [ -z "`getent passwd $UID`" ]; then \
      adduser -S -u $UID -G plausibleuser -s /bin/sh plausibleuser; \
    else \
      usermod -l plausibleuser -g $GID -d /home/plausibleuser -m `getent passwd $UID | cut -d: -f1`; \
    fi



# Set the build directory.
RUN mkdir /opt/app
WORKDIR /opt/app

# copy files or ideally check out master branch with git
# and install dependencies with apk (before switching to unprivileged user)

RUN chown plausibleuser:plausibleuser /opt/app

USER plausibleuser

# everthing after the above line will be run as plausibleuser

There are also many posts on this topic about the why's and the hows (i.e. 1, 2, 3).

2. Cleanup of some unneeded comments

It seems that there are some legacy comments, right? 🤔

3. Use production environment

I tried to make a release but that didn't work as it appears use the environment variables at build time when running the release.

I'm not an Elixir dev neither but can you explain what's happening?
I mean apart from adding env variables like PGPASSWORD and POSTGRES_HOST to the other environments as well (maybe PGPASSWORD should be renamed to POSTGRES_PASSWORD to match the variables of the postgres image?)

Also it's probably better to set/export MIX_ENV just once. Currently the entrypoint will set it explicitly to dev a few times. So I guess this might lead to errors in case someone forgets about it.

@weyert
Copy link
Author

weyert commented Apr 14, 2020

Glad to hear you liked it @alexanderadam, yeah, fair point not to run it as root. I will have a look at that don't think that shouldn't be too difficult but we are still talking about computers ;)

PGPASSWORD and POSTGRES_HOST environment variables are the names used by PostgresSQL CLI tools. The idea was to keep waiting until the PostgresSQL database becomes available and then run your migration scripts at startup.

Yeah, regarding production environment, the main problem I had here was it didn't use my process environment variables when running your application instead it got all compiled at compile time and kept using those. I seem to have resolved this in the other branch in my repo.

Do you have experiences with mix release? My other branch is using that to try the runtime environment vars config working. I will keep cracking on it :)

@alexanderadam
Copy link

alexanderadam commented Apr 14, 2020

Do you have experiences with mix release?

I'm afraid not, sorry.

The idea was to keep waiting until the PostgresSQL database becomes available and then run your migration scripts at startup.

You probably mean

until psql -h postgres -U "postgres" -c '\q' 2>/dev/null; do

in the entrypoint.sh? Shouldn't this easily to make consistent by adding a line like

export PGPASSWORD="${PGPASSWORD:-POSTGRES_PASSWORD}"

before?

@ghost
Copy link

ghost commented Apr 15, 2020

First to use release you will have to setup the initial release configuration by adding a release parameter to your mix.exs file. Have a look at https://hexdocs.pm/mix/Mix.Tasks.Release.html

You should add a release.exs file in the config folder for the variables that should be passed at run time. https://hexdocs.pm/mix/Mix.Tasks.Release.html#module-runtime-configuration

The entrypoint file is more complex that it should be, after the release task you should not have to use mix but just start the server with:

 _build/prod/rel/app/bin/app start

In order to run the migrations before the start I do:

_build/prod/rel/app/bin/app eval "App.Release.migrate";

With this following file added to the code.

defmodule App.Release do
  @app :app

  def migrate do
    for repo <- repos() do
      {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :up, all: true))
    end
  end

  def rollback(repo, version) do
    {:ok, _, _} = Ecto.Migrator.with_repo(repo, &Ecto.Migrator.run(&1, :down, to: version))
  end

  defp repos do
    Application.load(@app)
    Application.fetch_env!(@app, :ecto_repos)
  end
end

@weyert
Copy link
Author

weyert commented Apr 28, 2020

Thanks @steffenix. I will update my PR. I don't get the feeling the author of this solution appreciates this PR, though

@ukutaht
Copy link
Contributor

ukutaht commented May 1, 2020

Thanks to everyone on this.

Sorry, of course I appreciate this PR and I'm very thankful. I've planned to take a look at this but just haven't got to it yet.

My main concern is that I don't run the app on docker. Not locally nor in production. So it's likely that the docker setup will break and get out of sync at some point as I develop the codebase and infrastructure.

I will test this out and merge soon. Looking at the code, I want to make sure the changes to config/dev.ex don't break the standard local postgres setup.

Again, sorry for the delay on this.

@alexanderadam
Copy link

alexanderadam commented May 1, 2020

My main concern is that I don't run the app on docker. Not locally nor in production. So it's likely that the docker setup will break and get out of sync at some point as I develop the codebase and infrastructure.

Users will give feedback soon enough I guess.
Furthermore there's already a travis config and travis can use docker images anyway, so I guess it should be possible to get early feedback via travis, too.

I will test this out and merge soon.

As mentioned earlier I think this PR is not merge-ready yet :

  1. This PR requires a cleanup (many legacy comments, especially in Dockerfile)
  2. Changes for prod.exs and test.exs are still missing.
  3. ENV variable consistency would be nice
  4. Image is still running as root

@tckb
Copy link
Contributor

tckb commented May 4, 2020

@ukutaht Really appreciate the effort you put in to open source it. I was searching for this exact solution. Although there are other (OSS) products, either they are incomplete or have not open-sourced the "privacy" focused part.

I am forking the current repo in-order to "add-in" the missing Dockerfile , I already have some of personal elixir apps running on my personal k8s cluster, I plan to add this one too.

I plan to work on this one, once this is stable enough and I will (read may) a PR for merging it into the upstream. Feel free to join for contributions.

https://gitlab.com/tckb-public/plausible

@weyert
Copy link
Author

weyert commented May 9, 2020

Thanks @tckb, any plans to make a PR for it? I am happy to make the changes listed by @alexanderadam

@tckb
Copy link
Contributor

tckb commented May 9, 2020

Yes, I plan to do it but, I'm not sure of the changes are aligned with the author. As I mentioned in #26 (comment) I have few things to do. I am tracking the issues I will work in the coming days here: https://gitlab.com/tckb-public/plausible/-/issues

once this is finished I will create a PR

@ukutaht
Copy link
Contributor

ukutaht commented May 29, 2020

Closing this in favour of #64

@ukutaht ukutaht closed this May 29, 2020
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 this pull request may close these issues.

None yet

4 participants