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

Allow ports to be configured via environment variables #199

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kuhnchris
Copy link
Contributor

This PR solves a very homeopathic problem with shared networks on docker (in this case: pods in podman, or, in this very special case: networking via pasta in a pod with podman where other containers already use up the ports requested by the passbolt container).

Sadly I was not aware if we need to add the changes over at root-level scripts/ aswell, I only saw them taking effect in the dev/bin docker-entrypoint.

The change itself should be fairly self-explainatory, I check the 3 variables and set the ports with some sed replacements in the nginx/conf.d/default.conf and the /usr/local/etc/php-fpm.d/www.conf pool config.

Tested locally, but YMMV.

@dlen
Copy link
Member

dlen commented May 5, 2023

Hi @kuhnchris!

Thanks for your PR! I'm not very familiar with pasta but could you explain the scenario where you have conflicting ports?
Using sed is something I would like to get rid of, this could be a great opportunity to use envsubst the same way they use it on the official nginx image

@kuhnchris
Copy link
Contributor Author

Hi there @dlen - sorry for the late reply!


Ad Network/Pasta:
Technically it's docker-esque implementation, with pasta creating a unprivileged network namespace (technical stuff).
What's important for this is: All "containers" within the pasta network share the same ip, hence localhost in the passbolt container is the same localhost in another webserver container, so if the passbolt container occupies port 80, no other container within the same network can occupy port 80.
My conflict comes from exactly this issue, me running a Caddy proxy on port 443 and 80 (in combination with a load of other containers), so the passbolt container can't get up.


Ad getting rid of sed:
I took a look, technically envsubst is great! BUT! For our purpose we cannot use this - we can do it in the dev/ container, as we have everything in our hand (and can create a template .conf file for nginx), but the debian/ / "production" container actually uses the packaged conf file, which can be found in your other repository (https://github.com/passbolt/passbolt_packaging/tree/main/debian/conf) - sadly this means we would need to either add a 3rd config there to delivery with every package, or add the .conf with template directly with COPY in the debian/Dockerfile.
This is a architectual question, so I would need a decision how to proceed from your side before I can do any changes. :-)

Thanks for reading and all the best,
Chris

@dlen
Copy link
Member

dlen commented May 25, 2023

I see, is this relevant to your use case? containers/podman#7175 (comment)

@kuhnchris
Copy link
Contributor Author

Sadly no, as that issue is just, as far as I read, about remapping ports from the container to the host, i.e. the container has it's own "available ports" - my case is, that multiple containers share a network, and hence cannot occupy the same port more than once. ;-)

@Hussienfahmy
Copy link

I want this feature too, I have Nginx running on ports 80 and 443. and can't get around that.

@CLAassistant
Copy link

CLAassistant commented Aug 17, 2023

CLA assistant check
All committers have signed the CLA.

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