Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Update the dockerfile to be Debian based, not Alpine based #6373

Closed
wants to merge 10 commits into from

Conversation

hawkowl
Copy link
Contributor

@hawkowl hawkowl commented Nov 18, 2019

This means we don't use libressl instead of openssl, and we get to use a lot of the manylinux1 wheels vs building them with headers ourselves.

@hawkowl hawkowl requested a review from a team November 18, 2019 16:45
@hawkowl hawkowl added the A-Docker Docker images, or making it easier to run Synapse in a container. label Nov 18, 2019
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

seems like a good idea


# xmlsec is required for saml support
RUN apk add --no-cache --virtual .runtime_deps \
libffi \
Copy link
Member

Choose a reason for hiding this comment

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

we need a bunch of this stuff for features that people expect to work in the docker image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so because we use the available wheels, we don't, because cffi and etc ship what's required.

Copy link
Member

Choose a reason for hiding this comment

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

I think we still need xmlsec (or, as debian calls it, xmlsec1) at least. pysaml relies on the /usr/bin/xmlsec1 binary.

@hawkowl
Copy link
Contributor Author

hawkowl commented Nov 25, 2019

I added some --version functionality so that we can check that Synapse, as well as its optional dependencies, are functional. On build, I get:

Synapse/1.6.0rc1

Additional functionality:
matrix-synapse-ldap3   [  OK  ]
postgres               [  OK  ]
resources.consent      [  OK  ]
acme                   [  OK  ]
saml2                  [  OK  ]
systemd                [NOT OK]
url_preview            [  OK  ]
sentry                 [  OK  ]
opentracing            [  OK  ]
jwt                    [  OK  ]

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

when I run this I get:

$ docker run --rm -it -e SYNAPSE_SERVER_NAME=test -e SYNAPSE_REPORT_STATS=no matrixdotorg/synapse generate
Container running as UserID 0:0, ENV (or defaults) requests 991:991
Creating log config /data/test.log.config
Traceback (most recent call last):
  File "/start.py", line 259, in <module>
    main(sys.argv, os.environ)
  File "/start.py", line 201, in main
    return run_generate_config(environ, ownership)
  File "/start.py", line 176, in run_generate_config
    os.execv("/sbin/su-exec", args)
FileNotFoundError: [Errno 2] No such file or directory

I'm basically a bit concerned this hasn't been tested.


# xmlsec is required for saml support
RUN apk add --no-cache --virtual .runtime_deps \
libffi \
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need xmlsec (or, as debian calls it, xmlsec1) at least. pysaml relies on the /usr/bin/xmlsec1 binary.

@richvdh
Copy link
Member

richvdh commented Feb 26, 2020

for future reference: this is probably a thing we should do if only to bring the docker image into line with other distros wrt which ssl library we use (see https://cryptography.io/en/latest/installation/)

@richvdh
Copy link
Member

richvdh commented Feb 26, 2020

Having said that: can't we just build against openssl-dev rather than libressl-dev? (see https://pkgs.alpinelinux.org/contents?file=libssl.so&path=&name=&branch=edge&arch=x86_64)

@richvdh
Copy link
Member

richvdh commented Feb 26, 2020

related: #7000

@clokep
Copy link
Member

clokep commented Apr 21, 2020

I'm going to close this since it hasn't been updated in ~6 months. If this is necessary in the future we can re-open this PR.

@clokep clokep closed this Apr 21, 2020
richvdh pushed a commit that referenced this pull request Jul 17, 2020
As mentioned in #7397, switching to a debian base should help with multi-arch work to save time on compiling. This is unashamedly based on #6373, but without the extra functionality. Switch python version back to generic 3.7 to always pull the latest. Essentially, keeping this as small as possible. The image is bigger though unfortunately.
@clokep clokep deleted the hawkowl/docker-image-update branch July 17, 2020 16:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Docker Docker images, or making it easier to run Synapse in a container.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants