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

Adding data and config volumes #39

Merged
merged 1 commit into from Feb 29, 2020
Merged

Adding data and config volumes #39

merged 1 commit into from Feb 29, 2020

Conversation

@hairyhenderson
Copy link
Contributor

hairyhenderson commented Feb 29, 2020

This came up in #38. Right now persisting TLS certs and auto-saved config is undocumented and requires mounting volumes to /root/.local/share and /root/.config.

This sets the data and config paths to /data/caddy and /config/caddy, and configures them as VOLUMEs to prevent data in these directories from persisting in the container. A user would then use them like so:

$ docker run -p 80:80 -p 443:443 \
    -v caddy_data:/data -v caddy_config:/config \
    caddy/caddy

And this would persist data/config to volumes.

Another useful benefit is that the container can then be run in read-only mode (docker run --read-only) for some added security.

Signed-off-by: Dave Henderson dhenderson@gmail.com

Signed-off-by: Dave Henderson <dhenderson@gmail.com>
@hairyhenderson hairyhenderson requested a review from mholt Feb 29, 2020
@hairyhenderson

This comment has been minimized.

Copy link
Contributor Author

hairyhenderson commented Feb 29, 2020

(would love to request a review from @francislavoie too, but GitHub isn't letting me - maybe team membership?)

@francislavoie

This comment has been minimized.

Copy link
Member

francislavoie commented Feb 29, 2020

Yeah I'm not a member. @mholt wouldn't mind if you added me! 😁

@hairyhenderson

This comment has been minimized.

Copy link
Contributor Author

hairyhenderson commented Feb 29, 2020

Note: this is a breaking change as discussed in #38 - IMO it's worth it, but I want @mholt to sign off on that 😉

@mholt
mholt approved these changes Feb 29, 2020
Copy link
Member

mholt left a comment

Fine with me, esp. while we're still in beta -- and this image isn't bound to Caddy's compatibility goals anyway.

Copy link
Member

francislavoie left a comment

LGTM, simplifies the paths and makes it much easier to remember them!

Btw did you give it a quick test to make sure it works as expected? Is Caddy handling the env properly?

@hairyhenderson hairyhenderson mentioned this pull request Feb 29, 2020
@hairyhenderson

This comment has been minimized.

Copy link
Contributor Author

hairyhenderson commented Feb 29, 2020

Btw did you give it a quick test to make sure it works as expected? Is Caddy handling the env properly?

Yup:

$ docker run -it --rm caddy/caddy
...
2020/02/29 18:34:14.789 INFO    autosaved config        {"file": "/config/caddy/autosave.json"}
@hairyhenderson hairyhenderson merged commit f49f8e0 into master Feb 29, 2020
3 checks passed
3 checks passed
docker-build
Details
gen-stackbrew
Details
gen-dockerfiles
Details
@hairyhenderson hairyhenderson deleted the data-config-volumes branch Feb 29, 2020
@mholt

This comment has been minimized.

Copy link
Member

mholt commented Feb 29, 2020

You can also use caddy environ or caddy run --environ to have Caddy spit the entire environment and its paths and stuff for logging/debugging.

@hairyhenderson hairyhenderson added this to Done in Official Image Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.