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

[Core] Add internal port feature #2285

Merged
merged 2 commits into from
Oct 9, 2021
Merged

Conversation

Bockiii
Copy link
Contributor

@Bockiii Bockiii commented Oct 4, 2021

This fixes #2270

The addition of the feature does not change any behavior for current users. You have to manually add the environment variable "PORT" to the container to use this internal feature.

What it does:
If you add "PORT" to your docker container environment call, the script replaces the default port 80 in the config files to whichever port you assign.

I will add the how-to to the wiki as soon as it's merged.

@em92
Copy link
Contributor

em92 commented Oct 9, 2021

Only 80 port is exposed. If you switch it to other one, you won't be able to connect to it.

@em92
Copy link
Contributor

em92 commented Oct 9, 2021

Closing, as parent issue is resolvable without additional changes.

@em92 em92 closed this Oct 9, 2021
@Bockiii
Copy link
Contributor Author

Bockiii commented Oct 9, 2021

No, you misunderstood the query.

If you are using other network modes than "bridged" in docker, the environmental mapping is not available for some. So for example, if you want 2 containers in their own DMZ, there is no mapping at all between those containers and the host, but all ports are open between them. As if they were one machine.

The problem comes in when you have two containers that both use the same port (for example rss-bridge and a webserver container). Both listen on 80. If you now try to connect from a third container inside this DMZ to port 80, it doesn't know where to connect to.

It's a niche case, but still valid. One would be the case that the Issue opener provided: VPN container with nginx and rss-bridge.

In these networking scenarios, "exposed" also doesn't do anything. All ports are open inside the dmz.

@Bockiii
Copy link
Contributor Author

Bockiii commented Oct 9, 2021

On a sidenote: The "expose" feature is helpful for automated environments. You can say "auto-map exposed ports" and some container runtimes automatically attach a random port to the exposed ports, but in the end its just that: A help for automation. You can still map whatever port your heart desires to the container, doesn't matter if it's exposed or not. So changing the internal port to 5555, you only need to also map that port to the outside. eg docker run -e PORT=5555 -p 5555:5555 rss-bridge

@em92 em92 reopened this Oct 9, 2021
@em92 em92 merged commit 793c55f into RSS-Bridge:master Oct 9, 2021
@em92
Copy link
Contributor

em92 commented Oct 9, 2021

Sorry for misunderstanding the case and thank you for explanation!

I changed environment variable from PORT to HTTP_PORT as it is more commonly used. For example here https://symfony.com/doc/4.4/configuration/env_var_processors.html

@Bockiii
Copy link
Contributor Author

Bockiii commented Oct 9, 2021

If we want to switch the name, i would go for a different wording. I had the same issue with another application:

GoogleChrome/lighthouse-ci#258 (comment)

Rancher (think of it as kubernetes) auto creates a variable called SERVICE_PORT. So if someone uses Rancher and also has a service called "HTTP", theres going to be problems.

We can keep it like this for now since you already merged it. Just be aware of this if someone else opens an issue :)

@Bockiii
Copy link
Contributor Author

Bockiii commented Oct 9, 2021

tbh, its probably unlikely that someone names their kubernetes service "HTTP". Just be aware :D

floviolleau pushed a commit to floviolleau/rss-bridge that referenced this pull request Jan 25, 2022
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.

Change 80 port in config.ini.php file
2 participants