Skip to content

Conversation

@huzaifaedhi22
Copy link

This PR

  • adds Zammad image to stack.yml
  • adds zammad.localhost to dev certificates

TODO

  • A zammad role in postgres
  • A schema for zammad ( Draft )

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Zammad configuration support by defining several new services in stack.yml and updating development certificates to include zammad.localhost.

  • Introduces new Docker service definitions for Zammad components (elasticsearch, init, railsserver, scheduler, websocket, among others).
  • Adds a dedicated volume for Zammad's Elasticsearch data.
  • Updates mkcert.sh to generate certificates for zammad.localhost.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/development/stack.yml Adds new service definitions and volume for Zammad, configuring image, dependencies, and traefik labels for routing.
src/development/certificates/mkcert.sh Updates certificate generation to include zammad.localhost.
Comments suppressed due to low confidence (1)

src/development/stack.yml:697

  • [nitpick] Consider standardizing the naming convention between service names (e.g., 'zammad-elasticsearch') and the associated volume ('zammad_elasticsearch_data') for improved clarity.
zammad_elasticsearch_data: {}

- ../../../reccoom/:/srv/app/

zammad-elasticsearch:
image: zammad/zammad-docker-compose:latest
Copy link

Copilot AI May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the 'latest' tag for the Zammad image can lead to unpredictable deployments. Consider pinning the image to a specific version to ensure consistency.

Suggested change
image: zammad/zammad-docker-compose:latest
image: zammad/zammad-docker-compose:5.0.3

Copilot uses AI. Check for mistakes.
Copy link
Member

@dargmuesli dargmuesli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dargstack expects each service to have a comment explaining what the service does, compare to the existing services. Also, run dargstack rgen to generate an updated README for others to see what services are in use.

@dargmuesli
Copy link
Member

@huzaifaedhi22 please request review from @sthelemann instead of me to get a bit more into inter-team collaboration. I get notified about all new PRs either way and will comment if necessary. When you two are happy, you can request a final review from me.

Comment on lines 364 to 365
- POSTGRESQL_USER=${POSTGRES_USER}
- POSTGRESQL_PASS=${POSTGRES_PASSWORD}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of env variables we need to work with secrets, see postgres service definition.


secrets:
- source: postgres_user
target: POSTGRESQL_USER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? On https://docs.docker.com/reference/compose-file/services/#secrets it reads as if you have to name a file path for target.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right in Swarm mode target: under secrets: needs to be a file path, not a variable name. I'll update it.

Comment on lines -32 to -41
`# adminer` "adminer.localhost" \
`# grafana` "grafana.localhost" \
`# minio` "minio.localhost" \
`# portainer` "portainer.localhost" \
`# postgraphile` "postgraphile.localhost" \
`# prometheus` "prometheus.localhost" \
`# redpanda` "redpanda.localhost" \
`# traefik` "traefik.localhost" \
`# tusd` "tusd.localhost" \
`# vibetype` "localhost" "www.localhost" "127.0.0.1" "0.0.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to delete the comments

POSTGRES_PASSWORD_FILE: /run/secrets/postgres_password
POSTGRES_USER_FILE: /run/secrets/postgres_user
image: postgis/postgis:17-3.5-alpine
image: imresamu/postgis:17-3.5.2-alpine3.21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be reverted

- ../../../reccoom/:/srv/app/

zammad-elasticsearch:
image: bitnami/elasticsearch:${ELASTICSEARCH_VERSION:-8.18.0}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
image: bitnami/elasticsearch:${ELASTICSEARCH_VERSION:-8.18.0}
image: bitnami/elasticsearch:8.18.0


zammad-elasticsearch:
image: bitnami/elasticsearch:${ELASTICSEARCH_VERSION:-8.18.0}
restart: always
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restart does not exist for stack I believe, see deploy.restart_policy.condition (https://docs.docker.com/reference/compose-file/deploy/#restart_policy)

volumes:
- ../../../reccoom/:/srv/app/

zammad-elasticsearch:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can elasticsearch be used by Zammad only or can we have it as elasticsearch only, similar to postgres?

Reading https://github.com/zammad/zammad-docker-compose?tab=readme-ov-file#running-without-elasticsearch it seems like elasticsearch could be excluded from the stack for "small teams". It appears some maintenance may be required for upgrades which I'm not sure how complicated they are. Please check that a bit further.

zammad-init:
image: zammad/zammad-docker-compose:latest
restart: on-failure
depends_on:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depends_on does not exist for stacks I believe

restart: always

zammad-websocket:
image: zammad/zammad-docker-compose:latest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
image: zammad/zammad-docker-compose:latest
image: zammad/zammad-docker-compose:6.5.0-85

maybe?

# The message queue's data.
{}
zammad_elasticsearch_data: {}
zammad_storage: {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
zammad_storage: {}
zammad_data: {}

Copy link
Member

@dargmuesli dargmuesli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, why do the additions look so different compared to the source definition? Especially in that some services have some labels but not all? Isn't it possible to use x-shared?

Comment on lines 490 to 491
- NGINX_SERVER_NAME=zammad.${STACK_DOMAIN}
- REDIS_URL=redis://zammad-redis:6379
Copy link
Member

@dargmuesli dargmuesli Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I apply the same environment variables as for zammad-websocket the container starts nginx successfully. Also, port 8080 needs to be used, not 80.

target: POSTGRESQL_PASS
command: ["zammad-scheduler"]

zammad-redis:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to elasticsearch, can this just be called redis?

target: POSTGRESQL_PASS
command: ["zammad-scheduler"]

zammad-redis:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Postgraphile you see an unnamed volume:
image
Clicking it reveals that it is mounted by redis under /data:
image
So you can add a volume named redis_data on that path here too.

@sthelemann sthelemann force-pushed the feat/zammad branch 3 times, most recently from bc2a3ae to 032e6c0 Compare June 16, 2025 21:20
@sthelemann
Copy link
Contributor

No idea what is still wrong with the [CI / dargstack rgen / Build (ubuntu-latest, 22) (pull_request) test.

@dargmuesli
Copy link
Member

@sthelemann you have to run dargstack rgen to regenerate the README so that it accurately represents the stack contents for people interested in finding our configuration.

An x-shared section was added to the stack.yml file, similar as in the original docker-compose yaml file to reuse environment settings etc. in several Zammad services.
@CLAassistant
Copy link

CLAassistant commented Jun 28, 2025

CLA assistant check
All committers have signed the CLA.

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.

5 participants