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

Package with Docker #145

Merged
merged 4 commits into from
Mar 17, 2021
Merged

Package with Docker #145

merged 4 commits into from
Mar 17, 2021

Conversation

mildsunrise
Copy link

This PR adds .dockerignore and Dockerfile to allow it to be deployed with Docker. The Dockerfile contains instructions & examples of use.

This Dockerfile contains many improvements with respect to #110, see #110 (comment).

To allow the application to run correctly under docker, some extra changes have been made:

  • Allow the application to be configured by environment variables too (this is how Docker usually works)
  • config.ru: Set stdout to be always unbuffered (so logs appear immediately). This is useful for both Docker and other environments.
  • config.ru: Set PUBLIC_DIR in production environment too, otherwise public files are not served.

If you don't like those changes, tell me and I'll find a workaround.

@7olstoy
Copy link

7olstoy commented Nov 5, 2020

ruby:2.6 is the latest work version of ruby without errors

Copy link

@quentin-lpt quentin-lpt left a comment

Choose a reason for hiding this comment

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

all good

@quentin-lpt
Copy link

quentin-lpt commented Dec 8, 2020

Hello @mildsunrise
Could I add a docker-compose.yml directly in your Pull Request ?
I guess I would need some dev access to your repo ? (never done that before)

@mildsunrise
Copy link
Author

I think this PR already has many changes as-is... to avoid complicating it further, I'd suggest you start a separate PR based on my branch, and add docker-compose.yml there :)

@quentin-lpt
Copy link

Will do, thanks Alba :)

@whysthatso
Copy link

is there anything missing to move this pr? i'd like to use it :)

Copy link
Collaborator

@delano delano left a comment

Choose a reason for hiding this comment

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

Thanks so much for this. A bunch of folks will be happy to see this merged.

I had the one question re: loading the config file, but I'm going to go ahead and merge right away. I'm going to think about this change some more and I may save for a follow-on release so we can get the Docker config out the door.

Nice work!

@@ -135,7 +136,7 @@ module Config
attr_reader :env, :base, :bootstrap
def load path=self.path
raise ArgumentError, "Bad path (#{path})" unless File.readable?(path)
YAML.load_file path
YAML.load(ERB.new(File.read(path)).result)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. This allows the config file to be interpolated. Could you add a comment with a usage example?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I uncollapsed the config changes and see the obvious usecase 😹. All good

@delano delano merged commit d4d3ac4 into onetimesecret:master Mar 17, 2021
@mildsunrise mildsunrise deleted the docker branch March 17, 2021 20:26
@mildsunrise
Copy link
Author

Thank you! I see the config file question is resolved. Btw, I just noticed a typo in the help comments:

you should protect your Redis instance with authentication or Redis networks

Redis networks -> Docker networks

@FrederikNJS FrederikNJS mentioned this pull request Apr 21, 2021
@isAAAc
Copy link

isAAAc commented Jul 1, 2021

bump :)
is the docker version usable ?

@7olstoy
Copy link

7olstoy commented Jul 1, 2021

@isAAAc Sure, why not?

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

6 participants