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

Add Dockerfile (Updated) #290

Merged
merged 24 commits into from
Jun 11, 2020
Merged

Conversation

tristanlins
Copy link
Contributor

@tristanlins tristanlins commented Apr 5, 2020

This is the successor or a revised version of #80
From the original PR I picked the two essential / sensible commits.
Instead of using the environment variables at runtime, you can now overwrite the defaults. In particular, it's all about SYDENT_DB_PATH.

A build on top of v2.0.0 can be found here: https://github.com/tristanlins/sydent/tree/feature-docker-v2.0.0
A pre-build image can be found on docker hub: https://hub.docker.com/r/tril/matrixdotorg-sydent/tags

Changelog

(non docker file related changes)

  • Improve the database migration logging.
  • The environment variables SYDENT_SERVER_NAME, SYDENT_PID_FILE and SYDENT_DB_PATH can be used to adapt the automatically created configuration at startup.

turt2live and others added 12 commits April 5, 2020 11:15
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
… mount

Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
@tristanlins
Copy link
Contributor Author

I made some more optimisations on the dockerfile.

  • Image size reduced from ~394MB to ~197MB
  • Run as user sydent not root (for security)

Signed-off-by: Tristan Lins <tristan@lins.io>
@tristanlins
Copy link
Contributor Author

The user sydent is now a system user and has a fixed id (<1000).
This reduces access to the data by regular users.
And the fixed ID ensures that the user's ID does not change accidentally.

@tristanlins
Copy link
Contributor Author

tristanlins commented Apr 7, 2020

I still have a few problems with configuration via environment variables. However, I have the impression that the problem has nothing to do with Docker, but with the handling of the default values. 🤔

@tristanlins
Copy link
Contributor Author

tristanlins commented Apr 15, 2020

I could no longer reproduce my problem when testing, I don't know exactly what was going on.

This Docker image is already running productively for me 😎

@clokep clokep requested a review from a team April 15, 2020 19:13
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Thanks for this! Just a small change.

README.rst Outdated Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented Jun 5, 2020

@tristanlins are you still interested in working on this? It's been a few weeks since the last update here: would you be able to answer @anoadragon453's questions?

@tristanlins
Copy link
Contributor Author

Sorry, I hat lost focus on this issue 😁
I had already replied to @anoadragon453, but had received no reply.
I have marked it as done for now, but I can make adjustments again.

Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

I had already replied to @anoadragon453, but had received no reply.

Do you mean #290 (comment)? That was only posted yesterday, and your comment 20m later. That's not a lot of time to respond :D

Anywho, thank you for the explanation 👍 I've become more familiar with Dockerfiles over the past couple weeks and thus feel more confident in reviewing this now. As such, I've left a few comments and questions below. Thank you!

.dockerignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@tristanlins
Copy link
Contributor Author

Do you mean #290 (comment)? That was only posted yesterday, and your comment 20m later. That's not a lot of time to respond :D

I'm really sorry, I had written the comment for a long time, but somehow it wasn't published. 🙈 (The new review management from Github still confuses me regularly ...)

I will incorporate the suggestions later.

@anoadragon453
Copy link
Member

I'm really sorry, I had written the comment for a long time, but somehow it wasn't published.

Ahh, no worries, I've done the exact same thing before :)

tristanlins and others added 4 commits June 11, 2020 10:00
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Signed-off-by: Tristan Lins <tristan@lins.io>
Signed-off-by: Tristan Lins <tristan@lins.io>
tristanlins and others added 4 commits June 11, 2020 13:11
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Signed-off-by: Tristan Lins <tristan@lins.io>
@tristanlins
Copy link
Contributor Author

Done, @anoadragon453 I'm looking forward to your feedback 😀

Signed-off-by: Tristan Lins <tristan@lins.io>
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

This LGTM! Thank you very much!

…cker

* 'master' of github.com:matrix-org/sydent:
  changelog: OpenID auth tokens -> access tokens
  2.0.1
  Add newsfile
  Raise IncorrectSessionTokenException instead of returning it
  Fix documentation for testing skipping the deletion of 3PID invite tokens
  Cleanup and document tests for 3PID invites
  Fix checking for access_token query parameter (matrix-org#294)
  Update pyproject.toml
  Fix project name in pyproject.toml
  Add towncrier config
  Do a deep copy of the config default when parsing a dict
  Add test cases for invite deletion
  Add a config option to disable deleting tokens on bind
@anoadragon453 anoadragon453 merged commit de80990 into matrix-org:master Jun 11, 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.

4 participants