-
-
Notifications
You must be signed in to change notification settings - Fork 80
Support private/public IP in Realm response + ORM update + DB migration #1489
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
Open
Belnadifia
wants to merge
4
commits into
The-Alpha-Project:master
Choose a base branch
from
Belnadifia:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import os | ||
| import socket | ||
| import traceback | ||
| import ipaddress | ||
|
|
||
| from game.world.WorldSessionStateHandler import RealmDatabaseManager | ||
| from network.packet.PacketWriter import * | ||
|
|
@@ -9,6 +10,11 @@ | |
| from utils.Logger import Logger | ||
| from utils.constants import EnvVars | ||
|
|
||
| def is_private_ip(ip): | ||
| try: | ||
| return ipaddress.ip_address(ip).is_private | ||
| except ValueError: | ||
| return False | ||
|
|
||
| REALMLIST = {realm.realm_id: realm for realm in RealmDatabaseManager.realm_get_list()} | ||
|
|
||
|
|
@@ -18,20 +24,21 @@ class RealmManager: | |
|
|
||
| @staticmethod | ||
| def serve_realmlist(sck): | ||
| client_ip = sck.getpeername()[0] | ||
| realmlist_bytes = pack('<B', len(REALMLIST)) | ||
|
|
||
| for realm in REALMLIST.values(): | ||
| is_realm_local = config.Server.Connection.Realm.local_realm_id == realm.realm_id | ||
|
|
||
| name_bytes = PacketWriter.string_to_bytes(realm.realm_name) | ||
| # Only check if the forward address needs to be overriden if this realm is hosted on this same machine. | ||
| # Docker on Windows hackfix. | ||
| # https://discord.com/channels/628574828038717470/653374433636909077/840314080073351238 | ||
| if is_realm_local: | ||
| forward_address = os.getenv(EnvVars.EnvironmentalVariables.FORWARD_ADDRESS_OVERRIDE, | ||
| realm.proxy_address) | ||
| else: | ||
|
|
||
| if is_private_ip(client_ip): | ||
| forward_address = realm.proxy_address | ||
| Logger.debug(f'[{sck.getpeername()[0]}] Connection from {client_ip} -> Private') | ||
| else: | ||
| forward_address = realm.public_proxy_address | ||
| Logger.debug(f'[{sck.getpeername()[0]}] Connection from {client_ip} -> Public') | ||
|
|
||
|
|
||
| address_bytes = PacketWriter.string_to_bytes(f'{forward_address}:{realm.proxy_port}') | ||
| online_count = RealmDatabaseManager.realmlist_get_online_player_count(realm.realm_id) | ||
|
|
@@ -43,27 +50,33 @@ def serve_realmlist(sck): | |
| online_count | ||
| ) | ||
|
|
||
| Logger.debug(f'[{sck.getpeername()[0]}] Sending realmlist...') | ||
| Logger.debug(f'[{sck.getpeername()[0]}] Sending realmlist... (sent {forward_address}:{realm.proxy_port})') | ||
| sck.sendall(realmlist_bytes) | ||
|
|
||
| @staticmethod | ||
| def redirect_to_world(sck): | ||
| forward_address = os.getenv(EnvVars.EnvironmentalVariables.FORWARD_ADDRESS_OVERRIDE, | ||
| config.Server.Connection.WorldServer.host) | ||
| client_ip = sck.getpeername()[0] | ||
| local_realm = REALMLIST[config.Server.Connection.Realm.local_realm_id] | ||
|
|
||
| if is_private_ip(client_ip): | ||
| forward_address = local_realm.realm_address | ||
| else: | ||
| forward_address = local_realm.public_realm_address | ||
|
|
||
|
|
||
| world_bytes = PacketWriter.string_to_bytes(f'{forward_address}:{config.Server.Connection.WorldServer.port}') | ||
| packet = pack( | ||
| f'<{len(world_bytes)}s', | ||
| world_bytes | ||
| ) | ||
|
|
||
| Logger.debug(f'[{sck.getpeername()[0]}] Redirecting to world server...') | ||
| Logger.debug(f'[{client_ip}] Redirecting to world server... (sent {forward_address}:{config.Server.Connection.WorldServer.port})') | ||
| sck.sendall(packet) | ||
|
|
||
| @staticmethod | ||
| def start_realm(running, realm_server_ready): | ||
| local_realm = REALMLIST[config.Server.Connection.Realm.local_realm_id] | ||
| with SocketBuilder.build_socket(local_realm.realm_address, local_realm.realm_port, timeout=2) as server_socket: | ||
| with SocketBuilder.build_socket('0.0.0.0', local_realm.realm_port, timeout=2) as server_socket: | ||
| server_socket.listen() | ||
| real_binding = server_socket.getsockname() | ||
| # Make sure all characters have online = 0 on realm start. | ||
|
|
@@ -90,7 +103,7 @@ def start_realm(running, realm_server_ready): | |
| @staticmethod | ||
| def start_proxy(running, proxy_server_ready): | ||
| local_realm = REALMLIST[config.Server.Connection.Realm.local_realm_id] | ||
| with SocketBuilder.build_socket(local_realm.proxy_address, local_realm.proxy_port, timeout=2) as server_socket: | ||
| with SocketBuilder.build_socket('0.0.0.0', local_realm.proxy_port, timeout=2) as server_socket: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| server_socket.listen() | ||
| real_binding = server_socket.getsockname() | ||
| Logger.success(f'Proxy server started, listening on {real_binding[0]}:{real_binding[1]}') | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why bind to 0.0.0.0 here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When setting up a server socket, you might think to bind directly to a specific interface or IP address. However, in most modern deployments—especially in Kubernetes or cloud environments—binding to
0.0.0.0is the correct and robust choice.Why not bind to a specific IP?
Pods in Kubernetes have their own internal IPs (e.g.,
10.x.x.x), which are only reachable from inside the cluster.Externally, your service is typically accessed through a Load Balancer or NodePort with a different IP (e.g.,
172.16.x.x).Your application (or database, etc.) configuration might advertise the external (load balancer) IP, but your pod/container does not have that IP as a local interface!
If you bind your server to this external IP, the server won’t actually receive any connections, because that IP does not exist inside the pod.
Example scenario
Suppose:
Your pod’s internal IP is
10.0.0.1(only reachable from within the cluster).Your service is exposed externally at
172.16.0.54(load balancer or worker node).In your database or config, you have to advertise
172.16.0.54as the address clients should use.If you bind your socket to
172.16.0.54, your server will not listen on any actual interface inside the pod—because that IP does not exist inside the pod.The correct solution: Bind to
0.0.0.0Binding to
0.0.0.0means “listen on all available interfaces and all incoming destination IPs”.This ensures:
Your server can accept connections, no matter which network interface or cluster routing path is used.
Clients (inside or outside the cluster) can reach your service as long as the port is exposed.
It works for both internal and external clients, across NAT, service mesh, and Kubernetes routing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that, but it shouldn't be hardcoded; listening on 0.0.0.0 might not be suitable for everyone, depending on their server setup. Also, since
FORWARD_ADDRESS_OVERRIDEbecomes obsolete with these changes, all references to it should be removed, including in the Docker setup script.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If think listening on 0.0.0.0 is suitable for almost 90% of users ^•^
Having multiple NIC is mostly rare for common user (well, if we put asside WLAN)
Using 0.0.0.0 will accept any connection, no matter the server setup.
I understand it's shouldn't be hardcoded for specific case (most of the time, we use 0.0.0.0) , but then you should create a new ENV Variable and prosion it from conf file with 0.0.0.0 as default value.
And for FORWARD_ADDRESS_OVERRIDE, been a certain so I haven't the code in mind at the moment.