zammad / zammad-docker-compose

Zammad Docker images for docker-compose
https://hub.docker.com/r/zammad/zammad-docker-compose/
GNU Affero General Public License v3.0
273 stars 223 forks source link

Fix passing of database credentials to containers. #366

Closed mgruner closed 1 year ago

mgruner commented 1 year ago

The current state has serious issues. Database env vars from .env don't get passed to the containers, and thus it will only work if the defaults are used - they are also assumed by docker-entrypoint.sh. If you attempt to change database username, host etc. it will fail to even install Zammad.

This update corrects that and makes sure all containers that potentially execute Zammad / Rails code get all required variables. With this, commands can be executed like: docker-compose run --rm zammad-railsserver rails r 'pp Setting.count'.

MrGeneration commented 1 year ago

So I've tested it and even though the environments are now available as required, there's one environment variable that is missing: DATABASE_URL which rails and rake seem to be looking for (if they can't find it they'll fallback to config/database.yml.

The only way I can get that variable set in a "quick" way by entering the container with docker exec -ti zammad-docker-compose-zammad-railsserver-1 bash followed by source /docker-entrypoint.sh which is suboptimal. One could add .bashrc with

ESCAPED_POSTGRESQL_PASS=$(echo "$POSTGRESQL_PASS" | sed -e 's/[\/&]/\\&/g')
export DATABASE_URL="postgres://${POSTGRESQL_USER}:${ESCAPED_POSTGRESQL_PASS}@${POSTGRESQL_HOST}:${POSTGRESQL_PORT}/${POSTGRESQL_DB}"

for a dirty workaround to get rid of the second step. However: The disadvantage on this approach clearly is that you always have to call bash and from there call rails.

Directly calling rails does not work (yet). I'm still looking for a solution on that.


Docker by default uses sh which also by default doesn't invoke as login shell. For this reason one can't use /etc/profile or /etc/profile.d/someenv.sh.

A very dirty workaround I found on stackoverflow was to overwrite sh with bash which I personally don't like too much.

mgruner commented 1 year ago

@MrGeneration thanks a lot for the testing. I'm aware of this limitation. That's why I suggested to use docker-compose run ... instead. This will go through the entrypoint, and thus provide the DATABASE_URL.

Alternatively, docker exec ... /opt/zammad/contrib/docker-entrypoint.sh rails ... can be used to achieve the same environment from the outside. Just pass the command to call to the entrypoint, and it will invoke that. This is made possible by a recent contribution to docker-entrypoint.sh.

We'd have to make sure this is clearly documented. What do you think?

MrGeneration commented 1 year ago

My versions are

Docker version 24.0.5, build ced0996
Docker Compose version v2.20.2

I dug deeper and noticed that the .env suggestion right now is tag version 66 which is fairly old (we're at 119 right now). Suddendly it works. The image version is very important and should be bumped together with this PR to reduce confusions for others just like I had.

With this the entrypoint variant with exec also works as expected:

root@dockertest:/opt/zammad-docker-compose# docker exec -ti zammad-docker-compose-zammad-railsserver-1 /docker-entrypoint.sh rails r "pp Setting.count"
I, [2023-08-23T18:53:15.098150 #32]  INFO -- : ActionCable is using the redis instance at redis://zammad-redis:6379.
I, [2023-08-23T18:53:15.114664#32-5380]  INFO -- : Using memcached as Rails cache store.
I, [2023-08-23T18:53:15.114785#32-5380]  INFO -- : Using the Redis back end for Zammad's web socket session store.
I, [2023-08-23T18:53:16.487167#32-5380]  INFO -- : Setting.set('models_searchable', ["Chat::Session", "KnowledgeBase::Answer::Translation", "Organization", "Ticket", "User"])
251

Sorry for the confusion on my end here.


We'd have to make sure this is clearly documented. What do you think?

Definitely should be documented. It's not straight forward.

We also could implement some kind of wrapper like we have in package installations zammad run .... Just as an idea for lazy people like me. That variant is still shorter than the entrypoint variant. But that's just me possibly.


Btw I also build a local container to test my bash variant by defining bash to be the shell to be in the container. That variant however was not solving my above points so I discarded it.

42web-ch commented 1 year ago

oh! A big thanks, now its works for me. I'm a new Zammad User, great Work.