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

docs: update Basic-usage.md #1156

Closed
wants to merge 1 commit into from
Closed

Conversation

yoriiis
Copy link

@yoriiis yoriiis commented Oct 1, 2024

There is a difference between docker compose example and docker run for the acme-companion service about the certificat volume.

https://github.com/nginx-proxy/acme-companion/blob/main/docs/Docker-Compose.md?plain=1#L45

I imagine that the docker compose is the right syntax

There is a difference between docker compose example and docker run for the acme-companion service about the certificat volume.

https://github.com/nginx-proxy/acme-companion/blob/main/docs/Docker-Compose.md?plain=1#L45

I imagine that the docker compose is the right syntax
@buchdag buchdag added the type/docs PR with documentation only changes label Oct 2, 2024
@buchdag buchdag changed the title Update Basic-usage.md docs: update Basic-usage.md Oct 2, 2024
@buchdag
Copy link
Member

buchdag commented Oct 2, 2024

Hi.

In the compose file example, the nginx-proxy container mount the certs volume as :ro, so it need to be explicitly mounted as :rw on the acme-companion container.

In the Docker run example, the command for the nginx-proxy container does not specify :ro or :rw for this volume, so it defaults to :rw and the acme-companion container get the volume with the correct read/write permission through --volumes-from nginx-prox alone.

Both versions will work equally well, it boils down to restricting nginx-proxy write permission to the certs volume or not.

@yoriiis
Copy link
Author

yoriiis commented Oct 14, 2024

Hello @buchdag thanks for the explanation.

It might be a good idea to harmonize/synchronize to have the same implementation, but I'll let you decide that.

Thanks, you're free to close the issue.

@yoriiis yoriiis closed this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/docs PR with documentation only changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants